From: Benny Halevy <bhalevy@panasas.com>
To: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: linux-nfs@vger.kernel.org
Subject: Re: [RFC 09/27] pnfs: support for non-rpc layout drivers
Date: Fri, 22 Apr 2011 12:03:32 +0300 [thread overview]
Message-ID: <4DB14464.8060207@panasas.com> (raw)
In-Reply-To: <1303331655.23206.46.camel@lade.trondhjem.org>
On 2011-04-20 23:34, Trond Myklebust wrote:
> On Wed, 2011-04-20 at 20:27 +0300, Benny Halevy wrote:
>> Non-rpc layout driver such as for objects and blocks
>> implement their own I/O path and error handling logic.
>> Therefore bypass NFS-based error handling for these layout drivers.
>>
>> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
>> ---
>> fs/nfs/internal.h | 2 +
>> fs/nfs/nfs4filelayout.c | 1 +
>> fs/nfs/nfs4proc.c | 14 +++++++++++-
>> fs/nfs/pnfs.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>> fs/nfs/pnfs.h | 7 +++++-
>> include/linux/nfs_xdr.h | 2 +
>> 6 files changed, 71 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
>> index ce118ce..1914d2f 100644
>> --- a/fs/nfs/internal.h
>> +++ b/fs/nfs/internal.h
>> @@ -310,6 +310,8 @@ extern int nfs_migrate_page(struct address_space *,
>> #endif
>>
>> /* nfs4proc.c */
>> +extern void __nfs4_read_done_cb(struct nfs_read_data *);
>> +extern void __nfs4_write_done_cb(struct nfs_write_data *);
>> extern void nfs4_reset_read(struct rpc_task *task, struct nfs_read_data *data);
>> extern int nfs4_init_client(struct nfs_client *clp,
>> const struct rpc_timeout *timeparms,
>> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
>> index 2feab7f..e67a0d4 100644
>> --- a/fs/nfs/nfs4filelayout.c
>> +++ b/fs/nfs/nfs4filelayout.c
>> @@ -859,6 +859,7 @@ static struct pnfs_layoutdriver_type filelayout_type = {
>> .id = LAYOUT_NFSV4_1_FILES,
>> .name = "LAYOUT_NFSV4_1_FILES",
>> .owner = THIS_MODULE,
>> + .flags = PNFS_USE_RPC_CODE,
>
> This isn't being used anywhere, so why do I need it in this patch?
>
Sorry, it's just leftovers from the previous version.
I'll get rid of it.
>> .alloc_lseg = filelayout_alloc_lseg,
>> .free_lseg = filelayout_free_lseg,
>> .pg_test = filelayout_pg_test,
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index d0eb50b..cc2cdcd 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -3149,6 +3149,11 @@ static int nfs4_proc_pathconf(struct nfs_server *server, struct nfs_fh *fhandle,
>> return err;
>> }
>>
>> +void __nfs4_read_done_cb(struct nfs_read_data *data)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^ why the wrapper?
To be called from nfs4_read_done_cb and from pnfs_read_done.
I can just call nfs_invalidate_atime from there but since
this is common logic I think the code, small as it is,
should be kept common.
By the way, what about filelayout_read_done_cb()?
shouldn't it invalidate the inode's atime too?
>> +{
>> + nfs_invalidate_atime(data->inode);
>> +}
>> +
>> static int nfs4_read_done_cb(struct rpc_task *task, struct nfs_read_data *data)
>> {
>> struct nfs_server *server = NFS_SERVER(data->inode);
>> @@ -3158,7 +3163,7 @@ static int nfs4_read_done_cb(struct rpc_task *task, struct nfs_read_data *data)
>> return -EAGAIN;
>> }
>>
>> - nfs_invalidate_atime(data->inode);
>> + __nfs4_read_done_cb(data);
>> if (task->tk_status > 0)
>> renew_lease(server, data->timestamp);
>> return 0;
>> @@ -3198,6 +3203,11 @@ void nfs4_reset_read(struct rpc_task *task, struct nfs_read_data *data)
>> }
>> EXPORT_SYMBOL_GPL(nfs4_reset_read);
>>
>> +void __nfs4_write_done_cb(struct nfs_write_data *data)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Again, why the wrapper?
>
>> +{
>> + nfs_post_op_update_inode_force_wcc(data->inode, data->res.fattr);
Sorry, my bad, the pnfs done path needs just pnfs_set_layoutcommit()
>> +}
>> +
>> static int nfs4_write_done_cb(struct rpc_task *task, struct nfs_write_data *data)
>> {
>> struct inode *inode = data->inode;
>> @@ -3208,7 +3218,7 @@ static int nfs4_write_done_cb(struct rpc_task *task, struct nfs_write_data *data
>> }
>> if (task->tk_status >= 0) {
>> renew_lease(NFS_SERVER(inode), data->timestamp);
>> - nfs_post_op_update_inode_force_wcc(inode, data->res.fattr);
>> + __nfs4_write_done_cb(data);
>> }
>> return 0;
>> }
>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>> index a5050d2..18ae397 100644
>> --- a/fs/nfs/pnfs.c
>> +++ b/fs/nfs/pnfs.c
>> @@ -1130,6 +1130,30 @@ pnfs_pageio_init_write(struct nfs_pageio_descriptor *pgio, struct inode *inode)
>> pgio->pg_test = (ld && ld->pg_test) ? pnfs_write_pg_test : NULL;
>> }
>>
>> +/*
>> + * Called by non rpc-based layout drivers
>> + */
>> +int
>> +pnfs_write_done(struct nfs_write_data *data)
> ^^^^^^^^^^^^^^^^^^ If this is not generic to all pnfs layout drivers,
> then why the apparently generic name?
>
Makes sense, how about pnfs_ld_write_done?
> Why isn't this being introduced together with a driver that actually
> uses the functionality? There is no way to review it outside of that
> context.
>
OK, will do in next version of the patch series.
Benny
>> +{
>> + int status;
>> +
>> + put_lseg(data->lseg);
>> + data->lseg = NULL;
>> + if (!data->pnfs_error) {
>> + __nfs4_write_done_cb(data);
>> + data->mds_ops->rpc_call_done(NULL, data);
>> + data->mds_ops->rpc_release(data);
>> + return 0;
>> + }
>> +
>> + dprintk("%s: pnfs_error=%d, retry via MDS\n", __func__,
>> + data->pnfs_error);
>> + status = nfs_initiate_write(data, NFS_CLIENT(data->inode), data->mds_ops, NFS_FILE_SYNC);
>> + return status ? : -EAGAIN;
>> +}
>> +EXPORT_SYMBOL_GPL(pnfs_write_done);
>> +
>> enum pnfs_try_status
>> pnfs_try_to_write_data(struct nfs_write_data *wdata,
>> const struct rpc_call_ops *call_ops, int how)
>> @@ -1155,6 +1179,30 @@ pnfs_try_to_write_data(struct nfs_write_data *wdata,
>> }
>>
>> /*
>> + * Called by non rpc-based layout drivers
>> + */
>> +int
>> +pnfs_read_done(struct nfs_read_data *data)
>> +{
>> + int status;
>> +
>> + put_lseg(data->lseg);
>> + data->lseg = NULL;
>> + if (!data->pnfs_error) {
>> + __nfs4_read_done_cb(data);
>> + data->mds_ops->rpc_call_done(NULL, data);
>> + data->mds_ops->rpc_release(data);
>> + return 0;
>> + }
>> +
>> + dprintk("%s: pnfs_error=%d, retry via MDS\n", __func__,
>> + data->pnfs_error);
>> + status = nfs_initiate_read(data, NFS_CLIENT(data->inode), data->mds_ops);
>> + return status ? : -EAGAIN;
>> +}
>> +EXPORT_SYMBOL_GPL(pnfs_read_done);
>> +
>> +/*
>> * Call the appropriate parallel I/O subsystem read function.
>> */
>> enum pnfs_try_status
>> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
>> index 9f8e970..18b84ce 100644
>> --- a/fs/nfs/pnfs.h
>> +++ b/fs/nfs/pnfs.h
>> @@ -65,8 +65,11 @@ enum {
>> };
>>
>> enum layoutdriver_policy_flags {
>> + /* Should the full nfs rpc cleanup code be used after io */
>> + PNFS_USE_RPC_CODE = 1 << 0,
>> +
>> /* Should the pNFS client commit and return the layout upon a setattr */
>> - PNFS_LAYOUTRET_ON_SETATTR = 1 << 0,
>> + PNFS_LAYOUTRET_ON_SETATTR = 1 << 1,
>> };
>>
>> /* Per-layout driver specific registration structure */
>> @@ -182,6 +185,8 @@ bool pnfs_roc_drain(struct inode *ino, u32 *barrier);
>> void pnfs_set_layoutcommit(struct nfs_write_data *wdata);
>> int pnfs_layoutcommit_inode(struct inode *inode, bool sync);
>> int _pnfs_return_layout(struct inode *, struct pnfs_layout_range *, bool wait);
>> +int pnfs_write_done(struct nfs_write_data *);
>> +int pnfs_read_done(struct nfs_read_data *);
>>
>> static inline int lo_fail_bit(u32 iomode)
>> {
>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>> index 01eb1ae..41f896a 100644
>> --- a/include/linux/nfs_xdr.h
>> +++ b/include/linux/nfs_xdr.h
>> @@ -1108,6 +1108,7 @@ struct nfs_read_data {
>> const struct rpc_call_ops *mds_ops;
>> int (*read_done_cb) (struct rpc_task *task, struct nfs_read_data *data);
>> __u64 mds_offset;
>> + int pnfs_error;
>> struct page *page_array[NFS_PAGEVEC_SIZE];
>> };
>>
>> @@ -1133,6 +1134,7 @@ struct nfs_write_data {
>> unsigned long timestamp; /* For lease renewal */
>> #endif
>> __u64 mds_offset; /* Filelayout dense stripe */
>> + int pnfs_error;
>> struct page *page_array[NFS_PAGEVEC_SIZE];
>> };
>>
>
next prev parent reply other threads:[~2011-04-22 9:03 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-20 16:46 [RFC 0/27] pnfs-submit for 2.6.40 Benny Halevy
2011-04-20 17:26 ` [RFC 01/27] pnfs: CB_NOTIFY_DEVICEID Benny Halevy
2011-04-20 19:41 ` Trond Myklebust
2011-04-22 6:22 ` Benny Halevy
2011-04-20 17:26 ` [RFC 02/27] pnfs: direct i/o Benny Halevy
2011-04-20 17:26 ` [RFC 03/27] pnfs: layoutreturn Benny Halevy
2011-04-20 19:53 ` Trond Myklebust
2011-04-22 6:52 ` Benny Halevy
2011-04-22 8:04 ` [PATCH 1/6] SQUASHME: call pnfs_return_layout right before pnfs_destroy_layout Benny Halevy
2011-04-22 8:04 ` [PATCH 2/6] SQUASHME: remove assert_spin_locked from pnfs_clear_lseg_list Benny Halevy
2011-04-22 8:04 ` [PATCH 3/6] SQUASHME: remove wait parameter from the layoutreturn path Benny Halevy
2011-04-22 8:31 ` Benny Halevy
2011-04-22 8:05 ` [PATCH 4/6] SQUASHME: remove return_type field from nfs4_layoutreturn_args Benny Halevy
2011-04-22 8:05 ` [PATCH 5/6] SQUASHME: remove range " Benny Halevy
2011-04-22 8:05 ` [PATCH 6/6] SQUASHME: no need to send layoutcommit from _pnfs_return_layout Benny Halevy
2011-04-20 17:26 ` [RFC 04/27] pnfs: layoutret_on_setattr Benny Halevy
2011-04-20 20:03 ` Trond Myklebust
2011-04-22 8:23 ` Benny Halevy
2011-04-20 17:26 ` [RFC 05/27] pnfs: Use byte-range layout segments Benny Halevy
2011-04-20 17:26 ` [RFC 06/27] pnfs: encode_layoutreturn Benny Halevy
2011-04-20 20:16 ` Trond Myklebust
2011-04-22 8:26 ` Benny Halevy
2011-04-20 17:27 ` [RFC 07/27] pnfs: encode_layoutcommit Benny Halevy
2011-04-20 20:18 ` Trond Myklebust
2011-04-22 8:48 ` Benny Halevy
2011-04-20 17:27 ` [RFC 08/27] pnfs: {setup,cleanup}_layoutcommit Benny Halevy
2011-04-20 20:22 ` Trond Myklebust
2011-04-20 17:27 ` [RFC 09/27] pnfs: support for non-rpc layout drivers Benny Halevy
2011-04-20 20:34 ` Trond Myklebust
2011-04-22 9:03 ` Benny Halevy [this message]
2011-04-20 17:27 ` [RFC 10/27] pnfs: {,un}set_layoutdriver methods Benny Halevy
2011-04-20 17:27 ` [RFC 11/27] pnfs: per mount layout driver private data Benny Halevy
2011-04-20 20:36 ` Trond Myklebust
2011-04-22 9:05 ` Benny Halevy
2011-04-20 17:27 ` [RFC 12/27] pnfs: alloc and free layout_hdr layoutdriver methods Benny Halevy
2011-04-20 20:43 ` Trond Myklebust
2011-04-22 9:09 ` Benny Halevy
2011-04-20 17:27 ` [RFC 13/27] pnfs: client stats Benny Halevy
2011-04-20 17:28 ` [RFC 14/27] pnfsd: introduce exp_xdr.h Benny Halevy
2011-04-20 17:28 ` [RFC 15/27] pnfs-obj: pnfs_osd XDR definitions Benny Halevy
2011-04-20 20:49 ` Trond Myklebust
2011-04-22 9:11 ` Benny Halevy
2011-04-20 17:28 ` [RFC 16/27] pnfs-obj: pnfs_osd XDR client implementations Benny Halevy
2011-04-20 17:28 ` [RFC 17/27] exofs: pnfs-tree: Remove pnfs-osd private definitions Benny Halevy
2011-04-20 17:28 ` [RFC 18/27] pnfs-obj: Define PNFS_OBJLAYOUT Kconfig option Benny Halevy
2011-04-20 17:28 ` [RFC 19/27] pnfs-obj: objlayout driver skeleton Benny Halevy
2011-04-20 17:28 ` [RFC 20/27] pnfs-obj: objio_osd device information retrieval and caching Benny Halevy
2011-04-20 17:28 ` [RFC 21/27] pnfs-obj: objio_osd real IO implementation Benny Halevy
2011-04-20 17:29 ` [RFC 22/27] sunrpc: New xdr_rewind_stream() Benny Halevy
2011-04-20 17:29 ` [RFC 23/27] pnfs-obj: objlayout_encode_layoutreturn Implementation Benny Halevy
2011-04-20 17:29 ` [RFC 24/27] pnfs-obj: objio_osd report osd_errors for layoutreturn Benny Halevy
2011-04-20 17:29 ` [RFC 25/27] pnfs-obj: objlayout_encode_layoutcommit implementation Benny Halevy
2011-04-20 17:29 ` [RFC 26/27] pnfs-obj: objio_osd: RAID0 support Benny Halevy
2011-04-20 17:29 ` [RFC 27/27] pnfs-obj: objio_osd: groups support Benny Halevy
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=4DB14464.8060207@panasas.com \
--to=bhalevy@panasas.com \
--cc=Trond.Myklebust@netapp.com \
--cc=linux-nfs@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.