On Mon, Sep 14, 2020 at 02:34:52PM -0400, Jeff Layton wrote:
> Ceph is also the only caller of pagevec_lookup_range_nr_tag(), so
> changing this code to use pagevec_lookup_range_tag() should allow us to
> eliminate that call as well. That may mean that we sometimes find more
> pages than are needed, but the extra references will just get put at the
> end regardless.
That was the part I wasn't clear about!
So, let's suppose max_pages is 10 and we get 15 pages back.
We'll run the
for (i = 0; i < pvec_pages && locked_pages < max_pages; i++) {
}
loop ten times, then hit:
if (i) {
for (j = 0; j < pvec_pages; j++) {
if (!pvec.pages[j])
continue;
OK, we do that ten times, then
if (n < j)
pvec.pages[n] = pvec.pages[j];
so we now have five pages clustered at the bottom of pvec
pvec.nr = n;
... then we do the new_request: stanza ...
ah, and then we call pagevec_release(&pvec);
and everything is good!
Excellent. I was overwhelmed by the amount of code in this function.
Glad to see the patch was so simple in the end.
Reviewed-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> Reported-by: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/ceph/addr.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> I'm still testing this, but it looks good so far. If it's OK, we'll get
> this in for v5.10, and then I'll send a patch to remove
> pagevec_lookup_range_nr_tag.
>
> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index 6ea761c84494..b03dbaa9d345 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -962,9 +962,8 @@ static int ceph_writepages_start(struct address_space *mapping,
> max_pages = wsize >> PAGE_SHIFT;
>
> get_more_pages:
> - pvec_pages = pagevec_lookup_range_nr_tag(&pvec, mapping, &index,
> - end, PAGECACHE_TAG_DIRTY,
> - max_pages - locked_pages);
> + pvec_pages = pagevec_lookup_range_tag(&pvec, mapping, &index,
> + end, PAGECACHE_TAG_DIRTY);
> dout("pagevec_lookup_range_tag got %d\n", pvec_pages);
> if (!pvec_pages && !locked_pages)
> break;
> --
> 2.26.2
>
