From: Benjamin Coddington <bcodding@redhat.com>
To: Olga Kornievskaia <olga.kornievskaia@gmail.com>
Cc: trond.myklebust@hammerspace.com, anna.schumaker@netapp.com,
linux-nfs@vger.kernel.org
Subject: Re: [PATCH 1/1] pNFS/filelayout: check layout segment range
Date: Wed, 08 May 2024 14:23:53 -0400 [thread overview]
Message-ID: <33B5E0C2-9B94-4C54-BD6F-475E4AC0EB75@redhat.com> (raw)
In-Reply-To: <CAN-5tyFWrqihMv8UwvAmrcxuFdM6S5qMvnZgWkJqWXm6iFJeVQ@mail.gmail.com>
On 8 May 2024, at 13:52, Olga Kornievskaia wrote:
> On Wed, May 8, 2024 at 10:50 AM Benjamin Coddington <bcodding@redhat.com> wrote:
>>
>> On 7 May 2024, at 15:59, Olga Kornievskaia wrote:
>>
>>> From: Olga Kornievskaia <kolga@netapp.com>
>>>
>>> Before doing the IO, check that we have the layout covering the range of
>>> IO.
>>>
>>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>>> ---
>>> fs/nfs/filelayout/filelayout.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/fs/nfs/filelayout/filelayout.c b/fs/nfs/filelayout/filelayout.c
>>> index 85d2dc9bc212..bf3ba2e98f33 100644
>>> --- a/fs/nfs/filelayout/filelayout.c
>>> +++ b/fs/nfs/filelayout/filelayout.c
>>> @@ -868,6 +868,7 @@ filelayout_pg_init_read(struct nfs_pageio_descriptor *pgio,
>>> struct nfs_page *req)
>>> {
>>> pnfs_generic_pg_check_layout(pgio);
>>> + pnfs_generic_pg_check_range(pgio, req);
>>> if (!pgio->pg_lseg) {
>>> pgio->pg_lseg = fl_pnfs_update_layout(pgio->pg_inode,
>>> nfs_req_openctx(req),
>>> @@ -892,6 +893,7 @@ filelayout_pg_init_write(struct nfs_pageio_descriptor *pgio,
>>> struct nfs_page *req)
>>> {
>>> pnfs_generic_pg_check_layout(pgio);
>>> + pnfs_generic_pg_check_range(pgio, req);
>>> if (!pgio->pg_lseg) {
>>> pgio->pg_lseg = fl_pnfs_update_layout(pgio->pg_inode,
>>> nfs_req_openctx(req),
>>> --
>>> 2.39.1
>>
>> Looks right, less duplication to just call pnfs_generic_pg_check_range()
>> from pnfs_generic_pg_check_layout() now.
>
> filelayout.c is not the only caller of pnfs_generic_pg_check_layout().
> flexfilelayout has a wrapper around those 2 functions and calls that.
> however, the argument about duplicated code frustrates me because
> currently the code has 4lines. but if we were to re-write the same
> with a function, it would be more lines used in total (flexfiles has 8
> lines for it).
sorry - not trying to frustrate. Since everyone is now calling both
pnfs_generic_pg_check_layout /and/ pnfs_generic_pg_check_range in the same
place, I thought you could just put
!pnfs_lseg_request_intersecting(pgio->pg_lseg, req) in the first test within
pnfs_generic_pg_check_layout(pgio, req), making that function do the work of
both. Then you can delete pnfs_generic_pg_check_range entirely for
everyone. The name still makes sense.
Ben
prev parent reply other threads:[~2024-05-08 18:24 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-07 19:59 [PATCH 1/1] pNFS/filelayout: check layout segment range Olga Kornievskaia
2024-05-08 14:50 ` Benjamin Coddington
2024-05-08 17:52 ` Olga Kornievskaia
2024-05-08 17:55 ` Olga Kornievskaia
2024-05-08 18:23 ` Benjamin Coddington [this message]
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=33B5E0C2-9B94-4C54-BD6F-475E4AC0EB75@redhat.com \
--to=bcodding@redhat.com \
--cc=anna.schumaker@netapp.com \
--cc=linux-nfs@vger.kernel.org \
--cc=olga.kornievskaia@gmail.com \
--cc=trond.myklebust@hammerspace.com \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).