Re: [PATCH v6] mm,hugetlb: remove mlock ulimit for SHM_HUGETLB

[Mike Kravetz <mike.kravetz@oracle.com>]

On 11/1/21 2:09 PM, zhangyiru wrote:
> commit 21a3c273f88c9cbbaf7e ("mm, hugetlb: add thread name and pid to
> SHM_HUGETLB mlock rlimit warning") marked this as deprecated in 2012,
> but it is not deleted yet.
> 
> Mike says he still see that message in log files on occasion,
> so maybe we should preserve this warning.
> 
> Also remove hugetlbfs related user_shm_unlock in ipc/shm.c.
> 
> Signed-off-by: zhangyiru <zhangyiru3@huawei.com>
> ---
> Changelog:
>  v6: fix errors in v5 and remove hugetlbfs related user_shm_unlock in ipc/shm.c
>  v5: delete ucounts variable, and function calls that use ucounts in
> additon
>  v4: modify context information of obsolete
>  v3: modify warning message to obsolete
>  v2: preserve warning message
>  v1: remove mlock ulimit for SHM_HUGETLB
> ---
>  fs/hugetlbfs/inode.c    | 11 ++++-------
>  include/linux/hugetlb.h |  3 +--
>  ipc/shm.c               |  8 +-------
>  mm/memfd.c              |  4 +---
>  mm/mmap.c               |  3 +--
>  5 files changed, 8 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index cdfb1ae78a3f..363eec008390 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -1446,19 +1446,19 @@ static int get_hstate_idx(int page_size_log)
>   * otherwise hugetlb_reserve_pages reserves one less hugepages than intended.
>   */
>  struct file *hugetlb_file_setup(const char *name, size_t size,
> -				vm_flags_t acctflag, struct ucounts **ucounts,
> -				int creat_flags, int page_size_log)
> +				vm_flags_t acctflag, int creat_flags,
> +				int page_size_log)
>  {
>  	struct inode *inode;
>  	struct vfsmount *mnt;
>  	int hstate_idx;
>  	struct file *file;
> +	struct ucounts **ucounts;

How about just moving this to the block where it is used?  And no need
for **ucounts, just struct *ucounts should do.

>  
>  	hstate_idx = get_hstate_idx(page_size_log);
>  	if (hstate_idx < 0)
>  		return ERR_PTR(-ENODEV);
>  
> -	*ucounts = NULL;
>  	mnt = hugetlbfs_vfsmount[hstate_idx];
>  	if (!mnt)
>  		return ERR_PTR(-ENOENT);
> @@ -1466,12 +1466,9 @@ struct file *hugetlb_file_setup(const char *name, size_t size,
>  	if (creat_flags == HUGETLB_SHMFS_INODE && !can_do_hugetlb_shm()) {
>  		*ucounts = current_ucounts();
>  		if (user_shm_lock(size, *ucounts)) {
> -			task_lock(current);
>  			pr_warn_once("%s (%d): Using mlock ulimits for SHM_HUGETLB is deprecated\n",

Please do change this to "... is obsolete\n".  I am not sure why there
was a v4 with the change back to deprecated?

>  				current->comm, current->pid);
> -			task_unlock(current);
> -		} else {
> -			*ucounts = NULL;
> +			user_shm_unlock(size, *ucounts);
>  			return ERR_PTR(-EPERM);

I believe this error return should be outside this 'if user_shm_lock()'
block.  After the following '}'.

>  		}

		Place return ERR_PTR(-EPERM); here.

>  	}

Also, need to remove the user_shm_unlock after out: in this function.

-- 
Mike Kravetz