Re: [PATCH] pipe: Prefer mempolicy node for metadata allocations

["Vlastimil Babka (SUSE)" <vbabka@kernel.org>]

On 11/11/26 07:48, Andrea Righi wrote:
> Hi Jan,
> 
> On Tue, Nov 10, 2026 at 01:27:53PM +0100, Jan Kara wrote:
>> On Tue 10-11-26 07:52:19, Andrea Righi wrote:
>> > Generic slab allocations can reuse partial slabs from another NUMA node,
>> > even when a pipe workload is bound to one node. Pipe metadata is shared
>> > between senders and receivers and accessed throughout the pipe lifetime.
>> > Reusing remote metadata can therefore make an IPC workload pay remote
>> > access and coherence costs on every exchange.
>> > 
>> > The target use case is a workload that binds its workers and memory to
>> > one NUMA node with numactl. The policy node is then a useful placement
>> > hint for shared pipe metadata.
>> > 
>> > Allocate pipe_inode_info and its descriptor array on the node selected
>> > by the caller's memory policy. Apply the same preference when replacing
>> > slab-sized descriptor arrays during ring resizing. An explicit node lets
>> > SLUB try local allocations before reusing remote partial slabs.
>> > 
>> > For descriptor arrays larger than KMALLOC_MAX_CACHE_SIZE, pass
>> > NUMA_NO_NODE to keep the policy-aware page allocation path. An explicit
>> > node would otherwise lose the MPOL_BIND nodemask on fallback. Select the
>> > policy node only for slab-sized arrays, avoiding an extra advance of an
>> > interleave policy for large allocations.
>> > 
>> > Moving a pipe's users to another node doesn't migrate its existing
>> > metadata, so migration can still result in remote accesses regardless of
>> > this change. Generic allocations may reuse remote partial slabs, making
>> > initial placement dependent on allocator state. In contrast, preferring
>> > the policy node leverages the workload's placement hint to improve
>> > locality when users remain on the intended node.
>> > 
>> > Testing has been performed on a dual-socket NVIDIA Vera system, covering
>> > hackbench, pipe transfers, pipe creation and resizing, pipe_bench, and
>> > targeted stress tests.
>> > 
>> > The primary hackbench reproducer is:
>> > 
>> >   $ numactl --physcpubind=0-87 --membind=0 \
>> >       hackbench -P -p -g 10 -f 20 -s 100 -l 100000
>> > 
>> > Results (over 10 measured runs per case):
>> > 
>> >               avg       stdev
>> >    before     9.811s    0.130s
>> >    after      5.770s    0.058s
>> > 
>> > Before this change, allocations could reuse remote pipe metadata. With
>> > this change, both pipe objects and descriptor arrays are allocated on
>> > the requested node, improving locality and reducing hackbench runtime by
>> > approximately 41%.
>> > 
>> > Reported-by: Quan Liu <qualiu@nvidia.com>
>> > Signed-off-by: Andrea Righi <arighi@nvidia.com>
>> 
>> To me this makes sense so feel free to add:
>> 
>> Acked-by: Jan Kara <jack@suse.cz>
>> 
>> But IMHO it deserves a look from MM guys since this is more of an MM thing
>> than a filesystem one... Added to CC.
> 
> Thanks for taking a look, and good point about getting MM feedback.
> 
> Vlastimil, Gregory, do you think this SLUB locality and mempolicy
> aspects makes sense here? In particular, does preferring the policy node
> for slab-sized pipe metadata, while retaining the policy-aware path for
> larger arrays, look reasonable?

It's not wrong, but feels a bit ad-hoc. If it's pipe metadata now, what's next?

Slab will generally return objects from the cpu's local node even if it's
not specified. It's not guaranteed, but it's not guaranteed also when it's
specified by kmalloc_node() (unless __GFP_THISNODE forces it).

So the results above are surprising to me. Are they on recent kernels? Are
cpus 0-87 all local to node 0, or is membind 0 there to force a node that's
not local to some of them? If they are all local, I would expect the objects
to be from node 0 even without kmalloc_node(). So maybe we have some unknown
deficiency that should be investigated.

Also note that it's possible to force this behavior of respecting mempolicy
for all slab objects, usigng the slab_strict_numa boot parameter. AFAIK this
was added for systems where the interconnect makes remote accesses
especially costly.

> Thanks,
> -Andrea
> 
>> 
>> 								Honza
>> 
>> > ---
>> >  fs/pipe.c | 26 ++++++++++++++++++++++----
>> >  1 file changed, 22 insertions(+), 4 deletions(-)
>> > 
>> > diff --git a/fs/pipe.c b/fs/pipe.c
>> > index 4fa69192e6fba..2a8f9e9f70f84 100644
>> > --- a/fs/pipe.c
>> > +++ b/fs/pipe.c
>> > @@ -24,6 +24,7 @@
>> >  #include <linux/syscalls.h>
>> >  #include <linux/fcntl.h>
>> >  #include <linux/memcontrol.h>
>> > +#include <linux/mempolicy.h>
>> >  #include <linux/watch_queue.h>
>> >  #include <linux/sysctl.h>
>> >  #include <linux/sort.h>
>> > @@ -874,6 +875,16 @@ bool pipe_is_unprivileged_user(void)
>> >  	return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
>> >  }
>> >  
>> > +/* Prefer the policy node over remote partial slabs, retaining fallback. */
>> > +static int pipe_slab_node(void)
>> > +{
>> > +#ifdef CONFIG_NUMA
>> > +	return mempolicy_slab_node();
>> > +#else
>> > +	return NUMA_NO_NODE;
>> > +#endif
>> > +}
>> > +
>> >  struct pipe_inode_info *alloc_pipe_info(void)
>> >  {
>> >  	struct pipe_inode_info *pipe;
>> > @@ -881,8 +892,9 @@ struct pipe_inode_info *alloc_pipe_info(void)
>> >  	struct user_struct *user = get_current_user();
>> >  	unsigned long user_bufs;
>> >  	unsigned int max_size = READ_ONCE(pipe_max_size);
>> > +	int node = pipe_slab_node();
>> >  
>> > -	pipe = kzalloc_obj(struct pipe_inode_info, GFP_KERNEL_ACCOUNT);
>> > +	pipe = kzalloc_node(sizeof(*pipe), GFP_KERNEL_ACCOUNT, node);
>> >  	if (pipe == NULL)
>> >  		goto out_free_uid;
>> >  
>> > @@ -899,8 +911,8 @@ struct pipe_inode_info *alloc_pipe_info(void)
>> >  	if (too_many_pipe_buffers_hard(user_bufs) && pipe_is_unprivileged_user())
>> >  		goto out_revert_acct;
>> >  
>> > -	pipe->bufs = kzalloc_objs(struct pipe_buffer, pipe_bufs,
>> > -				  GFP_KERNEL_ACCOUNT);
>> > +	pipe->bufs = kcalloc_node(pipe_bufs, sizeof(*pipe->bufs),
>> > +				  GFP_KERNEL_ACCOUNT, node);
>> >  
>> >  	if (pipe->bufs) {
>> >  		init_waitqueue_head(&pipe->rd_wait);
>> > @@ -1375,12 +1387,18 @@ int pipe_resize_ring(struct pipe_inode_info *pipe, unsigned int nr_slots)
>> >  {
>> >  	struct pipe_buffer *bufs;
>> >  	unsigned int head, tail, mask, n;
>> > +	int node = NUMA_NO_NODE;
>> >  
>> >  	/* nr_slots larger than limits of pipe->{head,tail} */
>> >  	if (unlikely(nr_slots > (pipe_index_t)-1u))
>> >  		return -EINVAL;
>> >  
>> > -	bufs = kzalloc_objs(*bufs, nr_slots, GFP_KERNEL_ACCOUNT | __GFP_NOWARN);
>> > +	/* Keep the policy nodemask for allocations that bypass slab caches. */
>> > +	if (nr_slots <= KMALLOC_MAX_CACHE_SIZE / sizeof(*bufs))
>> > +		node = pipe_slab_node();
>> > +
>> > +	bufs = kcalloc_node(nr_slots, sizeof(*bufs),
>> > +			    GFP_KERNEL_ACCOUNT | __GFP_NOWARN, node);
>> >  	if (unlikely(!bufs))
>> >  		return -ENOMEM;
>> >  
>> > -- 
>> > 2.56.0
>> > 
>> -- 
>> Jan Kara <jack@suse.com>
>> SUSE Labs, CR
>>