[Devel] [PATCH 2/4] fs/fuse kio: add pending kio requests to kqueue

Kirill Tkhai ktkhai at virtuozzo.com
Wed May 15 13:26:10 MSK 2019


Hi, Pavel,

On 15.05.2019 11:53, Pavel Butsykin wrote:
> Pending kio requests don't fall into kqueue list and therefore not tracked, this
> of course is a mistake. This patch fixes the mistake, making it possible to add
> pending requests to di->kq inside pcs_fuse_prep_rw(). It's also very important
> to be able to immediately interrupt pending kio requests and terminate it with
> error in order to maintain synchronization with fuse_invalidate_files(). By this
> reason pcs_fuse_prep_rw() will return -EIO in case FUSE_S_FAIL_IMMEDIATELY
> status was set to ff_state.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin at virtuozzo.com>
> ---
>  fs/fuse/kio/pcs/fuse_io.c          |  5 +++
>  fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 82 +++++++++++++++++++++++++-------------
>  2 files changed, 59 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/fuse/kio/pcs/fuse_io.c b/fs/fuse/kio/pcs/fuse_io.c
> index 219f4e3423af..ed5926eb5d4d 100644
> --- a/fs/fuse/kio/pcs/fuse_io.c
> +++ b/fs/fuse/kio/pcs/fuse_io.c
> @@ -253,10 +253,15 @@ void pcs_fuse_prep_io(struct pcs_fuse_req *r, unsigned short type, off_t offset,
>  static void falloc_req_complete(struct pcs_int_request *ireq)
>  {
>  	struct pcs_fuse_req * r = ireq->completion_data.priv;
> +	struct pcs_dentry_info *di = get_pcs_inode(r->req.io_inode);
>  	struct pcs_fuse_cluster *pfc = cl_from_req(r);
>  
>  	BUG_ON(ireq->type != PCS_IREQ_NOOP);
>  
> +	spin_lock(&di->kq_lock);
> +	list_del_init(&r->req.list);
> +	spin_unlock(&di->kq_lock);
> +
>  	DTRACE("do fuse_request_end req:%p op:%d err:%d\n", &r->req, r->req.in.h.opcode, r->req.out.h.error);
>  	fuse_stat_account(pfc->fc, KFUSE_OP_FALLOCATE, ktime_sub(ktime_get(), ireq->ts));
>  	inode_dio_end(r->req.io_inode);
> diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> index efa2b4054215..e75eaf090d99 100644
> --- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> +++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> @@ -815,6 +815,19 @@ static void wait_shrink(struct pcs_fuse_req *r, struct pcs_dentry_info *di)
>  	list_add_tail(&r->exec.ireq.list, &di->size.queue);
>  }
>  
> +static bool kqueue_insert(struct pcs_dentry_info *di, struct fuse_file *ff,
> +			  struct fuse_req *req)
> +{
> +	spin_lock(&di->kq_lock);
> +	if (ff && test_bit(FUSE_S_FAIL_IMMEDIATELY, &ff->ff_state)) {
> +		spin_unlock(&di->kq_lock);
> +		return false;
> +	}
> +	list_add_tail(&req->list, &di->kq);
> +	spin_unlock(&di->kq_lock);
> +	return true;
> +}
> +
>  /*
>   * Check i size boundary and deffer request if necessary
>   * Ret code
> @@ -822,7 +835,7 @@ static void wait_shrink(struct pcs_fuse_req *r, struct pcs_dentry_info *di)
>   * -1: should fail request
>   * 1: request placed to pended queue
>  */
> -static int pcs_fuse_prep_rw(struct pcs_fuse_req *r)
> +static int pcs_fuse_prep_rw(struct pcs_fuse_req *r, struct fuse_file *ff)
>  {
>  	struct fuse_inode *fi = get_fuse_inode(r->req.io_inode);
>  	struct pcs_dentry_info *di = pcs_inode_from_fuse(fi);
> @@ -832,8 +845,8 @@ static int pcs_fuse_prep_rw(struct pcs_fuse_req *r)
>  	/* Deffer all requests if shrink requested to prevent livelock */
>  	if (di->size.op == PCS_SIZE_SHRINK) {
>  		wait_shrink(r, di);
> -		spin_unlock(&di->lock);
> -		return 1;
> +		ret = 1;
> +		goto out;
>  	}
>  	if (r->req.in.h.opcode == FUSE_READ) {
>  		size_t size;
> @@ -843,8 +856,8 @@ static int pcs_fuse_prep_rw(struct pcs_fuse_req *r)
>  		if (in->offset + in->size > di->fileinfo.attr.size) {
>  			if (in->offset >= di->fileinfo.attr.size) {
>  				r->req.out.args[0].size = 0;
> -				spin_unlock(&di->lock);
> -				return -1;
> +				ret = -EPERM;
> +				goto out;
>  			}
>  			size = di->fileinfo.attr.size - in->offset;
>  		}
> @@ -853,6 +866,10 @@ static int pcs_fuse_prep_rw(struct pcs_fuse_req *r)
>  		struct fuse_write_in *in = &r->req.misc.write.in;
>  
>  		if (in->offset + in->size > di->fileinfo.attr.size) {
> +			if (!kqueue_insert(di, ff, &r->req)) {
> +				ret = -EIO;
> +				goto out;
> +			}
>  			wait_grow(r, di, in->offset + in->size);
>  			ret = 1;
>  		}
> @@ -868,8 +885,8 @@ static int pcs_fuse_prep_rw(struct pcs_fuse_req *r)
>  		size = in->fm_length;
>  		if (in->fm_start + size > di->fileinfo.attr.size) {
>  			if (in->fm_start >= di->fileinfo.attr.size) {
> -				spin_unlock(&di->lock);
> -				return -1;
> +				ret = -EPERM;

What is the reason, that EPERM is here?

> +				goto out;
>  			}
>  			size = di->fileinfo.attr.size - in->fm_start;
>  		}
> @@ -880,6 +897,10 @@ static int pcs_fuse_prep_rw(struct pcs_fuse_req *r)
>  		struct fuse_fallocate_in const *in = r->req.in.args[0].value;
>  
>  		if (in->offset + in->length > di->fileinfo.attr.size) {
> +			if (!kqueue_insert(di, ff, &r->req)) {
> +				ret = -EIO;
> +				goto out;
> +			}
>  			wait_grow(r, di, in->offset + in->length);
>  			ret = 1;
>  		}
> @@ -892,14 +913,14 @@ static int pcs_fuse_prep_rw(struct pcs_fuse_req *r)
>  			if (ret) {
>  				pcs_fuse_prep_fallocate(r);
>  			} else {
> -				spin_unlock(&di->lock);
> -				return -1;
> +				ret = -EPERM;
> +				goto out;
>  			}
>  		}
>  	}
>  	inode_dio_begin(r->req.io_inode);
> +out:
>  	spin_unlock(&di->lock);
> -
>  	return ret;
>  }
>  
> @@ -924,12 +945,15 @@ static void pcs_fuse_submit(struct pcs_fuse_cluster *pfc, struct fuse_req *req,
>  	switch (r->req.in.h.opcode) {
>  	case FUSE_WRITE:
>  	case FUSE_READ:
> -		ret = pcs_fuse_prep_rw(r);
> -		if (!ret)
> +		ret = pcs_fuse_prep_rw(r, ff);
> +		if (likely(!ret))
>  			goto submit;
>  		if (ret > 0)
> -			/* Pended, nothing to do. */
> -			return;
> +			return; /* Pended, nothing to do. */
> +		if (ret != -EPERM) {
> +			req->out.h.error = ret;
> +			goto error;
>
> +		}
>  		break;
>  	case FUSE_FALLOCATE: {
>  		struct fuse_fallocate_in *inarg = (void*) req->in.args[0].value;
> @@ -958,12 +982,15 @@ static void pcs_fuse_submit(struct pcs_fuse_cluster *pfc, struct fuse_req *req,
>  				inarg->length = di->fileinfo.attr.size - inarg->offset;
>  		}
>  
> -		ret = pcs_fuse_prep_rw(r);
> -		if (!ret)
> +		ret = pcs_fuse_prep_rw(r, ff);
> +		if (likely(!ret))
>  			goto submit;
>  		if (ret > 0)
> -			/* Pended, nothing to do. */
> -			return;
> +			return; /* Pended, nothing to do. */
> +		if (ret != -EPERM) {
> +			req->out.h.error = ret;
> +			goto error;
> +		}
>  		break;
>  	}
>  	case FUSE_FSYNC:
> @@ -976,12 +1003,15 @@ static void pcs_fuse_submit(struct pcs_fuse_cluster *pfc, struct fuse_req *req,
>  			goto error;
>  		}
>  
> -		ret = pcs_fuse_prep_rw(r);
> -		if (!ret)
> +		ret = pcs_fuse_prep_rw(r, ff);
> +		if (likely(!ret))
>  			goto submit;
>  		if (ret > 0)
> -			/* Pended, nothing to do. */
> -			return;
> +			return; /* Pended, nothing to do. */
> +		if (ret != -EPERM) {
> +			req->out.h.error = ret;
> +			goto error;

Hm, strange thing in generic fuse code we fail immediate requests with EIO
(see fuse_request_queue_background()), but there we finish them as successful.

Also, I'm not sure, that EPERM is good value to use. IMO, it confuses a lot.
Can't all pcs_fuse_prep_rw() fail with EIO?

> +		}
>  		break;
>  	}
>  	r->req.out.h.error = 0;
> @@ -996,14 +1026,10 @@ error:
>  	return;
>  
>  submit:
> -	spin_lock(&di->kq_lock);
> -	if (ff && test_bit(FUSE_S_FAIL_IMMEDIATELY, &ff->ff_state)) {
> -		spin_unlock(&di->kq_lock);
> +	if (!kqueue_insert(di, ff, req)) {
>  		req->out.h.error = -EIO;
>  		goto error;
>  	}
> -	list_add_tail(&req->list, &di->kq);
> -	spin_unlock(&di->kq_lock);
>  
>  	if (async)
>  		pcs_cc_submit(ireq->cc, ireq);
> @@ -1071,7 +1097,7 @@ static void _pcs_shrink_end(struct fuse_conn *fc, struct fuse_req *req)
>  
>  		TRACE("resubmit %p\n", &r->req);
>  		list_del_init(&ireq->list);
> -		pcs_fuse_submit(pfc, &r->req, NULL, true, false);
> +		pcs_fuse_submit(pfc, &r->req, r->req.ff, true, false);
>  	}
>  }
>  
> 



More information about the Devel mailing list