linux-kernel - Re: [PATCH 5/5] hugetlbfs: fix confusing hugetlbfs stat

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: <YtnvFBn8tSm7y/mV@monkey>
Date:   Thu, 21 Jul 2022 17:28:04 -0700
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     Miaohe Lin <linmiaohe@...wei.com>
Cc:     akpm@...ux-foundation.org, songmuchun@...edance.com,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/5] hugetlbfs: fix confusing hugetlbfs stat

On 07/21/22 21:16, Miaohe Lin wrote:
> When size option is not specified, f_blocks, f_bavail and f_bfree will be
> set to -1 instead of 0. Likewise, when nr_inodes is not specified, f_files
> and f_ffree will be set to -1 too. Check max_hpages and max_inodes against
> -1 first to make sure 0 is reported for max/free/used when no limit is set
> as the comment states.

Just curious, where are you seeing values reported as -1?  The check
for sbinfo->spool was supposed to handle these cases.  Seems like it
should handle the max_hpages == -1 case.  But, it doesn't look like it
considers the max_inodes == -1 case.

If I create/mount a hugetlb filesystem without specifying size or nr_inodes,
df seems to report zero instead of -1.

Just want to understand the reasoning behind the change.
-- 
Mike Kravetz

> 
> Signed-off-by: Miaohe Lin <linmiaohe@...wei.com>
> ---
>  fs/hugetlbfs/inode.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index 19fc62a9c2fe..44da9828e171 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -1083,16 +1083,20 @@ static int hugetlbfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  		/* If no limits set, just report 0 for max/free/used
>  		 * blocks, like simple_statfs() */
>  		if (sbinfo->spool) {
> -			long free_pages;
> -
>  			spin_lock_irq(&sbinfo->spool->lock);
> -			buf->f_blocks = sbinfo->spool->max_hpages;
> -			free_pages = sbinfo->spool->max_hpages
> -				- sbinfo->spool->used_hpages;
> -			buf->f_bavail = buf->f_bfree = free_pages;
> +			if (sbinfo->spool->max_hpages != -1) {
> +				long free_pages;
> +
> +				buf->f_blocks = sbinfo->spool->max_hpages;
> +				free_pages = sbinfo->spool->max_hpages
> +					     - sbinfo->spool->used_hpages;
> +				buf->f_bavail = buf->f_bfree = free_pages;
> +			}
>  			spin_unlock_irq(&sbinfo->spool->lock);
> -			buf->f_files = sbinfo->max_inodes;
> -			buf->f_ffree = sbinfo->free_inodes;
> +			if (sbinfo->max_inodes != -1) {
> +				buf->f_files = sbinfo->max_inodes;
> +				buf->f_ffree = sbinfo->free_inodes;
> +			}
>  		}
>  		spin_unlock(&sbinfo->stat_lock);
>  	}
> -- 
> 2.23.0
>