Received-SPF: pass (google.com: domain of linux-nfs+bounces-3213-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240507195933.45683-1-olga.kornievskaia@gmail.com>
 <9D9DB9E5-E772-462B-BD38-7F703459A0FC@redhat.com> <CAN-5tyFWrqihMv8UwvAmrcxuFdM6S5qMvnZgWkJqWXm6iFJeVQ@mail.gmail.com>
In-Reply-To: <CAN-5tyFWrqihMv8UwvAmrcxuFdM6S5qMvnZgWkJqWXm6iFJeVQ@mail.gmail.com>
From: Olga Kornievskaia <olga.kornievskaia@gmail.com>
Date: Wed, 8 May 2024 13:55:42 -0400
Message-ID: <CAN-5tyGx-Q1szxNvRK=+Msxu76-Xwiyg9u+rp0NxxT53ha-Q+Q@mail.gmail.com>
Subject: Re: [PATCH 1/1] pNFS/filelayout: check layout segment range
To: Benjamin Coddington <bcodding@redhat.com>
Cc: trond.myklebust@hammerspace.com, anna.schumaker@netapp.com, 
	linux-nfs@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, May 8, 2024 at 1:52=E2=80=AFPM Olga Kornievskaia
<olga.kornievskaia@gmail.com> wrote:
>
> On Wed, May 8, 2024 at 10:50=E2=80=AFAM Benjamin Coddington <bcodding@red=
hat.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.

I should add that this patch extends previously posted 2 patches by
Anna for the partial layout support for filelayout (as in they go
together).

> > >
> > > 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/filel=
ayout.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_descrip=
tor *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 =3D fl_pnfs_update_layout(pgio->pg_inode,
> > >                                                     nfs_req_openctx(r=
eq),
> > > @@ -892,6 +893,7 @@ filelayout_pg_init_write(struct nfs_pageio_descri=
ptor *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 =3D fl_pnfs_update_layout(pgio->pg_inode,
> > >                                                     nfs_req_openctx(r=
eq),
> > > --
> > > 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).
>
> >
> > Reviewed-by: Benjamin Coddington <bcodding@redhat.com>
> >
> > Ben
> >