| 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
| ||
|
Message-ID: <20200930121512.GT10896@quack2.suse.cz>
Date: Wed, 30 Sep 2020 14:15:12 +0200
From: Jan Kara <jack@...e.cz>
To: "Matthew Wilcox (Oracle)" <willy@...radead.org>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
Hugh Dickins <hughd@...gle.com>,
William Kucharski <william.kucharski@...cle.com>,
Johannes Weiner <hannes@...xchg.org>, Jan Kara <jack@...e.cz>,
Yang Shi <yang.shi@...ux.alibaba.com>,
Dave Chinner <dchinner@...hat.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 12/12] mm/filemap: Return only head pages from
find_get_entries
On Mon 14-09-20 14:00:42, Matthew Wilcox (Oracle) wrote:
> All callers now expect head (and base) pages, and can handle multiple
> head pages in a single batch, so make find_get_entries() behave that way.
> Also take the opportunity to make it use the pagevec infrastructure
> instead of open-coding how pvecs behave. This has the side-effect of
> being able to append to a pagevec with existing contents, although we
> don't make use of that functionality anywhere yet.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@...radead.org>
Looks good to me. You can add:
Reviewed-by: Jan Kara <jack@...e.cz>
I'm just curious: What has happened to pagevec_lookup_entries() call in
invalidate_inode_pages2_range()? Your series appears to be based on a tree
where the call already does not exist...
Honza
> ---
> include/linux/pagemap.h | 2 --
> mm/filemap.c | 36 ++++++++----------------------------
> mm/internal.h | 2 ++
> 3 files changed, 10 insertions(+), 30 deletions(-)
>
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index 788c280737e0..50d2c39b47ab 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -413,8 +413,6 @@ static inline struct page *find_subpage(struct page *head, pgoff_t index)
> return head + (index & (thp_nr_pages(head) - 1));
> }
>
> -unsigned find_get_entries(struct address_space *mapping, pgoff_t start,
> - pgoff_t end, struct pagevec *pvec, pgoff_t *indices);
> unsigned find_get_pages_range(struct address_space *mapping, pgoff_t *start,
> pgoff_t end, unsigned int nr_pages,
> struct page **pages);
> diff --git a/mm/filemap.c b/mm/filemap.c
> index a4ad294a34dc..dbe3f7050897 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1807,49 +1807,29 @@ static inline struct page *xas_find_get_entry(struct xa_state *xas,
> * the mapping. The entries are placed in @pvec. find_get_entries()
> * takes a reference on any actual pages it returns.
> *
> - * The search returns a group of mapping-contiguous page cache entries
> - * with ascending indexes. There may be holes in the indices due to
> - * not-present pages.
> + * The entries have ascending indexes. The indices may not be consecutive
> + * due to not-present entries or THPs.
> *
> * Any shadow entries of evicted pages, or swap entries from
> * shmem/tmpfs, are included in the returned array.
> *
> - * If it finds a Transparent Huge Page, head or tail, find_get_entries()
> - * stops at that page: the caller is likely to have a better way to handle
> - * the compound page as a whole, and then skip its extent, than repeatedly
> - * calling find_get_entries() to return all its tails.
> - *
> - * Return: the number of pages and shadow entries which were found.
> + * Return: The number of entries which were found.
> */
> unsigned find_get_entries(struct address_space *mapping, pgoff_t start,
> pgoff_t end, struct pagevec *pvec, pgoff_t *indices)
> {
> XA_STATE(xas, &mapping->i_pages, start);
> struct page *page;
> - unsigned int ret = 0;
> - unsigned nr_entries = PAGEVEC_SIZE;
>
> rcu_read_lock();
> while ((page = xas_find_get_entry(&xas, end, XA_PRESENT))) {
> - /*
> - * Terminate early on finding a THP, to allow the caller to
> - * handle it all at once; but continue if this is hugetlbfs.
> - */
> - if (!xa_is_value(page) && PageTransHuge(page) &&
> - !PageHuge(page)) {
> - page = find_subpage(page, xas.xa_index);
> - nr_entries = ret + 1;
> - }
> -
> - indices[ret] = xas.xa_index;
> - pvec->pages[ret] = page;
> - if (++ret == nr_entries)
> + indices[pvec->nr] = xas.xa_index;
> + if (!pagevec_add(pvec, page))
> break;
> }
> rcu_read_unlock();
>
> - pvec->nr = ret;
> - return ret;
> + return pagevec_count(pvec);
> }
>
> /**
> @@ -1868,8 +1848,8 @@ unsigned find_get_entries(struct address_space *mapping, pgoff_t start,
> * not returned.
> *
> * The entries have ascending indexes. The indices may not be consecutive
> - * due to not-present entries, THP pages, pages which could not be locked
> - * or pages under writeback.
> + * due to not-present entries, THPs, pages which could not be locked or
> + * pages under writeback.
> *
> * Return: The number of entries which were found.
> */
> diff --git a/mm/internal.h b/mm/internal.h
> index d17d9060902f..55da9860a54b 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -62,6 +62,8 @@ static inline void force_page_cache_readahead(struct address_space *mapping,
>
> struct page *find_get_entry(struct address_space *mapping, pgoff_t index);
> struct page *find_lock_entry(struct address_space *mapping, pgoff_t index);
> +unsigned find_get_entries(struct address_space *mapping, pgoff_t start,
> + pgoff_t end, struct pagevec *pvec, pgoff_t *indices);
> unsigned find_lock_entries(struct address_space *mapping, pgoff_t start,
> pgoff_t end, struct pagevec *pvec, pgoff_t *indices);
>
> --
> 2.28.0
>
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists