* [PATCH 1/6] fuse: Use list_first_entry() in flush_bg_queue()
2018-08-27 15:29 [PATCH 0/6] Extract bg queue logic out fuse_conn::lock Kirill Tkhai
@ 2018-08-27 15:29 ` Kirill Tkhai
2018-08-27 15:29 ` [PATCH 2/6] fuse: Move clear_bit() up in request_end() Kirill Tkhai
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Kirill Tkhai @ 2018-08-27 15:29 UTC (permalink / raw)
To: miklos, ktkhai, linux-fsdevel
This cleanup patch makes the function to use the primitive
instead of direct dereferencing.
Also, move fiq dereferencing out of cycle, since it's
always constant.
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
fs/fuse/dev.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 11ea2c4a38ab..214ce96b1d26 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -353,12 +353,13 @@ 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 = &fc->iq;
+
while (fc->active_background < fc->max_background &&
!list_empty(&fc->bg_queue)) {
struct fuse_req *req;
- struct fuse_iqueue *fiq = &fc->iq;
- req = list_entry(fc->bg_queue.next, struct fuse_req, list);
+ req = list_first_entry(&fc->bg_queue, struct fuse_req, list);
list_del(&req->list);
fc->active_background++;
spin_lock(&fiq->waitq.lock);
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/6] fuse: Move clear_bit() up in request_end()
2018-08-27 15:29 [PATCH 0/6] Extract bg queue logic out fuse_conn::lock Kirill Tkhai
2018-08-27 15:29 ` [PATCH 1/6] fuse: Use list_first_entry() in flush_bg_queue() Kirill Tkhai
@ 2018-08-27 15:29 ` Kirill Tkhai
2018-08-27 15:29 ` [PATCH 3/6] fuse: Underline congestion_threshold and max_background may be read w/o fc->lock Kirill Tkhai
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Kirill Tkhai @ 2018-08-27 15:29 UTC (permalink / raw)
To: miklos, ktkhai, linux-fsdevel
Nobody can clear FR_BACKGROUND bit on processing
request in parallel, so it's possible to do it
out of fc->lock.
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
fs/fuse/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 214ce96b1d26..ce07a41ff8fe 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -390,8 +390,8 @@ static void request_end(struct fuse_conn *fc, struct fuse_req *req)
WARN_ON(test_bit(FR_PENDING, &req->flags));
WARN_ON(test_bit(FR_SENT, &req->flags));
if (test_bit(FR_BACKGROUND, &req->flags)) {
- spin_lock(&fc->lock);
clear_bit(FR_BACKGROUND, &req->flags);
+ spin_lock(&fc->lock);
if (fc->num_background == fc->max_background)
fc->blocked = 0;
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/6] fuse: Underline congestion_threshold and max_background may be read w/o fc->lock
2018-08-27 15:29 [PATCH 0/6] Extract bg queue logic out fuse_conn::lock Kirill Tkhai
2018-08-27 15:29 ` [PATCH 1/6] fuse: Use list_first_entry() in flush_bg_queue() Kirill Tkhai
2018-08-27 15:29 ` [PATCH 2/6] fuse: Move clear_bit() up in request_end() Kirill Tkhai
@ 2018-08-27 15:29 ` Kirill Tkhai
2018-08-27 15:29 ` [PATCH 4/6] fuse: Lock fc->lock during changing num_background and congestion_threshold Kirill Tkhai
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Kirill Tkhai @ 2018-08-27 15:29 UTC (permalink / raw)
To: miklos, ktkhai, linux-fsdevel
Since they are of unsigned int type, it's allowed to read them
unlocked during reporting to userspace. Let's underline this fact
with READ_ONCE() macroses.
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
fs/fuse/control.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/fuse/control.c b/fs/fuse/control.c
index e09b9cd9c3fc..c7290625e171 100644
--- a/fs/fuse/control.c
+++ b/fs/fuse/control.c
@@ -107,7 +107,7 @@ static ssize_t fuse_conn_max_background_read(struct file *file,
if (!fc)
return 0;
- val = fc->max_background;
+ val = READ_ONCE(fc->max_background);
fuse_conn_put(fc);
return fuse_conn_limit_read(file, buf, len, ppos, val);
@@ -144,7 +144,7 @@ static ssize_t fuse_conn_congestion_threshold_read(struct file *file,
if (!fc)
return 0;
- val = fc->congestion_threshold;
+ val = READ_ONCE(fc->congestion_threshold);
fuse_conn_put(fc);
return fuse_conn_limit_read(file, buf, len, ppos, val);
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/6] fuse: Lock fc->lock during changing num_background and congestion_threshold
2018-08-27 15:29 [PATCH 0/6] Extract bg queue logic out fuse_conn::lock Kirill Tkhai
` (2 preceding siblings ...)
2018-08-27 15:29 ` [PATCH 3/6] fuse: Underline congestion_threshold and max_background may be read w/o fc->lock Kirill Tkhai
@ 2018-08-27 15:29 ` Kirill Tkhai
2018-08-27 15:29 ` [PATCH 5/6] fs: Introduce fuse_conn::bg_lock Kirill Tkhai
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Kirill Tkhai @ 2018-08-27 15:29 UTC (permalink / raw)
To: miklos, ktkhai, linux-fsdevel
Functions sequences like request_end()->flush_bg_queue()
require max_background and congestion_threshold are constant
during their execution. Otherwise, checks like
if (fc->num_background == fc->max_background)
made in different time may behave not like expected.
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
fs/fuse/control.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/fs/fuse/control.c b/fs/fuse/control.c
index c7290625e171..9206501bd94d 100644
--- a/fs/fuse/control.c
+++ b/fs/fuse/control.c
@@ -125,7 +125,12 @@ static ssize_t fuse_conn_max_background_write(struct file *file,
if (ret > 0) {
struct fuse_conn *fc = fuse_ctl_file_conn_get(file);
if (fc) {
+ spin_lock(&fc->lock);
fc->max_background = val;
+ fc->blocked = (fc->num_background >= fc->max_background);
+ if (!fc->blocked && waitqueue_active(&fc->blocked_waitq))
+ wake_up(&fc->blocked_waitq);
+ spin_unlock(&fc->lock);
fuse_conn_put(fc);
}
}
@@ -155,18 +160,31 @@ static ssize_t fuse_conn_congestion_threshold_write(struct file *file,
size_t count, loff_t *ppos)
{
unsigned uninitialized_var(val);
+ struct fuse_conn *fc;
ssize_t ret;
ret = fuse_conn_limit_write(file, buf, count, ppos, &val,
max_user_congthresh);
- if (ret > 0) {
- struct fuse_conn *fc = fuse_ctl_file_conn_get(file);
- if (fc) {
- fc->congestion_threshold = val;
- fuse_conn_put(fc);
+ if (ret <= 0)
+ goto out;
+ fc = fuse_ctl_file_conn_get(file);
+ if (!fc)
+ goto out;
+
+ spin_lock(&fc->lock);
+ fc->congestion_threshold = val;
+ if (fc->sb) {
+ if (fc->num_background < fc->congestion_threshold) {
+ clear_bdi_congested(fc->sb->s_bdi, BLK_RW_SYNC);
+ clear_bdi_congested(fc->sb->s_bdi, BLK_RW_ASYNC);
+ } else {
+ set_bdi_congested(fc->sb->s_bdi, BLK_RW_SYNC);
+ set_bdi_congested(fc->sb->s_bdi, BLK_RW_ASYNC);
}
}
-
+ spin_unlock(&fc->lock);
+ fuse_conn_put(fc);
+out:
return ret;
}
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/6] fs: Introduce fuse_conn::bg_lock
2018-08-27 15:29 [PATCH 0/6] Extract bg queue logic out fuse_conn::lock Kirill Tkhai
` (3 preceding siblings ...)
2018-08-27 15:29 ` [PATCH 4/6] fuse: Lock fc->lock during changing num_background and congestion_threshold Kirill Tkhai
@ 2018-08-27 15:29 ` Kirill Tkhai
2018-08-27 15:29 ` [PATCH 6/6] fuse: Do not take fuse_conn::lock on fuse_request_send_background() Kirill Tkhai
2018-09-11 10:14 ` [PATCH 0/6] Extract bg queue logic out fuse_conn::lock Kirill Tkhai
6 siblings, 0 replies; 13+ messages in thread
From: Kirill Tkhai @ 2018-08-27 15:29 UTC (permalink / raw)
To: miklos, ktkhai, linux-fsdevel
To reduce contention of fuse_conn::lock, this patch
introduces bg_lock for protection of fields related
to background queue. These are: max_background,
congestion_threshold, num_background, active_background,
bg_queue and blocked.
This allows next patch to make async reads not requiring
fuse_conn::lock, so async reads and writes will have
better performance executed in parallel.
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
fs/fuse/control.c | 8 ++++----
fs/fuse/dev.c | 20 ++++++++++++--------
fs/fuse/file.c | 2 +-
fs/fuse/fuse_i.h | 8 ++++++--
fs/fuse/inode.c | 3 +++
5 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/fs/fuse/control.c b/fs/fuse/control.c
index 9206501bd94d..63d0e9552140 100644
--- a/fs/fuse/control.c
+++ b/fs/fuse/control.c
@@ -125,12 +125,12 @@ static ssize_t fuse_conn_max_background_write(struct file *file,
if (ret > 0) {
struct fuse_conn *fc = fuse_ctl_file_conn_get(file);
if (fc) {
- spin_lock(&fc->lock);
+ spin_lock(&fc->bg_lock);
fc->max_background = val;
fc->blocked = (fc->num_background >= fc->max_background);
if (!fc->blocked && waitqueue_active(&fc->blocked_waitq))
wake_up(&fc->blocked_waitq);
- spin_unlock(&fc->lock);
+ spin_unlock(&fc->bg_lock);
fuse_conn_put(fc);
}
}
@@ -171,7 +171,7 @@ static ssize_t fuse_conn_congestion_threshold_write(struct file *file,
if (!fc)
goto out;
- spin_lock(&fc->lock);
+ spin_lock(&fc->bg_lock);
fc->congestion_threshold = val;
if (fc->sb) {
if (fc->num_background < fc->congestion_threshold) {
@@ -182,7 +182,7 @@ static ssize_t fuse_conn_congestion_threshold_write(struct file *file,
set_bdi_congested(fc->sb->s_bdi, BLK_RW_ASYNC);
}
}
- spin_unlock(&fc->lock);
+ spin_unlock(&fc->bg_lock);
fuse_conn_put(fc);
out:
return ret;
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index ce07a41ff8fe..9690e7c79df7 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -287,10 +287,10 @@ void fuse_put_request(struct fuse_conn *fc, struct fuse_req *req)
* We get here in the unlikely case that a background
* request was allocated but not sent
*/
- spin_lock(&fc->lock);
+ spin_lock(&fc->bg_lock);
if (!fc->blocked)
wake_up(&fc->blocked_waitq);
- spin_unlock(&fc->lock);
+ spin_unlock(&fc->bg_lock);
}
if (test_bit(FR_WAITING, &req->flags)) {
@@ -391,7 +391,7 @@ static void request_end(struct fuse_conn *fc, struct fuse_req *req)
WARN_ON(test_bit(FR_SENT, &req->flags));
if (test_bit(FR_BACKGROUND, &req->flags)) {
clear_bit(FR_BACKGROUND, &req->flags);
- spin_lock(&fc->lock);
+ spin_lock(&fc->bg_lock);
if (fc->num_background == fc->max_background)
fc->blocked = 0;
@@ -406,7 +406,7 @@ static void request_end(struct fuse_conn *fc, struct fuse_req *req)
fc->num_background--;
fc->active_background--;
flush_bg_queue(fc);
- spin_unlock(&fc->lock);
+ spin_unlock(&fc->bg_lock);
}
wake_up(&req->waitq);
if (req->end)
@@ -579,8 +579,8 @@ ssize_t fuse_simple_request(struct fuse_conn *fc, struct fuse_args *args)
*
* fc->connected must have been checked previously
*/
-void fuse_request_send_background_locked(struct fuse_conn *fc,
- struct fuse_req *req)
+void fuse_request_send_background_nocheck(struct fuse_conn *fc,
+ struct fuse_req *req)
{
BUG_ON(!test_bit(FR_BACKGROUND, &req->flags));
if (!test_bit(FR_WAITING, &req->flags)) {
@@ -588,6 +588,7 @@ void fuse_request_send_background_locked(struct fuse_conn *fc,
atomic_inc(&fc->num_waiting);
}
__set_bit(FR_ISREPLY, &req->flags);
+ spin_lock(&fc->bg_lock);
fc->num_background++;
if (fc->num_background == fc->max_background)
fc->blocked = 1;
@@ -597,6 +598,7 @@ void fuse_request_send_background_locked(struct fuse_conn *fc,
}
list_add_tail(&req->list, &fc->bg_queue);
flush_bg_queue(fc);
+ spin_unlock(&fc->bg_lock);
}
void fuse_request_send_background(struct fuse_conn *fc, struct fuse_req *req)
@@ -604,7 +606,7 @@ void fuse_request_send_background(struct fuse_conn *fc, struct fuse_req *req)
BUG_ON(!req->end);
spin_lock(&fc->lock);
if (fc->connected) {
- fuse_request_send_background_locked(fc, req);
+ fuse_request_send_background_nocheck(fc, req);
spin_unlock(&fc->lock);
} else {
spin_unlock(&fc->lock);
@@ -2105,7 +2107,6 @@ void fuse_abort_conn(struct fuse_conn *fc, bool is_abort)
LIST_HEAD(to_end);
fc->connected = 0;
- fc->blocked = 0;
fc->aborted = is_abort;
fuse_set_initialized(fc);
list_for_each_entry(fud, &fc->devices, entry) {
@@ -2127,8 +2128,11 @@ void fuse_abort_conn(struct fuse_conn *fc, bool is_abort)
list_splice_tail_init(&fpq->processing, &to_end);
spin_unlock(&fpq->lock);
}
+ spin_lock(&fc->bg_lock);
+ fc->blocked = 0;
fc->max_background = UINT_MAX;
flush_bg_queue(fc);
+ spin_unlock(&fc->bg_lock);
spin_lock(&fiq->waitq.lock);
fiq->connected = 0;
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 32d0b883e74f..a875959dd939 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1502,7 +1502,7 @@ __acquires(fc->lock)
req->in.args[1].size = inarg->size;
fi->writectr++;
- fuse_request_send_background_locked(fc, req);
+ fuse_request_send_background_nocheck(fc, req);
return;
out_free:
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index f78e9614bb5f..b2c6a1cd1e2c 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -500,6 +500,10 @@ struct fuse_conn {
/** The list of background requests set aside for later queuing */
struct list_head bg_queue;
+ /** Protects: max_background, congestion_threshold, num_background,
+ * active_background, bg_queue, blocked */
+ spinlock_t bg_lock;
+
/** Flag indicating that INIT reply has been received. Allocating
* any fuse request will be suspended until the flag is set */
int initialized;
@@ -857,8 +861,8 @@ ssize_t fuse_simple_request(struct fuse_conn *fc, struct fuse_args *args);
*/
void fuse_request_send_background(struct fuse_conn *fc, struct fuse_req *req);
-void fuse_request_send_background_locked(struct fuse_conn *fc,
- struct fuse_req *req);
+void fuse_request_send_background_nocheck(struct fuse_conn *fc,
+ struct fuse_req *req);
/* Abort all requests */
void fuse_abort_conn(struct fuse_conn *fc, bool is_abort);
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 9db8c0c745a9..a4285ec7c248 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -606,6 +606,7 @@ void fuse_conn_init(struct fuse_conn *fc, struct user_namespace *user_ns)
{
memset(fc, 0, sizeof(*fc));
spin_lock_init(&fc->lock);
+ spin_lock_init(&fc->bg_lock);
init_rwsem(&fc->killsb);
refcount_set(&fc->count, 1);
atomic_set(&fc->dev_count, 1);
@@ -853,6 +854,7 @@ static void process_init_limits(struct fuse_conn *fc, struct fuse_init_out *arg)
sanitize_global_limit(&max_user_bgreq);
sanitize_global_limit(&max_user_congthresh);
+ spin_lock(&fc->bg_lock);
if (arg->max_background) {
fc->max_background = arg->max_background;
@@ -866,6 +868,7 @@ static void process_init_limits(struct fuse_conn *fc, struct fuse_init_out *arg)
fc->congestion_threshold > max_user_congthresh)
fc->congestion_threshold = max_user_congthresh;
}
+ spin_unlock(&fc->bg_lock);
}
static void process_init_reply(struct fuse_conn *fc, struct fuse_req *req)
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 6/6] fuse: Do not take fuse_conn::lock on fuse_request_send_background()
2018-08-27 15:29 [PATCH 0/6] Extract bg queue logic out fuse_conn::lock Kirill Tkhai
` (4 preceding siblings ...)
2018-08-27 15:29 ` [PATCH 5/6] fs: Introduce fuse_conn::bg_lock Kirill Tkhai
@ 2018-08-27 15:29 ` Kirill Tkhai
2018-09-26 12:25 ` Miklos Szeredi
2018-09-11 10:14 ` [PATCH 0/6] Extract bg queue logic out fuse_conn::lock Kirill Tkhai
6 siblings, 1 reply; 13+ messages in thread
From: Kirill Tkhai @ 2018-08-27 15:29 UTC (permalink / raw)
To: miklos, ktkhai, linux-fsdevel
Currently, we take fc->lock there only to check for fc->connected.
But this flag is changed only on connection abort, which is very
rare operation. Good thing looks to make fuse_request_send_background()
faster, while fuse_abort_conn() slowler.
So, we make fuse_request_send_background() lockless and mark
(fc->connected == 1) region as RCU-protected. Abort function
just uses synchronize_sched() to wait till all pending background
requests is being queued, and then makes ordinary abort.
Note, that synchronize_sched() is used instead of synchronize_rcu(),
since we want to check for fc->connected without rcu_dereference()
in fuse_request_send_background() (i.e., not to add memory barriers
to this hot path).
Also, note fuse_conn::aborting field is introduced. It's aimed
to synchronize two sequential aborts executing in parallel:
we do not want the secondly called abort finishes before the first
one is done, after it sees fc->connected has already became 0.
Our storage test shows performance increase on parallel read and write
with the patchset (7 test runs, 300 seconds of execution of each, aio):
./io_iops --read --write --iops -u 48g -s 4k -p 24 -n 24 --aio -q 128 -t 300 -f /mnt/vstorage/file
Before (iops):
25721.58203 (worst)
26179.43359
26092.58594
25789.96484
26238.63477 (best)
25985.88867
25987.66406
After (iops):
26852.27539
27547.60742
26322.29688
27643.33398 (best)
26753.10547
26157.91016 (worst)
26694.63477
All *after* runs are better than any *before* run, except of one.
The best *after* run is 5% better than the best *before* run.
The worst *after* run is 1.7% better then the worst *before* run.
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
fs/fuse/dev.c | 17 ++++++++++++-----
fs/fuse/fuse_i.h | 3 +++
fs/fuse/inode.c | 1 +
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 9690e7c79df7..064a65ca7283 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -575,8 +575,6 @@ ssize_t fuse_simple_request(struct fuse_conn *fc, struct fuse_args *args)
}
/*
- * Called under fc->lock
- *
* fc->connected must have been checked previously
*/
void fuse_request_send_background_nocheck(struct fuse_conn *fc,
@@ -604,12 +602,12 @@ void fuse_request_send_background_nocheck(struct fuse_conn *fc,
void fuse_request_send_background(struct fuse_conn *fc, struct fuse_req *req)
{
BUG_ON(!req->end);
- spin_lock(&fc->lock);
+ rcu_read_lock_sched();
if (fc->connected) {
fuse_request_send_background_nocheck(fc, req);
- spin_unlock(&fc->lock);
+ rcu_read_unlock_sched();
} else {
- spin_unlock(&fc->lock);
+ rcu_read_unlock_sched();
req->out.h.error = -ENOTCONN;
req->end(fc, req);
fuse_put_request(fc, req);
@@ -2107,6 +2105,13 @@ void fuse_abort_conn(struct fuse_conn *fc, bool is_abort)
LIST_HEAD(to_end);
fc->connected = 0;
+ fc->aborting = true;
+ spin_unlock(&fc->lock);
+
+ /* Propagate fc->connected */
+ synchronize_sched();
+
+ spin_lock(&fc->lock);
fc->aborted = is_abort;
fuse_set_initialized(fc);
list_for_each_entry(fud, &fc->devices, entry) {
@@ -2145,12 +2150,14 @@ void fuse_abort_conn(struct fuse_conn *fc, bool is_abort)
spin_unlock(&fiq->waitq.lock);
kill_fasync(&fiq->fasync, SIGIO, POLL_IN);
end_polls(fc);
+ fc->aborting = false;
wake_up_all(&fc->blocked_waitq);
spin_unlock(&fc->lock);
end_requests(fc, &to_end);
} else {
spin_unlock(&fc->lock);
+ wait_event(fc->blocked_waitq, !fc->aborting);
}
}
EXPORT_SYMBOL_GPL(fuse_abort_conn);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index b2c6a1cd1e2c..b9a991b42c88 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -523,6 +523,9 @@ struct fuse_conn {
abort and device release */
unsigned connected;
+ /** Connection is now aborting */
+ bool aborting;
+
/** Connection aborted via sysfs */
bool aborted;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index a4285ec7c248..9612eb4bc609 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -624,6 +624,7 @@ void fuse_conn_init(struct fuse_conn *fc, struct user_namespace *user_ns)
fc->blocked = 0;
fc->initialized = 0;
fc->connected = 1;
+ fc->aborting = false;
fc->attr_version = 1;
get_random_bytes(&fc->scramble_key, sizeof(fc->scramble_key));
fc->pid_ns = get_pid_ns(task_active_pid_ns(current));
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 6/6] fuse: Do not take fuse_conn::lock on fuse_request_send_background()
2018-08-27 15:29 ` [PATCH 6/6] fuse: Do not take fuse_conn::lock on fuse_request_send_background() Kirill Tkhai
@ 2018-09-26 12:25 ` Miklos Szeredi
2018-09-26 15:18 ` Kirill Tkhai
0 siblings, 1 reply; 13+ messages in thread
From: Miklos Szeredi @ 2018-09-26 12:25 UTC (permalink / raw)
To: Kirill Tkhai; +Cc: linux-fsdevel
On Mon, Aug 27, 2018 at 06:29:56PM +0300, Kirill Tkhai wrote:
> Currently, we take fc->lock there only to check for fc->connected.
> But this flag is changed only on connection abort, which is very
> rare operation. Good thing looks to make fuse_request_send_background()
> faster, while fuse_abort_conn() slowler.
>
> So, we make fuse_request_send_background() lockless and mark
> (fc->connected == 1) region as RCU-protected. Abort function
> just uses synchronize_sched() to wait till all pending background
> requests is being queued, and then makes ordinary abort.
>
> Note, that synchronize_sched() is used instead of synchronize_rcu(),
> since we want to check for fc->connected without rcu_dereference()
> in fuse_request_send_background() (i.e., not to add memory barriers
> to this hot path).
Apart from the inaccuracies in the above (_sched variant is for scheduling and
NMI taking code; _sched variant requires rcu_dereference() as well;
rcu_dereference() does not add barriers; rcu_dereference() is only for pointers,
so we can't use it for an integer), wouldn't it be simpler to just use bg_lock
for checking ->connected, and lock bg_lock (as well as fc->lock) when setting
->connected?
Updated patch below (untested).
Thanks,
Miklos
---
Subject: fuse: do not take fc->lock in fuse_request_send_background()
From: Kirill Tkhai <ktkhai@virtuozzo.com>
Date: Mon, 27 Aug 2018 18:29:56 +0300
Currently, we take fc->lock there only to check for fc->connected.
But this flag is changed only on connection abort, which is very
rare operation.
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
fs/fuse/dev.c | 46 +++++++++++++++++++++++-----------------------
fs/fuse/file.c | 4 +++-
fs/fuse/fuse_i.h | 4 +---
3 files changed, 27 insertions(+), 27 deletions(-)
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -574,42 +574,38 @@ ssize_t fuse_simple_request(struct fuse_
return ret;
}
-/*
- * Called under fc->lock
- *
- * fc->connected must have been checked previously
- */
-void fuse_request_send_background_nocheck(struct fuse_conn *fc,
- struct fuse_req *req)
+bool fuse_request_queue_background(struct fuse_conn *fc, struct fuse_req *req)
{
- BUG_ON(!test_bit(FR_BACKGROUND, &req->flags));
+ bool queued = false;
+
+ WARN_ON(!test_bit(FR_BACKGROUND, &req->flags));
if (!test_bit(FR_WAITING, &req->flags)) {
__set_bit(FR_WAITING, &req->flags);
atomic_inc(&fc->num_waiting);
}
__set_bit(FR_ISREPLY, &req->flags);
spin_lock(&fc->bg_lock);
- fc->num_background++;
- if (fc->num_background == fc->max_background)
- fc->blocked = 1;
- if (fc->num_background == fc->congestion_threshold && fc->sb) {
- set_bdi_congested(fc->sb->s_bdi, BLK_RW_SYNC);
- set_bdi_congested(fc->sb->s_bdi, BLK_RW_ASYNC);
+ if (likely(fc->connected)) {
+ fc->num_background++;
+ if (fc->num_background == fc->max_background)
+ fc->blocked = 1;
+ if (fc->num_background == fc->congestion_threshold && fc->sb) {
+ set_bdi_congested(fc->sb->s_bdi, BLK_RW_SYNC);
+ set_bdi_congested(fc->sb->s_bdi, BLK_RW_ASYNC);
+ }
+ list_add_tail(&req->list, &fc->bg_queue);
+ flush_bg_queue(fc);
+ queued = true;
}
- list_add_tail(&req->list, &fc->bg_queue);
- flush_bg_queue(fc);
spin_unlock(&fc->bg_lock);
+
+ return queued;
}
void fuse_request_send_background(struct fuse_conn *fc, struct fuse_req *req)
{
- BUG_ON(!req->end);
- spin_lock(&fc->lock);
- if (fc->connected) {
- fuse_request_send_background_nocheck(fc, req);
- spin_unlock(&fc->lock);
- } else {
- spin_unlock(&fc->lock);
+ WARN_ON(!req->end);
+ if (!fuse_request_queue_background(fc, req)) {
req->out.h.error = -ENOTCONN;
req->end(fc, req);
fuse_put_request(fc, req);
@@ -2112,7 +2108,11 @@ void fuse_abort_conn(struct fuse_conn *f
struct fuse_req *req, *next;
LIST_HEAD(to_end);
+ /* Background queuing checks fc->connected under bg_lock */
+ spin_lock(&fc->bg_lock);
fc->connected = 0;
+ spin_unlock(&fc->bg_lock);
+
fc->aborted = is_abort;
fuse_set_initialized(fc);
list_for_each_entry(fud, &fc->devices, entry) {
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -863,9 +863,7 @@ ssize_t fuse_simple_request(struct fuse_
* Send a request in the background
*/
void fuse_request_send_background(struct fuse_conn *fc, struct fuse_req *req);
-
-void fuse_request_send_background_nocheck(struct fuse_conn *fc,
- struct fuse_req *req);
+bool fuse_request_queue_background(struct fuse_conn *fc, struct fuse_req *req);
/* Abort all requests */
void fuse_abort_conn(struct fuse_conn *fc, bool is_abort);
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1487,6 +1487,7 @@ __acquires(fc->lock)
struct fuse_inode *fi = get_fuse_inode(req->inode);
struct fuse_write_in *inarg = &req->misc.write.in;
__u64 data_size = req->num_pages * PAGE_SIZE;
+ bool queued;
if (!fc->connected)
goto out_free;
@@ -1502,7 +1503,8 @@ __acquires(fc->lock)
req->in.args[1].size = inarg->size;
fi->writectr++;
- fuse_request_send_background_nocheck(fc, req);
+ queued = fuse_request_queue_background(fc, req);
+ WARN_ON(!queued);
return;
out_free:
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 6/6] fuse: Do not take fuse_conn::lock on fuse_request_send_background()
2018-09-26 12:25 ` Miklos Szeredi
@ 2018-09-26 15:18 ` Kirill Tkhai
2018-09-27 8:37 ` Kirill Tkhai
0 siblings, 1 reply; 13+ messages in thread
From: Kirill Tkhai @ 2018-09-26 15:18 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel
On 26.09.2018 15:25, Miklos Szeredi wrote:
> On Mon, Aug 27, 2018 at 06:29:56PM +0300, Kirill Tkhai wrote:
>> Currently, we take fc->lock there only to check for fc->connected.
>> But this flag is changed only on connection abort, which is very
>> rare operation. Good thing looks to make fuse_request_send_background()
>> faster, while fuse_abort_conn() slowler.
>>
>> So, we make fuse_request_send_background() lockless and mark
>> (fc->connected == 1) region as RCU-protected. Abort function
>> just uses synchronize_sched() to wait till all pending background
>> requests is being queued, and then makes ordinary abort.
>>
>> Note, that synchronize_sched() is used instead of synchronize_rcu(),
>> since we want to check for fc->connected without rcu_dereference()
>> in fuse_request_send_background() (i.e., not to add memory barriers
>> to this hot path).
>
> Apart from the inaccuracies in the above (_sched variant is for scheduling and
> NMI taking code; _sched variant requires rcu_dereference() as well;
> rcu_dereference() does not add barriers; rcu_dereference() is only for pointers,
> so we can't use it for an integer),
Writing this I was inspired by expand_fdtable(). Yes, the description confuses,
and we don't need rcu_dereference() since we do not touch memory pointed by __rcu
pointer, we have no pointer at all. synchronize_sched() guarantees:
On systems with more than one CPU, when synchronize_sched() returns,
each CPU is guaranteed to have executed a full memory barrier since the
end of its last RCU-sched read-side critical section whose beginning
preceded the call to synchronize_sched().
(and rcu_dereference() unfolds in smp_read_barrier_depends(), which I mean as
added barriers)
But it does not so matter. I'm OK with the patch you updated.
>wouldn't it be simpler to just use bg_lock
> for checking ->connected, and lock bg_lock (as well as fc->lock) when setting
> ->connected?
>
> Updated patch below (untested).
Tested it. Works for me.
Thanks,
Kirill
>
> ---
> Subject: fuse: do not take fc->lock in fuse_request_send_background()
> From: Kirill Tkhai <ktkhai@virtuozzo.com>
> Date: Mon, 27 Aug 2018 18:29:56 +0300
>
> Currently, we take fc->lock there only to check for fc->connected.
> But this flag is changed only on connection abort, which is very
> rare operation.
>
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
> fs/fuse/dev.c | 46 +++++++++++++++++++++++-----------------------
> fs/fuse/file.c | 4 +++-
> fs/fuse/fuse_i.h | 4 +---
> 3 files changed, 27 insertions(+), 27 deletions(-)
>
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -574,42 +574,38 @@ ssize_t fuse_simple_request(struct fuse_
> return ret;
> }
>
> -/*
> - * Called under fc->lock
> - *
> - * fc->connected must have been checked previously
> - */
> -void fuse_request_send_background_nocheck(struct fuse_conn *fc,
> - struct fuse_req *req)
> +bool fuse_request_queue_background(struct fuse_conn *fc, struct fuse_req *req)
> {
> - BUG_ON(!test_bit(FR_BACKGROUND, &req->flags));
> + bool queued = false;
> +
> + WARN_ON(!test_bit(FR_BACKGROUND, &req->flags));
> if (!test_bit(FR_WAITING, &req->flags)) {
> __set_bit(FR_WAITING, &req->flags);
> atomic_inc(&fc->num_waiting);
> }
> __set_bit(FR_ISREPLY, &req->flags);
> spin_lock(&fc->bg_lock);
> - fc->num_background++;
> - if (fc->num_background == fc->max_background)
> - fc->blocked = 1;
> - if (fc->num_background == fc->congestion_threshold && fc->sb) {
> - set_bdi_congested(fc->sb->s_bdi, BLK_RW_SYNC);
> - set_bdi_congested(fc->sb->s_bdi, BLK_RW_ASYNC);
> + if (likely(fc->connected)) {
> + fc->num_background++;
> + if (fc->num_background == fc->max_background)
> + fc->blocked = 1;
> + if (fc->num_background == fc->congestion_threshold && fc->sb) {
> + set_bdi_congested(fc->sb->s_bdi, BLK_RW_SYNC);
> + set_bdi_congested(fc->sb->s_bdi, BLK_RW_ASYNC);
> + }
> + list_add_tail(&req->list, &fc->bg_queue);
> + flush_bg_queue(fc);
> + queued = true;
> }
> - list_add_tail(&req->list, &fc->bg_queue);
> - flush_bg_queue(fc);
> spin_unlock(&fc->bg_lock);
> +
> + return queued;
> }
>
> void fuse_request_send_background(struct fuse_conn *fc, struct fuse_req *req)
> {
> - BUG_ON(!req->end);
> - spin_lock(&fc->lock);
> - if (fc->connected) {
> - fuse_request_send_background_nocheck(fc, req);
> - spin_unlock(&fc->lock);
> - } else {
> - spin_unlock(&fc->lock);
> + WARN_ON(!req->end);
> + if (!fuse_request_queue_background(fc, req)) {
> req->out.h.error = -ENOTCONN;
> req->end(fc, req);
> fuse_put_request(fc, req);
> @@ -2112,7 +2108,11 @@ void fuse_abort_conn(struct fuse_conn *f
> struct fuse_req *req, *next;
> LIST_HEAD(to_end);
>
> + /* Background queuing checks fc->connected under bg_lock */
> + spin_lock(&fc->bg_lock);
> fc->connected = 0;
> + spin_unlock(&fc->bg_lock);
> +
> fc->aborted = is_abort;
> fuse_set_initialized(fc);
> list_for_each_entry(fud, &fc->devices, entry) {
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -863,9 +863,7 @@ ssize_t fuse_simple_request(struct fuse_
> * Send a request in the background
> */
> void fuse_request_send_background(struct fuse_conn *fc, struct fuse_req *req);
> -
> -void fuse_request_send_background_nocheck(struct fuse_conn *fc,
> - struct fuse_req *req);
> +bool fuse_request_queue_background(struct fuse_conn *fc, struct fuse_req *req);
>
> /* Abort all requests */
> void fuse_abort_conn(struct fuse_conn *fc, bool is_abort);
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1487,6 +1487,7 @@ __acquires(fc->lock)
> struct fuse_inode *fi = get_fuse_inode(req->inode);
> struct fuse_write_in *inarg = &req->misc.write.in;
> __u64 data_size = req->num_pages * PAGE_SIZE;
> + bool queued;
>
> if (!fc->connected)
> goto out_free;
> @@ -1502,7 +1503,8 @@ __acquires(fc->lock)
>
> req->in.args[1].size = inarg->size;
> fi->writectr++;
> - fuse_request_send_background_nocheck(fc, req);
> + queued = fuse_request_queue_background(fc, req);
> + WARN_ON(!queued);
> return;
>
> out_free:
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 6/6] fuse: Do not take fuse_conn::lock on fuse_request_send_background()
2018-09-26 15:18 ` Kirill Tkhai
@ 2018-09-27 8:37 ` Kirill Tkhai
2018-09-27 11:25 ` Miklos Szeredi
0 siblings, 1 reply; 13+ messages in thread
From: Kirill Tkhai @ 2018-09-27 8:37 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel
Hi, Miklos,
should I resend the series with the patch you changed,
or you are already taken it since there is your SoB?
Thanks,
Kirill
On 26.09.2018 18:18, Kirill Tkhai wrote:
> On 26.09.2018 15:25, Miklos Szeredi wrote:
>> On Mon, Aug 27, 2018 at 06:29:56PM +0300, Kirill Tkhai wrote:
>>> Currently, we take fc->lock there only to check for fc->connected.
>>> But this flag is changed only on connection abort, which is very
>>> rare operation. Good thing looks to make fuse_request_send_background()
>>> faster, while fuse_abort_conn() slowler.
>>>
>>> So, we make fuse_request_send_background() lockless and mark
>>> (fc->connected == 1) region as RCU-protected. Abort function
>>> just uses synchronize_sched() to wait till all pending background
>>> requests is being queued, and then makes ordinary abort.
>>>
>>> Note, that synchronize_sched() is used instead of synchronize_rcu(),
>>> since we want to check for fc->connected without rcu_dereference()
>>> in fuse_request_send_background() (i.e., not to add memory barriers
>>> to this hot path).
>>
>> Apart from the inaccuracies in the above (_sched variant is for scheduling and
>> NMI taking code; _sched variant requires rcu_dereference() as well;
>> rcu_dereference() does not add barriers; rcu_dereference() is only for pointers,
>> so we can't use it for an integer),
>
> Writing this I was inspired by expand_fdtable(). Yes, the description confuses,
> and we don't need rcu_dereference() since we do not touch memory pointed by __rcu
> pointer, we have no pointer at all. synchronize_sched() guarantees:
>
> On systems with more than one CPU, when synchronize_sched() returns,
> each CPU is guaranteed to have executed a full memory barrier since the
> end of its last RCU-sched read-side critical section whose beginning
> preceded the call to synchronize_sched().
>
> (and rcu_dereference() unfolds in smp_read_barrier_depends(), which I mean as
> added barriers)
>
> But it does not so matter. I'm OK with the patch you updated.
>
>> wouldn't it be simpler to just use bg_lock
>> for checking ->connected, and lock bg_lock (as well as fc->lock) when setting
>> ->connected?
>>
>> Updated patch below (untested).
>
> Tested it. Works for me.
>
> Thanks,
> Kirill
>
>>
>> ---
>> Subject: fuse: do not take fc->lock in fuse_request_send_background()
>> From: Kirill Tkhai <ktkhai@virtuozzo.com>
>> Date: Mon, 27 Aug 2018 18:29:56 +0300
>>
>> Currently, we take fc->lock there only to check for fc->connected.
>> But this flag is changed only on connection abort, which is very
>> rare operation.
>>
>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
>> ---
>> fs/fuse/dev.c | 46 +++++++++++++++++++++++-----------------------
>> fs/fuse/file.c | 4 +++-
>> fs/fuse/fuse_i.h | 4 +---
>> 3 files changed, 27 insertions(+), 27 deletions(-)
>>
>> --- a/fs/fuse/dev.c
>> +++ b/fs/fuse/dev.c
>> @@ -574,42 +574,38 @@ ssize_t fuse_simple_request(struct fuse_
>> return ret;
>> }
>>
>> -/*
>> - * Called under fc->lock
>> - *
>> - * fc->connected must have been checked previously
>> - */
>> -void fuse_request_send_background_nocheck(struct fuse_conn *fc,
>> - struct fuse_req *req)
>> +bool fuse_request_queue_background(struct fuse_conn *fc, struct fuse_req *req)
>> {
>> - BUG_ON(!test_bit(FR_BACKGROUND, &req->flags));
>> + bool queued = false;
>> +
>> + WARN_ON(!test_bit(FR_BACKGROUND, &req->flags));
>> if (!test_bit(FR_WAITING, &req->flags)) {
>> __set_bit(FR_WAITING, &req->flags);
>> atomic_inc(&fc->num_waiting);
>> }
>> __set_bit(FR_ISREPLY, &req->flags);
>> spin_lock(&fc->bg_lock);
>> - fc->num_background++;
>> - if (fc->num_background == fc->max_background)
>> - fc->blocked = 1;
>> - if (fc->num_background == fc->congestion_threshold && fc->sb) {
>> - set_bdi_congested(fc->sb->s_bdi, BLK_RW_SYNC);
>> - set_bdi_congested(fc->sb->s_bdi, BLK_RW_ASYNC);
>> + if (likely(fc->connected)) {
>> + fc->num_background++;
>> + if (fc->num_background == fc->max_background)
>> + fc->blocked = 1;
>> + if (fc->num_background == fc->congestion_threshold && fc->sb) {
>> + set_bdi_congested(fc->sb->s_bdi, BLK_RW_SYNC);
>> + set_bdi_congested(fc->sb->s_bdi, BLK_RW_ASYNC);
>> + }
>> + list_add_tail(&req->list, &fc->bg_queue);
>> + flush_bg_queue(fc);
>> + queued = true;
>> }
>> - list_add_tail(&req->list, &fc->bg_queue);
>> - flush_bg_queue(fc);
>> spin_unlock(&fc->bg_lock);
>> +
>> + return queued;
>> }
>>
>> void fuse_request_send_background(struct fuse_conn *fc, struct fuse_req *req)
>> {
>> - BUG_ON(!req->end);
>> - spin_lock(&fc->lock);
>> - if (fc->connected) {
>> - fuse_request_send_background_nocheck(fc, req);
>> - spin_unlock(&fc->lock);
>> - } else {
>> - spin_unlock(&fc->lock);
>> + WARN_ON(!req->end);
>> + if (!fuse_request_queue_background(fc, req)) {
>> req->out.h.error = -ENOTCONN;
>> req->end(fc, req);
>> fuse_put_request(fc, req);
>> @@ -2112,7 +2108,11 @@ void fuse_abort_conn(struct fuse_conn *f
>> struct fuse_req *req, *next;
>> LIST_HEAD(to_end);
>>
>> + /* Background queuing checks fc->connected under bg_lock */
>> + spin_lock(&fc->bg_lock);
>> fc->connected = 0;
>> + spin_unlock(&fc->bg_lock);
>> +
>> fc->aborted = is_abort;
>> fuse_set_initialized(fc);
>> list_for_each_entry(fud, &fc->devices, entry) {
>> --- a/fs/fuse/fuse_i.h
>> +++ b/fs/fuse/fuse_i.h
>> @@ -863,9 +863,7 @@ ssize_t fuse_simple_request(struct fuse_
>> * Send a request in the background
>> */
>> void fuse_request_send_background(struct fuse_conn *fc, struct fuse_req *req);
>> -
>> -void fuse_request_send_background_nocheck(struct fuse_conn *fc,
>> - struct fuse_req *req);
>> +bool fuse_request_queue_background(struct fuse_conn *fc, struct fuse_req *req);
>>
>> /* Abort all requests */
>> void fuse_abort_conn(struct fuse_conn *fc, bool is_abort);
>> --- a/fs/fuse/file.c
>> +++ b/fs/fuse/file.c
>> @@ -1487,6 +1487,7 @@ __acquires(fc->lock)
>> struct fuse_inode *fi = get_fuse_inode(req->inode);
>> struct fuse_write_in *inarg = &req->misc.write.in;
>> __u64 data_size = req->num_pages * PAGE_SIZE;
>> + bool queued;
>>
>> if (!fc->connected)
>> goto out_free;
>> @@ -1502,7 +1503,8 @@ __acquires(fc->lock)
>>
>> req->in.args[1].size = inarg->size;
>> fi->writectr++;
>> - fuse_request_send_background_nocheck(fc, req);
>> + queued = fuse_request_queue_background(fc, req);
>> + WARN_ON(!queued);
>> return;
>>
>> out_free:
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 6/6] fuse: Do not take fuse_conn::lock on fuse_request_send_background()
2018-09-27 8:37 ` Kirill Tkhai
@ 2018-09-27 11:25 ` Miklos Szeredi
0 siblings, 0 replies; 13+ messages in thread
From: Miklos Szeredi @ 2018-09-27 11:25 UTC (permalink / raw)
To: Kirill Tkhai; +Cc: linux-fsdevel
On Thu, Sep 27, 2018 at 10:37 AM, Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
> Hi, Miklos,
>
> should I resend the series with the patch you changed,
> or you are already taken it since there is your SoB?
I already took that series.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/6] Extract bg queue logic out fuse_conn::lock
2018-08-27 15:29 [PATCH 0/6] Extract bg queue logic out fuse_conn::lock Kirill Tkhai
` (5 preceding siblings ...)
2018-08-27 15:29 ` [PATCH 6/6] fuse: Do not take fuse_conn::lock on fuse_request_send_background() Kirill Tkhai
@ 2018-09-11 10:14 ` Kirill Tkhai
2018-10-01 9:23 ` Miklos Szeredi
6 siblings, 1 reply; 13+ messages in thread
From: Kirill Tkhai @ 2018-09-11 10:14 UTC (permalink / raw)
To: miklos, linux-fsdevel
Hi, Miklos,
Have you seen this? Some reaction on this?
Thanks,
Kirill
On 8/27/18 6:29 PM, Kirill Tkhai wrote:
> Hi, Miklos,
>
> the patchset makes background queue and background-related
> data be protected with new bg_lock. This allows async reads
> not take fc->lock in fuse_request_send_background(), and
> this gives performace-visible increase on out async read-write
> test (1.7-5 % better). See [6/6] for the test results.
>
> The new lock protects max_background, congestion_threshold,
> num_background, active_background, bg_queue and blocked
> fields. See [5/6] for the details.
>
> Patches [1-3/6] are preparations. I've already sent you
> two of them, and you said at least one is already applied,
> but since they are yet no in linux-next, I add them to make
> kbuild test robot happy.
>
> [4/6] is a fix of existing issue. I don't know whether
> stable needs this fix, so I don't send it in separate. Please,
> say if it's needed resend it in this way.
>
> The test results is for our proprietary vstorage fuse driver.
> Feel free to ask me, in case of you want the patchset to be
> tested on some other fuse driver and with some other test.
>
> Thanks,
> Kirill
> ---
>
> Kirill Tkhai (6):
> fuse: Use list_first_entry() in flush_bg_queue()
> fuse: Move clear_bit() up in request_end()
> fuse: Underline congestion_threshold and max_background may be read w/o fc->lock
> fuse: Lock fc->lock during changing num_background and congestion_threshold
> fs: Introduce fuse_conn::bg_lock
> fuse: Do not take fuse_conn::lock on fuse_request_send_background()
>
>
> fs/fuse/control.c | 34 ++++++++++++++++++++++++++--------
> fs/fuse/dev.c | 42 +++++++++++++++++++++++++++---------------
> fs/fuse/file.c | 2 +-
> fs/fuse/fuse_i.h | 11 +++++++++--
> fs/fuse/inode.c | 4 ++++
> 5 files changed, 67 insertions(+), 26 deletions(-)
>
> --
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>
^ permalink raw reply [flat|nested] 13+ messages in thread