[pnfs] [PATCH 06/10] pnfs: Isolate odirect rpc read/write/commit execution.
Dean Hildebrand
seattleplus at gmail.com
Thu Jul 10 01:16:25 EDT 2008
Benny Halevy wrote:
> On Jul. 09, 2008, 0:54 +0300, Dean Hildebrand <seattleplus at gmail.com> wrote:
>
>> This patch helps prepare for pNFS, which may not use rpc.
>>
>> Signed-off-by: Dean Hildebrand <dhildeb at us.ibm.com>
>> ---
>> fs/nfs/direct.c | 164 ++++++++++++++++++++++++++++++++-----------------------
>> 1 files changed, 96 insertions(+), 68 deletions(-)
>>
>> diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
>> index ada71d5..854ed47 100644
>> --- a/fs/nfs/direct.c
>> +++ b/fs/nfs/direct.c
>> @@ -266,6 +266,40 @@ static const struct rpc_call_ops nfs_read_direct_ops = {
>> .rpc_release = nfs_direct_read_release,
>> };
>>
>> +static long nfs_direct_read_execute(struct nfs_read_data *data,
>> + struct rpc_task_setup *task_setup_data,
>> + struct rpc_message *msg)
>> +{
>> + struct inode *inode = data->inode;
>> + struct rpc_task *task;
>> + long err;
>> +
>> + msg->rpc_argp = &data->args;
>> + msg->rpc_resp = &data->res;
>> +
>> + task_setup_data->task = &data->task;
>> + task_setup_data->callback_data = data;
>> + NFS_PROTO(inode)->read_setup(data, msg);
>> +
>> + task = rpc_run_task(task_setup_data);
>> + err = IS_ERR(task);
>> + if (err)
>> + goto out;
>>
>
> doing the following will percolate the error code up to the caller:
>
> if (IS_ERR(task))
> return PTR_ERR(task);
>
This would change current nfs behavior as it seems the function always
wants to return EFAULT in this case. Not sure if this was intentional
by Trond or not, but I'd rather move the change into a different patch
if you send this to Trond.
>
>> +
>> + rpc_put_task(task);
>> +
>> + dprintk("NFS: %5u initiated direct read call "
>> + "(req %s/%lld, %u bytes @ offset %llu)\n",
>> + data->task.tk_pid,
>> + inode->i_sb->s_id,
>> + (long long)NFS_FILEID(inode),
>> + data->args.count,
>> + (unsigned long long)data->args.offset);
>> +
>> +out:
>> + return err;
>>
>
> return 0;
> (cont. to the above)
>
>> +}
>> +
>> /*
>> * For each rsize'd chunk of the user's buffer, dispatch an NFS READ
>> * operation. If nfs_readdata_alloc() or get_user_pages() fails,
>> @@ -282,7 +316,6 @@ static ssize_t nfs_direct_read_schedule_segment(struct nfs_direct_req *dreq,
>> unsigned long user_addr = (unsigned long)iov->iov_base;
>> size_t count = iov->iov_len;
>> size_t rsize = NFS_SERVER(inode)->rsize;
>> - struct rpc_task *task;
>> struct rpc_message msg = {
>> .rpc_cred = ctx->cred,
>> };
>> @@ -343,25 +376,9 @@ static ssize_t nfs_direct_read_schedule_segment(struct nfs_direct_req *dreq,
>> data->res.fattr = &data->fattr;
>> data->res.eof = 0;
>> data->res.count = bytes;
>> - msg.rpc_argp = &data->args;
>> - msg.rpc_resp = &data->res;
>> -
>> - task_setup_data.task = &data->task;
>> - task_setup_data.callback_data = data;
>> - NFS_PROTO(inode)->read_setup(data, &msg);
>> -
>> - task = rpc_run_task(&task_setup_data);
>> - if (IS_ERR(task))
>> - break;
>> - rpc_put_task(task);
>>
>> - dprintk("NFS: %5u initiated direct read call "
>> - "(req %s/%Ld, %zu bytes @ offset %Lu)\n",
>> - data->task.tk_pid,
>> - inode->i_sb->s_id,
>> - (long long)NFS_FILEID(inode),
>> - bytes,
>> - (unsigned long long)data->args.offset);
>> + if (nfs_direct_read_execute(data, &task_setup_data, &msg))
>> + break;
>>
> ^^^^
> this should be a tab (I'll fix that up when submitting)
>
oops, thanks.
>> started += bytes;
>> user_addr += bytes;
>> @@ -447,12 +464,15 @@ static void nfs_direct_free_writedata(struct nfs_direct_req *dreq)
>> }
>>
>> #if defined(CONFIG_NFS_V3) || defined(CONFIG_NFS_V4)
>> +static long nfs_direct_write_execute(struct nfs_write_data *data,
>> + struct rpc_task_setup *task_setup_data,
>> + struct rpc_message *msg);
>> +
>> static void nfs_direct_write_reschedule(struct nfs_direct_req *dreq)
>> {
>> struct inode *inode = dreq->inode;
>> struct list_head *p;
>> struct nfs_write_data *data;
>> - struct rpc_task *task;
>> struct rpc_message msg = {
>> .rpc_cred = dreq->ctx->cred,
>> };
>> @@ -485,25 +505,7 @@ static void nfs_direct_write_reschedule(struct nfs_direct_req *dreq)
>> * Reuse data->task; data->args should not have changed
>> * since the original request was sent.
>> */
>> - task_setup_data.task = &data->task;
>> - task_setup_data.callback_data = data;
>> - msg.rpc_argp = &data->args;
>> - msg.rpc_resp = &data->res;
>> - NFS_PROTO(inode)->write_setup(data, &msg);
>> -
>> - /*
>> - * We're called via an RPC callback, so BKL is already held.
>> - */
>> - task = rpc_run_task(&task_setup_data);
>> - if (!IS_ERR(task))
>> - rpc_put_task(task);
>> -
>> - dprintk("NFS: %5u rescheduled direct write call (req %s/%Ld, %u bytes @ offset %Lu)\n",
>> - data->task.tk_pid,
>> - inode->i_sb->s_id,
>> - (long long)NFS_FILEID(inode),
>> - data->args.count,
>> - (unsigned long long)data->args.offset);
>> + nfs_direct_write_execute(data, &task_setup_data, &msg);
>> }
>>
>> if (put_dreq(dreq))
>> @@ -546,10 +548,28 @@ static const struct rpc_call_ops nfs_commit_direct_ops = {
>> .rpc_release = nfs_direct_commit_release,
>> };
>>
>> +static void nfs_direct_commit_execute(struct nfs_direct_req *dreq,
>> + struct nfs_write_data *data,
>> + struct rpc_task_setup *task_setup_data,
>> + struct rpc_message *msg)
>>
>
> to keep things similar how about returning status as in the read and write
> cases (even though it's not used in this case)
>
I agree that it is probably better, but I think it should also be in a
separate 2nd patch as it changes current behavior.
Dean
>
>> +{
>> + struct rpc_task *task;
>> +
>> + NFS_PROTO(data->inode)->commit_setup(data, msg);
>> +
>> + /* Note: task.tk_ops->rpc_release will free dreq->commit_data */
>> + dreq->commit_data = NULL;
>> +
>> + dprintk("NFS: %5u initiated commit call\n", data->task.tk_pid);
>> +
>> + task = rpc_run_task(task_setup_data);
>> + if (!IS_ERR(task))
>> + rpc_put_task(task);
>> +}
>> +
>> static void nfs_direct_commit_schedule(struct nfs_direct_req *dreq)
>> {
>> struct nfs_write_data *data = dreq->commit_data;
>> - struct rpc_task *task;
>> struct rpc_message msg = {
>> .rpc_argp = &data->args,
>> .rpc_resp = &data->res,
>> @@ -576,16 +596,7 @@ static void nfs_direct_commit_schedule(struct nfs_direct_req *dreq)
>> data->res.fattr = &data->fattr;
>> data->res.verf = &data->verf;
>>
>> - NFS_PROTO(data->inode)->commit_setup(data, &msg);
>> -
>> - /* Note: task.tk_ops->rpc_release will free dreq->commit_data */
>> - dreq->commit_data = NULL;
>> -
>> - dprintk("NFS: %5u initiated commit call\n", data->task.tk_pid);
>> -
>> - task = rpc_run_task(&task_setup_data);
>> - if (!IS_ERR(task))
>> - rpc_put_task(task);
>> + nfs_direct_commit_execute(dreq, data, &task_setup_data, &msg);
>> }
>>
>> static void nfs_direct_write_complete(struct nfs_direct_req *dreq, struct inode *inode)
>> @@ -687,6 +698,40 @@ static const struct rpc_call_ops nfs_write_direct_ops = {
>> .rpc_release = nfs_direct_write_release,
>> };
>>
>> +static long nfs_direct_write_execute(struct nfs_write_data *data,
>> + struct rpc_task_setup *task_setup_data,
>> + struct rpc_message *msg)
>> +{
>> + struct inode *inode = data->inode;
>> + struct rpc_task *task;
>> + long err;
>> +
>> + msg->rpc_argp = &data->args;
>> + msg->rpc_resp = &data->res;
>> +
>> + task_setup_data->task = &data->task;
>> + task_setup_data->callback_data = data;
>> + NFS_PROTO(inode)->write_setup(data, msg);
>> +
>> + task = rpc_run_task(task_setup_data);
>> + err = IS_ERR(task);
>> + if (err)
>> + goto out;
>>
>
> ditto as in the read case.
>
>
>> +
>> + rpc_put_task(task);
>> +
>> + dprintk("NFS: %5u initiated direct write call "
>> + "(req %s/%lld, %u bytes @ offset %llu)\n",
>> + data->task.tk_pid,
>> + inode->i_sb->s_id,
>> + (long long)NFS_FILEID(inode),
>> + data->args.count,
>> + (unsigned long long)data->args.offset);
>> +
>> +out:
>> + return err;
>> +}
>> +
>> /*
>> * For each wsize'd chunk of the user's buffer, dispatch an NFS WRITE
>> * operation. If nfs_writedata_alloc() or get_user_pages() fails,
>> @@ -702,7 +747,6 @@ static ssize_t nfs_direct_write_schedule_segment(struct nfs_direct_req *dreq,
>> struct inode *inode = ctx->path.dentry->d_inode;
>> unsigned long user_addr = (unsigned long)iov->iov_base;
>> size_t count = iov->iov_len;
>> - struct rpc_task *task;
>> struct rpc_message msg = {
>> .rpc_cred = ctx->cred,
>> };
>> @@ -767,24 +811,8 @@ static ssize_t nfs_direct_write_schedule_segment(struct nfs_direct_req *dreq,
>> data->res.count = bytes;
>> data->res.verf = &data->verf;
>>
>> - task_setup_data.task = &data->task;
>> - task_setup_data.callback_data = data;
>> - msg.rpc_argp = &data->args;
>> - msg.rpc_resp = &data->res;
>> - NFS_PROTO(inode)->write_setup(data, &msg);
>> -
>> - task = rpc_run_task(&task_setup_data);
>> - if (IS_ERR(task))
>> + if (nfs_direct_write_execute(data, &task_setup_data, &msg))
>> break;
>> - rpc_put_task(task);
>> -
>> - dprintk("NFS: %5u initiated direct write call "
>> - "(req %s/%Ld, %zu bytes @ offset %Lu)\n",
>> - data->task.tk_pid,
>> - inode->i_sb->s_id,
>> - (long long)NFS_FILEID(inode),
>> - bytes,
>> - (unsigned long long)data->args.offset);
>>
>> started += bytes;
>> user_addr += bytes;
>>
>
>
More information about the pNFS
mailing list