[v2,2/2] fs/fuse kio: simplify processing and sending kio requests

Submitted by Pavel Butsykin on May 27, 2019, 3:29 p.m.

Details

Message ID 20190527152951.13332-2-pbutsykin@virtuozzo.com
State New
Series "Series without cover letter"
Headers show

Commit Message

Pavel Butsykin May 27, 2019, 3:29 p.m.
This patch makes functions kpcs_req_classify() and pcs_fuse_submit() easier,
reducing the number of cases to handle kio requests.

Also, this patch allows us to get rid of the additional context switch, when
kio request is moved to cc->work_queue list under bg_lock. After this patch,
execution of kpcs_req_send() is available only without fc->bg_lock.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
---
 fs/fuse/dev.c                      | 24 ++++++-----
 fs/fuse/fuse_i.h                   |  2 +-
 fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 84 +++++++++++++-------------------------
 3 files changed, 43 insertions(+), 67 deletions(-)

Patch hide | download patch | download mbox

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 3e0fecbe8db3..b8a524f04298 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -393,9 +393,11 @@  void fuse_queue_forget(struct fuse_conn *fc, struct fuse_forget_link *forget,
 
 static void flush_bg_queue(struct fuse_conn *fc, struct fuse_iqueue *fiq)
 {
+	struct fuse_req *req, *next;
+	LIST_HEAD(kio_reqs);
+
 	while (fc->active_background < fc->max_background &&
 	       !list_empty(&fc->bg_queue)) {
-		struct fuse_req *req;
 
 		req = list_first_entry(&fc->bg_queue, struct fuse_req, list);
 		list_del_init(&req->list);
@@ -404,16 +406,24 @@  static void flush_bg_queue(struct fuse_conn *fc, struct fuse_iqueue *fiq)
 		if (fc->kio.op) {
 			int ret = fc->kio.op->req_classify(fc, req, true, true);
 			if (likely(!ret)) {
-				fc->kio.op->req_send(fc, req, NULL, true, true);
+				list_add_tail(&req->list, &kio_reqs);
 				continue;
 			} else if (ret < 0)
 				continue;
 		}
+
 		spin_lock(&fiq->waitq.lock);
 		req->in.h.unique = fuse_get_unique(fiq);
 		queue_request(fiq, req);
 		spin_unlock(&fiq->waitq.lock);
 	}
+
+	spin_unlock(&fc->bg_lock);
+	list_for_each_entry_safe(req, next, &kio_reqs, list) {
+		list_del_init(&req->list);
+		fc->kio.op->req_send(fc, req, NULL, true);
+	}
+	spin_lock(&fc->bg_lock);
 }
 
 /*
@@ -563,7 +573,7 @@  static void __fuse_request_send(struct fuse_conn *fc, struct fuse_req *req,
 	if (fc->kio.op) {
 		int ret = fc->kio.op->req_classify(fc, req, false, false);
 		if (likely(!ret))
-			return fc->kio.op->req_send(fc, req, ff, false, false);
+			return fc->kio.op->req_send(fc, req, ff, false);
 		else if (ret < 0)
 			return;
 	}
@@ -656,14 +666,6 @@  void fuse_request_send_background(struct fuse_conn *fc, struct fuse_req *req)
 {
 	WARN_ON(!req->end);
 
-	if (fc->kio.op) {
-		int ret = fc->kio.op->req_classify(fc, req, true, false);
-		if (likely(!ret))
-			return fc->kio.op->req_send(fc, req, NULL, true, false);
-		else if (ret < 0)
-			return;
-	}
-
 	if (!fuse_request_queue_background(fc, req)) {
 		if (!req->out.h.error)
 			req->out.h.error = -ENOTCONN;
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 7978a1d891d2..34a4317a9689 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -530,7 +530,7 @@  struct fuse_kio_ops {
 	int (*req_classify)(struct fuse_conn *fc, struct fuse_req *req, bool bg,
 			    bool locked);
 	void (*req_send)(struct fuse_conn *fc, struct fuse_req *req,
-			 struct fuse_file *ff, bool bg, bool locked);
+			 struct fuse_file *ff, bool bg);
 
 	/* Inode scope hooks */
 	int  (*file_open)(struct fuse_conn *fc, struct file *file,
diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
index a101d3950373..51e45911d6b8 100644
--- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
+++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
@@ -947,7 +947,7 @@  out:
 }
 
 static void pcs_fuse_submit(struct pcs_fuse_cluster *pfc, struct fuse_req *req,
-			    struct fuse_file *ff, bool async, bool lk)
+			    struct fuse_file *ff)
 {
 	struct pcs_fuse_req *r = pcs_req_from_fuse(req);
 	struct fuse_inode *fi = get_fuse_inode(req->io_inode);
@@ -1040,11 +1040,7 @@  static void pcs_fuse_submit(struct pcs_fuse_cluster *pfc, struct fuse_req *req,
 error:
 	DTRACE("do fuse_request_end req:%p op:%d err:%d\n", req, req->in.h.opcode, req->out.h.error);
 
-	if (lk)
-		spin_unlock(&pfc->fc->bg_lock);
 	request_end(pfc->fc, req);
-	if (lk)
-		spin_lock(&pfc->fc->bg_lock);
 	return;
 
 submit:
@@ -1052,11 +1048,7 @@  submit:
 		req->out.h.error = -EIO;
 		goto error;
 	}
-
-	if (async)
-		pcs_cc_submit(ireq->cc, ireq);
-	else
-		ireq_process(ireq);
+	ireq_process(ireq);
 }
 
 static void kpcs_setattr_end(struct fuse_conn *fc, struct fuse_req *req)
@@ -1119,7 +1111,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, r->req.ff, true, false);
+		pcs_fuse_submit(pfc, &r->req, r->req.ff);
 	}
 }
 
@@ -1232,64 +1224,46 @@  static int kpcs_req_classify(struct fuse_conn* fc, struct fuse_req *req,
 		return 1;
 
 	BUG_ON(!pfc);
-	/* At this point request can not belongs to any list
-	 * so we can avoid grab fc->lock here at all.
-	 */
-	BUG_ON(!list_empty(&req->list));
-
 	DTRACE("Classify req:%p op:%d end:%p bg:%d lk:%d\n", req, req->in.h.opcode,
 							  req->end, bg, lk);
 	ret = pcs_kio_classify_req(fc, req, lk);
-	if (ret) {
-		if (ret < 0) {
-			if (!bg)
-				atomic_inc(&req->count);
-			__clear_bit(FR_PENDING, &req->flags);
-			req->out.h.error = ret;
-			if (lk)
-				spin_unlock(&fc->bg_lock);
-			request_end(fc, req);
-			if (lk)
-				spin_lock(&fc->bg_lock);
-			return ret;
-		}
-		return 1;
-	}
+	if (likely(!ret))
+		return 0;
 
-	if (!lk) {
-		spin_lock(&fc->bg_lock);
-		if (fc->num_background + 1 >= fc->max_background ||
-		    !fc->connected) {
+	if (ret < 0) {
+		if (!bg)
+			atomic_inc(&req->count);
+		__clear_bit(FR_PENDING, &req->flags);
+		req->out.h.error = ret;
+		if (lk)
 			spin_unlock(&fc->bg_lock);
-			return 1;
-		}
-		fc->num_background++;
-		fc->active_background++;
-
-		if (fc->num_background == fc->congestion_threshold &&
-		    fc->bdi_initialized) {
-			set_bdi_congested(&fc->bdi, BLK_RW_SYNC);
-			set_bdi_congested(&fc->bdi, BLK_RW_ASYNC);
-		}
-		spin_unlock(&fc->bg_lock);
+		request_end(fc, req);
+		if (lk)
+			spin_lock(&fc->bg_lock);
+		return ret;
 	}
-       return 0;
+	return 1;
 }
 
 static void kpcs_req_send(struct fuse_conn* fc, struct fuse_req *req,
-			  struct fuse_file *ff, bool bg, bool lk)
+			  struct fuse_file *ff, bool bg)
 {
-       struct pcs_fuse_cluster *pfc = (struct pcs_fuse_cluster*)fc->kio.ctx;
+	struct pcs_fuse_cluster *pfc = (struct pcs_fuse_cluster*)fc->kio.ctx;
 
-       TRACE("Send req:%p op:%d end:%p bg:%d lk:%d\n",
-               req, req->in.h.opcode, req->end, bg, lk);
+	/* At this point request can not belongs to any list
+	 * so we can avoid grab fc->lock here at all.
+	 */
+	BUG_ON(!list_empty(&req->list));
 
-       /* request_end below will do fuse_put_request() */
-       if (!bg)
-               atomic_inc(&req->count);
+	TRACE("Send req:%p op:%d end:%p bg:%d\n",
+		req, req->in.h.opcode, req->end, bg);
+
+	/* request_end below will do fuse_put_request() */
+	if (!bg)
+		atomic_inc(&req->count);
 	__clear_bit(FR_PENDING, &req->flags);
 
-	pcs_fuse_submit(pfc, req, ff ? : req->ff, lk, lk);
+	pcs_fuse_submit(pfc, req, ff ? : req->ff);
 	if (!bg)
 		wait_event(req->waitq,
 			   test_bit(FR_FINISHED, &req->flags) && !req->end);

Comments

Kirill Tkhai May 27, 2019, 3:37 p.m.
On 27.05.2019 18:29, Pavel Butsykin wrote:
> This patch makes functions kpcs_req_classify() and pcs_fuse_submit() easier,
> reducing the number of cases to handle kio requests.
> 
> Also, this patch allows us to get rid of the additional context switch, when
> kio request is moved to cc->work_queue list under bg_lock. After this patch,
> execution of kpcs_req_send() is available only without fc->bg_lock.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>

Reviewed-by: Kirill Tkhai <ktkhai@virtuozzo.com>

> ---
>  fs/fuse/dev.c                      | 24 ++++++-----
>  fs/fuse/fuse_i.h                   |  2 +-
>  fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 84 +++++++++++++-------------------------
>  3 files changed, 43 insertions(+), 67 deletions(-)
> 
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 3e0fecbe8db3..b8a524f04298 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -393,9 +393,11 @@ void fuse_queue_forget(struct fuse_conn *fc, struct fuse_forget_link *forget,
>  
>  static void flush_bg_queue(struct fuse_conn *fc, struct fuse_iqueue *fiq)
>  {
> +	struct fuse_req *req, *next;
> +	LIST_HEAD(kio_reqs);
> +
>  	while (fc->active_background < fc->max_background &&
>  	       !list_empty(&fc->bg_queue)) {
> -		struct fuse_req *req;
>  
>  		req = list_first_entry(&fc->bg_queue, struct fuse_req, list);
>  		list_del_init(&req->list);
> @@ -404,16 +406,24 @@ static void flush_bg_queue(struct fuse_conn *fc, struct fuse_iqueue *fiq)
>  		if (fc->kio.op) {
>  			int ret = fc->kio.op->req_classify(fc, req, true, true);
>  			if (likely(!ret)) {
> -				fc->kio.op->req_send(fc, req, NULL, true, true);
> +				list_add_tail(&req->list, &kio_reqs);
>  				continue;
>  			} else if (ret < 0)
>  				continue;
>  		}
> +
>  		spin_lock(&fiq->waitq.lock);
>  		req->in.h.unique = fuse_get_unique(fiq);
>  		queue_request(fiq, req);
>  		spin_unlock(&fiq->waitq.lock);
>  	}
> +
> +	spin_unlock(&fc->bg_lock);
> +	list_for_each_entry_safe(req, next, &kio_reqs, list) {
> +		list_del_init(&req->list);
> +		fc->kio.op->req_send(fc, req, NULL, true);
> +	}
> +	spin_lock(&fc->bg_lock);
>  }
>  
>  /*
> @@ -563,7 +573,7 @@ static void __fuse_request_send(struct fuse_conn *fc, struct fuse_req *req,
>  	if (fc->kio.op) {
>  		int ret = fc->kio.op->req_classify(fc, req, false, false);
>  		if (likely(!ret))
> -			return fc->kio.op->req_send(fc, req, ff, false, false);
> +			return fc->kio.op->req_send(fc, req, ff, false);
>  		else if (ret < 0)
>  			return;
>  	}
> @@ -656,14 +666,6 @@ void fuse_request_send_background(struct fuse_conn *fc, struct fuse_req *req)
>  {
>  	WARN_ON(!req->end);
>  
> -	if (fc->kio.op) {
> -		int ret = fc->kio.op->req_classify(fc, req, true, false);
> -		if (likely(!ret))
> -			return fc->kio.op->req_send(fc, req, NULL, true, false);
> -		else if (ret < 0)
> -			return;
> -	}
> -
>  	if (!fuse_request_queue_background(fc, req)) {
>  		if (!req->out.h.error)
>  			req->out.h.error = -ENOTCONN;
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 7978a1d891d2..34a4317a9689 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -530,7 +530,7 @@ struct fuse_kio_ops {
>  	int (*req_classify)(struct fuse_conn *fc, struct fuse_req *req, bool bg,
>  			    bool locked);
>  	void (*req_send)(struct fuse_conn *fc, struct fuse_req *req,
> -			 struct fuse_file *ff, bool bg, bool locked);
> +			 struct fuse_file *ff, bool bg);
>  
>  	/* Inode scope hooks */
>  	int  (*file_open)(struct fuse_conn *fc, struct file *file,
> diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> index a101d3950373..51e45911d6b8 100644
> --- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> +++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> @@ -947,7 +947,7 @@ out:
>  }
>  
>  static void pcs_fuse_submit(struct pcs_fuse_cluster *pfc, struct fuse_req *req,
> -			    struct fuse_file *ff, bool async, bool lk)
> +			    struct fuse_file *ff)
>  {
>  	struct pcs_fuse_req *r = pcs_req_from_fuse(req);
>  	struct fuse_inode *fi = get_fuse_inode(req->io_inode);
> @@ -1040,11 +1040,7 @@ static void pcs_fuse_submit(struct pcs_fuse_cluster *pfc, struct fuse_req *req,
>  error:
>  	DTRACE("do fuse_request_end req:%p op:%d err:%d\n", req, req->in.h.opcode, req->out.h.error);
>  
> -	if (lk)
> -		spin_unlock(&pfc->fc->bg_lock);
>  	request_end(pfc->fc, req);
> -	if (lk)
> -		spin_lock(&pfc->fc->bg_lock);
>  	return;
>  
>  submit:
> @@ -1052,11 +1048,7 @@ submit:
>  		req->out.h.error = -EIO;
>  		goto error;
>  	}
> -
> -	if (async)
> -		pcs_cc_submit(ireq->cc, ireq);
> -	else
> -		ireq_process(ireq);
> +	ireq_process(ireq);
>  }
>  
>  static void kpcs_setattr_end(struct fuse_conn *fc, struct fuse_req *req)
> @@ -1119,7 +1111,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, r->req.ff, true, false);
> +		pcs_fuse_submit(pfc, &r->req, r->req.ff);
>  	}
>  }
>  
> @@ -1232,64 +1224,46 @@ static int kpcs_req_classify(struct fuse_conn* fc, struct fuse_req *req,
>  		return 1;
>  
>  	BUG_ON(!pfc);
> -	/* At this point request can not belongs to any list
> -	 * so we can avoid grab fc->lock here at all.
> -	 */
> -	BUG_ON(!list_empty(&req->list));
> -
>  	DTRACE("Classify req:%p op:%d end:%p bg:%d lk:%d\n", req, req->in.h.opcode,
>  							  req->end, bg, lk);
>  	ret = pcs_kio_classify_req(fc, req, lk);
> -	if (ret) {
> -		if (ret < 0) {
> -			if (!bg)
> -				atomic_inc(&req->count);
> -			__clear_bit(FR_PENDING, &req->flags);
> -			req->out.h.error = ret;
> -			if (lk)
> -				spin_unlock(&fc->bg_lock);
> -			request_end(fc, req);
> -			if (lk)
> -				spin_lock(&fc->bg_lock);
> -			return ret;
> -		}
> -		return 1;
> -	}
> +	if (likely(!ret))
> +		return 0;
>  
> -	if (!lk) {
> -		spin_lock(&fc->bg_lock);
> -		if (fc->num_background + 1 >= fc->max_background ||
> -		    !fc->connected) {
> +	if (ret < 0) {
> +		if (!bg)
> +			atomic_inc(&req->count);
> +		__clear_bit(FR_PENDING, &req->flags);
> +		req->out.h.error = ret;
> +		if (lk)
>  			spin_unlock(&fc->bg_lock);
> -			return 1;
> -		}
> -		fc->num_background++;
> -		fc->active_background++;
> -
> -		if (fc->num_background == fc->congestion_threshold &&
> -		    fc->bdi_initialized) {
> -			set_bdi_congested(&fc->bdi, BLK_RW_SYNC);
> -			set_bdi_congested(&fc->bdi, BLK_RW_ASYNC);
> -		}
> -		spin_unlock(&fc->bg_lock);
> +		request_end(fc, req);
> +		if (lk)
> +			spin_lock(&fc->bg_lock);
> +		return ret;
>  	}
> -       return 0;
> +	return 1;
>  }
>  
>  static void kpcs_req_send(struct fuse_conn* fc, struct fuse_req *req,
> -			  struct fuse_file *ff, bool bg, bool lk)
> +			  struct fuse_file *ff, bool bg)
>  {
> -       struct pcs_fuse_cluster *pfc = (struct pcs_fuse_cluster*)fc->kio.ctx;
> +	struct pcs_fuse_cluster *pfc = (struct pcs_fuse_cluster*)fc->kio.ctx;
>  
> -       TRACE("Send req:%p op:%d end:%p bg:%d lk:%d\n",
> -               req, req->in.h.opcode, req->end, bg, lk);
> +	/* At this point request can not belongs to any list
> +	 * so we can avoid grab fc->lock here at all.
> +	 */
> +	BUG_ON(!list_empty(&req->list));
>  
> -       /* request_end below will do fuse_put_request() */
> -       if (!bg)
> -               atomic_inc(&req->count);
> +	TRACE("Send req:%p op:%d end:%p bg:%d\n",
> +		req, req->in.h.opcode, req->end, bg);
> +
> +	/* request_end below will do fuse_put_request() */
> +	if (!bg)
> +		atomic_inc(&req->count);
>  	__clear_bit(FR_PENDING, &req->flags);
>  
> -	pcs_fuse_submit(pfc, req, ff ? : req->ff, lk, lk);
> +	pcs_fuse_submit(pfc, req, ff ? : req->ff);
>  	if (!bg)
>  		wait_event(req->waitq,
>  			   test_bit(FR_FINISHED, &req->flags) && !req->end);
>