linux-kernel - Re: [PATCH] hugetlbfs: show pagesize in unit of GB if possible

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c755cad3-1020-46bc-2c4c-7e382ad366f5@google.com>
Date:   Mon, 1 Feb 2021 13:31:53 -0800 (PST)
From:   David Rientjes <rientjes@...gle.com>
To:     Miaohe Lin <linmiaohe@...wei.com>
cc:     akpm@...ux-foundation.org, mike.kravetz@...cle.com,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hugetlbfs: show pagesize in unit of GB if possible

On Mon, 1 Feb 2021, Miaohe Lin wrote:

> >> Hugepage size in unit of GB is supported. We could show pagesize in unit of
> >> GB to make it more friendly to read. Also rework the calculation code of
> >> page size unit to make it more readable.
> >>
> >> Signed-off-by: Miaohe Lin <linmiaohe@...wei.com>
> >> ---
> >>  fs/hugetlbfs/inode.c | 12 ++++++++----
> >>  1 file changed, 8 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> >> index 3a08fbae3b53..40a9795f250a 100644
> >> --- a/fs/hugetlbfs/inode.c
> >> +++ b/fs/hugetlbfs/inode.c
> >> @@ -1014,11 +1014,15 @@ 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;
> >> -	mod = 'K';
> >> -	if (hpage_size >= 1024) {
> >> -		hpage_size /= 1024;
> >> +	if (hpage_size >= SZ_1G) {
> >> +		hpage_size /= SZ_1G;
> >> +		mod = 'G';
> >> +	} else if (hpage_size >= SZ_1M) {
> >> +		hpage_size /= SZ_1M;
> >>  		mod = 'M';
> >> +	} else {
> >> +		hpage_size /= SZ_1K;
> >> +		mod = 'K';
> >>  	}
> >>  	seq_printf(m, ",pagesize=%lu%c", hpage_size, mod);
> >>  	if (spool) {
> > 
> > NACK, this can break existing userspace parsing.
> > .
> > 
> 
> I see. I should take care of this. Many thanks.
> 

Thanks.  It's an important point to emphasize because it shows how 
important user-facing interfaces are.  Once the hugetlbfs mount options 
are printed in the way they are, it becomes very difficult to change them 
because there can be userspace in the wild that would break as a result.  
This is why it's crucial to be very careful when specifying user-facing 
interfaces the first time so they are extensible.