[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJaqyWfwPCOSrgJ=rBFm4oJph5wPG2xu4Gvr-WFntqoA0n4o=Q@mail.gmail.com>
Date: Mon, 24 Feb 2025 08:44:39 +0100
From: Eugenio Perez Martin <eperezma@...hat.com>
To: Jason Wang <jasowang@...hat.com>
Cc: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Stefan Hajnoczi <stefanha@...hat.com>,
Hanna Reitz <hreitz@...hat.com>, linux-kernel@...r.kernel.org,
German Maglione <gmaglione@...hat.com>, virtualization@...ts.linux.dev,
Stefano Garzarella <sgarzare@...hat.com>, yama@...hat.com, Vivek Goyal <vgoyal@...hat.com>,
Miklos Szeredi <miklos@...redi.hu>, mst@...hat.com
Subject: Re: [RFC v2 5/5] virtiofs: Disable notifications more aggresively
On Mon, Feb 24, 2025 at 7:44 AM Eugenio Perez Martin
<eperezma@...hat.com> wrote:
>
> On Mon, Feb 24, 2025 at 3:01 AM Jason Wang <jasowang@...hat.com> wrote:
> >
> > On Sat, Feb 22, 2025 at 1:07 AM Eugenio Pérez <eperezma@...hat.com> wrote:
> > >
> > > Disable notifications right before scheduling, instead of waiting until
> > > the work is running.
> > >
> > > After applying this patch, fio read test goes from 1191MiB/s to
> > > 1263.14Mib/s
> >
> > Let's describe more about the testing. E.g FIO parameters, test setups.
> >
> > >
> > > Signed-off-by: Eugenio Pérez <eperezma@...hat.com>
> > > ---
> > > fs/fuse/virtio_fs.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> > > index e19c78f2480e..3d6981c0f3c3 100644
> > > --- a/fs/fuse/virtio_fs.c
> > > +++ b/fs/fuse/virtio_fs.c
> > > @@ -915,6 +915,8 @@ static void virtio_fs_vq_done(struct virtqueue *vq)
> > >
> > > dev_dbg(&vq->vdev->dev, "%s %s\n", __func__, fsvq->name);
> > >
> > > + /* Shceduled, don't send more notifications */
> >
> > Typo, and I think we can just drop this commnet.
>
> Agree.
>
> >
> > > + virtqueue_disable_cb(vq);
> >
> > Do we need to tweak the virtio_fs_requests_done_work() as well?
>
> I don't think so, as virtqueue_disable_cb can be called multiple times
> without consequences.
>
> To control it in virtio_fs_requests_done_work imply to create a new
> boolean somewhere and add a conditional there, redundant with the
> conditional in virtio_fs_requests_done_work.
I meant it would be redundant with the conditional in
virtqueue_disable_cb_split and _packed, as they store the avail flags
set so the operation is idempotent :).
Is it worth reordering this patch so it can be profiled and applied in
current code?
Powered by blists - more mailing lists