Re: [PATCH v2] mm/swap: fix swap_info_struct race between swapoff and get_swap_pages()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andrew

On 4/5/23 3:26 AM, Andrew Morton wrote:
On Tue,  4 Apr 2023 23:47:16 +0800 Rongwei Wang <rongwei.wang@xxxxxxxxxxxxxxxxx> wrote:

The si->lock must be held when deleting the si from
the available list.

...

--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -679,6 +679,7 @@ static void __del_from_avail_list(struct swap_info_struct *p)
  {
  	int nid;
+ assert_spin_locked(&p->lock);
  	for_each_node(nid)
  		plist_del(&p->avail_lists[nid], &swap_avail_heads[nid]);
  }
@@ -2434,8 +2435,8 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
  		spin_unlock(&swap_lock);
  		goto out_dput;
  	}
-	del_from_avail_list(p);
  	spin_lock(&p->lock);
+	del_from_avail_list(p);
  	if (p->prio < 0) {
  		struct swap_info_struct *si = p;
  		int nid;
So we have

swap_avail_lock
swap_info_struct.lock
swap_cluster_info.lock

Is the ranking of these three clearly documented somewhere?

It seems have

swap_lock

swap_info_struct.lock

swap_avail_lock

I just summary the ranking of these three locks by reading code, not find any documents (maybe have).



Did you test this with lockdep fully enabled?


I'm thinking that Aaron's a2468cc9bfdff ("swap: choose swap device
according to numa node") is the appropriate Fixes: target - do you
agree?

Yes, I'm sure my latest test version has included Aaron's a2468cc9bfdff, and my test .config has enabled CONFIG

as below:

CONFIG_LOCK_DEBUGGING_SUPPORT=y CONFIG_PROVE_LOCKING=y CONFIG_DEBUG_SPINLOCK=y CONFIG_DEBUG_LOCK_ALLOC=y CONFIG_LOCKDEP=y CONFIG_DEBUG_LOCKDEP=y CONFIG_DEBUG_ATOMIC_SLEEP=y



These functions use identifier `p' for the swap_info_struct*, whereas
most other code uses the much more sensible `si'.  That's just rude.
But we shouldn't change that within this fix.

Indeed, It's confusing more or less to use both 'si' and 'p'. I can ready for another patch to replace 'p' with 'si'.

Thanks.





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux