convert read_kcore(), vread() to use iterators

Commit df04abfd181a ("fs/proc/kcore.c: Add bounce buffer for ktext data") introduced the use of a bounce buffer to retrieve kernel text data for /proc/kcore in order to avoid failures arising from hardened user copies enabled by CONFIG_HARDENED_USERCOPY in check_kernel_text_object().

We can avoid doing this if instead of copy_to_user() we use _copy_to_user() which bypasses the hardening check. This is more efficient than using a bounce buffer and simplifies the code.

We do so as part an overall effort to eliminate bounce buffer usage in the function with an eye to converting it an iterator read.

Signed-off-by Lorenzo Stoakes <[email protected]>

---
  fs/proc/kcore.c | 17 +++++------------
  1 file changed, 5 insertions(+), 12 deletions(-)

fs/proc/kcore.c +2 -8

index 71157ee35c1a..556f310d6aa4 100644
@@ -541,19 +541,12 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos)
  		case KCORE_VMEMMAP:
  		case KCORE_TEXT:
  			/*
-			 * Using bounce buffer to bypass the
-			 * hardened user copy kernel text checks.
+			 * We use _copy_to_user() to bypass usermode hardening
+			 * which would otherwise prevent this operation.
  			 */
-			if (copy_from_kernel_nofault(buf, (void *)start, tsz)) {
-				if (clear_user(buffer, tsz)) {
-					ret = -EFAULT;
-					goto out;
-				}
-			} else {

vmalloc() is, by design, not permitted to be used in atomic context and already contains components which may sleep, so avoiding spin locks is not a problem from the perspective of atomic context.

The global vmap_area_lock is held when the red/black tree rooted in vmap_are_root is accessed and thus is rather long-held and under potentially high contention. It is likely to be under contention for reads rather than write, so replace it with a rwsem.

Each individual vmap_block->lock is likely to be held for less time but under low contention, so a mutex is not an outrageous choice here.

A subset of test_vmalloc.sh performance results:-

fix_size_alloc_test 0.40%
full_fit_alloc_test 2.08%
long_busy_list_alloc_test 0.34%
random_size_alloc_test -0.25%
random_size_align_alloc_test 0.06%
...
all tests cycles 0.2%

This represents a tiny reduction in performance that sits barely above noise.

The reason for making this change is to build a basis for vread() to be usable asynchronously, this eliminating the need for a bounce buffer when copying data to userland in read_kcore() and allowing that to be converted to an iterator form.

Quoted 25 lines · On Sun, 19 Mar 2023 07:09:31 +0000 Lorenzo Stoakes <[email protected]> wrote:

vmalloc() is, by design, not permitted to be used in atomic context and already contains components which may sleep, so avoiding spin locks is not a problem from the perspective of atomic context.

The global vmap_area_lock is held when the red/black tree rooted in vmap_are_root is accessed and thus is rather long-held and under potentially high contention. It is likely to be under contention for reads rather than write, so replace it with a rwsem.

Each individual vmap_block->lock is likely to be held for less time but under low contention, so a mutex is not an outrageous choice here.

A subset of test_vmalloc.sh performance results:-

fix_size_alloc_test 0.40%
full_fit_alloc_test 2.08%
long_busy_list_alloc_test 0.34%
random_size_alloc_test -0.25%
random_size_align_alloc_test 0.06%
...
all tests cycles 0.2%

This represents a tiny reduction in performance that sits barely above noise.

Quoted 5 lines

The reason for making this change is to build a basis for vread() to be usable asynchronously, this eliminating the need for a bounce buffer when copying data to userland in read_kcore() and allowing that to be converted to an iterator form.

I'm not understanding the final paragraph. How and where is vread() used "asynchronously"?

The basis for saying asynchronous was based on Documentation/filesystems/vfs.rst describing read_iter() as 'possibly asynchronous read with iov_iter as destination', and read_iter() is what is (now) invoked when accessing /proc/kcore.

However I agree this is vague and it is clearer to refer to the fact that we are now directly writing to user memory and thus wish to avoid spinlocks as we may need to fault in user memory in doing so.

Would it be ok for you to go ahead and replace that final paragraph with the below?:-

The reason for making this change is to build a basis for vread() to write to user memory directly via an iterator; as a result we may cause page faults during which we must not hold a spinlock. Doing this eliminates the need for a bounce buffer in read_kcore() and thus permits that to be converted to also use an iterator, as a read_iter() handler.

Quoted 9 lines · Lorenzo Stoakes

The basis for saying asynchronous was based on Documentation/filesystems/vfs.rst describing read_iter() as 'possibly asynchronous read with iov_iter as destination', and read_iter() is what is (now) invoked when accessing /proc/kcore.

However I agree this is vague and it is clearer to refer to the fact that we are now directly writing to user memory and thus wish to avoid spinlocks as we may need to fault in user memory in doing so.

Quoted 8 lines

Would it be ok for you to go ahead and replace that final paragraph with the below?:-

The reason for making this change is to build a basis for vread() to write to user memory directly via an iterator; as a result we may cause page faults during which we must not hold a spinlock. Doing this eliminates the need for a bounce buffer in read_kcore() and thus permits that to be converted to also use an iterator, as a read_iter() handler.

I'd say the purpose of the iterator is to abstract whether we're accessing user memory, kernel memory or a pipe, so I'd suggest:

The reason for making this change is to build a basis for vread() to write to memory via an iterator; as a result we may cause page faults during which we must not hold a spinlock. Doing this eliminates the need for a bounce buffer in read_kcore() and thus permits that to be converted to also use an iterator, as a read_iter() handler.

I'm still undecided whether this change is really a good thing. I think we have line-of-sight to making vmalloc (and thus kvmalloc) usable from interrupt context, and this destroys that possibility.

I wonder if we can't do something like prefaulting the page before taking the spinlock, then use copy_page_to_iter_atomic()

Quoted 2 lines · Matthew Wilcox

I wonder if we can't do something like prefaulting the page before taking the spinlock, then use copy_page_to_iter_atomic()

vmalloc() is, by design, not permitted to be used in atomic context and already contains components which may sleep, so avoiding spin locks is not a problem from the perspective of atomic context.

The global vmap_area_lock is held when the red/black tree rooted in vmap_are_root is accessed and thus is rather long-held and under potentially high contention. It is likely to be under contention for reads rather than write, so replace it with a rwsem.

Each individual vmap_block->lock is likely to be held for less time but under low contention, so a mutex is not an outrageous choice here.

A subset of test_vmalloc.sh performance results:-

fix_size_alloc_test 0.40%
full_fit_alloc_test 2.08%
long_busy_list_alloc_test 0.34%
random_size_alloc_test -0.25%
random_size_align_alloc_test 0.06%
...
all tests cycles 0.2%

This represents a tiny reduction in performance that sits barely above noise.

Quoted 22 lines

vmalloc() is, by design, not permitted to be used in atomic context and already contains components which may sleep, so avoiding spin locks is not a problem from the perspective of atomic context.

The global vmap_area_lock is held when the red/black tree rooted in vmap_are_root is accessed and thus is rather long-held and under potentially high contention. It is likely to be under contention for reads rather than write, so replace it with a rwsem.

Each individual vmap_block->lock is likely to be held for less time but under low contention, so a mutex is not an outrageous choice here.

A subset of test_vmalloc.sh performance results:-

fix_size_alloc_test 0.40%
full_fit_alloc_test 2.08%
long_busy_list_alloc_test 0.34%
random_size_alloc_test -0.25%
random_size_align_alloc_test 0.06%
...
all tests cycles 0.2%

Quoted 3 lines

This represents a tiny reduction in performance that sits barely above noise.

How important to have many simultaneous users of vread()? I do not see a big reason to switch into mutexes due to performance impact and making it less atomic.

So, how important for you to have this change?

--
Uladzislau Rezki

Quoted 22 lines · Uladzislau Rezki

Quoted 22 lines

vmalloc() is, by design, not permitted to be used in atomic context and already contains components which may sleep, so avoiding spin locks is not a problem from the perspective of atomic context.

The global vmap_area_lock is held when the red/black tree rooted in vmap_are_root is accessed and thus is rather long-held and under potentially high contention. It is likely to be under contention for reads rather than write, so replace it with a rwsem.

Each individual vmap_block->lock is likely to be held for less time but under low contention, so a mutex is not an outrageous choice here.

A subset of test_vmalloc.sh performance results:-

fix_size_alloc_test 0.40%
full_fit_alloc_test 2.08%
long_busy_list_alloc_test 0.34%
random_size_alloc_test -0.25%
random_size_align_alloc_test 0.06%
...
all tests cycles 0.2%

Quoted 6 lines

Quoted 3 lines

This represents a tiny reduction in performance that sits barely above noise.

How important to have many simultaneous users of vread()? I do not see a big reason to switch into mutexes due to performance impact and making it less atomic.

It's less about simultaneous users of vread() and more about being able to write direct to user memory rather than via a bounce buffer and not hold a spinlock over possible page faults.

The performance impact is barely above noise (I got fairly widely varying results), so I don't think it's really much of a cost at all. I can't imagine there are many users critically dependent on a sub-single digit % reduction in speed in vmalloc() allocation.

As I was saying to Willy, the code is already not atomic, or rather needs rework to become atomic-safe (there are a smattering of might_sleep()'s throughout)

However, given your objection alongside Willy's, let me examine Willy's suggestion that we instead of doing this, prefault the user memory in advance of the vread call.

Quoted 3 lines

So, how important for you to have this change?

Personally, always very important :)

Quoted 39 lines · Lorenzo Stoakes

Quoted 22 lines · Uladzislau Rezki

Quoted 22 lines

vmalloc() is, by design, not permitted to be used in atomic context and already contains components which may sleep, so avoiding spin locks is not a problem from the perspective of atomic context.

The global vmap_area_lock is held when the red/black tree rooted in vmap_are_root is accessed and thus is rather long-held and under potentially high contention. It is likely to be under contention for reads rather than write, so replace it with a rwsem.

Each individual vmap_block->lock is likely to be held for less time but under low contention, so a mutex is not an outrageous choice here.

A subset of test_vmalloc.sh performance results:-

fix_size_alloc_test 0.40%
full_fit_alloc_test 2.08%
long_busy_list_alloc_test 0.34%
random_size_alloc_test -0.25%
random_size_align_alloc_test 0.06%
...
all tests cycles 0.2%

Quoted 6 lines

Quoted 3 lines

This represents a tiny reduction in performance that sits barely above noise.

How important to have many simultaneous users of vread()? I do not see a big reason to switch into mutexes due to performance impact and making it less atomic.

It's less about simultaneous users of vread() and more about being able to write direct to user memory rather than via a bounce buffer and not hold a spinlock over possible page faults.

The performance impact is barely above noise (I got fairly widely varying results), so I don't think it's really much of a cost at all. I can't imagine there are many users critically dependent on a sub-single digit % reduction in speed in vmalloc() allocation.

Quoted 7 lines

As I was saying to Willy, the code is already not atomic, or rather needs rework to become atomic-safe (there are a smattering of might_sleep()'s throughout)

However, given your objection alongside Willy's, let me examine Willy's suggestion that we instead of doing this, prefault the user memory in advance of the vread call.

Just a quick perf tests shows regression around 6%. 10 workers test_mask is 31:

# default
[  140.349731] All test took worker0=485061693537 cycles
[  140.386065] All test took worker1=486504572954 cycles
[  140.418452] All test took worker2=467204082542 cycles
[  140.435895] All test took worker3=512591010219 cycles
[  140.458316] All test took worker4=448583324125 cycles
[  140.494244] All test took worker5=501018129647 cycles
[  140.518144] All test took worker6=516224787767 cycles
[  140.535472] All test took worker7=442025617137 cycles
[  140.558249] All test took worker8=503337286539 cycles
[  140.590571] All test took worker9=494369561574 cycles

# patch
[  144.464916] All test took worker0=530373399067 cycles
[  144.492904] All test took worker1=522641540924 cycles
[  144.528999] All test took worker2=529711158267 cycles
[  144.552963] All test took worker3=527389011775 cycles
[  144.592951] All test took worker4=529583252449 cycles
[  144.610286] All test took worker5=523605706016 cycles
[  144.627690] All test took worker6=531494777011 cycles
[  144.653046] All test took worker7=527150114726 cycles
[  144.669818] All test took worker8=526599712235 cycles
[  144.693428] All test took worker9=526057490851 cycles

Quoted 6 lines

Quoted 3 lines

So, how important for you to have this change?

Personally, always very important :)

This is good. Personal opinion always wins :)

--
Uladzislau Rezki

Quoted 61 lines · Uladzislau Rezki

Quoted 39 lines · Lorenzo Stoakes

Quoted 22 lines · Uladzislau Rezki

Quoted 22 lines

vmalloc() is, by design, not permitted to be used in atomic context and already contains components which may sleep, so avoiding spin locks is not a problem from the perspective of atomic context.

The global vmap_area_lock is held when the red/black tree rooted in vmap_are_root is accessed and thus is rather long-held and under potentially high contention. It is likely to be under contention for reads rather than write, so replace it with a rwsem.

Each individual vmap_block->lock is likely to be held for less time but under low contention, so a mutex is not an outrageous choice here.

A subset of test_vmalloc.sh performance results:-

fix_size_alloc_test 0.40%
full_fit_alloc_test 2.08%
long_busy_list_alloc_test 0.34%
random_size_alloc_test -0.25%
random_size_align_alloc_test 0.06%
...
all tests cycles 0.2%

Quoted 6 lines

Quoted 3 lines

This represents a tiny reduction in performance that sits barely above noise.

How important to have many simultaneous users of vread()? I do not see a big reason to switch into mutexes due to performance impact and making it less atomic.

It's less about simultaneous users of vread() and more about being able to write direct to user memory rather than via a bounce buffer and not hold a spinlock over possible page faults.

The performance impact is barely above noise (I got fairly widely varying results), so I don't think it's really much of a cost at all. I can't imagine there are many users critically dependent on a sub-single digit % reduction in speed in vmalloc() allocation.

Quoted 7 lines

As I was saying to Willy, the code is already not atomic, or rather needs rework to become atomic-safe (there are a smattering of might_sleep()'s throughout)

However, given your objection alongside Willy's, let me examine Willy's suggestion that we instead of doing this, prefault the user memory in advance of the vread call.

Just a quick perf tests shows regression around 6%. 10 workers test_mask is 31:

# default
[  140.349731] All test took worker0=485061693537 cycles
[  140.386065] All test took worker1=486504572954 cycles
[  140.418452] All test took worker2=467204082542 cycles
[  140.435895] All test took worker3=512591010219 cycles
[  140.458316] All test took worker4=448583324125 cycles
[  140.494244] All test took worker5=501018129647 cycles
[  140.518144] All test took worker6=516224787767 cycles
[  140.535472] All test took worker7=442025617137 cycles
[  140.558249] All test took worker8=503337286539 cycles
[  140.590571] All test took worker9=494369561574 cycles

Quoted 12 lines

# patch
[  144.464916] All test took worker0=530373399067 cycles
[  144.492904] All test took worker1=522641540924 cycles
[  144.528999] All test took worker2=529711158267 cycles
[  144.552963] All test took worker3=527389011775 cycles
[  144.592951] All test took worker4=529583252449 cycles
[  144.610286] All test took worker5=523605706016 cycles
[  144.627690] All test took worker6=531494777011 cycles
[  144.653046] All test took worker7=527150114726 cycles
[  144.669818] All test took worker8=526599712235 cycles
[  144.693428] All test took worker9=526057490851 cycles

OK ouch, that's worse than I observed! Let me try this prefault approach and then we can revert back to spinlocks.

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

So, how important for you to have this change?

Quoted 4 lines

Quoted 2 lines

Personally, always very important :)

This is good. Personal opinion always wins :)

The heart always wins ;) well, an adaption here can make everybody's hearts happy I think.

vmalloc() is, by design, not permitted to be used in atomic context and already contains components which may sleep, so avoiding spin locks is not a problem from the perspective of atomic context.

The global vmap_area_lock is held when the red/black tree rooted in vmap_are_root is accessed and thus is rather long-held and under potentially high contention. It is likely to be under contention for reads rather than write, so replace it with a rwsem.

Each individual vmap_block->lock is likely to be held for less time but under low contention, so a mutex is not an outrageous choice here.

A subset of test_vmalloc.sh performance results:-

fix_size_alloc_test 0.40%
full_fit_alloc_test 2.08%
long_busy_list_alloc_test 0.34%
random_size_alloc_test -0.25%
random_size_align_alloc_test 0.06%
...
all tests cycles 0.2%

This represents a tiny reduction in performance that sits barely above noise.

Quoted 22 lines · Lorenzo Stoakes

vmalloc() is, by design, not permitted to be used in atomic context and already contains components which may sleep, so avoiding spin locks is not a problem from the perspective of atomic context.

The global vmap_area_lock is held when the red/black tree rooted in vmap_are_root is accessed and thus is rather long-held and under potentially high contention. It is likely to be under contention for reads rather than write, so replace it with a rwsem.

Each individual vmap_block->lock is likely to be held for less time but under low contention, so a mutex is not an outrageous choice here.

A subset of test_vmalloc.sh performance results:-

fix_size_alloc_test 0.40%
full_fit_alloc_test 2.08%
long_busy_list_alloc_test 0.34%
random_size_alloc_test -0.25%
random_size_align_alloc_test 0.06%
...
all tests cycles 0.2%

Quoted 2 lines

This represents a tiny reduction in performance that sits barely above noise.

I'm travelling right now, but give me a few days and I'll test this against the XFS workloads that hammer the global vmalloc spin lock really, really badly. XFS can use vm_map_ram and vmalloc really heavily for metadata buffers and hit the global spin lock from every CPU in the system at the same time (i.e. highly concurrent workloads). vmalloc is also heavily used in the hottest path throught the journal where we process and calculate delta changes to several million items every second, again spread across every CPU in the system at the same time.

We really need the global spinlock to go away completely, but in the mean time a shared read lock should help a little bit....

Quoted 36 lines · Dave Chinner

Quoted 22 lines · Lorenzo Stoakes

vmalloc() is, by design, not permitted to be used in atomic context and already contains components which may sleep, so avoiding spin locks is not a problem from the perspective of atomic context.

The global vmap_area_lock is held when the red/black tree rooted in vmap_are_root is accessed and thus is rather long-held and under potentially high contention. It is likely to be under contention for reads rather than write, so replace it with a rwsem.

Each individual vmap_block->lock is likely to be held for less time but under low contention, so a mutex is not an outrageous choice here.

A subset of test_vmalloc.sh performance results:-

fix_size_alloc_test 0.40%
full_fit_alloc_test 2.08%
long_busy_list_alloc_test 0.34%
random_size_alloc_test -0.25%
random_size_align_alloc_test 0.06%
...
all tests cycles 0.2%

Quoted 2 lines

This represents a tiny reduction in performance that sits barely above noise.

I'm travelling right now, but give me a few days and I'll test this against the XFS workloads that hammer the global vmalloc spin lock really, really badly. XFS can use vm_map_ram and vmalloc really heavily for metadata buffers and hit the global spin lock from every CPU in the system at the same time (i.e. highly concurrent workloads). vmalloc is also heavily used in the hottest path throught the journal where we process and calculate delta changes to several million items every second, again spread across every CPU in the system at the same time.

Quoted 3 lines

We really need the global spinlock to go away completely, but in the mean time a shared read lock should help a little bit....

I am working on it. I submitted a proposal how to eliminate it:

<snip>
Hello, LSF.

Title: Introduce a per-cpu-vmap-cache to eliminate a vmap lock contention

Description:
Currently the vmap code is not scaled to number of CPU cores in a system because a global vmap space is protected by a single spinlock. Such approach has a clear bottleneck if many CPUs simultaneously access to one resource.

In this talk i would like to describe a drawback, show some data related to contentions and places where those occur in a code. Apart of that i would like to share ideas how to eliminate it providing a few approaches and compare them.

Requirements:
* It should be a per-cpu approach;
* Search of freed ptrs should not interfere with other freeing(as much as we can);
* - offload allocated areas(buzy ones) per-cpu;
* Cache ready sized objects or merge them into one big per-cpu-space(split on demand);
* Lazily-freed areas either drained per-cpu individually or by one CPU for all;
* Prefetch a fixed size in front and allocate per-cpu

Goals:
* Implement a per-cpu way of allocation to eliminate a contention.

Thanks!
<snip>

--
Uladzislau Rezki

Quoted 37 lines · Uladzislau Rezki

Quoted 36 lines · Dave Chinner

Quoted 22 lines · Lorenzo Stoakes

vmalloc() is, by design, not permitted to be used in atomic context and already contains components which may sleep, so avoiding spin locks is not a problem from the perspective of atomic context.

The global vmap_area_lock is held when the red/black tree rooted in vmap_are_root is accessed and thus is rather long-held and under potentially high contention. It is likely to be under contention for reads rather than write, so replace it with a rwsem.

Each individual vmap_block->lock is likely to be held for less time but under low contention, so a mutex is not an outrageous choice here.

A subset of test_vmalloc.sh performance results:-

fix_size_alloc_test 0.40%
full_fit_alloc_test 2.08%
long_busy_list_alloc_test 0.34%
random_size_alloc_test -0.25%
random_size_align_alloc_test 0.06%
...
all tests cycles 0.2%

Quoted 2 lines

This represents a tiny reduction in performance that sits barely above noise.

I'm travelling right now, but give me a few days and I'll test this against the XFS workloads that hammer the global vmalloc spin lock really, really badly. XFS can use vm_map_ram and vmalloc really heavily for metadata buffers and hit the global spin lock from every CPU in the system at the same time (i.e. highly concurrent workloads). vmalloc is also heavily used in the hottest path throught the journal where we process and calculate delta changes to several million items every second, again spread across every CPU in the system at the same time.

Quoted 3 lines

Quoted 3 lines

We really need the global spinlock to go away completely, but in the mean time a shared read lock should help a little bit....

Hugely appreciated Dave, however I must disappoint on the rwsem as I have now reworked my patch set to use the original locks in order to satisfy Willy's desire to make vmalloc atomic in future, and Uladzislau's desire to not have a ~6% performance hit -
https://lore.kernel.org/all/[email protected]/

Quoted 32 lines

I am working on it. I submitted a proposal how to eliminate it:

<snip>
Hello, LSF.

Title: Introduce a per-cpu-vmap-cache to eliminate a vmap lock contention

Description:
Currently the vmap code is not scaled to number of CPU cores in a system because a global vmap space is protected by a single spinlock. Such approach has a clear bottleneck if many CPUs simultaneously access to one resource.

In this talk i would like to describe a drawback, show some data related to contentions and places where those occur in a code. Apart of that i would like to share ideas how to eliminate it providing a few approaches and compare them.

Requirements:
* It should be a per-cpu approach;
* Search of freed ptrs should not interfere with other freeing(as much as we can);
* - offload allocated areas(buzy ones) per-cpu;
* Cache ready sized objects or merge them into one big per-cpu-space(split on demand);
* Lazily-freed areas either drained per-cpu individually or by one CPU for all;
* Prefetch a fixed size in front and allocate per-cpu

Goals:
* Implement a per-cpu way of allocation to eliminate a contention.

Thanks!
<snip>

Quoted 3 lines

--
Uladzislau Rezki

That's really awesome! I will come to that talk at LSF/MM :) being able to sustain the lock in atomic context seems to be an aspect that is important going forward also.

Quoted 37 lines · Uladzislau Rezki

Quoted 36 lines · Dave Chinner

Quoted 22 lines · Lorenzo Stoakes

vmalloc() is, by design, not permitted to be used in atomic context and already contains components which may sleep, so avoiding spin locks is not a problem from the perspective of atomic context.

The global vmap_area_lock is held when the red/black tree rooted in vmap_are_root is accessed and thus is rather long-held and under potentially high contention. It is likely to be under contention for reads rather than write, so replace it with a rwsem.

Each individual vmap_block->lock is likely to be held for less time but under low contention, so a mutex is not an outrageous choice here.

A subset of test_vmalloc.sh performance results:-

fix_size_alloc_test 0.40%
full_fit_alloc_test 2.08%
long_busy_list_alloc_test 0.34%
random_size_alloc_test -0.25%
random_size_align_alloc_test 0.06%
...
all tests cycles 0.2%

Quoted 2 lines

This represents a tiny reduction in performance that sits barely above noise.

I'm travelling right now, but give me a few days and I'll test this against the XFS workloads that hammer the global vmalloc spin lock really, really badly. XFS can use vm_map_ram and vmalloc really heavily for metadata buffers and hit the global spin lock from every CPU in the system at the same time (i.e. highly concurrent workloads). vmalloc is also heavily used in the hottest path throught the journal where we process and calculate delta changes to several million items every second, again spread across every CPU in the system at the same time.

Quoted 3 lines

Quoted 3 lines

We really need the global spinlock to go away completely, but in the mean time a shared read lock should help a little bit....

Hugely appreciated Dave, however I must disappoint on the rwsem as I have now reworked my patch set to use the original locks in order to satisfy Willy's desire to make vmalloc atomic in future, and Uladzislau's desire to not have a ~6% performance hit -
https://lore.kernel.org/all/[email protected]/

Quoted 13 lines

I am working on it. I submitted a proposal how to eliminate it:

<snip>
Hello, LSF.

Title: Introduce a per-cpu-vmap-cache to eliminate a vmap lock contention

Description:
Currently the vmap code is not scaled to number of CPU cores in a system because a global vmap space is protected by a single spinlock. Such approach has a clear bottleneck if many CPUs simultaneously access to one resource.

Quoted 4 lines

In this talk i would like to describe a drawback, show some data related to contentions and places where those occur in a code. Apart of that i would like to share ideas how to eliminate it providing a few approaches and compare them.

Quoted 2 lines

Requirements:
* It should be a per-cpu approach;

Quoted 5 lines

* Search of freed ptrs should not interfere with other freeing(as much as we can);
* - offload allocated areas(buzy ones) per-cpu;
* Cache ready sized objects or merge them into one big per-cpu-space(split on demand);
* Lazily-freed areas either drained per-cpu individually or by one CPU for all;
* Prefetch a fixed size in front and allocate per-cpu

Quoted 2 lines

Goals:
* Implement a per-cpu way of allocation to eliminate a contention.

Quoted 42 lines · Lorenzo Stoakes

Quoted 37 lines · Uladzislau Rezki

Quoted 36 lines · Dave Chinner

Quoted 22 lines · Lorenzo Stoakes

vmalloc() is, by design, not permitted to be used in atomic context and already contains components which may sleep, so avoiding spin locks is not a problem from the perspective of atomic context.

The global vmap_area_lock is held when the red/black tree rooted in vmap_are_root is accessed and thus is rather long-held and under potentially high contention. It is likely to be under contention for reads rather than write, so replace it with a rwsem.

Each individual vmap_block->lock is likely to be held for less time but under low contention, so a mutex is not an outrageous choice here.

A subset of test_vmalloc.sh performance results:-

fix_size_alloc_test 0.40%
full_fit_alloc_test 2.08%
long_busy_list_alloc_test 0.34%
random_size_alloc_test -0.25%
random_size_align_alloc_test 0.06%
...
all tests cycles 0.2%

Quoted 2 lines

This represents a tiny reduction in performance that sits barely above noise.

I'm travelling right now, but give me a few days and I'll test this against the XFS workloads that hammer the global vmalloc spin lock really, really badly. XFS can use vm_map_ram and vmalloc really heavily for metadata buffers and hit the global spin lock from every CPU in the system at the same time (i.e. highly concurrent workloads). vmalloc is also heavily used in the hottest path throught the journal where we process and calculate delta changes to several million items every second, again spread across every CPU in the system at the same time.

Quoted 3 lines

Quoted 3 lines

We really need the global spinlock to go away completely, but in the mean time a shared read lock should help a little bit....

Quoted 5 lines

Hugely appreciated Dave, however I must disappoint on the rwsem as I have now reworked my patch set to use the original locks in order to satisfy Willy's desire to make vmalloc atomic in future, and Uladzislau's desire to not have a ~6% performance hit -
https://lore.kernel.org/all/[email protected]/

Yeah, I'd already read that.

What I want to do, though, is to determine whether the problem shared access contention or exclusive access contention. If it's exclusive access contention, then an rwsem will do nothing to alleviate the problem, and that's kinda critical to know before any fix for the contention problems are worked out...

Quoted 13 lines

Quoted 13 lines

I am working on it. I submitted a proposal how to eliminate it:

<snip>
Hello, LSF.

Title: Introduce a per-cpu-vmap-cache to eliminate a vmap lock contention

Description:
Currently the vmap code is not scaled to number of CPU cores in a system because a global vmap space is protected by a single spinlock. Such approach has a clear bottleneck if many CPUs simultaneously access to one resource.

Quoted 4 lines

Quoted 4 lines

In this talk i would like to describe a drawback, show some data related to contentions and places where those occur in a code. Apart of that i would like to share ideas how to eliminate it providing a few approaches and compare them.

If you want data about contention problems with vmalloc

Quoted 2 lines

Quoted 2 lines

Requirements:
* It should be a per-cpu approach;

Hmmmm. My 2c worth on this: That is not a requirement.

That's a -solution-.

The requirement is that independent concurrent vmalloc/vfree operations do not severely contend with each other.

Yes, the solution will probably involve sharding the resource space across mulitple independent structures (as we do in filesystems with block groups, allocations groups, etc) but that does not necessarily need the structures to be per-cpu.

e.g per-node vmalloc arenas might be sufficient and allow more expensive but more efficient indexing structures to be used because we don't have to care about the explosion of memory that fine-grained per-cpu indexing generally entails. This may also fit in to the existing per-node structure of the memory reclaim infrastructure to manage things like compaction, balancing, etc of vmalloc space assigned to the given node.

Hence I think saying "per-cpu is a requirement" kinda prevents exploration of other novel solutions that may have advantages other than "just solves the concurrency problem"...

Quoted 5 lines

Quoted 5 lines

* Search of freed ptrs should not interfere with other freeing(as much as we can);
* - offload allocated areas(buzy ones) per-cpu;
* Cache ready sized objects or merge them into one big per-cpu-space(split on demand);
* Lazily-freed areas either drained per-cpu individually or by one CPU for all;
* Prefetch a fixed size in front and allocate per-cpu

I'd call these desired traits and/or potential optimisations, not hard requirements.

Quoted 2 lines

Quoted 2 lines

Goals:
* Implement a per-cpu way of allocation to eliminate a contention.

The goal should be to "allow contention-free vmalloc operations", not that we implement a specific solution.

I'm travelling right now, but give me a few days and I'll test this against the XFS workloads that hammer the global vmalloc spin lock really, really badly. XFS can use vm_map_ram and vmalloc really heavily for metadata buffers and hit the global spin lock from every CPU in the system at the same time (i.e. highly concurrent workloads). vmalloc is also heavily used in the hottest path throught the journal where we process and calculate delta changes to several million items every second, again spread across every CPU in the system at the same time.

We really need the global spinlock to go away completely, but in the mean time a shared read lock should help a little bit....

Hello, Dave.

Quoted 11 lines

I'm travelling right now, but give me a few days and I'll test this against the XFS workloads that hammer the global vmalloc spin lock really, really badly. XFS can use vm_map_ram and vmalloc really heavily for metadata buffers and hit the global spin lock from every CPU in the system at the same time (i.e. highly concurrent workloads). vmalloc is also heavily used in the hottest path throught the journal where we process and calculate delta changes to several million items every second, again spread across every CPU in the system at the same time.

Quoted 3 lines

We really need the global spinlock to go away completely, but in the mean time a shared read lock should help a little bit....

Could you please share some steps how to run your workloads in order to touch vmalloc() code. I would like to have a look at it in more detail just for understanding the workloads.

Meanwhile my grep agains xfs shows:

<snip>

urezki@pc638:~/data/raid0/coding/linux-rcu.git/fs/xfs$ grep -rn vmalloc ./

Quoted 16 lines · Uladzislau Rezki

Hello, Dave.

Quoted 11 lines

I'm travelling right now, but give me a few days and I'll test this against the XFS workloads that hammer the global vmalloc spin lock really, really badly. XFS can use vm_map_ram and vmalloc really heavily for metadata buffers and hit the global spin lock from every CPU in the system at the same time (i.e. highly concurrent workloads). vmalloc is also heavily used in the hottest path throught the journal where we process and calculate delta changes to several million items every second, again spread across every CPU in the system at the same time.

Quoted 3 lines

We really need the global spinlock to go away completely, but in the mean time a shared read lock should help a little bit....

Quoted 8 lines

Could you please share some steps how to run your workloads in order to touch vmalloc() code. I would like to have a look at it in more detail just for understanding the workloads.

Meanwhile my grep agains xfs shows:

<snip>

urezki@pc638:~/data/raid0/coding/linux-rcu.git/fs/xfs$ grep -rn vmalloc ./

You're missing:

fs/xfs/xfs_buf.c: bp->b_addr = vm_map_ram(bp->b_pages, bp->b_page_count,

which i suspect is the majority of Dave's workload. That will almost certainly take the vb_alloc() path.

Quoted 26 lines · Matthew Wilcox

Quoted 16 lines · Uladzislau Rezki

Hello, Dave.

Quoted 11 lines

I'm travelling right now, but give me a few days and I'll test this against the XFS workloads that hammer the global vmalloc spin lock really, really badly. XFS can use vm_map_ram and vmalloc really heavily for metadata buffers and hit the global spin lock from every CPU in the system at the same time (i.e. highly concurrent workloads). vmalloc is also heavily used in the hottest path throught the journal where we process and calculate delta changes to several million items every second, again spread across every CPU in the system at the same time.

Quoted 3 lines

We really need the global spinlock to go away completely, but in the mean time a shared read lock should help a little bit....

Quoted 8 lines

Could you please share some steps how to run your workloads in order to touch vmalloc() code. I would like to have a look at it in more detail just for understanding the workloads.

Meanwhile my grep agains xfs shows:

<snip>

urezki@pc638:~/data/raid0/coding/linux-rcu.git/fs/xfs$ grep -rn vmalloc ./

Quoted 7 lines

You're missing:

fs/xfs/xfs_buf.c: bp->b_addr = vm_map_ram(bp->b_pages, bp->b_page_count,

which i suspect is the majority of Dave's workload. That will almost certainly take the vb_alloc() path.

Then it has nothing to do with vmalloc contention(i mean global KVA allocator), IMHO. Unless:

<snip>
void *vm_map_ram(struct page **pages, unsigned int count, int node)

{
	unsigned long size = (unsigned long)count << PAGE_SHIFT;
	unsigned long addr;
	void *mem;

	if (likely(count <= VMAP_MAX_ALLOC)) {
		mem = vb_alloc(size, GFP_KERNEL);
		if (IS_ERR(mem))
			return NULL;
		addr = (unsigned long)mem;
	} else {
		struct vmap_area *va;
		va = alloc_vmap_area(size, PAGE_SIZE,
				VMALLOC_START, VMALLOC_END, node, GFP_KERNEL);
		if (IS_ERR(va))
			return NULL;

<snip>

number of pages > VMAP_MAX_ALLOC.

That is why i have asked about workloads because i would like to understand where a "problem" is. A vm_map_ram() access the global vmap space but it happens when a new vmap block is required and i also think it is not a problem.

But who knows, therefore it makes sense to have a lock at workload.

Quoted 57 lines · Uladzislau Rezki

Quoted 26 lines · Matthew Wilcox

Quoted 16 lines · Uladzislau Rezki

Hello, Dave.

Quoted 11 lines

I'm travelling right now, but give me a few days and I'll test this against the XFS workloads that hammer the global vmalloc spin lock really, really badly. XFS can use vm_map_ram and vmalloc really heavily for metadata buffers and hit the global spin lock from every CPU in the system at the same time (i.e. highly concurrent workloads). vmalloc is also heavily used in the hottest path throught the journal where we process and calculate delta changes to several million items every second, again spread across every CPU in the system at the same time.

Quoted 3 lines

We really need the global spinlock to go away completely, but in the mean time a shared read lock should help a little bit....

Quoted 8 lines

Could you please share some steps how to run your workloads in order to touch vmalloc() code. I would like to have a look at it in more detail just for understanding the workloads.

Meanwhile my grep agains xfs shows:

<snip>

urezki@pc638:~/data/raid0/coding/linux-rcu.git/fs/xfs$ grep -rn vmalloc ./

Quoted 7 lines

You're missing:

fs/xfs/xfs_buf.c: bp->b_addr = vm_map_ram(bp->b_pages, bp->b_page_count,

which i suspect is the majority of Dave's workload. That will almost certainly take the vb_alloc() path.

Then it has nothing to do with vmalloc contention(i mean global KVA allocator), IMHO. Unless:

<snip>
void *vm_map_ram(struct page **pages, unsigned int count, int node)

{
	unsigned long size = (unsigned long)count << PAGE_SHIFT;
	unsigned long addr;
	void *mem;

	if (likely(count <= VMAP_MAX_ALLOC)) {
		mem = vb_alloc(size, GFP_KERNEL);
		if (IS_ERR(mem))
			return NULL;
		addr = (unsigned long)mem;
	} else {
		struct vmap_area *va;
		va = alloc_vmap_area(size, PAGE_SIZE,
				VMALLOC_START, VMALLOC_END, node, GFP_KERNEL);
		if (IS_ERR(va))
			return NULL;

<snip>

Quoted 8 lines

number of pages > VMAP_MAX_ALLOC.

That is why i have asked about workloads because i would like to understand where a "problem" is. A vm_map_ram() access the global vmap space but it happens when a new vmap block is required and i also think it is not a problem.

But who knows, therefore it makes sense to have a lock at workload.

There is a lock-stat statistics for vm_map_ram()/vm_unmap_ram() test. I did it on 64 CPUs system with running 64 threads doing mapping/unmapping of 1 page. Each thread does 10 000 000 mapping + unmapping in a loop:

<snip>

root@pc638:/home/urezki# cat /proc/lock_stat
lock_stat version 0.4
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
class name    con-bounces    contentions   waittime-min   waittime-max waittime-total   waittime-avg    acq-bounces   acquisitions   holdtime-min   holdtime-max holdtime-total   holdtime-avg
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

vmap_area_lock: 2554079 2554276 0.06 213.61 11719647.67 4.59 2986513 3005712 0.05 67.02 3573323.37 1.19
--------------

  vmap_area_lock        1297948          [<00000000dd41cbaa>] alloc_vmap_area+0x1c7/0x910
  vmap_area_lock        1256330          [<000000009d927bf3>] free_vmap_block+0x4a/0xe0
  vmap_area_lock              1          [<00000000c95c05a7>] find_vm_area+0x16/0x70
  --------------
  vmap_area_lock        1738590          [<00000000dd41cbaa>] alloc_vmap_area+0x1c7/0x910
  vmap_area_lock         815688          [<000000009d927bf3>] free_vmap_block+0x4a/0xe0
  vmap_area_lock              1          [<00000000c1d619d7>] __get_vm_area_node+0xd2/0x170

.....................................................................................................................................................................................................

vmap_blocks.xa_lock: 862689 862698 0.05 77.74 849325.39 0.98 3005156 3005709 0.12 31.11 1920242.82 0.64
-------------------

  vmap_blocks.xa_lock         378418          [<00000000625a5626>] vm_map_ram+0x359/0x4a0
  vmap_blocks.xa_lock         484280          [<00000000caa2ef03>] xa_erase+0xe/0x30
  -------------------
  vmap_blocks.xa_lock         576226          [<00000000caa2ef03>] xa_erase+0xe/0x30
  vmap_blocks.xa_lock         286472          [<00000000625a5626>] vm_map_ram+0x359/0x4a0

....................................................................................................................................................................................................

free_vmap_area_lock: 394960 394961 0.05 124.78 448241.23 1.13 1514508 1515077 0.12 30.48 1179167.01 0.78
-------------------

  free_vmap_area_lock         385970          [<00000000955bd641>] alloc_vmap_area+0xe5/0x910
  free_vmap_area_lock           4692          [<00000000230abf7e>] __purge_vmap_area_lazy+0x10a/0x7d0
  free_vmap_area_lock           4299          [<00000000eed9ff9e>] alloc_vmap_area+0x497/0x910
  -------------------
  free_vmap_area_lock         371734          [<00000000955bd641>] alloc_vmap_area+0xe5/0x910
  free_vmap_area_lock          17007          [<00000000230abf7e>] __purge_vmap_area_lazy+0x10a/0x7d0
  free_vmap_area_lock           6220          [<00000000eed9ff9e>] alloc_vmap_area+0x497/0x910

.....................................................................................................................................................................................................

purge_vmap_area_lock: 169307 169312 0.05 31.08 81655.21 0.48 1514794 1515078 0.05 67.73 912391.12 0.60
--------------------
purge_vmap_area_lock 166409 [<0000000050938075>] free_vmap_area_noflush+0x65/0x370
purge_vmap_area_lock 2903 [<00000000fb8b57f7>] __purge_vmap_area_lazy+0x47/0x7d0
--------------------
purge_vmap_area_lock 167511 [<0000000050938075>] free_vmap_area_noflush+0x65/0x370
purge_vmap_area_lock 1801 [<00000000fb8b57f7>] __purge_vmap_area_lazy+0x47/0x7d0
<snip>

alloc_vmap_area is a top and second one is xa_lock. But the test i have done is pretty high concurrent scenario.

Hello, Dave.

Quoted 11 lines

I'm travelling right now, but give me a few days and I'll test this against the XFS workloads that hammer the global vmalloc spin lock really, really badly. XFS can use vm_map_ram and vmalloc really heavily for metadata buffers and hit the global spin lock from every CPU in the system at the same time (i.e. highly concurrent workloads). vmalloc is also heavily used in the hottest path throught the journal where we process and calculate delta changes to several million items every second, again spread across every CPU in the system at the same time.

Quoted 3 lines

We really need the global spinlock to go away completely, but in the mean time a shared read lock should help a little bit....

Could you please share some steps how to run your workloads in order to touch vmalloc() code. I would like to have a look at it in more detail just for understanding the workloads.

<snip>

urezki@pc638:~/data/raid0/coding/linux-rcu.git/fs/xfs$ grep -rn vmalloc ./
./xfs_log_priv.h:675: * Log vector and shadow buffers can be large, so we need to use kvmalloc() here
./xfs_log_priv.h:676: * to ensure success. Unfortunately, kvmalloc() only allows GFP_KERNEL contexts
./xfs_log_priv.h:677: * to fall back to vmalloc, so we can't actually do anything useful with gfp
./xfs_log_priv.h:678: * flags to control the kmalloc() behaviour within kvmalloc(). Hence kmalloc()
./xfs_log_priv.h:681: * vmalloc if it can't get somethign straight away from the free lists or
./xfs_log_priv.h:682: * buddy allocator. Hence we have to open code kvmalloc outselves here.
./xfs_log_priv.h:686: * allocations. This is actually the only way to make vmalloc() do GFP_NOFS
./xfs_log_priv.h:691:xlog_kvmalloc(

Did you read the comment above this function? I mean, it's all about how poorly kvmalloc() works for the highly concurrent, fail-fast context that occurs in the journal commit fast path, and how we open code it with kmalloc and vmalloc to work "ok" in this path.

Then if you go look at the commits related to it, you might find that XFS developers tend to write properly useful changelogs to document things like "it's better, but vmalloc will soon have lock contention problems if we hit it any harder"....

Quoted 9 lines · Dave Chinner

Did you read the comment above this function? I mean, it's all about how poorly kvmalloc() works for the highly concurrent, fail-fast context that occurs in the journal commit fast path, and how we open code it with kmalloc and vmalloc to work "ok" in this path.

Then if you go look at the commits related to it, you might find that XFS developers tend to write properly useful changelogs to document things like "it's better, but vmalloc will soon have lock contention problems if we hit it any harder"....

The problem with writing whinges like this is that mm developers don't read XFS changelogs. I certainly had no idea this was a problem, and I doubt anybody else who could make a start at fixing this problem had any idea either. Why go to all this effort instead of sending an email to linux-mm?

So, this patch open codes the kvmalloc() in the commit path to have the above described behaviour. The result is we more than halve the CPU time spend doing kvmalloc() in this path and transaction commits with 64kB objects in them more than doubles. i.e. we get ~5x reduction in CPU usage per costly-sized kvmalloc() invocation and the profile looks like this:

- 37.60% xlog_cil_commit
16.01% memcpy_erms
- 8.45% __kmalloc
- 8.04% kmalloc_order_trace
- 8.03% kmalloc_order
- 7.93% alloc_pages
- 7.90% __alloc_pages
- 4.05% __alloc_pages_slowpath.constprop.0
- 2.18% get_page_from_freelist
- 1.77% wake_all_kswapds
....
- __wake_up_common_lock
- 0.94% _raw_spin_lock_irqsave
- 3.72% get_page_from_freelist
- 2.43% _raw_spin_lock_irqsave
- 5.72% vmalloc

- 5.72% __vmalloc_node_range
- 4.81% __get_vm_area_node.constprop.0
- 3.26% alloc_vmap_area
- 2.52% _raw_spin_lock
- 1.46% _raw_spin_lock
0.56% __alloc_pages_bulk
- 4.66% kvfree
- 3.25% vfree

Quoted 23 lines

So, this patch open codes the kvmalloc() in the commit path to have the above described behaviour. The result is we more than halve the CPU time spend doing kvmalloc() in this path and transaction commits with 64kB objects in them more than doubles. i.e. we get ~5x reduction in CPU usage per costly-sized kvmalloc() invocation and the profile looks like this:

- 37.60% xlog_cil_commit
16.01% memcpy_erms
- 8.45% __kmalloc
- 8.04% kmalloc_order_trace
- 8.03% kmalloc_order
- 7.93% alloc_pages
- 7.90% __alloc_pages
- 4.05% __alloc_pages_slowpath.constprop.0
- 2.18% get_page_from_freelist
- 1.77% wake_all_kswapds
....
- __wake_up_common_lock
- 0.94% _raw_spin_lock_irqsave
- 3.72% get_page_from_freelist
- 2.43% _raw_spin_lock_irqsave
- 5.72% vmalloc

Quoted 8 lines

- 5.72% __vmalloc_node_range
- 4.81% __get_vm_area_node.constprop.0
- 3.26% alloc_vmap_area
- 2.52% _raw_spin_lock
- 1.46% _raw_spin_lock
0.56% __alloc_pages_bulk
- 4.66% kvfree
- 3.25% vfree

OK, i see. I tried to use the fs_mark in different configurations. For example:

<snip>
time fs_mark -D 10000 -S0 -n 100000 -s 0 -L 32 -d ./scratch/0 -d ./scratch/1 -d ./scratch/2 \

(quoted diff) +0 -3

-d ./scratch/3 -d ./scratch/4 -d ./scratch/5 -d ./scratch/6 -d ./scratch/7 -d ./scratch/8 \
-d ./scratch/9 -d ./scratch/10 -d ./scratch/11 -d ./scratch/12 -d ./scratch/13 \
-d ./scratch/14 -d ./scratch/15 -t 64 -F
<snip>

But i did not manage to trigger xlog_cil_commit() to fallback to vmalloc code. I think i should reduce an amount of memory on my kvm-pc and repeat the tests!

Quoted 41 lines · Uladzislau Rezki

Quoted 23 lines

So, this patch open codes the kvmalloc() in the commit path to have the above described behaviour. The result is we more than halve the CPU time spend doing kvmalloc() in this path and transaction commits with 64kB objects in them more than doubles. i.e. we get ~5x reduction in CPU usage per costly-sized kvmalloc() invocation and the profile looks like this:

- 37.60% xlog_cil_commit
16.01% memcpy_erms
- 8.45% __kmalloc
- 8.04% kmalloc_order_trace
- 8.03% kmalloc_order
- 7.93% alloc_pages
- 7.90% __alloc_pages
- 4.05% __alloc_pages_slowpath.constprop.0
- 2.18% get_page_from_freelist
- 1.77% wake_all_kswapds
....
- __wake_up_common_lock
- 0.94% _raw_spin_lock_irqsave
- 3.72% get_page_from_freelist
- 2.43% _raw_spin_lock_irqsave
- 5.72% vmalloc

Quoted 8 lines

- 5.72% __vmalloc_node_range
- 4.81% __get_vm_area_node.constprop.0
- 3.26% alloc_vmap_area
- 2.52% _raw_spin_lock
- 1.46% _raw_spin_lock
0.56% __alloc_pages_bulk
- 4.66% kvfree
- 3.25% vfree

OK, i see. I tried to use the fs_mark in different configurations. For example:

<snip>
time fs_mark -D 10000 -S0 -n 100000 -s 0 -L 32 -d ./scratch/0 -d ./scratch/1 -d ./scratch/2 \

(quoted diff) +0 -3

-d ./scratch/3 -d ./scratch/4 -d ./scratch/5 -d ./scratch/6 -d ./scratch/7 -d ./scratch/8 \
-d ./scratch/9 -d ./scratch/10 -d ./scratch/11 -d ./scratch/12 -d ./scratch/13 \
-d ./scratch/14 -d ./scratch/15 -t 64 -F
<snip>

Quoted 3 lines

But i did not manage to trigger xlog_cil_commit() to fallback to vmalloc code. I think i should reduce an amount of memory on my kvm-pc and repeat the tests!

Simple way of doing is to use directory blocks that are larger than page size:

mkfs.xfs -n size=64k ....

We can hit that path in other ways - large attributes will hit it in the attr buffer allocation path, enabling the new attribute intent-based logging mode will hit it in the xlog_cil_commit path as well. IIRC, the above profile comes from the latter case, creating lots of zero length files with 64kB xattrs attached via fsmark.