[PATCH v2 0/2] mm/swapfile: fix and cleanup swap list iterations

[PATCH v2 0/2] mm/swapfile: fix and cleanup swap list iterations

[Youngjun Park]

This series fixes a potential list iteration issue in swap_sync_discard()
when devices are removed, and includes a cleanup for
__folio_throttle_swaprate().

v2 -> v1:
- Keep the original return value style. (Thank you Kairui)
- Clarified commit message based on discussion. (Thank you Baoquan He)
- Collect tags.
- Updated only patch [1/2], base commit unchanged. (minor fix only).

Link v1: https://lore.kernel.org/linux-mm/aSfkrpV7T7g5mhpI@yjaykim-PowerEdge-T330/T/#m101dc552904259e26d6595f91d9cbee37b6da7ac

Youngjun Park (2):
  mm/swapfile: fix list iteration when next node is removed during
    discard
  mm/swapfile: use plist_for_each_entry in __folio_throttle_swaprate

 mm/swapfile.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

-- 
base-commit: 624c4d7b8fac3fdf9894a5f89f84709771db4dbb
2.34.1

Best regards,
--
Youngjun Park

[PATCH v2 1/2] mm/swapfile: fix list iteration when next node is removed during discard

[Youngjun Park]

When the next node is removed from the plist (e.g. by swapoff),
plist_del() makes the node point to itself, causing the iteration to
loop on the same entry indefinitely.

Add a plist_node_empty() check to detect this case and restart
iteration, allowing swap_sync_discard() to continue processing
remaining swap devices that still have pending discard entries.

Additionally, switch from swap_avail_lock/swap_avail_head to
swap_lock/swap_active_head so that iteration is only affected by
swapoff operations rather than frequent availability changes,
reducing exceptional condition checks and lock contention.

Fixes: 686ea517f471 ("mm, swap: do not perform synchronous discard during allocation")
Suggested-by: Kairui Song <kasong@tencent.com>
Acked-by: Kairui Song <kasong@tencent.com>
Signed-off-by: Youngjun Park <youngjun.park@lge.com>
---
 mm/swapfile.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index d12332423a06..8116f36e440b 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1387,9 +1387,10 @@ static bool swap_sync_discard(void)
 	bool ret = false;
 	struct swap_info_struct *si, *next;
 
-	spin_lock(&swap_avail_lock);
-	plist_for_each_entry_safe(si, next, &swap_avail_head, avail_list) {
-		spin_unlock(&swap_avail_lock);
+	spin_lock(&swap_lock);
+start_over:
+	plist_for_each_entry_safe(si, next, &swap_active_head, list) {
+		spin_unlock(&swap_lock);
 		if (get_swap_device_info(si)) {
 			if (si->flags & SWP_PAGE_DISCARD)
 				ret = swap_do_scheduled_discard(si);
@@ -1397,9 +1398,12 @@ static bool swap_sync_discard(void)
 		}
 		if (ret)
 			return true;
-		spin_lock(&swap_avail_lock);
+
+		spin_lock(&swap_lock);
+		if (plist_node_empty(&next->list))
+			goto start_over;
 	}
-	spin_unlock(&swap_avail_lock);
+	spin_unlock(&swap_lock);
 
 	return false;
 }
-- 
2.34.1

Re: [PATCH v2 1/2] mm/swapfile: fix list iteration when next node is removed during discard

[Baoquan He]

On 11/27/25 at 07:03pm, Youngjun Park wrote:
> When the next node is removed from the plist (e.g. by swapoff),
> plist_del() makes the node point to itself, causing the iteration to
> loop on the same entry indefinitely.
> 
> Add a plist_node_empty() check to detect this case and restart
> iteration, allowing swap_sync_discard() to continue processing
> remaining swap devices that still have pending discard entries.
> 
> Additionally, switch from swap_avail_lock/swap_avail_head to
> swap_lock/swap_active_head so that iteration is only affected by
> swapoff operations rather than frequent availability changes,
> reducing exceptional condition checks and lock contention.
> 
> Fixes: 686ea517f471 ("mm, swap: do not perform synchronous discard during allocation")
> Suggested-by: Kairui Song <kasong@tencent.com>
> Acked-by: Kairui Song <kasong@tencent.com>
> Signed-off-by: Youngjun Park <youngjun.park@lge.com>
> ---
>  mm/swapfile.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)

Reviewed-by: Baoquan He <bhe@redhat.com>

> 
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index d12332423a06..8116f36e440b 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1387,9 +1387,10 @@ static bool swap_sync_discard(void)
>  	bool ret = false;
>  	struct swap_info_struct *si, *next;
>  
> -	spin_lock(&swap_avail_lock);
> -	plist_for_each_entry_safe(si, next, &swap_avail_head, avail_list) {
> -		spin_unlock(&swap_avail_lock);
> +	spin_lock(&swap_lock);
> +start_over:
> +	plist_for_each_entry_safe(si, next, &swap_active_head, list) {
> +		spin_unlock(&swap_lock);
>  		if (get_swap_device_info(si)) {
>  			if (si->flags & SWP_PAGE_DISCARD)
>  				ret = swap_do_scheduled_discard(si);
> @@ -1397,9 +1398,12 @@ static bool swap_sync_discard(void)
>  		}
>  		if (ret)
>  			return true;
> -		spin_lock(&swap_avail_lock);
> +
> +		spin_lock(&swap_lock);
> +		if (plist_node_empty(&next->list))
> +			goto start_over;
>  	}
> -	spin_unlock(&swap_avail_lock);
> +	spin_unlock(&swap_lock);
>  
>  	return false;
>  }
> -- 
> 2.34.1
> 

[PATCH v2 2/2] mm/swapfile: use plist_for_each_entry in __folio_throttle_swaprate

[Youngjun Park]

The loop breaks immediately after finding the first swap device and
never modifies the list. Replace plist_for_each_entry_safe() with
plist_for_each_entry() and remove the unused next variable.

Signed-off-by: Youngjun Park <youngjun.park@lge.com>
---
 mm/swapfile.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 8116f36e440b..46d2008e4b99 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -4023,7 +4023,7 @@ static bool __has_usable_swap(void)
 
 void __folio_throttle_swaprate(struct folio *folio, gfp_t gfp)
 {
-	struct swap_info_struct *si, *next;
+	struct swap_info_struct *si;
 
 	if (!(gfp & __GFP_IO))
 		return;
@@ -4042,8 +4042,7 @@ void __folio_throttle_swaprate(struct folio *folio, gfp_t gfp)
 		return;
 
 	spin_lock(&swap_avail_lock);
-	plist_for_each_entry_safe(si, next, &swap_avail_head,
-				  avail_list) {
+	plist_for_each_entry(si, &swap_avail_head, avail_list) {
 		if (si->bdev) {
 			blkcg_schedule_throttle(si->bdev->bd_disk, true);
 			break;
-- 
2.34.1

Re: [PATCH v2 2/2] mm/swapfile: use plist_for_each_entry in __folio_throttle_swaprate

[Baoquan He]

On 11/27/25 at 07:03pm, Youngjun Park wrote:
> The loop breaks immediately after finding the first swap device and
> never modifies the list. Replace plist_for_each_entry_safe() with
> plist_for_each_entry() and remove the unused next variable.

Yeah, and the lock is always held. So this looks good to me.

Reviewed-by: Baoquan He <bhe@redhat.com>

> 
> Signed-off-by: Youngjun Park <youngjun.park@lge.com>
> ---
>  mm/swapfile.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 8116f36e440b..46d2008e4b99 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -4023,7 +4023,7 @@ static bool __has_usable_swap(void)
>  
>  void __folio_throttle_swaprate(struct folio *folio, gfp_t gfp)
>  {
> -	struct swap_info_struct *si, *next;
> +	struct swap_info_struct *si;
>  
>  	if (!(gfp & __GFP_IO))
>  		return;
> @@ -4042,8 +4042,7 @@ void __folio_throttle_swaprate(struct folio *folio, gfp_t gfp)
>  		return;
>  
>  	spin_lock(&swap_avail_lock);
> -	plist_for_each_entry_safe(si, next, &swap_avail_head,
> -				  avail_list) {
> +	plist_for_each_entry(si, &swap_avail_head, avail_list) {
>  		if (si->bdev) {
>  			blkcg_schedule_throttle(si->bdev->bd_disk, true);
>  			break;
> -- 
> 2.34.1
> 

Re: [PATCH v2 2/2] mm/swapfile: use plist_for_each_entry in __folio_throttle_swaprate

[Kairui Song]

On Thu, Nov 27, 2025 at 6:06 PM Youngjun Park <youngjun.park@lge.com> wrote:
>
> The loop breaks immediately after finding the first swap device and
> never modifies the list. Replace plist_for_each_entry_safe() with
> plist_for_each_entry() and remove the unused next variable.
>
> Signed-off-by: Youngjun Park <youngjun.park@lge.com>
> ---
>  mm/swapfile.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 8116f36e440b..46d2008e4b99 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -4023,7 +4023,7 @@ static bool __has_usable_swap(void)
>
>  void __folio_throttle_swaprate(struct folio *folio, gfp_t gfp)
>  {
> -       struct swap_info_struct *si, *next;
> +       struct swap_info_struct *si;
>
>         if (!(gfp & __GFP_IO))
>                 return;
> @@ -4042,8 +4042,7 @@ void __folio_throttle_swaprate(struct folio *folio, gfp_t gfp)
>                 return;
>
>         spin_lock(&swap_avail_lock);
> -       plist_for_each_entry_safe(si, next, &swap_avail_head,
> -                                 avail_list) {
> +       plist_for_each_entry(si, &swap_avail_head, avail_list) {
>                 if (si->bdev) {
>                         blkcg_schedule_throttle(si->bdev->bd_disk, true);
>                         break;
> --
> 2.34.1
>

Thanks!

Acked-by: Kairui Song <kasong@tencent.com>

Re: [PATCH v2 2/2] mm/swapfile: use plist_for_each_entry in __folio_throttle_swaprate

[Chris Li]

Hi Youngjun,

Thanks for the patch!

Acked-by: Chris Li <chrisl@kernel.org>

Chris

On Thu, Nov 27, 2025 at 2:03 PM Youngjun Park <youngjun.park@lge.com> wrote:
>
> The loop breaks immediately after finding the first swap device and
> never modifies the list. Replace plist_for_each_entry_safe() with
> plist_for_each_entry() and remove the unused next variable.
>
> Signed-off-by: Youngjun Park <youngjun.park@lge.com>
> ---
>  mm/swapfile.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 8116f36e440b..46d2008e4b99 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -4023,7 +4023,7 @@ static bool __has_usable_swap(void)
>
>  void __folio_throttle_swaprate(struct folio *folio, gfp_t gfp)
>  {
> -       struct swap_info_struct *si, *next;
> +       struct swap_info_struct *si;
>
>         if (!(gfp & __GFP_IO))
>                 return;
> @@ -4042,8 +4042,7 @@ void __folio_throttle_swaprate(struct folio *folio, gfp_t gfp)
>                 return;
>
>         spin_lock(&swap_avail_lock);
> -       plist_for_each_entry_safe(si, next, &swap_avail_head,
> -                                 avail_list) {
> +       plist_for_each_entry(si, &swap_avail_head, avail_list) {
>                 if (si->bdev) {
>                         blkcg_schedule_throttle(si->bdev->bd_disk, true);
>                         break;
> --
> 2.34.1
>