On Tue, Jun 14, 2011 at 5:19 AM, 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.
>>
>> 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);
>
> The layout driver better be passed the status on the done method
> rather than on release so that it can roll back on error.
>
> Although it is quite complicated to roll back after permanent errors like
> NFS4ERR_BADLAYOUT where the client is really screwed and it
> essentially needs to redirty and rewrite the data (to the MDS
> to simplify the error handling path), rolling back from
> transient errors like NFS4ERR_DELAY should be fairly easy.
I agree that it can be put in layoutcommit_done. But why is it related
to rolling back in error case? IMHO, layoutcommit error handling
should be implemented in generic code. e.g., for NFS4ERR_DELAY,
current code will retry layoutcommit in generic layer.
pnfs_cleanup_layoutcommit is simply an interface for layout driver to
cleanup its private data associated with this layoutcommit operation.
For block layout specifically, clean up commiting extent list.
Thanks,
Tao
>
> Benny
>
>> Â Â Â /* 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 */
>> Â Â Â *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;
>> Â Â Â 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,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âconst struct nfs4_layoutreturn_args *args);
>>
>> + Â Â Â Âvoid (*cleanup_layoutcommit) (struct pnfs_layout_hdr *layoutid,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âstruct nfs4_layoutcommit_data *data);
>> +
>> Â Â Â 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;
>> Â};
>>
>> Â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
>
--
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
