On 5/29/24 11:52 PM, Miklos Szeredi wrote:
> Virtiofs has its own queing mechanism, but still requests are first queued
> on fiq->pending to be immediately dequeued and queued onto the virtio
> queue.
>
> The queuing on fiq->pending is unnecessary and might even have some
> performance impact due to being a contention point.
>
> Forget requests are handled similarly.
>
> Move the queuing of requests and forgets into the fiq->ops->*.
> fuse_iqueue_ops are renamed to reflect the new semantics.
>
> Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxxxxx>
> ---
[...]
> +static void fuse_dev_queue_interrupt(struct fuse_iqueue *fiq, struct fuse_req *req)
> +{
> + spin_lock(&fiq->lock);
> + if (list_empty(&req->intr_entry)) {
> + list_add_tail(&req->intr_entry, &fiq->interrupts);
> + /*
> + * Pairs with smp_mb() implied by test_and_set_bit()
> + * from fuse_request_end().
> + */
> + smp_mb();
> + if (test_bit(FR_FINISHED, &req->flags)) {
> + list_del_init(&req->intr_entry);
> + spin_unlock(&fiq->lock ^
missing "return" here?
> + }
> + fuse_dev_wake_and_unlock(fiq);
> + } else {
> + spin_unlock(&fiq->lock);
> + }
> +}
[...]
> static void fuse_adjust_compat(struct fuse_conn *fc, struct fuse_args *args)
> @@ -581,7 +605,6 @@ static int fuse_simple_notify_reply(struct fuse_mount *fm,
> {
> struct fuse_req *req;
> struct fuse_iqueue *fiq = &fm->fc->iq;
> - int err = 0;
>
> req = fuse_get_req(fm, false);
> if (IS_ERR(req))> @@ -592,16 +615,9 @@ static int fuse_simple_notify_reply(struct
fuse_mount *fm,
>
> fuse_args_to_req(req, args);
>
> - spin_lock(&fiq->lock);
> - if (fiq->connected) {
> - queue_request_and_unlock(fiq, req);
> - } else {
> - err = -ENODEV;
> - spin_unlock(&fiq->lock);
> - fuse_put_request(req);
> - }
> + fuse_send_one(fiq, req);
>
> - return err;
> + return 0;
> }
There's a minor changed behavior visible to users. Prior to the patch,
the FUSE_NOTIFY_RETRIEVE will returns -ENODEV when the connection is
aborted, but now it returns 0.
It seems only example/notify_store_retrieve.c has used
FUSE_NOTIFY_RETRIEVE in libfuse. I'm not sure if this change really
matters.
Maybe we could check req->out.h.error after fuse_send_one() returns?
--
Thanks,
Jingbo
