mm/hugetlbfs: use macro SZ_1K to replace 1024

[PATCH] mm/hugetlbfs: use macro SZ_1K to replace 1024

Posted by Xin Hao 3 years, 6 months ago

Using macro SZ_1K in hugetlbfs_show_options() has no any functional
changes, just makes code more readable.

Signed-off-by: Xin Hao <xhao@linux.alibaba.com>
---
 fs/hugetlbfs/inode.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index f7a5b5124d8a..9b9784ffe8de 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -1023,10 +1023,10 @@ static int hugetlbfs_show_options(struct seq_file *m, struct dentry *root)
 	if (sbinfo->max_inodes != -1)
 		seq_printf(m, ",nr_inodes=%lu", sbinfo->max_inodes);

-	hpage_size /= 1024;
+	hpage_size /= SZ_1K;
 	mod = 'K';
-	if (hpage_size >= 1024) {
-		hpage_size /= 1024;
+	if (hpage_size >= SZ_1K) {
+		hpage_size /= SZ_1K;
 		mod = 'M';
 	}
 	seq_printf(m, ",pagesize=%lu%c", hpage_size, mod);
--
2.31.0

Re: [PATCH] mm/hugetlbfs: use macro SZ_1K to replace 1024

Posted by SeongJae Park 3 years, 6 months ago

On Wed, 14 Sep 2022 19:57:23 +0800 Xin Hao <xhao@linux.alibaba.com> wrote:

> Using macro SZ_1K in hugetlbfs_show_options() has no any functional
> changes, just makes code more readable.
> 
> Signed-off-by: Xin Hao <xhao@linux.alibaba.com>

Reviewed-by: SeongJae Park <sj@kernel.org>


Thanks,
SJ

> ---
>  fs/hugetlbfs/inode.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index f7a5b5124d8a..9b9784ffe8de 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -1023,10 +1023,10 @@ static int hugetlbfs_show_options(struct seq_file *m, struct dentry *root)
>  	if (sbinfo->max_inodes != -1)
>  		seq_printf(m, ",nr_inodes=%lu", sbinfo->max_inodes);
> 
> -	hpage_size /= 1024;
> +	hpage_size /= SZ_1K;
>  	mod = 'K';
> -	if (hpage_size >= 1024) {
> -		hpage_size /= 1024;
> +	if (hpage_size >= SZ_1K) {
> +		hpage_size /= SZ_1K;
>  		mod = 'M';
>  	}
>  	seq_printf(m, ",pagesize=%lu%c", hpage_size, mod);
> --
> 2.31.0

Re: [PATCH] mm/hugetlbfs: use macro SZ_1K to replace 1024

Posted by Al Viro 3 years, 6 months ago

On Wed, Sep 14, 2022 at 07:57:23PM +0800, Xin Hao wrote:
> Using macro SZ_1K in hugetlbfs_show_options() has no any functional
> changes, just makes code more readable.

Why is it any more readable that way?

> Signed-off-by: Xin Hao <xhao@linux.alibaba.com>
> ---
>  fs/hugetlbfs/inode.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index f7a5b5124d8a..9b9784ffe8de 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -1023,10 +1023,10 @@ static int hugetlbfs_show_options(struct seq_file *m, struct dentry *root)
>  	if (sbinfo->max_inodes != -1)
>  		seq_printf(m, ",nr_inodes=%lu", sbinfo->max_inodes);
> 
> -	hpage_size /= 1024;
> +	hpage_size /= SZ_1K;
>  	mod = 'K';
> -	if (hpage_size >= 1024) {
> -		hpage_size /= 1024;
> +	if (hpage_size >= SZ_1K) {
> +		hpage_size /= SZ_1K;
>  		mod = 'M';
>  	}
>  	seq_printf(m, ",pagesize=%lu%c", hpage_size, mod);
> --
> 2.31.0

Re: [PATCH] mm/hugetlbfs: use macro SZ_1K to replace 1024

Posted by haoxin 3 years, 6 months ago

在 2022/9/15 上午12:43, Al Viro 写道:
> On Wed, Sep 14, 2022 at 07:57:23PM +0800, Xin Hao wrote:
>> Using macro SZ_1K in hugetlbfs_show_options() has no any functional
>> changes, just makes code more readable.
> Why is it any more readable that way?
The main purpose of this code is to convert size to 'K', and 'M'. I 
think using SZ_1k does look more appropriate than 1024, that's all.
>> Signed-off-by: Xin Hao <xhao@linux.alibaba.com>
>> ---
>>   fs/hugetlbfs/inode.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
>> index f7a5b5124d8a..9b9784ffe8de 100644
>> --- a/fs/hugetlbfs/inode.c
>> +++ b/fs/hugetlbfs/inode.c
>> @@ -1023,10 +1023,10 @@ static int hugetlbfs_show_options(struct seq_file *m, struct dentry *root)
>>   	if (sbinfo->max_inodes != -1)
>>   		seq_printf(m, ",nr_inodes=%lu", sbinfo->max_inodes);
>>
>> -	hpage_size /= 1024;
>> +	hpage_size /= SZ_1K;
>>   	mod = 'K';
>> -	if (hpage_size >= 1024) {
>> -		hpage_size /= 1024;
>> +	if (hpage_size >= SZ_1K) {
>> +		hpage_size /= SZ_1K;
>>   		mod = 'M';
>>   	}
>>   	seq_printf(m, ",pagesize=%lu%c", hpage_size, mod);
>> --
>> 2.31.0

RE: [PATCH] mm/hugetlbfs: use macro SZ_1K to replace 1024

Posted by David Laight 3 years, 6 months ago

From: haoxin
> Sent: 15 September 2022 03:45
> 
> åœ¨ 2022/9/15 ä¸Šå�ˆ12:43, Al Viro å†™é�“:
> > On Wed, Sep 14, 2022 at 07:57:23PM +0800, Xin Hao wrote:
> >> Using macro SZ_1K in hugetlbfs_show_options() has no any functional
> >> changes, just makes code more readable.
> > Why is it any more readable that way?

> The main purpose of this code is to convert size to 'K', and 'M'. I
> think using SZ_1k does look more appropriate than 1024, that's all.

You'd need to use SZ_1M/SZ_1K if you want to be descriptive.
But really it just uses up more brain cycles to read.

Using named constants for things that are absolutely constant
isn't really necessary.

If you have '#define ONE 1' are you ever going to set it to
a different value?

And don't even think about what happens to systems that had
small integer constants at 'well known' addresses and then
managed to change one of them.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)