* [PATCH] mm/mempool.c: set @elems slots to NULL once freed
@ 2025-12-23 23:10 klourencodev
2025-12-23 23:47 ` Vlastimil Babka
2026-01-05 16:46 ` Vlastimil Babka
0 siblings, 2 replies; 6+ messages in thread
From: klourencodev @ 2025-12-23 23:10 UTC (permalink / raw)
To: linux-mm; +Cc: akpm, vbabka, Kevin Lourenco
From: Kevin Lourenco <klourencodev@gmail.com>
This is documented in the function comment as "...and sets their
slots in @elems to NULL.", but it was not followed.
We need to follow the NULL assignment, because elements newly returned
to the pool must not be touched under any circumstances by the user.
Signed-off-by: Kevin Lourenco <klourencodev@gmail.com>
---
mm/mempool.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/mm/mempool.c b/mm/mempool.c
index c290e5261b47..1a2060304fd4 100644
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -674,7 +674,8 @@ unsigned int mempool_free_bulk(struct mempool *pool, void **elems,
if (unlikely(READ_ONCE(pool->curr_nr) < pool->min_nr)) {
spin_lock_irqsave(&pool->lock, flags);
while (pool->curr_nr < pool->min_nr && freed < count) {
- add_element(pool, elems[freed++]);
+ add_element(pool, elems[freed]);
+ elems[freed++] = NULL;
added = true;
}
spin_unlock_irqrestore(&pool->lock, flags);
@@ -683,7 +684,8 @@ unsigned int mempool_free_bulk(struct mempool *pool, void **elems,
/* Handle the min_nr = 0 edge case: */
spin_lock_irqsave(&pool->lock, flags);
if (likely(pool->curr_nr == 0)) {
- add_element(pool, elems[freed++]);
+ add_element(pool, elems[freed]);
+ elems[freed++] = NULL;
added = true;
}
spin_unlock_irqrestore(&pool->lock, flags);
--
2.47.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/mempool.c: set @elems slots to NULL once freed
2025-12-23 23:10 [PATCH] mm/mempool.c: set @elems slots to NULL once freed klourencodev
@ 2025-12-23 23:47 ` Vlastimil Babka
2026-01-05 16:46 ` Vlastimil Babka
1 sibling, 0 replies; 6+ messages in thread
From: Vlastimil Babka @ 2025-12-23 23:47 UTC (permalink / raw)
To: klourencodev, linux-mm, Christoph Hellwig; +Cc: akpm
+Cc Christoph
On 12/24/25 00:10, klourencodev@gmail.com wrote:
> From: Kevin Lourenco <klourencodev@gmail.com>
>
> This is documented in the function comment as "...and sets their
> slots in @elems to NULL.", but it was not followed.
>
> We need to follow the NULL assignment, because elements newly returned
> to the pool must not be touched under any circumstances by the user.
>
> Signed-off-by: Kevin Lourenco <klourencodev@gmail.com>
> ---
> mm/mempool.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/mm/mempool.c b/mm/mempool.c
> index c290e5261b47..1a2060304fd4 100644
> --- a/mm/mempool.c
> +++ b/mm/mempool.c
> @@ -674,7 +674,8 @@ unsigned int mempool_free_bulk(struct mempool *pool, void **elems,
> if (unlikely(READ_ONCE(pool->curr_nr) < pool->min_nr)) {
> spin_lock_irqsave(&pool->lock, flags);
> while (pool->curr_nr < pool->min_nr && freed < count) {
> - add_element(pool, elems[freed++]);
> + add_element(pool, elems[freed]);
> + elems[freed++] = NULL;
> added = true;
> }
> spin_unlock_irqrestore(&pool->lock, flags);
> @@ -683,7 +684,8 @@ unsigned int mempool_free_bulk(struct mempool *pool, void **elems,
> /* Handle the min_nr = 0 edge case: */
> spin_lock_irqsave(&pool->lock, flags);
> if (likely(pool->curr_nr == 0)) {
> - add_element(pool, elems[freed++]);
> + add_element(pool, elems[freed]);
> + elems[freed++] = NULL;
> added = true;
> }
> spin_unlock_irqrestore(&pool->lock, flags);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/mempool.c: set @elems slots to NULL once freed
2025-12-23 23:10 [PATCH] mm/mempool.c: set @elems slots to NULL once freed klourencodev
2025-12-23 23:47 ` Vlastimil Babka
@ 2026-01-05 16:46 ` Vlastimil Babka
2026-01-06 6:22 ` Christoph Hellwig
1 sibling, 1 reply; 6+ messages in thread
From: Vlastimil Babka @ 2026-01-05 16:46 UTC (permalink / raw)
To: klourencodev, linux-mm, Christoph Hellwig; +Cc: akpm
On 12/24/25 00:10, klourencodev@gmail.com wrote:
> From: Kevin Lourenco <klourencodev@gmail.com>
>
> This is documented in the function comment as "...and sets their
> slots in @elems to NULL.", but it was not followed.
>
> We need to follow the NULL assignment, because elements newly returned
> to the pool must not be touched under any circumstances by the user.
Well alternatively we could update the comment if no user needs to rely on
this. Since it was modeled after __alloc_pages_bulk()/release_pages() and
release_pages() doesn't do this, we could also avoid it for perf reasons.
Christoph, what do you think?
> Signed-off-by: Kevin Lourenco <klourencodev@gmail.com>
> ---
> mm/mempool.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/mm/mempool.c b/mm/mempool.c
> index c290e5261b47..1a2060304fd4 100644
> --- a/mm/mempool.c
> +++ b/mm/mempool.c
> @@ -674,7 +674,8 @@ unsigned int mempool_free_bulk(struct mempool *pool, void **elems,
> if (unlikely(READ_ONCE(pool->curr_nr) < pool->min_nr)) {
> spin_lock_irqsave(&pool->lock, flags);
> while (pool->curr_nr < pool->min_nr && freed < count) {
> - add_element(pool, elems[freed++]);
> + add_element(pool, elems[freed]);
> + elems[freed++] = NULL;
> added = true;
> }
> spin_unlock_irqrestore(&pool->lock, flags);
> @@ -683,7 +684,8 @@ unsigned int mempool_free_bulk(struct mempool *pool, void **elems,
> /* Handle the min_nr = 0 edge case: */
> spin_lock_irqsave(&pool->lock, flags);
> if (likely(pool->curr_nr == 0)) {
> - add_element(pool, elems[freed++]);
> + add_element(pool, elems[freed]);
> + elems[freed++] = NULL;
> added = true;
> }
> spin_unlock_irqrestore(&pool->lock, flags);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/mempool.c: set @elems slots to NULL once freed
2026-01-05 16:46 ` Vlastimil Babka
@ 2026-01-06 6:22 ` Christoph Hellwig
2026-01-06 8:00 ` Vlastimil Babka
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2026-01-06 6:22 UTC (permalink / raw)
To: Vlastimil Babka; +Cc: klourencodev, linux-mm, Christoph Hellwig, akpm
On Mon, Jan 05, 2026 at 05:46:47PM +0100, Vlastimil Babka wrote:
> On 12/24/25 00:10, klourencodev@gmail.com wrote:
> > From: Kevin Lourenco <klourencodev@gmail.com>
> >
> > This is documented in the function comment as "...and sets their
> > slots in @elems to NULL.", but it was not followed.
> >
> > We need to follow the NULL assignment, because elements newly returned
> > to the pool must not be touched under any circumstances by the user.
>
> Well alternatively we could update the comment if no user needs to rely on
> this. Since it was modeled after __alloc_pages_bulk()/release_pages() and
> release_pages() doesn't do this, we could also avoid it for perf reasons.
> Christoph, what do you think?
Not sure. I hate the magic null skipping behavior in
__alloc_pages_bulk()/release_pages(), but given that we try to stay
compatible it might be best to do this.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/mempool.c: set @elems slots to NULL once freed
2026-01-06 6:22 ` Christoph Hellwig
@ 2026-01-06 8:00 ` Vlastimil Babka
2026-01-06 8:09 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Vlastimil Babka @ 2026-01-06 8:00 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: klourencodev, linux-mm, akpm
On 1/6/26 07:22, Christoph Hellwig wrote:
> On Mon, Jan 05, 2026 at 05:46:47PM +0100, Vlastimil Babka wrote:
>> On 12/24/25 00:10, klourencodev@gmail.com wrote:
>> > From: Kevin Lourenco <klourencodev@gmail.com>
>> >
>> > This is documented in the function comment as "...and sets their
>> > slots in @elems to NULL.", but it was not followed.
>> >
>> > We need to follow the NULL assignment, because elements newly returned
>> > to the pool must not be touched under any circumstances by the user.
>>
>> Well alternatively we could update the comment if no user needs to rely on
>> this. Since it was modeled after __alloc_pages_bulk()/release_pages() and
>> release_pages() doesn't do this, we could also avoid it for perf reasons.
>> Christoph, what do you think?
>
> Not sure. I hate the magic null skipping behavior in
> __alloc_pages_bulk()/release_pages(), but given that we try to stay
> compatible it might be best to do this.
Well but release_pages() is null skipping but not NULL creating?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/mempool.c: set @elems slots to NULL once freed
2026-01-06 8:00 ` Vlastimil Babka
@ 2026-01-06 8:09 ` Christoph Hellwig
0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2026-01-06 8:09 UTC (permalink / raw)
To: Vlastimil Babka; +Cc: Christoph Hellwig, klourencodev, linux-mm, akpm
On Tue, Jan 06, 2026 at 09:00:21AM +0100, Vlastimil Babka wrote:
> > Not sure. I hate the magic null skipping behavior in
> > __alloc_pages_bulk()/release_pages(), but given that we try to stay
> > compatible it might be best to do this.
>
> Well but release_pages() is null skipping but not NULL creating?
True. Let me think about this a bit more.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-01-06 8:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-23 23:10 [PATCH] mm/mempool.c: set @elems slots to NULL once freed klourencodev
2025-12-23 23:47 ` Vlastimil Babka
2026-01-05 16:46 ` Vlastimil Babka
2026-01-06 6:22 ` Christoph Hellwig
2026-01-06 8:00 ` Vlastimil Babka
2026-01-06 8:09 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox