[pnfs] [PATCH 10/10] pnfs: Enable O_DIRECT write path.

Benny Halevy bhalevy at panasas.com
Wed Jul 9 04:06:48 EDT 2008


On Jul. 09, 2008, 0:54 +0300, Dean Hildebrand <seattleplus at gmail.com> wrote:
> Signed-off-by: Dean Hildebrand <dhildeb at us.ibm.com>
> ---
>  fs/nfs/direct.c |   43 ++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 42 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
> index a397a9c..fe291ad 100644
> --- a/fs/nfs/direct.c
> +++ b/fs/nfs/direct.c
> @@ -515,6 +515,7 @@ static void nfs_direct_write_reschedule(struct nfs_direct_req *dreq)
>  		.workqueue = nfsiod_workqueue,
>  		.flags = RPC_TASK_ASYNC,
>  	};
> +	int result;
>  
>  	dreq->count = 0;
>  	get_dreq(dreq);
> @@ -538,6 +539,13 @@ 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.
>  		 */
> +		result = pnfs_try_to_write_data(data, &nfs_write_direct_ops,
> +						NFS_FILE_SYNC);
> +#if defined(CONFIG_PNFS)
> +		if (result == PNFS_ATTEMPTED && !data->pdata.pnfs_error)
> +			continue;
> +#endif /* CONFIG_PNFS */
> +
>  		nfs_direct_write_execute(data, &task_setup_data, &msg);
>  	}
>  
> @@ -617,6 +625,7 @@ static void nfs_direct_commit_schedule(struct nfs_direct_req *dreq)
>  		.workqueue = nfsiod_workqueue,
>  		.flags = RPC_TASK_ASYNC,
>  	};
> +	int result;
>  
>  	data->inode = dreq->inode;
>  	data->cred = msg.rpc_cred;
> @@ -629,6 +638,12 @@ static void nfs_direct_commit_schedule(struct nfs_direct_req *dreq)
>  	data->res.fattr = &data->fattr;
>  	data->res.verf = &data->verf;
>  
> +	result = pnfs_try_to_commit(data, &nfs_commit_direct_ops,
> +				    RPC_TASK_ASYNC);
> +	dprintk("%s: pnfs commit result %d\n", __func__, result);
> +	if (result <= PNFS_ATTEMPTED)

I don't like the fact that some pnfs_try_to methods may return
an (negative) error status and some may not.  If the return type
is enum pnfs_try_status then that should be it.  In this case,
pnfs_try_to_commit should just return PNFS_ATTEMPTED and the
error status, if needed, can be extracted from data->pdata.pnfs_error;

Benny

> +		return;
> +
>  	nfs_direct_commit_execute(dreq, data, &task_setup_data, &msg);
>  }
>  
> @@ -677,6 +692,9 @@ static void nfs_direct_write_result(struct rpc_task *task, void *calldata)
>  {
>  	struct nfs_write_data *data = calldata;
>  
> +	dprintk("%s: verf: %d stable %d\n", __func__,
> +		data->res.verf->committed, data->args.stable);
> +
>  	if (nfs_writeback_done(task, data) != 0)
>  		return;
>  }
> @@ -794,6 +812,16 @@ static ssize_t nfs_direct_write_schedule_segment(struct nfs_direct_req *dreq,
>  	unsigned int pgbase;
>  	int result;
>  	ssize_t started = 0;
> +	size_t pnfs_stripe_rem = count;
> +
> +	/* pnfs_stripe_rem will be set to the remaining bytes in
> +	 * the first stripe_unit (which for standard nfs is count)
> +	 */
> +	pnfs_direct_init_io(inode, ctx, count, pos, 1,
> +			    &wsize, &pnfs_stripe_rem);
> +
> +	dprintk("%s: pos %llu count %Zu wsize %Zu\n",
> +		__func__, pos, count, wsize);
>  
>  	do {
>  		struct nfs_write_data *data;
> @@ -802,6 +830,10 @@ static ssize_t nfs_direct_write_schedule_segment(struct nfs_direct_req *dreq,
>  		pgbase = user_addr & ~PAGE_MASK;
>  		bytes = min(wsize,count);
>  
> +#if defined(CONFIG_PNFS)
> +		bytes = min(bytes, pnfs_stripe_rem);
> +		pnfs_stripe_rem = wsize;
> +#endif /* CONFIG_PNFS */
>  		result = -ENOMEM;
>  		data = nfs_writedata_alloc(nfs_page_array_len(pgbase, bytes));
>  		if (unlikely(!data))
> @@ -844,7 +876,16 @@ static ssize_t nfs_direct_write_schedule_segment(struct nfs_direct_req *dreq,
>  		data->res.count = bytes;
>  		data->res.verf = &data->verf;
>  
> -		if (nfs_direct_write_execute(data, &task_setup_data, &msg))
> +		result = pnfs_try_to_write_data(data, &nfs_write_direct_ops,
> +						sync);
> +#if defined(CONFIG_PNFS)
> +		if (result == PNFS_ATTEMPTED && data->pdata.pnfs_error) {
> +			result = data->pdata.pnfs_error;

I'm not sure if this is the right thing to do since in the non-pnfs
path the error status is not returned.

Benny

> +			break;
> +		}
> +#endif /* CONFIG_PNFS */
> +		if (result == PNFS_NOT_ATTEMPTED &&
> +		    nfs_direct_write_execute(data, &task_setup_data, &msg))
>  			break;
>  
>  		started += bytes;



More information about the pNFS mailing list