From: Peng Tao <bergwolf@gmail.com>
To: Benny Halevy <bhalevy.lists@gmail.com>
Cc: Jim Rees <rees@umich.edu>,
linux-nfs@vger.kernel.org, peter honeyman <honey@citi.umich.edu>
Subject: Re: [PATCH 06/34] pnfs: cleanup_layoutcommit
Date: Tue, 14 Jun 2011 23:16:51 +0800 [thread overview]
Message-ID: <BANLkTinB__v1EOv05sknuqATRUYmdDAM_w@mail.gmail.com> (raw)
In-Reply-To: <4DF67EF6.6040707@gmail.com>
On Tue, Jun 14, 2011 at 5:19 AM, Benny Halevy <bhalevy.lists@gmail.com> wrote:
> On 2011-06-12 19:44, Jim Rees wrote:
>> From: Peng Tao <bergwolf@gmail.com>
>>
>> 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@netapp.com>
>> [fixup layout header pointer for layoutcommit]
>> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
>> Signed-off-by: Peng Tao <bergwolf@gmail.com>
>> ---
>> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
next prev parent reply other threads:[~2011-06-14 15:17 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-12 23:43 [PATCH 00/34] pnfs block layout driver based on v3.0-rc2 Jim Rees
2011-06-12 23:43 ` [PATCH 01/34] pnfs: GETDEVICELIST Jim Rees
2011-06-12 23:43 ` [PATCH 02/34] pnfs: add set-clear layoutdriver interface Jim Rees
2011-06-12 23:43 ` [PATCH 03/34] pnfs: let layoutcommit code handle multiple segments Jim Rees
2011-06-13 14:36 ` Fred Isaman
2011-06-14 10:40 ` tao.peng
2011-06-14 13:58 ` Fred Isaman
2011-06-14 14:28 ` Benny Halevy
2011-06-12 23:43 ` [PATCH 04/34] pnfs: hook nfs_write_begin/end to allow layout driver manipulation Jim Rees
2011-06-13 14:44 ` Fred Isaman
2011-06-14 11:01 ` tao.peng
2011-06-14 14:05 ` Fred Isaman
2011-06-14 15:53 ` Peng Tao
2011-06-14 16:02 ` Fred Isaman
2011-06-12 23:43 ` [PATCH 05/34] pnfs: ask for layout_blksize and save it in nfs_server Jim Rees
2011-06-14 15:01 ` Benny Halevy
2011-06-14 15:08 ` Peng Tao
2011-06-12 23:44 ` [PATCH 06/34] pnfs: cleanup_layoutcommit Jim Rees
2011-06-13 21:19 ` Benny Halevy
2011-06-14 15:16 ` Peng Tao [this message]
2011-06-14 15:10 ` Benny Halevy
2011-06-14 15:21 ` Peng Tao
2011-06-14 15:19 ` Benny Halevy
2011-06-12 23:44 ` [PATCH 07/34] pnfsblock: define PNFS_BLOCK Kconfig option Jim Rees
2011-06-14 15:13 ` Benny Halevy
2011-06-12 23:44 ` [PATCH 08/34] pnfsblock: blocklayout stub Jim Rees
2011-06-12 23:44 ` [PATCH 09/34] pnfsblock: layout alloc and free Jim Rees
2011-06-12 23:44 ` [PATCH 10/34] Add support for simple rpc pipefs Jim Rees
2011-06-12 23:44 ` [PATCH 11/34] pnfs-block: Add block device discovery pipe Jim Rees
2011-06-12 23:44 ` [PATCH 12/34] pnfsblock: basic extent code Jim Rees
2011-06-12 23:44 ` [PATCH 13/34] pnfsblock: add device operations Jim Rees
2011-06-12 23:44 ` [PATCH 14/34] pnfsblock: remove " Jim Rees
2011-06-12 23:44 ` [PATCH 15/34] pnfsblock: lseg alloc and free Jim Rees
2011-06-12 23:44 ` [PATCH 16/34] pnfsblock: merge extents Jim Rees
2011-06-12 23:44 ` [PATCH 17/34] pnfsblock: call and parse getdevicelist Jim Rees
2011-06-14 15:36 ` Benny Halevy
2011-06-12 23:44 ` [PATCH 18/34] pnfsblock: allow use of PG_owner_priv_1 flag Jim Rees
2011-06-13 15:56 ` Fred Isaman
2011-06-12 23:44 ` [PATCH 19/34] pnfsblock: xdr decode pnfs_block_layout4 Jim Rees
2011-06-12 23:44 ` [PATCH 20/34] pnfsblock: find_get_extent Jim Rees
2011-06-12 23:44 ` [PATCH 21/34] pnfsblock: SPLITME: add extent manipulation functions Jim Rees
2011-06-14 15:40 ` Benny Halevy
2011-06-12 23:44 ` [PATCH 22/34] pnfsblock: merge rw extents Jim Rees
2011-06-12 23:44 ` [PATCH 23/34] pnfsblock: encode_layoutcommit Jim Rees
2011-06-14 15:44 ` Benny Halevy
2011-06-12 23:44 ` [PATCH 24/34] pnfsblock: cleanup_layoutcommit Jim Rees
2011-06-12 23:44 ` [PATCH 25/34] pnfsblock: bl_read_pagelist Jim Rees
2011-06-12 23:44 ` [PATCH 26/34] pnfsblock: write_begin Jim Rees
2011-06-12 23:44 ` [PATCH 27/34] pnfsblock: write_end Jim Rees
2011-06-12 23:44 ` [PATCH 28/34] pnfsblock: write_end_cleanup Jim Rees
2011-06-12 23:45 ` [PATCH 29/34] pnfsblock: bl_write_pagelist support functions Jim Rees
2011-06-12 23:45 ` [PATCH 30/34] pnfsblock: bl_write_pagelist Jim Rees
2011-06-12 23:45 ` [PATCH 31/34] pnfsblock: note written INVAL areas for layoutcommit Jim Rees
2011-06-12 23:45 ` [PATCH 32/34] pnfsblock: Implement release_inval_marks Jim Rees
2011-06-12 23:45 ` [PATCH 33/34] Add configurable prefetch size for layoutget Jim Rees
2011-06-12 23:45 ` [PATCH 34/34] NFS41: do not update isize if inode needs layoutcommit Jim Rees
2011-06-14 16:15 ` Benny Halevy
2011-06-14 16:22 ` Fred Isaman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BANLkTinB__v1EOv05sknuqATRUYmdDAM_w@mail.gmail.com \
--to=bergwolf@gmail.com \
--cc=bhalevy.lists@gmail.com \
--cc=honey@citi.umich.edu \
--cc=linux-nfs@vger.kernel.org \
--cc=rees@umich.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).