On Tue, Jun 14, 2011 at 11:10 PM, Benny Halevy <bhalevy.lists@xxxxxxxxx> wrote:
> On 2011-06-12 19:44, Jim Rees wrote:
>> From: Peng Tao <bergwolf@xxxxxxxxx>
>>
>> This gives layout driver a chance to cleanup structures they put in.
>> Also ensure layoutcommit does not commit more than isize, as block layout
>> driver may dirty pages beyond EOF.
>
> let's separate the latter matter into a different patch so we can
> discuss the problem and the solution orthogonally to cleanup_layoutcommit.
>
>>
>> Signed-off-by: Andy Adamson <andros@xxxxxxxxxx>
>> [fixup layout header pointer for layoutcommit]
>> Signed-off-by: Benny Halevy <bhalevy@xxxxxxxxxxx>
>> Signed-off-by: Peng Tao <bergwolf@xxxxxxxxx>
>> ---
>> Âfs/nfs/nfs4proc.c    |  Â1 +
>> Âfs/nfs/nfs4xdr.c    Â|  Â3 ++-
>> Âfs/nfs/pnfs.c      |  15 +++++++++++++++
>> Âfs/nfs/pnfs.h      |  Â4 ++++
>> Âinclude/linux/nfs_xdr.h | Â Â1 +
>> Â5 files changed, 23 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 5246db8..e27a648 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -5890,6 +5890,7 @@ static void nfs4_layoutcommit_release(void *calldata)
>> Â{
>> Â Â Â struct nfs4_layoutcommit_data *data = calldata;
>>
>> + Â Â pnfs_cleanup_layoutcommit(data->args.inode, data);
>> Â Â Â /* Matched by references in pnfs_set_layoutcommit */
>> Â Â Â put_lseg(data->lseg);
>> Â Â Â put_rpccred(data->cred);
>> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
>> index fdcbd8f..57295d1 100644
>> --- a/fs/nfs/nfs4xdr.c
>> +++ b/fs/nfs/nfs4xdr.c
>> @@ -1963,7 +1963,7 @@ encode_layoutcommit(struct xdr_stream *xdr,
>> Â Â Â *p++ = cpu_to_be32(OP_LAYOUTCOMMIT);
>> Â Â Â /* Only whole file layouts */
>> Â Â Â p = xdr_encode_hyper(p, 0); /* offset */
>> - Â Â p = xdr_encode_hyper(p, NFS4_MAX_UINT64); /* length */
>> + Â Â p = xdr_encode_hyper(p, args->lastbytewritten+1); /* length */
>
> This is unrelated to this particular patch and it should be discussed separately.
> (and dropped altogether :)
>
>> Â Â Â *p++ = cpu_to_be32(0); /* reclaim */
>> Â Â Â p = xdr_encode_opaque_fixed(p, args->stateid.data, NFS4_STATEID_SIZE);
>> Â Â Â *p++ = cpu_to_be32(1); /* newoffset = TRUE */
>> @@ -5467,6 +5467,7 @@ static int decode_layoutcommit(struct xdr_stream *xdr,
>> Â Â Â int status;
>>
>> Â Â Â status = decode_op_hdr(xdr, OP_LAYOUTCOMMIT);
>> + Â Â res->status = status;
>
> What is res->status used for?
block layout driver use it to determine if it should clean up
commiting extent list or put them back to dirty extent list (which is
probably wrong but it remains to be seen when layoutcommit error
handling for layoutcommit is implemented in generic layer).
>
>> Â Â Â if (status)
>> Â Â Â Â Â Â Â return status;
>>
>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>> index e693718..48a06a1 100644
>> --- a/fs/nfs/pnfs.c
>> +++ b/fs/nfs/pnfs.c
>> @@ -1248,6 +1248,7 @@ pnfs_set_layoutcommit(struct nfs_write_data *wdata)
>> Â{
>> Â Â Â struct nfs_inode *nfsi = NFS_I(wdata->inode);
>> Â Â Â loff_t end_pos = wdata->mds_offset + wdata->res.count;
>> + Â Â loff_t isize = i_size_read(wdata->inode);
>> Â Â Â bool mark_as_dirty = false;
>>
>> Â Â Â spin_lock(&nfsi->vfs_inode.i_lock);
>> @@ -1261,9 +1262,13 @@ pnfs_set_layoutcommit(struct nfs_write_data *wdata)
>> Â Â Â Â Â Â Â dprintk("%s: Set layoutcommit for inode %lu ",
>> Â Â Â Â Â Â Â Â Â Â Â __func__, wdata->inode->i_ino);
>> Â Â Â }
>> + Â Â if (end_pos > isize)
>> + Â Â Â Â Â Â end_pos = isize;
>> Â Â Â if (end_pos > wdata->lseg->pls_end_pos)
>> Â Â Â Â Â Â Â wdata->lseg->pls_end_pos = end_pos;
>> Â Â Â spin_unlock(&nfsi->vfs_inode.i_lock);
>> + Â Â dprintk("%s: lseg %p end_pos %llu\n",
>> + Â Â Â Â Â Â __func__, wdata->lseg, wdata->lseg->pls_end_pos);
>>
>> Â Â Â /* if pnfs_layoutcommit_inode() runs between inode locks, the next one
>> Â Â Â Â* will be a noop because NFS_INO_LAYOUTCOMMIT will not be set */
>> @@ -1272,6 +1277,16 @@ pnfs_set_layoutcommit(struct nfs_write_data *wdata)
>> Â}
>> ÂEXPORT_SYMBOL_GPL(pnfs_set_layoutcommit);
>>
>> +void pnfs_cleanup_layoutcommit(struct inode *inode,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct nfs4_layoutcommit_data *data)
>> +{
>> + Â Â Â Âstruct nfs_server *nfss = NFS_SERVER(inode);
>> +
>> + Â Â Â Âif (nfss->pnfs_curr_ld->cleanup_layoutcommit)
>> + Â Â Â Â Â Â Â Ânfss->pnfs_curr_ld->cleanup_layoutcommit(
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ÂNFS_I(inode)->layout, data);
>> +}
>> +
>> Âvoid pnfs_free_fsdata(struct pnfs_fsdata *fsdata)
>> Â{
>> Â Â Â /* lseg refcounting handled directly in nfs_write_end */
>> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
>> index 525ec55..5048898 100644
>> --- a/fs/nfs/pnfs.h
>> +++ b/fs/nfs/pnfs.h
>> @@ -127,6 +127,9 @@ struct pnfs_layoutdriver_type {
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âstruct xdr_stream *xdr,it:
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âconst struct nfs4_layoutreturn_args *args);
>>
>> + Â Â Â Âvoid (*cleanup_layoutcommit) (struct pnfs_layout_hdr *layoutid,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âstruct nfs4_layoutcommit_data *data);
>> +
>
> nit: whitespace cleanup required...
>
>> Â Â Â void (*encode_layoutcommit) (struct pnfs_layout_hdr *layoutid,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âstruct xdr_stream *xdr,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âconst struct nfs4_layoutcommit_args *args);
>> @@ -213,6 +216,7 @@ void pnfs_roc_release(struct inode *ino);
>> Âvoid pnfs_roc_set_barrier(struct inode *ino, u32 barrier);
>> Âbool pnfs_roc_drain(struct inode *ino, u32 *barrier);
>> Âvoid pnfs_set_layoutcommit(struct nfs_write_data *wdata);
>> +void pnfs_cleanup_layoutcommit(struct inode *inode, struct nfs4_layoutcommit_data *data);
>> Âint pnfs_layoutcommit_inode(struct inode *inode, bool sync);
>> Âint _pnfs_return_layout(struct inode *);
>> Âint pnfs_ld_write_done(struct nfs_write_data *);
>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>> index a9c43ba..2c3ffda 100644
>> --- a/include/linux/nfs_xdr.h
>> +++ b/include/linux/nfs_xdr.h
>> @@ -270,6 +270,7 @@ struct nfs4_layoutcommit_res {
>> Â Â Â struct nfs_fattr *fattr;
>> Â Â Â const struct nfs_server *server;
>> Â Â Â struct nfs4_sequence_res seq_res;
>> + Â Â int status;
>
> This seems to be unused in this patch...
>
> Benny
>
>> Â};
>>
>> Âstruct nfs4_layoutcommit_data {
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
>
--
Thanks,
-Bergwolf
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
