2023-09-13 01:23:38

by Kemeng Shi

[permalink] [raw]
Subject: [PATCH v2 0/2] Remove usage of FR_WAITING

v1->v2:
-Fix typo in patch 1
-Drop patch "fuse: move fuse_put_request a bit to remove forward
declaration"

This series aims to remove unneeded usage of FR_WATING flag. More
details can be found in respective patches. Thanks!

Kemeng Shi (2):
fuse: move FR_WAITING set from fuse_request_queue_background to
fuse_simple_background
fuse: remove usage of FR_WATING flag

fs/fuse/dev.c | 15 ++++-----------
fs/fuse/fuse_i.h | 2 --
2 files changed, 4 insertions(+), 13 deletions(-)

--
2.30.0


2023-09-13 01:24:06

by Kemeng Shi

[permalink] [raw]
Subject: [PATCH v2 2/2] fuse: remove usage of FR_WATING flag

Each allocated request from fuse_request_alloc counts to num_waiting
before request is freed.
Simply drop num_waiting without FR_WAITING flag check in fuse_put_request
to remove unneeded usage of FR_WAITING flag.

Signed-off-by: Kemeng Shi <[email protected]>
---
fs/fuse/dev.c | 9 +--------
fs/fuse/fuse_i.h | 2 --
2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 59e1357d4880..4f49b1946635 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -139,7 +139,6 @@ static struct fuse_req *fuse_get_req(struct fuse_mount *fm, bool for_background)
req->in.h.gid = from_kgid(fc->user_ns, current_fsgid());
req->in.h.pid = pid_nr_ns(task_pid(current), fc->pid_ns);

- __set_bit(FR_WAITING, &req->flags);
if (for_background)
__set_bit(FR_BACKGROUND, &req->flags);

@@ -171,11 +170,7 @@ static void fuse_put_request(struct fuse_req *req)
spin_unlock(&fc->bg_lock);
}

- if (test_bit(FR_WAITING, &req->flags)) {
- __clear_bit(FR_WAITING, &req->flags);
- fuse_drop_waiting(fc);
- }
-
+ fuse_drop_waiting(fc);
fuse_request_free(req);
}
}
@@ -495,7 +490,6 @@ ssize_t fuse_simple_request(struct fuse_mount *fm, struct fuse_args *args)
if (!args->nocreds)
fuse_force_creds(req);

- __set_bit(FR_WAITING, &req->flags);
__set_bit(FR_FORCE, &req->flags);
} else {
WARN_ON(args->nocreds);
@@ -556,7 +550,6 @@ int fuse_simple_background(struct fuse_mount *fm, struct fuse_args *args,
if (!req)
return -ENOMEM;
atomic_inc(&fc->num_waiting);
- __set_bit(FR_WAITING, &req->flags);
__set_bit(FR_BACKGROUND, &req->flags);
} else {
WARN_ON(args->nocreds);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index bf0b85d0b95c..a78764cef313 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -312,7 +312,6 @@ struct fuse_io_priv {
* FR_ISREPLY: set if the request has reply
* FR_FORCE: force sending of the request even if interrupted
* FR_BACKGROUND: request is sent in the background
- * FR_WAITING: request is counted as "waiting"
* FR_ABORTED: the request was aborted
* FR_INTERRUPTED: the request has been interrupted
* FR_LOCKED: data is being copied to/from the request
@@ -326,7 +325,6 @@ enum fuse_req_flag {
FR_ISREPLY,
FR_FORCE,
FR_BACKGROUND,
- FR_WAITING,
FR_ABORTED,
FR_INTERRUPTED,
FR_LOCKED,
--
2.30.0

2023-09-13 01:32:14

by Kemeng Shi

[permalink] [raw]
Subject: [PATCH v2 1/2] fuse: move FR_WAITING set from fuse_request_queue_background to fuse_simple_background

Current way to set FR_WAITING in fuse_simple_background:
fuse_simple_background
if (args->force)
fuse_request_alloc
/* need to increase num_waiting before request is queued */
else
fuse_get_req
atomic_inc(&fc->num_waiting);
__set_bit(FR_WAITING, &req->flags);

fuse_request_queue_background
if (!test_bit(FR_WAITING, &req->flags)
__set_bit(FR_WAITING, &req->flags);
atomic_inc(&fc->num_waiting);

We only need to increase num_waiting for force allocated reqeust in
fuse_request_queue_background. Simply increase num_waiting in force block
to remove unnecessary test_bit(FR_WAITING).
This patch also makes it more intuitive to remove FR_WAITING usage in next
commit.

Signed-off-by: Kemeng Shi <[email protected]>
---
fs/fuse/dev.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 1a8f82f478cb..59e1357d4880 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -528,10 +528,6 @@ static bool fuse_request_queue_background(struct fuse_req *req)
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);
if (likely(fc->connected)) {
@@ -553,10 +549,14 @@ int fuse_simple_background(struct fuse_mount *fm, struct fuse_args *args,
struct fuse_req *req;

if (args->force) {
+ struct fuse_conn *fc = fm->fc;
+
WARN_ON(!args->nocreds);
req = fuse_request_alloc(fm, gfp_flags);
if (!req)
return -ENOMEM;
+ atomic_inc(&fc->num_waiting);
+ __set_bit(FR_WAITING, &req->flags);
__set_bit(FR_BACKGROUND, &req->flags);
} else {
WARN_ON(args->nocreds);
--
2.30.0

2023-09-16 17:56:43

by Bernd Schubert

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] fuse: remove usage of FR_WATING flag



On 9/13/23 11:22, Kemeng Shi wrote:
> Each allocated request from fuse_request_alloc counts to num_waiting
> before request is freed.
> Simply drop num_waiting without FR_WAITING flag check in fuse_put_request
> to remove unneeded usage of FR_WAITING flag.
>
> Signed-off-by: Kemeng Shi <[email protected]>
> ---
> fs/fuse/dev.c | 9 +--------
> fs/fuse/fuse_i.h | 2 --
> 2 files changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 59e1357d4880..4f49b1946635 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -139,7 +139,6 @@ static struct fuse_req *fuse_get_req(struct fuse_mount *fm, bool for_background)
> req->in.h.gid = from_kgid(fc->user_ns, current_fsgid());
> req->in.h.pid = pid_nr_ns(task_pid(current), fc->pid_ns);
>
> - __set_bit(FR_WAITING, &req->flags);
> if (for_background)
> __set_bit(FR_BACKGROUND, &req->flags);
>
> @@ -171,11 +170,7 @@ static void fuse_put_request(struct fuse_req *req)
> spin_unlock(&fc->bg_lock);
> }
>
> - if (test_bit(FR_WAITING, &req->flags)) {
> - __clear_bit(FR_WAITING, &req->flags);
> - fuse_drop_waiting(fc);
> - }
> -
> + fuse_drop_waiting(fc);
> fuse_request_free(req);
> }
> }
> @@ -495,7 +490,6 @@ ssize_t fuse_simple_request(struct fuse_mount *fm, struct fuse_args *args)
> if (!args->nocreds)
> fuse_force_creds(req);
>
> - __set_bit(FR_WAITING, &req->flags);
> __set_bit(FR_FORCE, &req->flags);
> } else {
> WARN_ON(args->nocreds);
> @@ -556,7 +550,6 @@ int fuse_simple_background(struct fuse_mount *fm, struct fuse_args *args,
> if (!req)
> return -ENOMEM;
> atomic_inc(&fc->num_waiting);
> - __set_bit(FR_WAITING, &req->flags);
> __set_bit(FR_BACKGROUND, &req->flags);
> } else {
> WARN_ON(args->nocreds);
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index bf0b85d0b95c..a78764cef313 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -312,7 +312,6 @@ struct fuse_io_priv {
> * FR_ISREPLY: set if the request has reply
> * FR_FORCE: force sending of the request even if interrupted
> * FR_BACKGROUND: request is sent in the background
> - * FR_WAITING: request is counted as "waiting"
> * FR_ABORTED: the request was aborted
> * FR_INTERRUPTED: the request has been interrupted
> * FR_LOCKED: data is being copied to/from the request
> @@ -326,7 +325,6 @@ enum fuse_req_flag {
> FR_ISREPLY,
> FR_FORCE,
> FR_BACKGROUND,
> - FR_WAITING,
> FR_ABORTED,
> FR_INTERRUPTED,
> FR_LOCKED,

Thanks, looks good to me.
Reviewed-by: Bernd Schubert <[email protected]>

2023-09-16 19:46:52

by Bernd Schubert

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] fuse: move FR_WAITING set from fuse_request_queue_background to fuse_simple_background



On 9/13/23 11:22, Kemeng Shi wrote:
> Current way to set FR_WAITING in fuse_simple_background:
> fuse_simple_background
> if (args->force)
> fuse_request_alloc
> /* need to increase num_waiting before request is queued */
> else
> fuse_get_req
> atomic_inc(&fc->num_waiting);
> __set_bit(FR_WAITING, &req->flags);
>
> fuse_request_queue_background
> if (!test_bit(FR_WAITING, &req->flags)
> __set_bit(FR_WAITING, &req->flags);
> atomic_inc(&fc->num_waiting);
>
> We only need to increase num_waiting for force allocated reqeust in
> fuse_request_queue_background. Simply increase num_waiting in force block
> to remove unnecessary test_bit(FR_WAITING).
> This patch also makes it more intuitive to remove FR_WAITING usage in next
> commit.
>
> Signed-off-by: Kemeng Shi <[email protected]>
> ---
> fs/fuse/dev.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 1a8f82f478cb..59e1357d4880 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -528,10 +528,6 @@ static bool fuse_request_queue_background(struct fuse_req *req)
> 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);
> if (likely(fc->connected)) {
> @@ -553,10 +549,14 @@ int fuse_simple_background(struct fuse_mount *fm, struct fuse_args *args,
> struct fuse_req *req;
>
> if (args->force) {
> + struct fuse_conn *fc = fm->fc;
> +
> WARN_ON(!args->nocreds);
> req = fuse_request_alloc(fm, gfp_flags);
> if (!req)
> return -ENOMEM;
> + atomic_inc(&fc->num_waiting);
> + __set_bit(FR_WAITING, &req->flags);
> __set_bit(FR_BACKGROUND, &req->flags);
> } else {
> WARN_ON(args->nocreds);

Looks good to me.
Reviewed-by: Bernd Schubert <[email protected]>