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

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

[Boaz Harrosh]

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 <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;
 }
-- 
1.7.3.4

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

[Benny Halevy]

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

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

[Boaz Harrosh]

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;
>>  }
> 

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

[Boaz Harrosh]

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

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

[Boaz Harrosh]

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

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

[Boaz Harrosh]

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