linux-next: manual merge of the slab tree with the mm-unstable tree

Posted by Stephen Rothwell 2 months, 3 weeks ago

Hi all,

Today's linux-next merge of the slab tree got a conflict in:

  mm/mempool.c

between commit:

  25c4d8d29dbb ("mempool: clarify behavior of mempool_alloc_preallocated()")

from the mm-unstable tree and commit:

  5c829783e5f8 ("mempool: improve kerneldoc comments")

from the slab tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc mm/mempool.c
index cceb09b75ebe,e14d1cf46c72..000000000000
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@@ -456,13 -568,12 +568,12 @@@ EXPORT_SYMBOL(mempool_alloc_noprof)
  
  /**
   * mempool_alloc_preallocated - allocate an element from preallocated elements
-  *                              belonging to a specific memory pool
-  * @pool:      pointer to the memory pool which was allocated via
-  *             mempool_create().
+  *                              belonging to a memory pool
+  * @pool:	pointer to the memory pool
   *
-  * This function is similar to mempool_alloc, but it only attempts allocating
+  * This function is similar to mempool_alloc(), but it only attempts allocating
 - * an element from the preallocated elements. It does not sleep and immediately
 - * returns if no preallocated elements are available.
 + * an element from the preallocated elements. It only takes a single spinlock_t
 + * and immediately returns if no preallocated elements are available.
   *
   * Return: pointer to the allocated element or %NULL if no elements are
   * available.

Re: linux-next: manual merge of the slab tree with the mm-unstable tree

Posted by Christoph Hellwig 2 months, 3 weeks ago

On Fri, Nov 14, 2025 at 03:13:21PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the slab tree got a conflict in:
> 
>   mm/mempool.c
> 
> between commit:
> 
>   25c4d8d29dbb ("mempool: clarify behavior of mempool_alloc_preallocated()")
> 
> from the mm-unstable tree and commit:
> 
>   5c829783e5f8 ("mempool: improve kerneldoc comments")

Hmm, I guess we need to agree on which tree takes mempool patches, then
we can just rebase one side.

I also find 25c4d8d29dbb odd.  Yes, with PREEMPT_RT anything taking
spinlocks could sleep in the normal sense, but pretty much everything
in Linux assumes spinlocks as spinning.  So if we want to update that
we should agree on global conventions for it and not starting to update
random little functions individually.

Re: linux-next: manual merge of the slab tree with the mm-unstable tree

Posted by Vlastimil Babka 2 months, 3 weeks ago

On 11/14/25 06:06, Christoph Hellwig wrote:
> On Fri, Nov 14, 2025 at 03:13:21PM +1100, Stephen Rothwell wrote:
>> Hi all,
>> 
>> Today's linux-next merge of the slab tree got a conflict in:
>> 
>>   mm/mempool.c
>> 
>> between commit:
>> 
>>   25c4d8d29dbb ("mempool: clarify behavior of mempool_alloc_preallocated()")
>> 
>> from the mm-unstable tree and commit:
>> 
>>   5c829783e5f8 ("mempool: improve kerneldoc comments")
> 
> Hmm, I guess we need to agree on which tree takes mempool patches, then
> we can just rebase one side.

I can take that patch as mm-unstable (not -stable) means it's still
droppable there at this point.

> I also find 25c4d8d29dbb odd.  Yes, with PREEMPT_RT anything taking
> spinlocks could sleep in the normal sense, but pretty much everything
> in Linux assumes spinlocks as spinning.  So if we want to update that
> we should agree on global conventions for it and not starting to update
> random little functions individually.

That's also true. Thomas, is this case special or what motivated the patch
in the first place?

Re: linux-next: manual merge of the slab tree with the mm-unstable tree

Posted by Thomas Weißschuh 2 months, 3 weeks ago

On Fri, Nov 14, 2025 at 09:46:30AM +0100, Vlastimil Babka wrote:
> On 11/14/25 06:06, Christoph Hellwig wrote:
> > On Fri, Nov 14, 2025 at 03:13:21PM +1100, Stephen Rothwell wrote:
> >> Hi all,
> >> 
> >> Today's linux-next merge of the slab tree got a conflict in:
> >> 
> >>   mm/mempool.c
> >> 
> >> between commit:
> >> 
> >>   25c4d8d29dbb ("mempool: clarify behavior of mempool_alloc_preallocated()")
> >> 
> >> from the mm-unstable tree and commit:
> >> 
> >>   5c829783e5f8 ("mempool: improve kerneldoc comments")
> > 
> > Hmm, I guess we need to agree on which tree takes mempool patches, then
> > we can just rebase one side.
> 
> I can take that patch as mm-unstable (not -stable) means it's still
> droppable there at this point.
> 
> > I also find 25c4d8d29dbb odd.  Yes, with PREEMPT_RT anything taking
> > spinlocks could sleep in the normal sense, but pretty much everything
> > in Linux assumes spinlocks as spinning.

This is why the new documentation explicitly mentions the spinlock.
All callers can interpret this relative to their own usecase.

> > So if we want to update that
> > we should agree on global conventions for it and not starting to update
> > random little functions individually.

The behaviour of different locks under various kernel configurations is
already documented extensively. My change explicitly tried to defer to that.

We have the 'Context:' tag in kdoc. What about the following?

	Context: Any context. Takes and releases pool->lock.

> That's also true. Thomas, is this case special

No, not special. Just one of the few places which promises to "never sleep".

> or what motivated the patch in the first place?

I used the function in a tracepoint handler [0] and trusted its documentation
to "never sleep". That turned out to be incorrect.
Also see the discussion on the patch submission [1] about just this point,
where we didn't come up with better wording.

[0] https://lore.kernel.org/lkml/20250919-rv-reactor-signal-v1-1-fb0012034158@linutronix.de/
[1] https://lore.kernel.org/lkml/20251014-mempool-doc-v1-1-bc9ebf169700@linutronix.de/

Thomas

Re: linux-next: manual merge of the slab tree with the mm-unstable tree

Posted by Christoph Hellwig 2 months, 3 weeks ago

On Fri, Nov 14, 2025 at 10:13:40AM +0100, Thomas Weißschuh wrote:
> We have the 'Context:' tag in kdoc. What about the following?
> 
> 	Context: Any context. Takes and releases pool->lock.

Which in this case would be ok.  But what about functions that take
non-irqsave spinlocks?

> I used the function in a tracepoint handler [0] and trusted its documentation
> to "never sleep". That turned out to be incorrect.

Heh, you'll find a lot of those..

> Also see the discussion on the patch submission [1] about just this point,
> where we didn't come up with better wording.

Can we please start a discussion on this on say linux-doc and
linux-kernel?  I don't really have a good answer, but this current
idea feels a bit lacking.  I don't meant that as trying to block
this patch, but I think we need to come up with a better convention.

Re: linux-next: manual merge of the slab tree with the mm-unstable tree

Posted by Thomas Weißschuh 2 months, 2 weeks ago

On Fri, Nov 14, 2025 at 12:55:38PM +0100, Christoph Hellwig wrote:
> On Fri, Nov 14, 2025 at 10:13:40AM +0100, Thomas Weißschuh wrote:
> > We have the 'Context:' tag in kdoc. What about the following?
> > 
> > 	Context: Any context. Takes and releases pool->lock.
> 
> Which in this case would be ok.  But what about functions that take
> non-irqsave spinlocks?
> 
> > I used the function in a tracepoint handler [0] and trusted its documentation
> > to "never sleep". That turned out to be incorrect.
> 
> Heh, you'll find a lot of those..

Yeah... But people are working on fixing them.

> > Also see the discussion on the patch submission [1] about just this point,
> > where we didn't come up with better wording.
> 
> Can we please start a discussion on this on say linux-doc and
> linux-kernel?  I don't really have a good answer, but this current
> idea feels a bit lacking.  I don't meant that as trying to block
> this patch, but I think we need to come up with a better convention.

Make sense. Right now I don't really have the capacity to see this through,
but hopefully I get to it later.
My patch not really critical, so if it gets in the way it can be dropped.
Nearly nobody is using this function anyways.


Thomas

Re: linux-next: manual merge of the slab tree with the mm-unstable tree

Posted by Vlastimil Babka 2 months, 2 weeks ago

On 11/20/25 14:31, Thomas Weißschuh wrote:
> On Fri, Nov 14, 2025 at 12:55:38PM +0100, Christoph Hellwig wrote:
>> On Fri, Nov 14, 2025 at 10:13:40AM +0100, Thomas Weißschuh wrote:
>> > We have the 'Context:' tag in kdoc. What about the following?
>> > 
>> > 	Context: Any context. Takes and releases pool->lock.
>> 
>> Which in this case would be ok.  But what about functions that take
>> non-irqsave spinlocks?
>> 
>> > I used the function in a tracepoint handler [0] and trusted its documentation
>> > to "never sleep". That turned out to be incorrect.
>> 
>> Heh, you'll find a lot of those..
> 
> Yeah... But people are working on fixing them.
> 
>> > Also see the discussion on the patch submission [1] about just this point,
>> > where we didn't come up with better wording.
>> 
>> Can we please start a discussion on this on say linux-doc and
>> linux-kernel?  I don't really have a good answer, but this current
>> idea feels a bit lacking.  I don't meant that as trying to block
>> this patch, but I think we need to come up with a better convention.
> 
> Make sense. Right now I don't really have the capacity to see this through,
> but hopefully I get to it later.
> My patch not really critical, so if it gets in the way it can be dropped.
> Nearly nobody is using this function anyways.
I've left your patch in the slab/for-next as-is.