Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Vivek Goyal <vgoyal@redhat.com>
To:     linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        virtio-fs@redhat.com
Cc:     virtualization@lists.linux-foundation.org, vgoyal@redhat.com,
        miklos@szeredi.hu, stefanha@redhat.com, dgilbert@redhat.com
Subject: [PATCH 3/3] virtiofs: Use completions while waiting for queue to be drained
Date:   Wed, 30 Oct 2019 11:07:19 -0400
Message-Id: <20191030150719.29048-4-vgoyal@redhat.com>
In-Reply-To: <20191030150719.29048-1-vgoyal@redhat.com>
References: <20191030150719.29048-1-vgoyal@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=WINDOWS-1252
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

While we wait for queue to finish draining, use completions instead of
uslee_range(). This is better way of waiting for event.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/fuse/virtio_fs.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 43224db8d9ed..b5ba83ef1914 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -35,6 +35,7 @@ struct virtio_fs_vq {
 =09struct fuse_dev *fud;
 =09bool connected;
 =09long in_flight;
+=09struct completion in_flight_zero; /* No inflight requests */
 =09char name[24];
 } ____cacheline_aligned_in_smp;
=20
@@ -85,6 +86,8 @@ static inline void dec_in_flight_req(struct virtio_fs_vq =
*fsvq)
 {
 =09WARN_ON(fsvq->in_flight <=3D 0);
 =09fsvq->in_flight--;
+=09if (!fsvq->in_flight)
+=09=09complete(&fsvq->in_flight_zero);
 }
=20
 static void release_virtio_fs_obj(struct kref *ref)
@@ -115,22 +118,23 @@ static void virtio_fs_drain_queue(struct virtio_fs_vq=
 *fsvq)
 =09WARN_ON(fsvq->in_flight < 0);
=20
 =09/* Wait for in flight requests to finish.*/
-=09while (1) {
-=09=09spin_lock(&fsvq->lock);
-=09=09if (!fsvq->in_flight) {
-=09=09=09spin_unlock(&fsvq->lock);
-=09=09=09break;
-=09=09}
+=09spin_lock(&fsvq->lock);
+=09if (fsvq->in_flight) {
+=09=09/* We are holding virtio_fs_mutex. There should not be any
+=09=09 * waiters waiting for completion.
+=09=09 */
+=09=09reinit_completion(&fsvq->in_flight_zero);
+=09=09spin_unlock(&fsvq->lock);
+=09=09wait_for_completion(&fsvq->in_flight_zero);
+=09} else {
 =09=09spin_unlock(&fsvq->lock);
-=09=09/* TODO use completion instead of timeout */
-=09=09usleep_range(1000, 2000);
 =09}
=20
 =09flush_work(&fsvq->done_work);
 =09flush_delayed_work(&fsvq->dispatch_work);
 }
=20
-static void virtio_fs_drain_all_queues(struct virtio_fs *fs)
+static void virtio_fs_drain_all_queues_locked(struct virtio_fs *fs)
 {
 =09struct virtio_fs_vq *fsvq;
 =09int i;
@@ -141,6 +145,19 @@ static void virtio_fs_drain_all_queues(struct virtio_f=
s *fs)
 =09}
 }
=20
+static void virtio_fs_drain_all_queues(struct virtio_fs *fs)
+{
+=09/* Provides mutual exclusion between ->remove and ->kill_sb
+=09 * paths. We don't want both of these draining queue at the
+=09 * same time. Current completion logic reinits completion
+=09 * and that means there should not be any other thread
+=09 * doing reinit or waiting for completion already.
+=09 */
+=09mutex_lock(&virtio_fs_mutex);
+=09virtio_fs_drain_all_queues_locked(fs);
+=09mutex_unlock(&virtio_fs_mutex);
+}
+
 static void virtio_fs_start_all_queues(struct virtio_fs *fs)
 {
 =09struct virtio_fs_vq *fsvq;
@@ -581,6 +598,7 @@ static int virtio_fs_setup_vqs(struct virtio_device *vd=
ev,
 =09INIT_LIST_HEAD(&fs->vqs[VQ_HIPRIO].end_reqs);
 =09INIT_DELAYED_WORK(&fs->vqs[VQ_HIPRIO].dispatch_work,
 =09=09=09virtio_fs_hiprio_dispatch_work);
+=09init_completion(&fs->vqs[VQ_HIPRIO].in_flight_zero);
 =09spin_lock_init(&fs->vqs[VQ_HIPRIO].lock);
=20
 =09/* Initialize the requests virtqueues */
@@ -591,6 +609,7 @@ static int virtio_fs_setup_vqs(struct virtio_device *vd=
ev,
 =09=09=09=09  virtio_fs_request_dispatch_work);
 =09=09INIT_LIST_HEAD(&fs->vqs[i].queued_reqs);
 =09=09INIT_LIST_HEAD(&fs->vqs[i].end_reqs);
+=09=09init_completion(&fs->vqs[i].in_flight_zero);
 =09=09snprintf(fs->vqs[i].name, sizeof(fs->vqs[i].name),
 =09=09=09 "requests.%u", i - VQ_REQUEST);
 =09=09callbacks[i] =3D virtio_fs_vq_done;
@@ -684,7 +703,7 @@ static void virtio_fs_remove(struct virtio_device *vdev=
)
 =09/* This device is going away. No one should get new reference */
 =09list_del_init(&fs->list);
 =09virtio_fs_stop_all_queues(fs);
-=09virtio_fs_drain_all_queues(fs);
+=09virtio_fs_drain_all_queues_locked(fs);
 =09vdev->config->reset(vdev);
 =09virtio_fs_cleanup_vqs(vdev, fs);
=20
--=20
2.20.1