[LU-9796] Speedup file creation under heavy concurrency

• Type: Improvement
• Resolution: Fixed
• Priority: Minor
• Fix Version/s: Lustre 2.11.0, Lustre 2.10.5
• Labels: patch
• Assignee: Hongchao Zhang
• Reporter: Shuichi Ihara (Inactive)

Description

In general, there are some metadata performance regression with/without quota.
Now, Project quota introduced, it's time to measure metadata and improve metadata performance when quota is enabled.
Also, it seems upstream kernel has some performance optimizations for ext4 when quota enabled. It might be possible to get those optimization for Lustre.

Comments

[Bob Glossman (Inactive)]

I see recent mods in master have landed for this issue, but only for RHEL 7.x
Is this not needed for RHEL 6.x, SLES 11/12, Ubuntu ?

[Peter Jones]

Does the remaining patch need to land too or can that be abandoned and this ticket marked resolved?

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/29032/
Subject: LU-9796 ldiskfs: improve inode allocation performance
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 3f0a7241c434d9556308299eea069628715816c2

[Shuichi Ihara (Inactive)]

attached is test mds-survey resutls (File creation) with/withotu patch. https://jira.hpdd.intel.com/secure/attachment/28338/LU-9796.png
1 x MDS(1 x Platinum 8160 CPU, 128GB DDR4 memory)
1 x MDT(SFA7700X, RAID10 with 4 x Toshiba RI SSD)
Lustre-2.10.1CR (user/group quota enabled) on CentOS7.3

patch helps file creation speedup especially at high number of thread. we are now reaching ~250K ops/sec for file creation with single MDT on RHEL7 kernel.

[Gerrit Updater]

Wang Shilong ([e-mail]) uploaded a new patch: https://review.whamcloud.com/29032
Subject: LU-9796 ldiskfs: improve inode allocation performace
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 0f94e45786dc92f9ec048908234153a547d64462

[Patrick Farrell (Inactive)]

Ah, I like that much better! And it looks like it's much faster too, at least for creates. Nice. (In fact, faster than the original code before the regression, isn't it?)

[Wang Shilong (Inactive)]

Hello Patrick,

I have refreshed the patch to use new approach which looks better for me.
https://patchwork.ozlabs.org/patch/799014/

I don't like a timed sleep too.^_

[Patrick Farrell (Inactive)]

Ah, sorry for my misunderstanding.

So then it seems your problem is "inode stealing", where another thread uses the inode bit/number in the group before you can do it. So you end up contending for that lock because you're having to try over and over... (Which also is why it makes some sense that you count attempts rather than look directly at lock contention.)

I don't know what the comments on the list have been (it looks like silence so far), but it really bothers me to see "insert an arbitrary timer delay" as the solution here. That doesn't seem very future proof. Isn't there something we can do directly about the stealing? Increase lock coverage, change how we find the bit, set it to in use before we do all the testing and unset it if the testing fails, that sort of thing? It's hard to say exactly what would be safe.

Because it looks like the current problem is every thread is doing work for every inode, but only one is really getting to use the work it does. So perhaps we should lock around the find_next_zero_bit and set the bit there. That complicates the error path, but it seems like it would (mostly) guarantee forward progress for each thread. Perhaps that's not safe for other reasons, perhaps we can't set that bit (even temporarily) until we know the other things we check... I don't know...

But inserting a timed sleep...

[Shuichi Ihara (Inactive)]

Patrick,
What you said queued spinlock is CONFIG_QUEUED_SPINLOCKS, right?
Good to know, we didn't aware of it, but that was enabled in our all upstream testing.

# grep -i spinlocks .config 
CONFIG_ARCH_USE_QUEUED_SPINLOCKS=y
CONFIG_QUEUED_SPINLOCKS=y
CONFIG_PARAVIRT_SPINLOCKS=y
# Lock Debugging (spinlocks, mutexes, etc...)

But we saw same contentions even with that is enabled.

[Wang Shilong (Inactive)]

Hello Patrick,

We did do same benchmark test for RHEL7 and Latest upstream kernel, the result is same, we hit
same lock contention problem, and patch consistently improved the performace, at least there is
some benchmark numbers in patch changelog that you did not notice...

Thanks,
Shilong

[Patrick Farrell (Inactive)]

Shilong,

I'm not on the ext4 list so I won't comment there, but that code should almost certainly use ext4_fs_is_busy rather than counting attempts directly.

Separately, did you try that patch on the current upstream kernel?

Is the problem spinlock contention (most time spent in lock/unlock, not just waiting for the lock - a problem which is fixed in newer kernels) or is it actually waiting for the lock? (Most time spent waiting for lock, because it really is held.)

Anyway, if you haven't, you should try this on a newer kernel - Spinlock contention as a peformance problem is more or less fixed with queued spinlocks. (Multiple waiters for a spinlock now has minimal performance impact on lock/unlock, whereas in earlier kernels, multiple waiters cause locking and unlocking to take many times longer. It won't fix the problem of having to wait for the lock, but removes lock contention itself as the cause of performance issues.) RHEL7 doesn't have them, sadly.

If your problem really is spinlock contention, it's not going to show up on newer kernels. We might still want the patch for RHEL7.