2011-06-15 21:43:47

by Boaz Harrosh

[permalink] [raw]
Subject: [PATCH] FIXME: BUG wdata->mds_offset never gets set


The fileslayout and blockslayout drivers had a set of
wdata->mds_offset in their .write_pagelist member.

The objects driver did not. Which breaks layout_commit.

FIXME: Since all drivers set mds_offset in exactly the same place
to the same value. And then never touch it. It calls for the
generic layer to take care of it.
(I'll send the fix tomorrow)

Signed-off-by: Boaz Harrosh <[email protected]>
---
fs/nfs/objlayout/objlayout.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c
index dc3956c..96dd474 100644
--- a/fs/nfs/objlayout/objlayout.c
+++ b/fs/nfs/objlayout/objlayout.c
@@ -430,6 +430,8 @@ objlayout_write_pagelist(struct nfs_write_data *wdata,
status = objio_write_pagelist(state, how & FLUSH_STABLE);
out:
dprintk("%s: Return status %Zd\n", __func__, status);
+ /* pnfs_set_layoutcommit needs this */
+ wdata->mds_offset = wdata->args.offset;
wdata->pnfs_error = status;
return PNFS_ATTEMPTED;
}
--
1.7.3.4



2011-06-16 12:52:22

by Boaz Harrosh

[permalink] [raw]
Subject: Re: [PATCH] FIXME: BUG wdata->mds_offset never gets set

On 06/15/2011 10:50 PM, Benny Halevy wrote:
> On 2011-06-15 17:43, Boaz Harrosh wrote:
>>
>> The fileslayout and blockslayout drivers had a set of
>> wdata->mds_offset in their .write_pagelist member.
>>
>> The objects driver did not. Which breaks layout_commit.
>>
>> FIXME: Since all drivers set mds_offset in exactly the same place
>> to the same value. And then never touch it. It calls for the
>> generic layer to take care of it.
>> (I'll send the fix tomorrow)
>
> Thanks, that's indeed a fallout from 4b8ee2b
> "nfs41: Correct offset for LAYOUTCOMMIT"
>
> I merged this also for pnfs-all-2.6.39
>
> Benny
>

I talked to both Fred And Andy and they agreed with me that we should
move the assignment to the generic layer. It's a layering violation
this way. I'll send you a patch today

Thanks
Boaz

>>
>> Signed-off-by: Boaz Harrosh <[email protected]>
>> ---
>> fs/nfs/objlayout/objlayout.c | 2 ++
>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c
>> index dc3956c..96dd474 100644
>> --- a/fs/nfs/objlayout/objlayout.c
>> +++ b/fs/nfs/objlayout/objlayout.c
>> @@ -430,6 +430,8 @@ objlayout_write_pagelist(struct nfs_write_data *wdata,
>> status = objio_write_pagelist(state, how & FLUSH_STABLE);
>> out:
>> dprintk("%s: Return status %Zd\n", __func__, status);
>> + /* pnfs_set_layoutcommit needs this */
>> + wdata->mds_offset = wdata->args.offset;
>> wdata->pnfs_error = status;
>> return PNFS_ATTEMPTED;
>> }
>


2011-06-16 02:50:16

by Benny Halevy

[permalink] [raw]
Subject: Re: [PATCH] FIXME: BUG wdata->mds_offset never gets set

On 2011-06-15 17:43, Boaz Harrosh wrote:
>
> The fileslayout and blockslayout drivers had a set of
> wdata->mds_offset in their .write_pagelist member.
>
> The objects driver did not. Which breaks layout_commit.
>
> FIXME: Since all drivers set mds_offset in exactly the same place
> to the same value. And then never touch it. It calls for the
> generic layer to take care of it.
> (I'll send the fix tomorrow)

Thanks, that's indeed a fallout from 4b8ee2b
"nfs41: Correct offset for LAYOUTCOMMIT"

I merged this also for pnfs-all-2.6.39

Benny

>
> Signed-off-by: Boaz Harrosh <[email protected]>
> ---
> fs/nfs/objlayout/objlayout.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c
> index dc3956c..96dd474 100644
> --- a/fs/nfs/objlayout/objlayout.c
> +++ b/fs/nfs/objlayout/objlayout.c
> @@ -430,6 +430,8 @@ objlayout_write_pagelist(struct nfs_write_data *wdata,
> status = objio_write_pagelist(state, how & FLUSH_STABLE);
> out:
> dprintk("%s: Return status %Zd\n", __func__, status);
> + /* pnfs_set_layoutcommit needs this */
> + wdata->mds_offset = wdata->args.offset;
> wdata->pnfs_error = status;
> return PNFS_ATTEMPTED;
> }

--
Benny Halevy
CTO, Tonian Inc.

Tel: +972-54-802-8340
[email protected]

2011-06-16 15:38:35

by Boaz Harrosh

[permalink] [raw]
Subject: [PATCH 3/3] SQUASHME: pnfs-obj: mds_offset is set in the generic layer


Benny you can just remove the previous patch that adds it:
b5157c1 FIXME: BUG wdata->mds_offset never gets set

This patch is just a reminder

Signed-off-by: Boaz Harrosh <[email protected]>
---
fs/nfs/objlayout/objlayout.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c
index e76cdab..1d06f8e 100644
--- a/fs/nfs/objlayout/objlayout.c
+++ b/fs/nfs/objlayout/objlayout.c
@@ -430,8 +430,6 @@ objlayout_write_pagelist(struct nfs_write_data *wdata,
status = objio_write_pagelist(state, how & FLUSH_STABLE);
out:
dprintk("%s: Return status %Zd\n", __func__, status);
- /* pnfs_set_layoutcommit needs this */
- wdata->mds_offset = wdata->args.offset;
wdata->pnfs_error = status;
return PNFS_ATTEMPTED;
}
--
1.7.3.4



2011-06-16 15:37:28

by Boaz Harrosh

[permalink] [raw]
Subject: [PATCH 2/3] SQUASHME pnfs-blocks: mds_offset is set in the generic layer


Signed-off-by: Boaz Harrosh <[email protected]>
---
fs/nfs/blocklayout/blocklayout.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index 8843df4..8531fd7 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -495,8 +495,6 @@ bl_write_pagelist(struct nfs_write_data *wdata,
wdata->res.count = (isect << 9) - (offset);
if (count < wdata->res.count)
wdata->res.count = count;
- /* pnfs_set_layoutcommit needs this */
- wdata->mds_offset = offset;
put_extent(be);
bl_submit_bio(WRITE, bio);
put_parallel(par);
--
1.7.3.4



2011-06-16 15:35:49

by Boaz Harrosh

[permalink] [raw]
Subject: [PATCH 1/3] pnfs: write: Set mds_offset in the generic layer - it is needed by all LDs


In current pnfs tree, all the layouts set mds_offset in their
.write_pagelist member.
mds_offset is only used by generic layer and should be handled by it.

This patch is for upstream. It is needed in this -rc series to fix a
bug in objects layout_commit.

I'll send patches for objects and blocks to be
squashed into current pnfs tree.

TODO: It looks like the read path needs the same patch.

Signed-off-by: Boaz Harrosh <[email protected]>
---
fs/nfs/nfs4filelayout.c | 1 -
fs/nfs/write.c | 2 ++
2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
index dcde4e0..472f81f 100644
--- a/fs/nfs/nfs4filelayout.c
+++ b/fs/nfs/nfs4filelayout.c
@@ -396,7 +396,6 @@ filelayout_write_pagelist(struct nfs_write_data *data, int sync)
* this offset and save the original offset.
*/
data->args.offset = filelayout_get_dserver_offset(lseg, offset);
- data->mds_offset = offset;

/* Perform an asynchronous write */
status = nfs_initiate_write(data, ds->ds_clp->cl_rpcclient,
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 383ee81..1185262 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -868,6 +868,8 @@ static int nfs_write_rpcsetup(struct nfs_page *req,

data->args.fh = NFS_FH(inode);
data->args.offset = req_offset(req) + offset;
+ /* pnfs_set_layoutcommit needs this */
+ data->mds_offset = data->args.offset;
data->args.pgbase = req->wb_pgbase + offset;
data->args.pages = data->pagevec;
data->args.count = count;
--
1.7.3.4