* Re: [PATCH] FIXME: BUG wdata->mds_offset never gets set
2011-06-15 21:43 [PATCH] FIXME: BUG wdata->mds_offset never gets set Boaz Harrosh
@ 2011-06-16 2:50 ` Benny Halevy
2011-06-16 12:52 ` Boaz Harrosh
2011-06-16 15:35 ` [PATCH 1/3] pnfs: write: Set mds_offset in the generic layer - it is needed by all LDs Boaz Harrosh
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Benny Halevy @ 2011-06-16 2:50 UTC (permalink / raw)
To: Boaz Harrosh; +Cc: Benny Halevy, NFS list
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 <bharrosh@panasas.com>
> ---
> 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
benny@tonian.com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] FIXME: BUG wdata->mds_offset never gets set
2011-06-16 2:50 ` Benny Halevy
@ 2011-06-16 12:52 ` Boaz Harrosh
0 siblings, 0 replies; 6+ messages in thread
From: Boaz Harrosh @ 2011-06-16 12:52 UTC (permalink / raw)
To: Benny Halevy; +Cc: Benny Halevy, NFS list
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 <bharrosh@panasas.com>
>> ---
>> 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;
>> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] pnfs: write: Set mds_offset in the generic layer - it is needed by all LDs
2011-06-15 21:43 [PATCH] FIXME: BUG wdata->mds_offset never gets set Boaz Harrosh
2011-06-16 2:50 ` Benny Halevy
@ 2011-06-16 15:35 ` Boaz Harrosh
2011-06-16 15:37 ` [PATCH 2/3] SQUASHME pnfs-blocks: mds_offset is set in the generic layer Boaz Harrosh
2011-06-16 15:38 ` [PATCH 3/3] SQUASHME: pnfs-obj: " Boaz Harrosh
3 siblings, 0 replies; 6+ messages in thread
From: Boaz Harrosh @ 2011-06-16 15:35 UTC (permalink / raw)
To: Benny Halevy, NFS list, Trond Myklebust
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 <bharrosh@panasas.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] SQUASHME pnfs-blocks: mds_offset is set in the generic layer
2011-06-15 21:43 [PATCH] FIXME: BUG wdata->mds_offset never gets set Boaz Harrosh
2011-06-16 2:50 ` Benny Halevy
2011-06-16 15:35 ` [PATCH 1/3] pnfs: write: Set mds_offset in the generic layer - it is needed by all LDs Boaz Harrosh
@ 2011-06-16 15:37 ` Boaz Harrosh
2011-06-16 15:38 ` [PATCH 3/3] SQUASHME: pnfs-obj: " Boaz Harrosh
3 siblings, 0 replies; 6+ messages in thread
From: Boaz Harrosh @ 2011-06-16 15:37 UTC (permalink / raw)
To: Benny Halevy, NFS list
Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] SQUASHME: pnfs-obj: mds_offset is set in the generic layer
2011-06-15 21:43 [PATCH] FIXME: BUG wdata->mds_offset never gets set Boaz Harrosh
` (2 preceding siblings ...)
2011-06-16 15:37 ` [PATCH 2/3] SQUASHME pnfs-blocks: mds_offset is set in the generic layer Boaz Harrosh
@ 2011-06-16 15:38 ` Boaz Harrosh
3 siblings, 0 replies; 6+ messages in thread
From: Boaz Harrosh @ 2011-06-16 15:38 UTC (permalink / raw)
To: Benny Halevy, NFS list
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 <bharrosh@panasas.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread