From: Olga Kornievskaia <[email protected]>
Before doing the IO, check that we have the layout covering the range of
IO.
Signed-off-by: Olga Kornievskaia <[email protected]>
---
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
On 7 May 2024, at 15:59, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <[email protected]>
>
> Before doing the IO, check that we have the layout covering the range of
> IO.
>
> Signed-off-by: Olga Kornievskaia <[email protected]>
> ---
> 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.
Reviewed-by: Benjamin Coddington <[email protected]>
Ben
On Wed, May 8, 2024 at 10:50 AM Benjamin Coddington <[email protected]> wrote:
>
> On 7 May 2024, at 15:59, Olga Kornievskaia wrote:
>
> > From: Olga Kornievskaia <[email protected]>
> >
> > Before doing the IO, check that we have the layout covering the range of
> > IO.
> >
> > Signed-off-by: Olga Kornievskaia <[email protected]>
> > ---
> > 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).
>
> Reviewed-by: Benjamin Coddington <[email protected]>
>
> Ben
>
On Wed, May 8, 2024 at 1:52 PM Olga Kornievskaia
<[email protected]> wrote:
>
> On Wed, May 8, 2024 at 10:50 AM Benjamin Coddington <[email protected]> wrote:
> >
> > On 7 May 2024, at 15:59, Olga Kornievskaia wrote:
> >
> > > From: Olga Kornievskaia <[email protected]>
> > >
> > > 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 <[email protected]>
> > > ---
> > > 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).
>
> >
> > Reviewed-by: Benjamin Coddington <[email protected]>
> >
> > Ben
> >
On 8 May 2024, at 13:52, Olga Kornievskaia wrote:
> On Wed, May 8, 2024 at 10:50 AM Benjamin Coddington <[email protected]> wrote:
>>
>> On 7 May 2024, at 15:59, Olga Kornievskaia wrote:
>>
>>> From: Olga Kornievskaia <[email protected]>
>>>
>>> Before doing the IO, check that we have the layout covering the range of
>>> IO.
>>>
>>> Signed-off-by: Olga Kornievskaia <[email protected]>
>>> ---
>>> 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