[PATCH 00/15] A few cleanup patches for swap

[PATCH 00/15] A few cleanup patches for swap

[Miaohe Lin]

Hi everyone,
This series contains a few patches to fix the comment, remove unneeded
return value, use some helpers and so on. More details can be found in
the respective changelogs. Thanks!

Miaohe Lin (15):
  mm/swap: use helper is_swap_pte() in swap_vma_readahead
  mm/swap: use helper macro __ATTR_RW
  mm/swap: fold __swap_info_get() into its sole caller
  mm/swap: remove unneeded return value of free_swap_slot
  mm/swap: print bad swap offset entry in get_swap_device
  mm/swap: remove buggy cache->nr check in refill_swap_slots_cache
  mm/swap: remove unneeded p != NULL check in __swap_duplicate
  mm/swap: make page_swapcount and __lru_add_drain_all
  mm/swap: avoid calling swp_swap_info when try to check
    SWP_STABLE_WRITES
  mm/swap: break the loop if matching device is found
  mm/swap: add helper swap_offset_available()
  mm/swap: fix the obsolete comment for SWP_TYPE_SHIFT
  mm/swap: clean up the comment of find_next_to_unuse
  mm/swap: fix the comment of get_kernel_pages
  mm/swap: fix comment about swap extent

 include/linux/swap.h       | 11 +----
 include/linux/swap_slots.h |  2 +-
 include/linux/swapops.h    |  4 +-
 mm/memory.c                |  2 +-
 mm/swap.c                  |  6 +--
 mm/swap_slots.c            |  6 +--
 mm/swap_state.c            |  8 +---
 mm/swapfile.c              | 86 ++++++++++++++++----------------------
 8 files changed, 50 insertions(+), 75 deletions(-)

-- 
2.23.0

[PATCH 01/15] mm/swap: use helper is_swap_pte() in swap_vma_readahead

[Miaohe Lin]

Use helper is_swap_pte() to check whether pte is swap entry to make code
more clear. Minor readability improvement.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/swap_state.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/mm/swap_state.c b/mm/swap_state.c
index 577c2848ae49..240b39ed5922 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -818,9 +818,7 @@ static struct page *swap_vma_readahead(swp_entry_t fentry, gfp_t gfp_mask,
 	for (i = 0, pte = ra_info.ptes; i < ra_info.nr_pte;
 	     i++, pte++) {
 		pentry = *pte;
-		if (pte_none(pentry))
-			continue;
-		if (pte_present(pentry))
+		if (!is_swap_pte(pentry))
 			continue;
 		entry = pte_to_swp_entry(pentry);
 		if (unlikely(non_swap_entry(entry)))
-- 
2.23.0

Re: [PATCH 01/15] mm/swap: use helper is_swap_pte() in swap_vma_readahead

[David Hildenbrand]

On 09.05.22 15:14, Miaohe Lin wrote:
> Use helper is_swap_pte() to check whether pte is swap entry to make code
> more clear. Minor readability improvement.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: David Hildenbrand <david@redhat.com>


-- 
Thanks,

David / dhildenb

Re: [PATCH 01/15] mm/swap: use helper is_swap_pte() in swap_vma_readahead

[Oscar Salvador]

On Mon, May 09, 2022 at 09:14:02PM +0800, Miaohe Lin wrote:
> Use helper is_swap_pte() to check whether pte is swap entry to make code
> more clear. Minor readability improvement.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/swap_state.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 577c2848ae49..240b39ed5922 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -818,9 +818,7 @@ static struct page *swap_vma_readahead(swp_entry_t fentry, gfp_t gfp_mask,
>  	for (i = 0, pte = ra_info.ptes; i < ra_info.nr_pte;
>  	     i++, pte++) {
>  		pentry = *pte;
> -		if (pte_none(pentry))
> -			continue;
> -		if (pte_present(pentry))
> +		if (!is_swap_pte(pentry))
>  			continue;
>  		entry = pte_to_swp_entry(pentry);
>  		if (unlikely(non_swap_entry(entry)))
> -- 
> 2.23.0
> 
> 

-- 
Oscar Salvador
SUSE Labs

[PATCH 02/15] mm/swap: use helper macro __ATTR_RW

[Miaohe Lin]

Use helper macro __ATTR_RW to define vma_ra_enabled_attr to make code more
clear. Minor readability improvement.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/swap_state.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/mm/swap_state.c b/mm/swap_state.c
index 240b39ed5922..9f99d8137ffd 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -881,9 +881,7 @@ static ssize_t vma_ra_enabled_store(struct kobject *kobj,
 
 	return count;
 }
-static struct kobj_attribute vma_ra_enabled_attr =
-	__ATTR(vma_ra_enabled, 0644, vma_ra_enabled_show,
-	       vma_ra_enabled_store);
+static struct kobj_attribute vma_ra_enabled_attr = __ATTR_RW(vma_ra_enabled);
 
 static struct attribute *swap_attrs[] = {
 	&vma_ra_enabled_attr.attr,
-- 
2.23.0

Re: [PATCH 02/15] mm/swap: use helper macro __ATTR_RW

[David Hildenbrand]

On 09.05.22 15:14, Miaohe Lin wrote:
> Use helper macro __ATTR_RW to define vma_ra_enabled_attr to make code more
> clear. Minor readability improvement.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH 02/15] mm/swap: use helper macro __ATTR_RW

[Oscar Salvador]

On Mon, May 09, 2022 at 09:14:03PM +0800, Miaohe Lin wrote:
> Use helper macro __ATTR_RW to define vma_ra_enabled_attr to make code more
> clear. Minor readability improvement.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/swap_state.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 240b39ed5922..9f99d8137ffd 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -881,9 +881,7 @@ static ssize_t vma_ra_enabled_store(struct kobject *kobj,
>  
>  	return count;
>  }
> -static struct kobj_attribute vma_ra_enabled_attr =
> -	__ATTR(vma_ra_enabled, 0644, vma_ra_enabled_show,
> -	       vma_ra_enabled_store);
> +static struct kobj_attribute vma_ra_enabled_attr = __ATTR_RW(vma_ra_enabled);
>  
>  static struct attribute *swap_attrs[] = {
>  	&vma_ra_enabled_attr.attr,
> -- 
> 2.23.0
> 
> 

-- 
Oscar Salvador
SUSE Labs

[PATCH 03/15] mm/swap: fold __swap_info_get() into its sole caller

[Miaohe Lin]

Fold __swap_info_get() into its sole caller to make code more clear.
Minor readability improvement.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/swapfile.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 05ca79e68d63..0aee6286d6a7 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1123,7 +1123,7 @@ int get_swap_pages(int n_goal, swp_entry_t swp_entries[], int entry_size)
 	return n_ret;
 }
 
-static struct swap_info_struct *__swap_info_get(swp_entry_t entry)
+static struct swap_info_struct *_swap_info_get(swp_entry_t entry)
 {
 	struct swap_info_struct *p;
 	unsigned long offset;
@@ -1138,8 +1138,13 @@ static struct swap_info_struct *__swap_info_get(swp_entry_t entry)
 	offset = swp_offset(entry);
 	if (offset >= p->max)
 		goto bad_offset;
+	if (data_race(!p->swap_map[swp_offset(entry)]))
+		goto bad_free;
 	return p;
 
+bad_free:
+	pr_err("%s: %s%08lx\n", __func__, Unused_offset, entry.val);
+	goto out;
 bad_offset:
 	pr_err("%s: %s%08lx\n", __func__, Bad_offset, entry.val);
 	goto out;
@@ -1152,23 +1157,6 @@ static struct swap_info_struct *__swap_info_get(swp_entry_t entry)
 	return NULL;
 }
 
-static struct swap_info_struct *_swap_info_get(swp_entry_t entry)
-{
-	struct swap_info_struct *p;
-
-	p = __swap_info_get(entry);
-	if (!p)
-		goto out;
-	if (data_race(!p->swap_map[swp_offset(entry)]))
-		goto bad_free;
-	return p;
-
-bad_free:
-	pr_err("%s: %s%08lx\n", __func__, Unused_offset, entry.val);
-out:
-	return NULL;
-}
-
 static struct swap_info_struct *swap_info_get_cont(swp_entry_t entry,
 					struct swap_info_struct *q)
 {
-- 
2.23.0

Re: [PATCH 03/15] mm/swap: fold __swap_info_get() into its sole caller

[David Hildenbrand]

On 09.05.22 15:14, Miaohe Lin wrote:
> Fold __swap_info_get() into its sole caller to make code more clear.
> Minor readability improvement.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: David Hildenbrand <david@redhat.com>


-- 
Thanks,

David / dhildenb

Re: [PATCH 03/15] mm/swap: fold __swap_info_get() into its sole caller

[Oscar Salvador]

On Mon, May 09, 2022 at 09:14:04PM +0800, Miaohe Lin wrote:
> Fold __swap_info_get() into its sole caller to make code more clear.
> Minor readability improvement.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/swapfile.c | 24 ++++++------------------
>  1 file changed, 6 insertions(+), 18 deletions(-)
> 
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 05ca79e68d63..0aee6286d6a7 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1123,7 +1123,7 @@ int get_swap_pages(int n_goal, swp_entry_t swp_entries[], int entry_size)
>  	return n_ret;
>  }
>  
> -static struct swap_info_struct *__swap_info_get(swp_entry_t entry)
> +static struct swap_info_struct *_swap_info_get(swp_entry_t entry)
>  {
>  	struct swap_info_struct *p;
>  	unsigned long offset;
> @@ -1138,8 +1138,13 @@ static struct swap_info_struct *__swap_info_get(swp_entry_t entry)
>  	offset = swp_offset(entry);
>  	if (offset >= p->max)
>  		goto bad_offset;
> +	if (data_race(!p->swap_map[swp_offset(entry)]))
> +		goto bad_free;
>  	return p;
>  
> +bad_free:
> +	pr_err("%s: %s%08lx\n", __func__, Unused_offset, entry.val);
> +	goto out;
>  bad_offset:
>  	pr_err("%s: %s%08lx\n", __func__, Bad_offset, entry.val);
>  	goto out;
> @@ -1152,23 +1157,6 @@ static struct swap_info_struct *__swap_info_get(swp_entry_t entry)
>  	return NULL;
>  }
>  
> -static struct swap_info_struct *_swap_info_get(swp_entry_t entry)
> -{
> -	struct swap_info_struct *p;
> -
> -	p = __swap_info_get(entry);
> -	if (!p)
> -		goto out;
> -	if (data_race(!p->swap_map[swp_offset(entry)]))
> -		goto bad_free;
> -	return p;
> -
> -bad_free:
> -	pr_err("%s: %s%08lx\n", __func__, Unused_offset, entry.val);
> -out:
> -	return NULL;
> -}
> -
>  static struct swap_info_struct *swap_info_get_cont(swp_entry_t entry,
>  					struct swap_info_struct *q)
>  {
> -- 
> 2.23.0
> 
> 

-- 
Oscar Salvador
SUSE Labs

[PATCH 04/15] mm/swap: remove unneeded return value of free_swap_slot

[Miaohe Lin]

The return value of free_swap_slot is always 0 and also ignored now.
Remove it to clean up the code.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 include/linux/swap_slots.h | 2 +-
 mm/swap_slots.c            | 4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/linux/swap_slots.h b/include/linux/swap_slots.h
index 347f1a304190..15adfb8c813a 100644
--- a/include/linux/swap_slots.h
+++ b/include/linux/swap_slots.h
@@ -24,7 +24,7 @@ struct swap_slots_cache {
 void disable_swap_slots_cache_lock(void);
 void reenable_swap_slots_cache_unlock(void);
 void enable_swap_slots_cache(void);
-int free_swap_slot(swp_entry_t entry);
+void free_swap_slot(swp_entry_t entry);
 
 extern bool swap_slot_cache_enabled;
 
diff --git a/mm/swap_slots.c b/mm/swap_slots.c
index 0218ec1cd24c..2f877e6f87d7 100644
--- a/mm/swap_slots.c
+++ b/mm/swap_slots.c
@@ -269,7 +269,7 @@ static int refill_swap_slots_cache(struct swap_slots_cache *cache)
 	return cache->nr;
 }
 
-int free_swap_slot(swp_entry_t entry)
+void free_swap_slot(swp_entry_t entry)
 {
 	struct swap_slots_cache *cache;
 
@@ -297,8 +297,6 @@ int free_swap_slot(swp_entry_t entry)
 direct_free:
 		swapcache_free_entries(&entry, 1);
 	}
-
-	return 0;
 }
 
 swp_entry_t folio_alloc_swap(struct folio *folio)
-- 
2.23.0

Re: [PATCH 04/15] mm/swap: remove unneeded return value of free_swap_slot

[David Hildenbrand]

On 09.05.22 15:14, Miaohe Lin wrote:
> The return value of free_swap_slot is always 0 and also ignored now.
> Remove it to clean up the code.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: David Hildenbrand <david@redhat.com>


-- 
Thanks,

David / dhildenb

Re: [PATCH 04/15] mm/swap: remove unneeded return value of free_swap_slot

[Oscar Salvador]

On Mon, May 09, 2022 at 09:14:05PM +0800, Miaohe Lin wrote:
> The return value of free_swap_slot is always 0 and also ignored now.
> Remove it to clean up the code.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  include/linux/swap_slots.h | 2 +-
>  mm/swap_slots.c            | 4 +---
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/swap_slots.h b/include/linux/swap_slots.h
> index 347f1a304190..15adfb8c813a 100644
> --- a/include/linux/swap_slots.h
> +++ b/include/linux/swap_slots.h
> @@ -24,7 +24,7 @@ struct swap_slots_cache {
>  void disable_swap_slots_cache_lock(void);
>  void reenable_swap_slots_cache_unlock(void);
>  void enable_swap_slots_cache(void);
> -int free_swap_slot(swp_entry_t entry);
> +void free_swap_slot(swp_entry_t entry);
>  
>  extern bool swap_slot_cache_enabled;
>  
> diff --git a/mm/swap_slots.c b/mm/swap_slots.c
> index 0218ec1cd24c..2f877e6f87d7 100644
> --- a/mm/swap_slots.c
> +++ b/mm/swap_slots.c
> @@ -269,7 +269,7 @@ static int refill_swap_slots_cache(struct swap_slots_cache *cache)
>  	return cache->nr;
>  }
>  
> -int free_swap_slot(swp_entry_t entry)
> +void free_swap_slot(swp_entry_t entry)
>  {
>  	struct swap_slots_cache *cache;
>  
> @@ -297,8 +297,6 @@ int free_swap_slot(swp_entry_t entry)
>  direct_free:
>  		swapcache_free_entries(&entry, 1);
>  	}
> -
> -	return 0;
>  }
>  
>  swp_entry_t folio_alloc_swap(struct folio *folio)
> -- 
> 2.23.0
> 
> 

-- 
Oscar Salvador
SUSE Labs

[PATCH 05/15] mm/swap: print bad swap offset entry in get_swap_device

[Miaohe Lin]

If offset exceeds the si->max, print bad swap offset entry to help debug
the unexpected case.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/swapfile.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 0aee6286d6a7..d4b81ca887c0 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1272,6 +1272,7 @@ struct swap_info_struct *get_swap_device(swp_entry_t entry)
 out:
 	return NULL;
 put_out:
+	pr_err("%s: %s%08lx\n", __func__, Bad_offset, entry.val);
 	percpu_ref_put(&si->users);
 	return NULL;
 }
-- 
2.23.0

Re: [PATCH 05/15] mm/swap: print bad swap offset entry in get_swap_device

[David Hildenbrand]

On 09.05.22 15:14, Miaohe Lin wrote:
> If offset exceeds the si->max, print bad swap offset entry to help debug
> the unexpected case.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: David Hildenbrand <david@redhat.com>


-- 
Thanks,

David / dhildenb

Re: [PATCH 05/15] mm/swap: print bad swap offset entry in get_swap_device

[Oscar Salvador]

On Mon, May 09, 2022 at 09:14:06PM +0800, Miaohe Lin wrote:
> If offset exceeds the si->max, print bad swap offset entry to help debug
> the unexpected case.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/swapfile.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 0aee6286d6a7..d4b81ca887c0 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1272,6 +1272,7 @@ struct swap_info_struct *get_swap_device(swp_entry_t entry)
>  out:
>  	return NULL;
>  put_out:
> +	pr_err("%s: %s%08lx\n", __func__, Bad_offset, entry.val);
>  	percpu_ref_put(&si->users);
>  	return NULL;
>  }
> -- 
> 2.23.0
> 
> 

-- 
Oscar Salvador
SUSE Labs

[PATCH 06/15] mm/swap: remove buggy cache->nr check in refill_swap_slots_cache

[Miaohe Lin]

refill_swap_slots_cache is always called when cache->nr is 0. And if
cache->nr != 0, we should return cache->nr instead of 0. So remove
such buggy and confusing check.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/swap_slots.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/swap_slots.c b/mm/swap_slots.c
index 2f877e6f87d7..2a65a89b5b4d 100644
--- a/mm/swap_slots.c
+++ b/mm/swap_slots.c
@@ -258,7 +258,7 @@ void enable_swap_slots_cache(void)
 /* called with swap slot cache's alloc lock held */
 static int refill_swap_slots_cache(struct swap_slots_cache *cache)
 {
-	if (!use_swap_slot_cache || cache->nr)
+	if (!use_swap_slot_cache)
 		return 0;
 
 	cache->cur = 0;
-- 
2.23.0

Re: [PATCH 06/15] mm/swap: remove buggy cache->nr check in refill_swap_slots_cache

[David Hildenbrand]

On 09.05.22 15:14, Miaohe Lin wrote:
> refill_swap_slots_cache is always called when cache->nr is 0. And if
> cache->nr != 0, we should return cache->nr instead of 0. So remove
> such buggy and confusing check.

Not sure about the "cache->nr != 0, we should return cache->nr instead
of 0" part, I'd just drop that from the patch description. We'd actually
end up overwriting cache->nr after your change, which doesn't sound
right and also different to what you describe here.

> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  mm/swap_slots.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/swap_slots.c b/mm/swap_slots.c
> index 2f877e6f87d7..2a65a89b5b4d 100644
> --- a/mm/swap_slots.c
> +++ b/mm/swap_slots.c
> @@ -258,7 +258,7 @@ void enable_swap_slots_cache(void)
>  /* called with swap slot cache's alloc lock held */
>  static int refill_swap_slots_cache(struct swap_slots_cache *cache)
>  {
> -	if (!use_swap_slot_cache || cache->nr)
> +	if (!use_swap_slot_cache)
>  		return 0;
>  
>  	cache->cur = 0;

I feel like if this function would be called with cache->nr, it would be
a BUG. So I'm fine with removing it, but we could also think about
turning it into some sort of WARN/BG to make it clearer that this is
unexpected.


Anyhow,

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH 06/15] mm/swap: remove buggy cache->nr check in refill_swap_slots_cache

[Miaohe Lin]

On 2022/5/12 21:37, David Hildenbrand wrote:
> On 09.05.22 15:14, Miaohe Lin wrote:
>> refill_swap_slots_cache is always called when cache->nr is 0. And if
>> cache->nr != 0, we should return cache->nr instead of 0. So remove
>> such buggy and confusing check.
> 
> Not sure about the "cache->nr != 0, we should return cache->nr instead
> of 0" part, I'd just drop that from the patch description. We'd actually
> end up overwriting cache->nr after your change, which doesn't sound
> right and also different to what you describe here.

Will do.

> 
>>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
>>  mm/swap_slots.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/swap_slots.c b/mm/swap_slots.c
>> index 2f877e6f87d7..2a65a89b5b4d 100644
>> --- a/mm/swap_slots.c
>> +++ b/mm/swap_slots.c
>> @@ -258,7 +258,7 @@ void enable_swap_slots_cache(void)
>>  /* called with swap slot cache's alloc lock held */
>>  static int refill_swap_slots_cache(struct swap_slots_cache *cache)
>>  {
>> -	if (!use_swap_slot_cache || cache->nr)
>> +	if (!use_swap_slot_cache)
>>  		return 0;
>>  
>>  	cache->cur = 0;
> 
> I feel like if this function would be called with cache->nr, it would be
> a BUG. So I'm fine with removing it, but we could also think about
> turning it into some sort of WARN/BG to make it clearer that this is
> unexpected.

Since refill_swap_slots_cache is only called by folio_alloc_swap when cache->nr == 0. I think
it might be too overkill to add a WARN/BG.

> 
> 
> Anyhow,
> 
> Acked-by: David Hildenbrand <david@redhat.com>

Many thanks for comment and Acked-by tag!

> 

Re: [PATCH 06/15] mm/swap: remove buggy cache->nr check in refill_swap_slots_cache

[Andrew Morton]

On Mon, 16 May 2022 10:00:43 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:

> On 2022/5/12 21:37, David Hildenbrand wrote:
> > On 09.05.22 15:14, Miaohe Lin wrote:
> >> refill_swap_slots_cache is always called when cache->nr is 0. And if
> >> cache->nr != 0, we should return cache->nr instead of 0. So remove
> >> such buggy and confusing check.
> > 
> > Not sure about the "cache->nr != 0, we should return cache->nr instead
> > of 0" part, I'd just drop that from the patch description. We'd actually
> > end up overwriting cache->nr after your change, which doesn't sound
> > right and also different to what you describe here.
> 
> Will do.

I've updated the changleog to simply

: refill_swap_slots_cache is always called when cache->nr is 0.  So remove
: such buggy and confusing check.

Re: [PATCH 06/15] mm/swap: remove buggy cache->nr check in refill_swap_slots_cache

[Oscar Salvador]

On Mon, May 09, 2022 at 09:14:07PM +0800, Miaohe Lin wrote:
> refill_swap_slots_cache is always called when cache->nr is 0. And if
> cache->nr != 0, we should return cache->nr instead of 0. So remove
> such buggy and confusing check.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

With Andrew's ammendment:

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/swap_slots.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/swap_slots.c b/mm/swap_slots.c
> index 2f877e6f87d7..2a65a89b5b4d 100644
> --- a/mm/swap_slots.c
> +++ b/mm/swap_slots.c
> @@ -258,7 +258,7 @@ void enable_swap_slots_cache(void)
>  /* called with swap slot cache's alloc lock held */
>  static int refill_swap_slots_cache(struct swap_slots_cache *cache)
>  {
> -	if (!use_swap_slot_cache || cache->nr)
> +	if (!use_swap_slot_cache)
>  		return 0;
>  
>  	cache->cur = 0;
> -- 
> 2.23.0
> 
> 

-- 
Oscar Salvador
SUSE Labs

[PATCH 07/15] mm/swap: remove unneeded p != NULL check in __swap_duplicate

[Miaohe Lin]

If p is NULL, __swap_duplicate will already return -EINVAL. So if we
reach here, p must be non-NULL.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/swapfile.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index d4b81ca887c0..7b4c99ca2aea 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -3336,8 +3336,7 @@ static int __swap_duplicate(swp_entry_t entry, unsigned char usage)
 
 unlock_out:
 	unlock_cluster_or_swap_info(p, ci);
-	if (p)
-		put_swap_device(p);
+	put_swap_device(p);
 	return err;
 }
 
-- 
2.23.0

Re: [PATCH 07/15] mm/swap: remove unneeded p != NULL check in __swap_duplicate

[David Hildenbrand]

On 09.05.22 15:14, Miaohe Lin wrote:
> If p is NULL, __swap_duplicate will already return -EINVAL. So if we
> reach here, p must be non-NULL.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: David Hildenbrand <david@redhat.com>


-- 
Thanks,

David / dhildenb

Re: [PATCH 07/15] mm/swap: remove unneeded p != NULL check in __swap_duplicate

[Oscar Salvador]

On Mon, May 09, 2022 at 09:14:08PM +0800, Miaohe Lin wrote:
> If p is NULL, __swap_duplicate will already return -EINVAL. So if we
> reach here, p must be non-NULL.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/swapfile.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index d4b81ca887c0..7b4c99ca2aea 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -3336,8 +3336,7 @@ static int __swap_duplicate(swp_entry_t entry, unsigned char usage)
>  
>  unlock_out:
>  	unlock_cluster_or_swap_info(p, ci);
> -	if (p)
> -		put_swap_device(p);
> +	put_swap_device(p);
>  	return err;
>  }
>  
> -- 
> 2.23.0
> 
> 

-- 
Oscar Salvador
SUSE Labs

[PATCH 08/15] mm/swap: make page_swapcount and __lru_add_drain_all

[Miaohe Lin]

Make page_swapcount and __lru_add_drain_all static. They are only used
within the file now.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 include/linux/swap.h | 7 -------
 mm/swap.c            | 2 +-
 mm/swapfile.c        | 2 +-
 3 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 999c7d79c2d5..8772132d21dc 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -490,7 +490,6 @@ int swap_type_of(dev_t device, sector_t offset);
 int find_first_swap(dev_t *device);
 extern unsigned int count_swap_pages(int, int);
 extern sector_t swapdev_block(int, pgoff_t);
-extern int page_swapcount(struct page *);
 extern int __swap_count(swp_entry_t entry);
 extern int __swp_swapcount(swp_entry_t entry);
 extern int swp_swapcount(swp_entry_t entry);
@@ -562,12 +561,6 @@ static inline void put_swap_page(struct page *page, swp_entry_t swp)
 {
 }
 
-
-static inline int page_swapcount(struct page *page)
-{
-	return 0;
-}
-
 static inline int __swap_count(swp_entry_t entry)
 {
 	return 0;
diff --git a/mm/swap.c b/mm/swap.c
index 7e320ec08c6a..6d2c37f781f8 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -748,7 +748,7 @@ static void lru_add_drain_per_cpu(struct work_struct *dummy)
  * Calling this function with cpu hotplug locks held can actually lead
  * to obscure indirect dependencies via WQ context.
  */
-inline void __lru_add_drain_all(bool force_all_cpus)
+static inline void __lru_add_drain_all(bool force_all_cpus)
 {
 	/*
 	 * lru_drain_gen - Global pages generation number
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 7b4c99ca2aea..133e03fea104 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1430,7 +1430,7 @@ void swapcache_free_entries(swp_entry_t *entries, int n)
  * This does not give an exact answer when swap count is continued,
  * but does include the high COUNT_CONTINUED flag to allow for that.
  */
-int page_swapcount(struct page *page)
+static int page_swapcount(struct page *page)
 {
 	int count = 0;
 	struct swap_info_struct *p;
-- 
2.23.0

Re: [PATCH 08/15] mm/swap: make page_swapcount and __lru_add_drain_all

[David Hildenbrand]

On 09.05.22 15:14, Miaohe Lin wrote:
> Make page_swapcount and __lru_add_drain_all static. They are only used
> within the file now.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH 08/15] mm/swap: make page_swapcount and __lru_add_drain_all

[Oscar Salvador]

On Mon, May 09, 2022 at 09:14:09PM +0800, Miaohe Lin wrote:
> Make page_swapcount and __lru_add_drain_all static. They are only used
> within the file now.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

I think the commit message is missing the "static" word.

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  include/linux/swap.h | 7 -------
>  mm/swap.c            | 2 +-
>  mm/swapfile.c        | 2 +-
>  3 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 999c7d79c2d5..8772132d21dc 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -490,7 +490,6 @@ int swap_type_of(dev_t device, sector_t offset);
>  int find_first_swap(dev_t *device);
>  extern unsigned int count_swap_pages(int, int);
>  extern sector_t swapdev_block(int, pgoff_t);
> -extern int page_swapcount(struct page *);
>  extern int __swap_count(swp_entry_t entry);
>  extern int __swp_swapcount(swp_entry_t entry);
>  extern int swp_swapcount(swp_entry_t entry);
> @@ -562,12 +561,6 @@ static inline void put_swap_page(struct page *page, swp_entry_t swp)
>  {
>  }
>  
> -
> -static inline int page_swapcount(struct page *page)
> -{
> -	return 0;
> -}
> -
>  static inline int __swap_count(swp_entry_t entry)
>  {
>  	return 0;
> diff --git a/mm/swap.c b/mm/swap.c
> index 7e320ec08c6a..6d2c37f781f8 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -748,7 +748,7 @@ static void lru_add_drain_per_cpu(struct work_struct *dummy)
>   * Calling this function with cpu hotplug locks held can actually lead
>   * to obscure indirect dependencies via WQ context.
>   */
> -inline void __lru_add_drain_all(bool force_all_cpus)
> +static inline void __lru_add_drain_all(bool force_all_cpus)
>  {
>  	/*
>  	 * lru_drain_gen - Global pages generation number
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 7b4c99ca2aea..133e03fea104 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1430,7 +1430,7 @@ void swapcache_free_entries(swp_entry_t *entries, int n)
>   * This does not give an exact answer when swap count is continued,
>   * but does include the high COUNT_CONTINUED flag to allow for that.
>   */
> -int page_swapcount(struct page *page)
> +static int page_swapcount(struct page *page)
>  {
>  	int count = 0;
>  	struct swap_info_struct *p;
> -- 
> 2.23.0
> 
> 

-- 
Oscar Salvador
SUSE Labs

Re: [PATCH 08/15] mm/swap: make page_swapcount and __lru_add_drain_all

[Miaohe Lin]

On 2022/5/18 17:46, Oscar Salvador wrote:
> On Mon, May 09, 2022 at 09:14:09PM +0800, Miaohe Lin wrote:
>> Make page_swapcount and __lru_add_drain_all static. They are only used
>> within the file now.
>>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> 
> I think the commit message is missing the "static" word.

Yes, you're right. The commit message should be "mm/swap: make page_swapcount and __lru_add_drain_all static". :)

Hi Andrew,
Could you please help tweak the commit message? Or I will send a new version soon if requested.
Thanks a lot!

> 
> Reviewed-by: Oscar Salvador <osalvador@suse.de>

Many thanks for your review and comment!

> 
>> ---
>>  include/linux/swap.h | 7 -------
>>  mm/swap.c            | 2 +-
>>  mm/swapfile.c        | 2 +-
>>  3 files changed, 2 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/linux/swap.h b/include/linux/swap.h
>> index 999c7d79c2d5..8772132d21dc 100644
>> --- a/include/linux/swap.h
>> +++ b/include/linux/swap.h
>> @@ -490,7 +490,6 @@ int swap_type_of(dev_t device, sector_t offset);
>>  int find_first_swap(dev_t *device);
>>  extern unsigned int count_swap_pages(int, int);
>>  extern sector_t swapdev_block(int, pgoff_t);
>> -extern int page_swapcount(struct page *);
>>  extern int __swap_count(swp_entry_t entry);
>>  extern int __swp_swapcount(swp_entry_t entry);
>>  extern int swp_swapcount(swp_entry_t entry);
>> @@ -562,12 +561,6 @@ static inline void put_swap_page(struct page *page, swp_entry_t swp)
>>  {
>>  }
>>  
>> -
>> -static inline int page_swapcount(struct page *page)
>> -{
>> -	return 0;
>> -}
>> -
>>  static inline int __swap_count(swp_entry_t entry)
>>  {
>>  	return 0;
>> diff --git a/mm/swap.c b/mm/swap.c
>> index 7e320ec08c6a..6d2c37f781f8 100644
>> --- a/mm/swap.c
>> +++ b/mm/swap.c
>> @@ -748,7 +748,7 @@ static void lru_add_drain_per_cpu(struct work_struct *dummy)
>>   * Calling this function with cpu hotplug locks held can actually lead
>>   * to obscure indirect dependencies via WQ context.
>>   */
>> -inline void __lru_add_drain_all(bool force_all_cpus)
>> +static inline void __lru_add_drain_all(bool force_all_cpus)
>>  {
>>  	/*
>>  	 * lru_drain_gen - Global pages generation number
>> diff --git a/mm/swapfile.c b/mm/swapfile.c
>> index 7b4c99ca2aea..133e03fea104 100644
>> --- a/mm/swapfile.c
>> +++ b/mm/swapfile.c
>> @@ -1430,7 +1430,7 @@ void swapcache_free_entries(swp_entry_t *entries, int n)
>>   * This does not give an exact answer when swap count is continued,
>>   * but does include the high COUNT_CONTINUED flag to allow for that.
>>   */
>> -int page_swapcount(struct page *page)
>> +static int page_swapcount(struct page *page)
>>  {
>>  	int count = 0;
>>  	struct swap_info_struct *p;
>> -- 
>> 2.23.0
>>
>>
> 

[PATCH 09/15] mm/swap: avoid calling swp_swap_info when try to check SWP_STABLE_WRITES

[Miaohe Lin]

Use flags of si directly to check SWP_STABLE_WRITES to avoid possible
READ_ONCE and thus save some cpu cycles.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index 9c3e7e6ac202..89dd15504f3d 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3892,7 +3892,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 			 */
 			exclusive = true;
 		} else if (exclusive && PageWriteback(page) &&
-			  (swp_swap_info(entry)->flags & SWP_STABLE_WRITES)) {
+			  (si->flags & SWP_STABLE_WRITES)) {
 			/*
 			 * This is tricky: not all swap backends support
 			 * concurrent page modifications while under writeback.
-- 
2.23.0

Re: [PATCH 09/15] mm/swap: avoid calling swp_swap_info when try to check SWP_STABLE_WRITES

[NeilBrown]

On Mon, 09 May 2022, Miaohe Lin wrote:
> Use flags of si directly to check SWP_STABLE_WRITES to avoid possible
> READ_ONCE and thus save some cpu cycles.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  mm/memory.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 9c3e7e6ac202..89dd15504f3d 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3892,7 +3892,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>  			 */
>  			exclusive = true;
>  		} else if (exclusive && PageWriteback(page) &&
> -			  (swp_swap_info(entry)->flags & SWP_STABLE_WRITES)) {
> +			  (si->flags & SWP_STABLE_WRITES)) {

Should this have a data_race() annotation.  Other bit-tests of si->flags
do because scan_swap_map_slots can update it asynchronously, but we know
that won't matter in practice.

Thanks,
NeilBrown


>  			/*
>  			 * This is tricky: not all swap backends support
>  			 * concurrent page modifications while under writeback.
> -- 
> 2.23.0
> 
> 

Re: [PATCH 09/15] mm/swap: avoid calling swp_swap_info when try to check SWP_STABLE_WRITES

[Miaohe Lin]

On 2022/5/10 8:28, NeilBrown wrote:
> On Mon, 09 May 2022, Miaohe Lin wrote:
>> Use flags of si directly to check SWP_STABLE_WRITES to avoid possible
>> READ_ONCE and thus save some cpu cycles.
>>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
>>  mm/memory.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/memory.c b/mm/memory.c
>> index 9c3e7e6ac202..89dd15504f3d 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -3892,7 +3892,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>>  			 */
>>  			exclusive = true;
>>  		} else if (exclusive && PageWriteback(page) &&
>> -			  (swp_swap_info(entry)->flags & SWP_STABLE_WRITES)) {
>> +			  (si->flags & SWP_STABLE_WRITES)) {
> 
> Should this have a data_race() annotation.  Other bit-tests of si->flags
> do because scan_swap_map_slots can update it asynchronously, but we know
> that won't matter in practice.

Yes, you're right. scan_swap_map_slots can update si->flags asynchronously while
do_swap_page tests SWP_STABLE_WRITES here. We know this is harmless because
SWP_STABLE_WRITES is only changed at swapon/swapoff.

Will add data_race() annotation in next version to avoid possible KCSAN data-race complaint.

Many thanks for pointing this out! :)

> 
> Thanks,
> NeilBrown
> 
> 
>>  			/*
>>  			 * This is tricky: not all swap backends support
>>  			 * concurrent page modifications while under writeback.
>> -- 
>> 2.23.0
>>
>>
> .
> 

Re: [PATCH 09/15] mm/swap: avoid calling swp_swap_info when try to check SWP_STABLE_WRITES

[Andrew Morton]

On Tue, 10 May 2022 10:21:21 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:

> On 2022/5/10 8:28, NeilBrown wrote:
> > On Mon, 09 May 2022, Miaohe Lin wrote:
> >> Use flags of si directly to check SWP_STABLE_WRITES to avoid possible
> >> READ_ONCE and thus save some cpu cycles.
> >>
> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> >> ---
> >>  mm/memory.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/mm/memory.c b/mm/memory.c
> >> index 9c3e7e6ac202..89dd15504f3d 100644
> >> --- a/mm/memory.c
> >> +++ b/mm/memory.c
> >> @@ -3892,7 +3892,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> >>  			 */
> >>  			exclusive = true;
> >>  		} else if (exclusive && PageWriteback(page) &&
> >> -			  (swp_swap_info(entry)->flags & SWP_STABLE_WRITES)) {
> >> +			  (si->flags & SWP_STABLE_WRITES)) {
> > 
> > Should this have a data_race() annotation.  Other bit-tests of si->flags
> > do because scan_swap_map_slots can update it asynchronously, but we know
> > that won't matter in practice.
> 
> Yes, you're right. scan_swap_map_slots can update si->flags asynchronously while
> do_swap_page tests SWP_STABLE_WRITES here. We know this is harmless because
> SWP_STABLE_WRITES is only changed at swapon/swapoff.
> 
> Will add data_race() annotation in next version to avoid possible KCSAN data-race complaint.
> 

I did this:

--- a/mm/memory.c~mm-swap-avoid-calling-swp_swap_info-when-try-to-check-swp_stable_writes-fix
+++ a/mm/memory.c
@@ -3889,7 +3889,7 @@ vm_fault_t do_swap_page(struct vm_fault
 			 */
 			exclusive = true;
 		} else if (exclusive && PageWriteback(page) &&
-			  (si->flags & SWP_STABLE_WRITES)) {
+			  data_race(si->flags & SWP_STABLE_WRITES)) {
 			/*
 			 * This is tricky: not all swap backends support
 			 * concurrent page modifications while under writeback.
_

[PATCH 10/15] mm/swap: break the loop if matching device is found

[Miaohe Lin]

We can break the loop if matching device is found to save some possible
cpu cycles because there should be only one matching device and there is
no need to continue if the matching one is already found.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/swapfile.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 133e03fea104..c90298a0561a 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1692,6 +1692,8 @@ int swap_type_of(dev_t device, sector_t offset)
 				spin_unlock(&swap_lock);
 				return type;
 			}
+
+			break;
 		}
 	}
 	spin_unlock(&swap_lock);
-- 
2.23.0

Re: [PATCH 10/15] mm/swap: break the loop if matching device is found

[Andrew Morton]

On Mon, 9 May 2022 21:14:11 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:

> We can break the loop if matching device is found to save some possible
> cpu cycles because there should be only one matching device and there is
> no need to continue if the matching one is already found.
> 
> ...
>
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1692,6 +1692,8 @@ int swap_type_of(dev_t device, sector_t offset)
>  				spin_unlock(&swap_lock);
>  				return type;
>  			}
> +
> +			break;
>  		}
>  	}
>  	spin_unlock(&swap_lock);

Are you sure?  If we have two S_ISREG swapfiles on the same device,
don't they have the same sis->bdev?

If not, why bother passing `offset' into this function at all?

Re: [PATCH 10/15] mm/swap: break the loop if matching device is found

[Miaohe Lin]

On 2022/5/10 5:16, Andrew Morton wrote:
> On Mon, 9 May 2022 21:14:11 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:
> 
>> We can break the loop if matching device is found to save some possible
>> cpu cycles because there should be only one matching device and there is
>> no need to continue if the matching one is already found.
>>
>> ...
>>
>> --- a/mm/swapfile.c
>> +++ b/mm/swapfile.c
>> @@ -1692,6 +1692,8 @@ int swap_type_of(dev_t device, sector_t offset)
>>  				spin_unlock(&swap_lock);
>>  				return type;
>>  			}
>> +
>> +			break;
>>  		}
>>  	}
>>  	spin_unlock(&swap_lock);
> 
> Are you sure?  If we have two S_ISREG swapfiles on the same device,
> don't they have the same sis->bdev?

Oh, I missed this use case. Sorry about it! :(

> 
> If not, why bother passing `offset' into this function at all?

Yes, you're right. 'offset' indicates the swap header location. Will drop this patch.

Thanks!

> .
> 

[PATCH 11/15] mm/swap: add helper swap_offset_available()

[Miaohe Lin]

Add helper swap_offset_available() to remove some duplicated codes.
Minor readability improvement.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/swapfile.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index c90298a0561a..d5d3e2d03d28 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -776,6 +776,21 @@ static void set_cluster_next(struct swap_info_struct *si, unsigned long next)
 	this_cpu_write(*si->cluster_next_cpu, next);
 }
 
+static inline bool swap_offset_available(struct swap_info_struct *si, unsigned long offset)
+{
+	if (data_race(!si->swap_map[offset])) {
+		spin_lock(&si->lock);
+		return true;
+	}
+
+	if (vm_swap_full() && READ_ONCE(si->swap_map[offset]) == SWAP_HAS_CACHE) {
+		spin_lock(&si->lock);
+		return true;
+	}
+
+	return false;
+}
+
 static int scan_swap_map_slots(struct swap_info_struct *si,
 			       unsigned char usage, int nr,
 			       swp_entry_t slots[])
@@ -953,15 +968,8 @@ static int scan_swap_map_slots(struct swap_info_struct *si,
 scan:
 	spin_unlock(&si->lock);
 	while (++offset <= READ_ONCE(si->highest_bit)) {
-		if (data_race(!si->swap_map[offset])) {
-			spin_lock(&si->lock);
+		if (swap_offset_available(si, offset))
 			goto checks;
-		}
-		if (vm_swap_full() &&
-		    READ_ONCE(si->swap_map[offset]) == SWAP_HAS_CACHE) {
-			spin_lock(&si->lock);
-			goto checks;
-		}
 		if (unlikely(--latency_ration < 0)) {
 			cond_resched();
 			latency_ration = LATENCY_LIMIT;
@@ -970,15 +978,8 @@ static int scan_swap_map_slots(struct swap_info_struct *si,
 	}
 	offset = si->lowest_bit;
 	while (offset < scan_base) {
-		if (data_race(!si->swap_map[offset])) {
-			spin_lock(&si->lock);
+		if (swap_offset_available(si, offset))
 			goto checks;
-		}
-		if (vm_swap_full() &&
-		    READ_ONCE(si->swap_map[offset]) == SWAP_HAS_CACHE) {
-			spin_lock(&si->lock);
-			goto checks;
-		}
 		if (unlikely(--latency_ration < 0)) {
 			cond_resched();
 			latency_ration = LATENCY_LIMIT;
-- 
2.23.0

Re: [PATCH 11/15] mm/swap: add helper swap_offset_available()

[NeilBrown]

On Mon, 09 May 2022, Miaohe Lin wrote:
> Add helper swap_offset_available() to remove some duplicated codes.
> Minor readability improvement.

I don't think that putting the spin_lock() inside the inline helper is
good for readability.
If the function was called
   swap_offset_available_and_locked()

it might be ok.  Otherwise I would rather the spin_lock() was called
when the function returned true.

Thanks,
NeilBrown

> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  mm/swapfile.c | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index c90298a0561a..d5d3e2d03d28 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -776,6 +776,21 @@ static void set_cluster_next(struct swap_info_struct *si, unsigned long next)
>  	this_cpu_write(*si->cluster_next_cpu, next);
>  }
>  
> +static inline bool swap_offset_available(struct swap_info_struct *si, unsigned long offset)
> +{
> +	if (data_race(!si->swap_map[offset])) {
> +		spin_lock(&si->lock);
> +		return true;
> +	}
> +
> +	if (vm_swap_full() && READ_ONCE(si->swap_map[offset]) == SWAP_HAS_CACHE) {
> +		spin_lock(&si->lock);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
>  static int scan_swap_map_slots(struct swap_info_struct *si,
>  			       unsigned char usage, int nr,
>  			       swp_entry_t slots[])
> @@ -953,15 +968,8 @@ static int scan_swap_map_slots(struct swap_info_struct *si,
>  scan:
>  	spin_unlock(&si->lock);
>  	while (++offset <= READ_ONCE(si->highest_bit)) {
> -		if (data_race(!si->swap_map[offset])) {
> -			spin_lock(&si->lock);
> +		if (swap_offset_available(si, offset))
>  			goto checks;
> -		}
> -		if (vm_swap_full() &&
> -		    READ_ONCE(si->swap_map[offset]) == SWAP_HAS_CACHE) {
> -			spin_lock(&si->lock);
> -			goto checks;
> -		}
>  		if (unlikely(--latency_ration < 0)) {
>  			cond_resched();
>  			latency_ration = LATENCY_LIMIT;
> @@ -970,15 +978,8 @@ static int scan_swap_map_slots(struct swap_info_struct *si,
>  	}
>  	offset = si->lowest_bit;
>  	while (offset < scan_base) {
> -		if (data_race(!si->swap_map[offset])) {
> -			spin_lock(&si->lock);
> +		if (swap_offset_available(si, offset))
>  			goto checks;
> -		}
> -		if (vm_swap_full() &&
> -		    READ_ONCE(si->swap_map[offset]) == SWAP_HAS_CACHE) {
> -			spin_lock(&si->lock);
> -			goto checks;
> -		}
>  		if (unlikely(--latency_ration < 0)) {
>  			cond_resched();
>  			latency_ration = LATENCY_LIMIT;
> -- 
> 2.23.0
> 
> 

Re: [PATCH 11/15] mm/swap: add helper swap_offset_available()

[Miaohe Lin]

On 2022/5/10 8:45, NeilBrown wrote:
> On Mon, 09 May 2022, Miaohe Lin wrote:
>> Add helper swap_offset_available() to remove some duplicated codes.
>> Minor readability improvement.
> 
> I don't think that putting the spin_lock() inside the inline helper is
> good for readability.
> If the function was called
>    swap_offset_available_and_locked()

Yes, swap_offset_available_and_locked should be more suitable as we do the spin_lock
inside it. Will do this in next version.

Thanks!

> 
> it might be ok.  Otherwise I would rather the spin_lock() was called
> when the function returned true.
> 
> Thanks,
> NeilBrown
> 
>>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
>>  mm/swapfile.c | 33 +++++++++++++++++----------------
>>  1 file changed, 17 insertions(+), 16 deletions(-)
>>
>> diff --git a/mm/swapfile.c b/mm/swapfile.c
>> index c90298a0561a..d5d3e2d03d28 100644
>> --- a/mm/swapfile.c
>> +++ b/mm/swapfile.c
>> @@ -776,6 +776,21 @@ static void set_cluster_next(struct swap_info_struct *si, unsigned long next)
>>  	this_cpu_write(*si->cluster_next_cpu, next);
>>  }
>>  
>> +static inline bool swap_offset_available(struct swap_info_struct *si, unsigned long offset)
>> +{
>> +	if (data_race(!si->swap_map[offset])) {
>> +		spin_lock(&si->lock);
>> +		return true;
>> +	}
>> +
>> +	if (vm_swap_full() && READ_ONCE(si->swap_map[offset]) == SWAP_HAS_CACHE) {
>> +		spin_lock(&si->lock);
>> +		return true;
>> +	}
>> +
>> +	return false;
>> +}
>> +
>>  static int scan_swap_map_slots(struct swap_info_struct *si,
>>  			       unsigned char usage, int nr,
>>  			       swp_entry_t slots[])
>> @@ -953,15 +968,8 @@ static int scan_swap_map_slots(struct swap_info_struct *si,
>>  scan:
>>  	spin_unlock(&si->lock);
>>  	while (++offset <= READ_ONCE(si->highest_bit)) {
>> -		if (data_race(!si->swap_map[offset])) {
>> -			spin_lock(&si->lock);
>> +		if (swap_offset_available(si, offset))
>>  			goto checks;
>> -		}
>> -		if (vm_swap_full() &&
>> -		    READ_ONCE(si->swap_map[offset]) == SWAP_HAS_CACHE) {
>> -			spin_lock(&si->lock);
>> -			goto checks;
>> -		}
>>  		if (unlikely(--latency_ration < 0)) {
>>  			cond_resched();
>>  			latency_ration = LATENCY_LIMIT;
>> @@ -970,15 +978,8 @@ static int scan_swap_map_slots(struct swap_info_struct *si,
>>  	}
>>  	offset = si->lowest_bit;
>>  	while (offset < scan_base) {
>> -		if (data_race(!si->swap_map[offset])) {
>> -			spin_lock(&si->lock);
>> +		if (swap_offset_available(si, offset))
>>  			goto checks;
>> -		}
>> -		if (vm_swap_full() &&
>> -		    READ_ONCE(si->swap_map[offset]) == SWAP_HAS_CACHE) {
>> -			spin_lock(&si->lock);
>> -			goto checks;
>> -		}
>>  		if (unlikely(--latency_ration < 0)) {
>>  			cond_resched();
>>  			latency_ration = LATENCY_LIMIT;
>> -- 
>> 2.23.0
>>
>>
> .
> 

Re: [PATCH 11/15] mm/swap: add helper swap_offset_available()

[Andrew Morton]

On Tue, 10 May 2022 10:03:19 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:

> On 2022/5/10 8:45, NeilBrown wrote:
> > On Mon, 09 May 2022, Miaohe Lin wrote:
> >> Add helper swap_offset_available() to remove some duplicated codes.
> >> Minor readability improvement.
> > 
> > I don't think that putting the spin_lock() inside the inline helper is
> > good for readability.
> > If the function was called
> >    swap_offset_available_and_locked()
> 
> Yes, swap_offset_available_and_locked should be more suitable as we do the spin_lock
> inside it. Will do this in next version.
> 

--- a/mm/swapfile.c~mm-swap-add-helper-swap_offset_available-fix
+++ a/mm/swapfile.c
@@ -775,7 +775,8 @@ static void set_cluster_next(struct swap
 	this_cpu_write(*si->cluster_next_cpu, next);
 }
 
-static inline bool swap_offset_available(struct swap_info_struct *si, unsigned long offset)
+static bool swap_offset_available_and_locked(struct swap_info_struct *si,
+					     unsigned long offset)
 {
 	if (data_race(!si->swap_map[offset])) {
 		spin_lock(&si->lock);
@@ -967,7 +968,7 @@ done:
 scan:
 	spin_unlock(&si->lock);
 	while (++offset <= READ_ONCE(si->highest_bit)) {
-		if (swap_offset_available(si, offset))
+		if (swap_offset_available_and_locked(si, offset))
 			goto checks;
 		if (unlikely(--latency_ration < 0)) {
 			cond_resched();
@@ -977,7 +978,7 @@ scan:
 	}
 	offset = si->lowest_bit;
 	while (offset < scan_base) {
-		if (swap_offset_available(si, offset))
+		if (swap_offset_available_and_locked(si, offset))
 			goto checks;
 		if (unlikely(--latency_ration < 0)) {
 			cond_resched();
_

[PATCH 12/15] mm/swap: fix the obsolete comment for SWP_TYPE_SHIFT

[Miaohe Lin]

Since commit 3159f943aafd ("xarray: Replace exceptional entries"), there
is only one bit of 'type' can be shifted up. Update the corresponding
comment.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 include/linux/swapops.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/swapops.h b/include/linux/swapops.h
index 7db424e2dcb1..bb7afd03a324 100644
--- a/include/linux/swapops.h
+++ b/include/linux/swapops.h
@@ -13,10 +13,10 @@
  * get good packing density in that tree, so the index should be dense in
  * the low-order bits.
  *
- * We arrange the `type' and `offset' fields so that `type' is at the seven
+ * We arrange the `type' and `offset' fields so that `type' is at the six
  * high-order bits of the swp_entry_t and `offset' is right-aligned in the
  * remaining bits.  Although `type' itself needs only five bits, we allow for
- * shmem/tmpfs to shift it all up a further two bits: see swp_to_radix_entry().
+ * shmem/tmpfs to shift it all up a further one bit: see swp_to_radix_entry().
  *
  * swp_entry_t's are *never* stored anywhere in their arch-dependent format.
  */
-- 
2.23.0

Re: [PATCH 12/15] mm/swap: fix the obsolete comment for SWP_TYPE_SHIFT

[David Hildenbrand]

On 09.05.22 15:14, Miaohe Lin wrote:
> Since commit 3159f943aafd ("xarray: Replace exceptional entries"), there
> is only one bit of 'type' can be shifted up. Update the corresponding
> comment.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  include/linux/swapops.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/swapops.h b/include/linux/swapops.h
> index 7db424e2dcb1..bb7afd03a324 100644
> --- a/include/linux/swapops.h
> +++ b/include/linux/swapops.h
> @@ -13,10 +13,10 @@
>   * get good packing density in that tree, so the index should be dense in
>   * the low-order bits.
>   *
> - * We arrange the `type' and `offset' fields so that `type' is at the seven
> + * We arrange the `type' and `offset' fields so that `type' is at the six
>   * high-order bits of the swp_entry_t and `offset' is right-aligned in the
>   * remaining bits.  Although `type' itself needs only five bits, we allow for
> - * shmem/tmpfs to shift it all up a further two bits: see swp_to_radix_entry().
> + * shmem/tmpfs to shift it all up a further one bit: see swp_to_radix_entry().

Actually, the shift magic is in xa_mk_value() :)

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

[PATCH 13/15] mm/swap: clean up the comment of find_next_to_unuse

[Miaohe Lin]

Since commit 10a9c496789f ("mm: simplify try_to_unuse"), frontswap
parameter is removed. Update the corresponding comment.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/swapfile.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index d5d3e2d03d28..7ead5fb96d9d 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -2007,9 +2007,9 @@ static int unuse_mm(struct mm_struct *mm, unsigned int type)
 }
 
 /*
- * Scan swap_map (or frontswap_map if frontswap parameter is true)
- * from current position to next entry still in use. Return 0
- * if there are no inuse entries after prev till end of the map.
+ * Scan swap_map from current position to next entry still in use.
+ * Return 0 if there are no inuse entries after prev till end of
+ * the map.
  */
 static unsigned int find_next_to_unuse(struct swap_info_struct *si,
 					unsigned int prev)
-- 
2.23.0

Re: [PATCH 13/15] mm/swap: clean up the comment of find_next_to_unuse

[David Hildenbrand]

On 09.05.22 15:14, Miaohe Lin wrote:
> Since commit 10a9c496789f ("mm: simplify try_to_unuse"), frontswap
> parameter is removed. Update the corresponding comment.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: David Hildenbrand <david@redhat.com>


-- 
Thanks,

David / dhildenb

[PATCH 14/15] mm/swap: fix the comment of get_kernel_pages

[Miaohe Lin]

If no pages were pinned, 0 is returned in fact. Fix the corresponding
comment.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/swap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/swap.c b/mm/swap.c
index 6d2c37f781f8..236b37663a1a 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -168,8 +168,8 @@ EXPORT_SYMBOL(put_pages_list);
  *
  * Returns number of pages pinned. This may be fewer than the number
  * requested. If nr_pages is 0 or negative, returns 0. If no pages
- * were pinned, returns -errno. Each page returned must be released
- * with a put_page() call when it is finished with.
+ * were pinned, returns 0. Each page returned must be released with
+ * a put_page() call when it is finished with.
  */
 int get_kernel_pages(const struct kvec *kiov, int nr_segs, int write,
 		struct page **pages)
-- 
2.23.0

Re: [PATCH 14/15] mm/swap: fix the comment of get_kernel_pages

[David Hildenbrand]

On 09.05.22 15:14, Miaohe Lin wrote:
> If no pages were pinned, 0 is returned in fact. Fix the corresponding
> comment.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  mm/swap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/swap.c b/mm/swap.c
> index 6d2c37f781f8..236b37663a1a 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -168,8 +168,8 @@ EXPORT_SYMBOL(put_pages_list);
>   *
>   * Returns number of pages pinned. This may be fewer than the number
>   * requested. If nr_pages is 0 or negative, returns 0. If no pages

Ehm, there is only "nr_segs", no "nr_pages" :/ Want to fix that up in
the same patch?

> - * were pinned, returns -errno. Each page returned must be released
> - * with a put_page() call when it is finished with.
> + * were pinned, returns 0. Each page returned must be released with
> + * a put_page() call when it is finished with.
>   */
>  int get_kernel_pages(const struct kvec *kiov, int nr_segs, int write,
>  		struct page **pages)

Apart from that

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH 14/15] mm/swap: fix the comment of get_kernel_pages

[Miaohe Lin]

On 2022/5/12 21:45, David Hildenbrand wrote:
> On 09.05.22 15:14, Miaohe Lin wrote:
>> If no pages were pinned, 0 is returned in fact. Fix the corresponding
>> comment.
>>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
>>  mm/swap.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/swap.c b/mm/swap.c
>> index 6d2c37f781f8..236b37663a1a 100644
>> --- a/mm/swap.c
>> +++ b/mm/swap.c
>> @@ -168,8 +168,8 @@ EXPORT_SYMBOL(put_pages_list);
>>   *
>>   * Returns number of pages pinned. This may be fewer than the number
>>   * requested. If nr_pages is 0 or negative, returns 0. If no pages
> 
> Ehm, there is only "nr_segs", no "nr_pages" :/ Want to fix that up in
> the same patch?

Will do.

> 
>> - * were pinned, returns -errno. Each page returned must be released
>> - * with a put_page() call when it is finished with.
>> + * were pinned, returns 0. Each page returned must be released with
>> + * a put_page() call when it is finished with.
>>   */
>>  int get_kernel_pages(const struct kvec *kiov, int nr_segs, int write,
>>  		struct page **pages)
> 
> Apart from that
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>

Many thanks for review and comment!

> 

Re: [PATCH 14/15] mm/swap: fix the comment of get_kernel_pages

[Andrew Morton]

On Fri, 13 May 2022 14:15:38 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:

> On 2022/5/12 21:45, David Hildenbrand wrote:
> > On 09.05.22 15:14, Miaohe Lin wrote:
> >> If no pages were pinned, 0 is returned in fact. Fix the corresponding
> >> comment.
> >>
> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> >> ---
> >>  mm/swap.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/mm/swap.c b/mm/swap.c
> >> index 6d2c37f781f8..236b37663a1a 100644
> >> --- a/mm/swap.c
> >> +++ b/mm/swap.c
> >> @@ -168,8 +168,8 @@ EXPORT_SYMBOL(put_pages_list);
> >>   *
> >>   * Returns number of pages pinned. This may be fewer than the number
> >>   * requested. If nr_pages is 0 or negative, returns 0. If no pages
> > 
> > Ehm, there is only "nr_segs", no "nr_pages" :/ Want to fix that up in
> > the same patch?
> 
> Will do.

I also reflowed that comment to use more columns.

--- a/mm/swap.c~mm-swap-fix-the-comment-of-get_kernel_pages-fix
+++ a/mm/swap.c
@@ -166,10 +166,10 @@ EXPORT_SYMBOL(put_pages_list);
  * @pages:	array that receives pointers to the pages pinned.
  *		Should be at least nr_segs long.
  *
- * Returns number of pages pinned. This may be fewer than the number
- * requested. If nr_pages is 0 or negative, returns 0. If no pages
- * were pinned, returns 0. Each page returned must be released with
- * a put_page() call when it is finished with.
+ * Returns number of pages pinned. This may be fewer than the number requested.
+ * If nr_segs is 0 or negative, returns 0.  If no pages were pinned, returns 0.
+ * Each page returned must be released with a put_page() call when it is
+ * finished with.
  */
 int get_kernel_pages(const struct kvec *kiov, int nr_segs, int write,
 		struct page **pages)
_

[PATCH 15/15] mm/swap: fix comment about swap extent

[Miaohe Lin]

Since commit 4efaceb1c5f8 ("mm, swap: use rbtree for swap_extent"), rbtree
is used for swap extent. Also curr_swap_extent is removed at that time.
Update the corresponding comment.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 include/linux/swap.h |  4 ++--
 mm/swapfile.c        | 15 ++++++---------
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 8772132d21dc..e1f3201dec6f 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -173,8 +173,8 @@ struct zone;
 
 /*
  * A swap extent maps a range of a swapfile's PAGE_SIZE pages onto a range of
- * disk blocks.  A list of swap extents maps the entire swapfile.  (Where the
- * term `swapfile' refers to either a blockdevice or an IS_REG file.  Apart
+ * disk blocks.  A rbtree of swap extents maps the entire swapfile (Where the
+ * term `swapfile' refers to either a blockdevice or an IS_REG file). Apart
  * from setup, they're handled identically.
  *
  * We always assume that blocks are of size PAGE_SIZE.
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 7ead5fb96d9d..b3f977d9c83e 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -2225,8 +2225,8 @@ EXPORT_SYMBOL_GPL(add_swap_extent);
 
 /*
  * A `swap extent' is a simple thing which maps a contiguous range of pages
- * onto a contiguous range of disk blocks.  An ordered list of swap extents
- * is built at swapon time and is then used at swap_writepage/swap_readpage
+ * onto a contiguous range of disk blocks.  A rbtree of swap extents is
+ * built at swapon time and is then used at swap_writepage/swap_readpage
  * time for locating where on disk a page belongs.
  *
  * If the swapfile is an S_ISBLK block device, a single extent is installed.
@@ -2234,12 +2234,12 @@ EXPORT_SYMBOL_GPL(add_swap_extent);
  * swap files identically.
  *
  * Whether the swapdev is an S_ISREG file or an S_ISBLK blockdev, the swap
- * extent list operates in PAGE_SIZE disk blocks.  Both S_ISREG and S_ISBLK
+ * extent rbtree operates in PAGE_SIZE disk blocks.  Both S_ISREG and S_ISBLK
  * swapfiles are handled *identically* after swapon time.
  *
  * For S_ISREG swapfiles, setup_swap_extents() will walk all the file's blocks
- * and will parse them into an ordered extent list, in PAGE_SIZE chunks.  If
- * some stray blocks are found which do not fall within the PAGE_SIZE alignment
+ * and will parse them into a rbtree, in PAGE_SIZE chunks.  If some stray
+ * blocks are found which do not fall within the PAGE_SIZE alignment
  * requirements, they are simply tossed out - we will never use those blocks
  * for swapping.
  *
@@ -2248,10 +2248,7 @@ EXPORT_SYMBOL_GPL(add_swap_extent);
  *
  * The amount of disk space which a single swap extent represents varies.
  * Typically it is in the 1-4 megabyte range.  So we can have hundreds of
- * extents in the list.  To avoid much list walking, we cache the previous
- * search location in `curr_swap_extent', and start new searches from there.
- * This is extremely effective.  The average number of iterations in
- * map_swap_page() has been measured at about 0.3 per page.  - akpm.
+ * extents in the rbtree. - akpm.
  */
 static int setup_swap_extents(struct swap_info_struct *sis, sector_t *span)
 {
-- 
2.23.0

Re: [PATCH 00/15] A few cleanup patches for swap

[Andrew Morton]

On Mon, 9 May 2022 21:14:01 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:

> This series contains a few patches to fix the comment, remove unneeded
> return value, use some helpers and so on. More details can be found in
> the respective changelogs. 

After dropping [10/14] and with the four fixlets I just sent out, I
believe this series is ready to be moved into mm-stable.  Is there
anything outstanding?

Re: [PATCH 00/15] A few cleanup patches for swap

[Miaohe Lin]

On 2022/5/18 7:42, Andrew Morton wrote:
> On Mon, 9 May 2022 21:14:01 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:
> 
>> This series contains a few patches to fix the comment, remove unneeded
>> return value, use some helpers and so on. More details can be found in
>> the respective changelogs. 
> 
> After dropping [10/14] and with the four fixlets I just sent out, I
> believe this series is ready to be moved into mm-stable.  Is there

Sorry, I am focus on fixing the deadloop when swapin error occurs at swapoff recently.
Many thanks for doing this and above changes look good to me. :)

> anything outstanding?
> 

That's all. Thanks a lot!

> 
> .
>