From: Charan Teja Kalla <quic_charante@quicinc.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: Mark Hemment <markhemm@googlemail.com>, <hughd@google.com>,
Andrew Morton <akpm@linux-foundation.org>, <vbabka@suse.cz>,
<rientjes@google.com>, <mhocko@suse.com>, <surenb@google.com>,
<shakeelb@google.com>, <linux-mm@kvack.org>,
<linux-kernel@vger.kernel.org>,
Charan Teja Reddy <charante@codeaurora.org>
Subject: Re: [PATCH v3 RESEND] mm: shmem: implement POSIX_FADV_[WILL|DONT]NEED for shmem
Date: Wed, 12 Jan 2022 19:05:47 +0530 [thread overview]
Message-ID: <eb056201-a6e2-a58e-dd77-666f124d1896@quicinc.com> (raw)
In-Reply-To: <Yd7VZId4IlKd4VpC@casper.infradead.org>
Thanks Matthew for the review.
On 1/12/2022 6:49 PM, Matthew Wilcox wrote:
> On Wed, Jan 12, 2022 at 01:51:55PM +0530, Charan Teja Kalla wrote:
>>>>> + rcu_read_lock();
>>>>> + xas_for_each(&xas, page, end) {
>>>>> + if (!xa_is_value(page))
>>>>> + continue;
>>>>> + xas_pause(&xas);
>>>>> + rcu_read_unlock();
>>>>> +
>>>>> + page = shmem_read_mapping_page(mapping, xas.xa_index);
>>>>> + if (!IS_ERR(page))
>>>>> + put_page(page);
>>>>> +
>>>>> + rcu_read_lock();
>>>>> + if (need_resched()) {
>>>>> + xas_pause(&xas);
>>>>> + cond_resched_rcu();
>>>>> + }
>>>>> + }
>>>>> + rcu_read_unlock();
>>> Even the xarray documentation says that: If most entries found during a
>>> walk require you to call xas_pause(), the xa_for_each() iterator may be
>>> more appropriate.
>
> Yes. This should obviously be an xa_for_each() loop.
>
ACK.
>>> Since every value entry found in the xarray requires me to do the
>>> xas_pause(), I do agree that xa_for_each() is the appropriate call here.
>>> Will switch to this in the next spin. Waiting for further review
>>> comments on this patch.
>>
>> I also found the below documentation:
>> xa_for_each() will spin if it hits a retry entry; if you intend to see
>> retry entries, you should use the xas_for_each() iterator instead.
>>
>> Since retry entries are expected, I should be using the xas_for_each()
>> with the corrections you had pointed out. Isn't it?
>
> No. You aren't handling retry entries at all; you clearly don't
> expect to see them. Just let the XArray code handle them itself.
ACK.
>
next prev parent reply other threads:[~2022-01-12 13:36 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-06 17:05 [PATCH v3 RESEND] mm: shmem: implement POSIX_FADV_[WILL|DONT]NEED for shmem Charan Teja Reddy
2022-01-07 12:10 ` Mark Hemment
2022-01-10 10:21 ` Charan Teja Kalla
2022-01-12 8:21 ` Charan Teja Kalla
2022-01-12 11:34 ` Mark Hemment
2022-01-12 13:19 ` Matthew Wilcox
2022-01-12 13:35 ` Charan Teja Kalla [this message]
2022-01-18 11:35 ` Charan Teja Kalla
2022-01-18 13:27 ` Matthew Wilcox
2022-01-10 12:36 ` Mark Hemment
2022-01-10 15:14 ` Charan Teja Kalla
2022-01-12 11:38 ` Mark Hemment
2022-01-12 15:43 ` Charan Teja Kalla
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=eb056201-a6e2-a58e-dd77-666f124d1896@quicinc.com \
--to=quic_charante@quicinc.com \
--cc=akpm@linux-foundation.org \
--cc=charante@codeaurora.org \
--cc=hughd@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=markhemm@googlemail.com \
--cc=mhocko@suse.com \
--cc=rientjes@google.com \
--cc=shakeelb@google.com \
--cc=surenb@google.com \
--cc=vbabka@suse.cz \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).