diff mbox series

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

Message ID 20220509131416.17553-11-linmiaohe@huawei.com (mailing list archive)
State New
Headers show
Series A few cleanup patches for swap | expand

Commit Message

Miaohe Lin May 9, 2022, 1:14 p.m. UTC
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(+)

Comments

Andrew Morton May 9, 2022, 9:16 p.m. UTC | #1
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?
Miaohe Lin May 10, 2022, 2:10 a.m. UTC | #2
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!

> .
>
diff mbox series

Patch

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);