Received-SPF: pass (google.com: domain of linux-kernel+bounces-167033-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240409082631.187483-1-21cnbao@gmail.com> <20240409082631.187483-2-21cnbao@gmail.com>
 <87y19f2lq3.fsf@yhuang6-desk2.ccr.corp.intel.com> <CAGsJ_4yPQyQpWvB2KmuOh2vyUQA0A6hNHsypEC9Gptp9XtuDgg@mail.gmail.com>
 <87jzkz2g3t.fsf@yhuang6-desk2.ccr.corp.intel.com> <CAGsJ_4wm4SfkyTBHpU46EPTFvhq8e54F3KRkKj6gTBcnOjCw1g@mail.gmail.com>
 <87bk6b2elo.fsf@yhuang6-desk2.ccr.corp.intel.com> <CAGsJ_4zH4RHhZembtXOqzBDL75MA5NiEjMHx7eCN-a1ifnKTBw@mail.gmail.com>
 <877cgy2ifu.fsf@yhuang6-desk2.ccr.corp.intel.com> <CAGsJ_4yim-PQW0JBjZD8dpGBG3FQm=xZb1V51+Nr37nJEwEPzA@mail.gmail.com>
 <87ttk20zns.fsf@yhuang6-desk2.ccr.corp.intel.com> <CAGsJ_4xigDba15o9K84z9nR_ZW1nXsUWa0LRNmUAfDdxcqsddw@mail.gmail.com>
 <87jzkw25hl.fsf@yhuang6-desk2.ccr.corp.intel.com> <CAGsJ_4wTsJe2yK3k+Tif6obgL9LUUuqXEBxhqMSZSFZpbvrzmA@mail.gmail.com>
 <CAGsJ_4ymv-tmpmH0s1D5+GF13UOPv5UdRFrLOxVE5X+xNUHveg@mail.gmail.com>
 <87le5bxd6n.fsf@yhuang6-desk2.ccr.corp.intel.com> <CAGsJ_4wtpkGrfAP23+LgUuUi0BDqS3jjdGu6Y2JDbwTskgA3yQ@mail.gmail.com>
In-Reply-To: <CAGsJ_4wtpkGrfAP23+LgUuUi0BDqS3jjdGu6Y2JDbwTskgA3yQ@mail.gmail.com>
From: Barry Song <21cnbao@gmail.com>
Date: Fri, 3 May 2024 11:05:13 +1200
Message-ID: <CAGsJ_4yyvmAwCjXN4Y2m9+i2WuXF2fVv6G_4HB3uuER+2wkfJg@mail.gmail.com>
Subject: Re: [PATCH v2 1/5] mm: swap: introduce swap_free_nr() for batched swap_free()
To: "Huang, Ying" <ying.huang@intel.com>
Cc: akpm@linux-foundation.org, linux-mm@kvack.org, 
	baolin.wang@linux.alibaba.com, chrisl@kernel.org, david@redhat.com, 
	hanchuanhua@oppo.com, hannes@cmpxchg.org, hughd@google.com, 
	kasong@tencent.com, ryan.roberts@arm.com, surenb@google.com, 
	v-songbaohua@oppo.com, willy@infradead.org, xiang@kernel.org, 
	yosryahmed@google.com, yuzhao@google.com, ziy@nvidia.com, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"

[snip]

> >
> > My idea is something as below.  It's only build tested.
> >
> > static void cluster_swap_free_nr(struct swap_info_struct *sis,
> >                                  unsigned long offset, int nr_pages)
> > {
> >         struct swap_cluster_info *ci;
> >         DECLARE_BITMAP(to_free, BITS_PER_LONG) = { 0 };
> >         int i, nr;
> >
> >         ci = lock_cluster_or_swap_info(sis, offset);
> >         while (nr_pages) {
> >                 nr = min(BITS_PER_LONG, nr_pages);
> >                 for (i = 0; i < nr; i++) {
> >                         if (!__swap_entry_free_locked(sis, offset + i, 1))
> >                                 bitmap_set(to_free, i, 1);
> >                 }
> >                 if (!bitmap_empty(to_free, BITS_PER_LONG)) {
> >                         unlock_cluster_or_swap_info(sis, ci);
> >                         for_each_set_bit(i, to_free, BITS_PER_LONG)
> >                                 free_swap_slot(swp_entry(sis->type, offset + i));
> >                         if (nr == nr_pages)
> >                                 return;
> >                         bitmap_clear(to_free, 0, BITS_PER_LONG);
> >                         ci = lock_cluster_or_swap_info(sis, offset);
> >                 }
> >                 offset += nr;
> >                 nr_pages -= nr;
> >         }
> >         unlock_cluster_or_swap_info(sis, ci);
> > }
> >
> > void swap_free_nr(swp_entry_t entry, int nr_pages)
> > {
> >         int nr;
> >         struct swap_info_struct *sis;
> >         unsigned long offset = swp_offset(entry);
> >
> >         sis = _swap_info_get(entry);
> >         if (!sis)
> >                 return;
> >
> >         while (nr_pages >= 0) {

this should be "while (nr_pages) " exactly like
"cluster_swap_free_nr", otherwise, we get

[  383.652632] EXT4-fs (vda): error count since last fsck: 85
[  383.653453] EXT4-fs (vda): initial error at time 1709536044:
mb_free_blocks:1937: block 704527
[  383.654947] EXT4-fs (vda): last error at time 1714689579:
ext4_mb_generate_buddy:1213
[  398.564002] watchdog: BUG: soft lockup - CPU#1 stuck for 26s! [a.out:104]
[  398.564679] Modules linked in:
[  398.565239] irq event stamp: 0
[  398.565648] hardirqs last  enabled at (0): [<0000000000000000>] 0x0
[  398.566888] hardirqs last disabled at (0): [<ffff8000800ade2c>]
copy_process+0x654/0x19a8
[  398.568255] softirqs last  enabled at (0): [<ffff8000800ade2c>]
copy_process+0x654/0x19a8
[  398.568813] softirqs last disabled at (0): [<0000000000000000>] 0x0
[  398.569481] CPU: 1 PID: 104 Comm: a.out Not tainted
6.9.0-rc4-g3c9251435c61-dirty #216
[  398.570076] Hardware name: linux,dummy-virt (DT)
[  398.570600] pstate: 01401005 (nzcv daif +PAN -UAO -TCO +DIT +SSBS BTYPE=--)
[  398.571148] pc : lock_acquire+0x3c/0x88
[  398.571636] lr : lock_acquire+0x3c/0x88
[  398.572075] sp : ffff800086f13af0
[  398.572484] x29: ffff800086f13af0 x28: 0000000000000000 x27: ffff0000c1096800
[  398.573349] x26: 0000000000000001 x25: ffff8000803e4d60 x24: 0000000000000000
[  398.574139] x23: 0000000000000001 x22: 0000000000000000 x21: 0000000000000000
[  398.574915] x20: 0000000000000000 x19: ffff0000c3eb0918 x18: 0000000000000000
[  398.576009] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
[  398.576895] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
[  398.577732] x11: 0000000000000000 x10: 0000000000000000 x9 : ffff8000814cf7a0
[  398.578668] x8 : ffff800086f13ab8 x7 : 0000000000000000 x6 : ffff8000803e4d60
[  398.579705] x5 : 0000000000000000 x4 : 0000000000000001 x3 : ffff800082f39008
[  398.580528] x2 : 0000000000000003 x1 : 0000000000000002 x0 : 0000000000000001
[  398.581387] Call trace:
[  398.581740]  lock_acquire+0x3c/0x88
[  398.582159]  _raw_spin_lock+0x50/0x70
[  398.582556]  swap_free_nr+0x98/0x2a0
[  398.582946]  do_swap_page+0x568/0xd00
[  398.583337]  __handle_mm_fault+0x76c/0x16d0
[  398.583853]  handle_mm_fault+0x7c/0x3c8
[  398.584276]  do_page_fault+0x188/0x698
[  398.584731]  do_translation_fault+0xb4/0xd8
[  398.585142]  do_mem_abort+0x4c/0xa8
[  398.585541]  el0_da+0x58/0x128
[  398.585923]  el0t_64_sync_handler+0xe4/0x158
[  398.586355]  el0t_64_sync+0x1a4/0x1a8
[  398.651930] watchdog: BUG: soft lockup - CPU#2 stuck for 26s! [a.out:103]
[  398.652819] Modules linked in:
[  398.653682] irq event stamp: 0
[  398.654213] hardirqs last  enabled at (0): [<0000000000000000>] 0x0
[  398.654714] hardirqs last disabled at (0): [<ffff8000800ade2c>]
copy_process+0x654/0x19a8
[  398.655242] softirqs last  enabled at (0): [<ffff8000800ade2c>]
copy_process+0x654/0x19a8
[  398.655835] softirqs last disabled at (0): [<0000000000000000>] 0x0
[  398.656657] CPU: 2 PID: 103 Comm: a.out Tainted: G             L
 6.9.0-rc4-g3c9251435c61-dirty #216
[  398.657705] Hardware name: linux,dummy-virt (DT)
[  398.658273] pstate: 01401005 (nzcv daif +PAN -UAO -TCO +DIT +SSBS BTYPE=--)
[  398.658743] pc : queued_spin_lock_slowpath+0x5c/0x528
[  398.659146] lr : do_raw_spin_lock+0xc8/0x120
[  398.659575] sp : ffff800086d03a60
[  398.659946] x29: ffff800086d03a60 x28: 0000000000000120 x27: 0800000103050003
[  398.661238] x26: ffff0000c27d5d40 x25: fffffdffc0000000 x24: ffff800082270d08
[  398.662431] x23: ffff800083f73a31 x22: ffff800086d03d48 x21: 0000ffff9bf70000
[  398.663358] x20: ffff800082f39008 x19: ffff0000c27d5d40 x18: 0000000000000000
[  398.664353] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
[  398.665825] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
[  398.666715] x11: 0000000000000000 x10: 0000000000000000 x9 : ffff8000801420f8
[  398.667528] x8 : ffff800086d03a48 x7 : 0000000000000000 x6 : ffff8000803abef0
[  398.668548] x5 : 0000000000000000 x4 : 0000000000000001 x3 : ffff800082f39008
[  398.670018] x2 : ffff80012ac81000 x1 : 0000000000000000 x0 : 0000000000000001
[  398.670831] Call trace:
[  398.671136]  queued_spin_lock_slowpath+0x5c/0x528
[  398.671604]  do_raw_spin_lock+0xc8/0x120
[  398.672144]  _raw_spin_lock+0x58/0x70
[  398.672860]  __pte_offset_map_lock+0x98/0x210
[  398.673695]  filemap_map_pages+0x10c/0x7f8
[  398.674495]  __handle_mm_fault+0x11e4/0x16d0
[  398.674926]  handle_mm_fault+0x7c/0x3c8
[  398.675336]  do_page_fault+0x100/0x698
[  398.675863]  do_translation_fault+0xb4/0xd8
[  398.676425]  do_mem_abort+0x4c/0xa8
[  398.677225]  el0_ia+0x80/0x188
[  398.678004]  el0t_64_sync_handler+0x100/0x158
[  398.678778]  el0t_64_sync+0x1a4/0x1a8
[  422.563920] watchdog: BUG: soft lockup - CPU#1 stuck for 48s! [a.out:104]
[  422.564528] Modules linked in:
[  422.564991] irq event stamp: 0


> >                 nr = min_t(int, nr_pages, SWAPFILE_CLUSTER - offset % SWAPFILE_CLUSTER);
> >                 cluster_swap_free_nr(sis, offset, nr);
> >                 offset += nr;
> >                 nr_pages -= nr;
> >         }
> > }
> > --
> > Best Regards,
> > Huang, Ying
>

Thanks
Barry