linux-kernel - Re: [PATCH v2 4/9] hugetlb: create remove_inode_single

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]

Date:   Sat, 24 Sep 2022 20:36:53 +0800
From:   Miaohe Lin <linmiaohe@...wei.com>
To:     Mike Kravetz <mike.kravetz@...cle.com>, <linux-mm@...ck.org>,
        <linux-kernel@...r.kernel.org>
CC:     Muchun Song <songmuchun@...edance.com>,
        David Hildenbrand <david@...hat.com>,
        Sven Schnelle <svens@...ux.ibm.com>,
        Michal Hocko <mhocko@...e.com>, Peter Xu <peterx@...hat.com>,
        Naoya Horiguchi <naoya.horiguchi@...ux.dev>,
        "Aneesh Kumar K . V" <aneesh.kumar@...ux.vnet.ibm.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Davidlohr Bueso <dave@...olabs.net>,
        Prakash Sangappa <prakash.sangappa@...cle.com>,
        James Houghton <jthoughton@...gle.com>,
        Mina Almasry <almasrymina@...gle.com>,
        Pasha Tatashin <pasha.tatashin@...een.com>,
        Axel Rasmussen <axelrasmussen@...gle.com>,
        Ray Fucillo <Ray.Fucillo@...ersystems.com>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v2 4/9] hugetlb: create remove_inode_single_folio to
 remove single file folio

On 2022/9/15 6:18, Mike Kravetz wrote:
> Create the new routine remove_inode_single_folio that will remove a
> single folio from a file.  This is refactored code from
> remove_inode_hugepages.  It checks for the uncommon case in which the
> folio is still mapped and unmaps.
> 
> No functional change.  This refactoring will be put to use and expanded
> upon in a subsequent patches.
> 
> Signed-off-by: Mike Kravetz <mike.kravetz@...cle.com>

LGTM with one nit below.

Reviewed-by: Miaohe Lin <linmiaohe@...wei.com>

> ---
>  fs/hugetlbfs/inode.c | 105 ++++++++++++++++++++++++++-----------------
>  1 file changed, 63 insertions(+), 42 deletions(-)
> 
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index edd69cc43ca5..7112a9a9f54d 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -411,6 +411,60 @@ hugetlb_vmdelete_list(struct rb_root_cached *root, pgoff_t start, pgoff_t end,
>  	}
>  }
>  
> +/*
> + * Called with hugetlb fault mutex held.
> + * Returns true if page was actually removed, false otherwise.
> + */
> +static bool remove_inode_single_folio(struct hstate *h, struct inode *inode,
> +					struct address_space *mapping,
> +					struct folio *folio, pgoff_t index,
> +					bool truncate_op)
> +{
> +	bool ret = false;
> +
> +	/*
> +	 * If folio is mapped, it was faulted in after being
> +	 * unmapped in caller.  Unmap (again) while holding
> +	 * the fault mutex.  The mutex will prevent faults
> +	 * until we finish removing the folio.
> +	 */
> +	if (unlikely(folio_mapped(folio))) {
> +		i_mmap_lock_write(mapping);
> +		hugetlb_vmdelete_list(&mapping->i_mmap,
> +			index * pages_per_huge_page(h),
> +			(index + 1) * pages_per_huge_page(h),
> +			ZAP_FLAG_DROP_MARKER);
> +		i_mmap_unlock_write(mapping);
> +	}
> +
> +	folio_lock(folio);
> +	/*
> +	 * After locking page, make sure mapping is the same.
> +	 * We could have raced with page fault populate and
> +	 * backout code.

Is this needed? remove_inode_single_folio() is called with hugetlb fault mutex held so it can't
race with page fault code?

Thanks,
Miaohe Lin