[PATCH] mm/z3fold:remove unneeded spinlock in z3fold_alloc

[PATCH] mm/z3fold:remove unneeded spinlock in z3fold_alloc

[Zhongkun He]

The spinklock in z3fold_alloc() is used to protect page->lru,
but now it was removed in commit 'e774a7bc7f0ad', so remove
the spinlock too.

Signed-off-by: Zhongkun He <hezhongkun.hzk@bytedance.com>
---
 mm/z3fold.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mm/z3fold.c b/mm/z3fold.c
index 7f608c0667f3..58946cacbfbb 100644
--- a/mm/z3fold.c
+++ b/mm/z3fold.c
@@ -1068,9 +1068,7 @@ static int z3fold_alloc(struct z3fold_pool *pool, size_t size, gfp_t gfp,
 	add_to_unbuddied(pool, zhdr);
 
 headless:
-	spin_lock(&pool->lock);
 	*handle = encode_handle(zhdr, bud);
-	spin_unlock(&pool->lock);
 	if (bud != HEADLESS)
 		z3fold_page_unlock(zhdr);
 
-- 
2.20.1

Re: [PATCH] mm/z3fold:remove unneeded spinlock in z3fold_alloc

[Johannes Weiner]

On Sun, Feb 04, 2024 at 09:15:43PM +0800, Zhongkun He wrote:
> The spinklock in z3fold_alloc() is used to protect page->lru,
> but now it was removed in commit 'e774a7bc7f0ad', so remove
> the spinlock too.

The pool->lock clearly protects things other than page->lru.

I'm not saying that you're wrong, but your changelog doesn't make a
strong case that you're right, either ;)

Please CC Vitaly (scripts/get_maintainer.pl) on these patches.

Re: [External] Re: [PATCH] mm/z3fold:remove unneeded spinlock in z3fold_alloc

[Zhongkun He]

On Thu, Feb 8, 2024 at 11:34 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> On Sun, Feb 04, 2024 at 09:15:43PM +0800, Zhongkun He wrote:
> > The spinklock in z3fold_alloc() is used to protect page->lru,
> > but now it was removed in commit 'e774a7bc7f0ad', so remove
> > the spinlock too.
>
> The pool->lock clearly protects things other than page->lru.

Yes, it protects pool unbuddied lists now.
I mean the following page-lru is removed in z3fold_alloc:
 headless:
        spin_lock(&pool->lock);
-       /* Add/move z3fold page to beginning of LRU */
-       if (!list_empty(&page->lru))
-               list_del(&page->lru);
-
-       list_add(&page->lru, &pool->lru);
-
        *handle = encode_handle(zhdr, bud);
        spin_unlock(&pool->lock);
        if (bud != HEADLESS)
            z3fold_page_unlock(zhdr);

For more detail please see the commit 'e774a7bc7f0ad'.

>
> I'm not saying that you're wrong, but your changelog doesn't make a
> strong case that you're right, either ;)
>

Hi Johannes, I am confused by the annotation :
The __encode_handle() annotation show:
* Pool lock should be held as this function accesses first_num.
but I did not find any use cases where first_num was protected by pool lock.
Furthermore, encode_handle() is protected by z3fold_page_lock() as I can
see.

> Please CC Vitaly (scripts/get_maintainer.pl) on these patches.

Got it, thanks.

Re: [PATCH] mm/z3fold:remove unneeded spinlock in z3fold_alloc

[Andrew Morton]

On Sun,  4 Feb 2024 21:15:43 +0800 Zhongkun He <hezhongkun.hzk@bytedance.com> wrote:

> The spinklock in z3fold_alloc() is used to protect page->lru,
> but now it was removed in commit 'e774a7bc7f0ad', so remove
> the spinlock too.
> 

I think I'll drop this patch and "mm/z3fold: remove unneeded spinlock"
pending some more clarity and agreement on whether these are
safe.  Thanks.

Re: [External] Re: [PATCH] mm/z3fold:remove unneeded spinlock in z3fold_alloc

[Zhongkun He]

>
> I think I'll drop this patch and "mm/z3fold: remove unneeded spinlock"
> pending some more clarity and agreement on whether these are
> safe.  Thanks.

Hi Andrew.
Per my understanding, these patches are safe and I hope they can be
reviewed by some experts.

Thanks.