search for: done_work

...c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c index 4d8d4f16c727..a676297db09b 100644 --- a/fs/fuse/virtio_fs.c +++ b/fs/fuse/virtio_fs.c @@ -45,7 +45,7 @@ struct virtio_fs_vq { struct work_struct done_work; struct list_head queued_reqs; struct list_head end_reqs; /* End these requests */ - struct delayed_work dispatch_work; + struct work_struct dispatch_work; struct fuse_dev *fud; bool connected; long in_flight; @@ -202,7 +202,7 @@ static void virtio_fs_drain_queue(struct virtio_fs_vq *fsvq...

...c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c index 4d8d4f16c727..a676297db09b 100644 --- a/fs/fuse/virtio_fs.c +++ b/fs/fuse/virtio_fs.c @@ -45,7 +45,7 @@ struct virtio_fs_vq { struct work_struct done_work; struct list_head queued_reqs; struct list_head end_reqs; /* End these requests */ - struct delayed_work dispatch_work; + struct work_struct dispatch_work; struct fuse_dev *fud; bool connected; long in_flight; @@ -202,7 +202,7 @@ static void virtio_fs_drain_queue(struct virtio_fs_vq *fsvq...

...'s a circular dependency between the completion of two or more async replies. Such a deadlock can be reproduced with xfstests:generic/503 if TEST_DIR == SCRATCH_MNT (which is a misconfiguration): - Process A is waiting for page lock in worker thread context and blocked (virtio_fs_requests_done_work()). - Process B is holding page lock and waiting for pending writes to finish (fuse_wait_on_page_writeback()). - Write requests are waiting in virtqueue and can't complete because worker thread is blocked on page lock (process A). Fix this by creating a unique work_struct for each asyn...

...'s a circular dependency between the completion of two or more async replies. Such a deadlock can be reproduced with xfstests:generic/503 if TEST_DIR == SCRATCH_MNT (which is a misconfiguration): - Process A is waiting for page lock in worker thread context and blocked (virtio_fs_requests_done_work()). - Process B is holding page lock and waiting for pending writes to finish (fuse_wait_on_page_writeback()). - Write requests are waiting in virtqueue and can't complete because worker thread is blocked on page lock (process A). Fix this by creating a unique work_struct for each asyn...

...f --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c > > >> index 4d8d4f16c727..a676297db09b 100644 > > >> --- a/fs/fuse/virtio_fs.c > > >> +++ b/fs/fuse/virtio_fs.c > > >> @@ -45,7 +45,7 @@ struct virtio_fs_vq { > > >> struct work_struct done_work; > > >> struct list_head queued_reqs; > > >> struct list_head end_reqs; /* End these requests */ > > >> - struct delayed_work dispatch_work; > > >> + struct work_struct dispatch_work; > > >> struct fuse_dev *fud; > > >>...

...gt;> > >> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c > >> index 4d8d4f16c727..a676297db09b 100644 > >> --- a/fs/fuse/virtio_fs.c > >> +++ b/fs/fuse/virtio_fs.c > >> @@ -45,7 +45,7 @@ struct virtio_fs_vq { > >> struct work_struct done_work; > >> struct list_head queued_reqs; > >> struct list_head end_reqs; /* End these requests */ > >> - struct delayed_work dispatch_work; > >> + struct work_struct dispatch_work; > >> struct fuse_dev *fud; > >> bool connected; > >>...

...fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c >>>>> index 4d8d4f16c727..a676297db09b 100644 >>>>> --- a/fs/fuse/virtio_fs.c >>>>> +++ b/fs/fuse/virtio_fs.c >>>>> @@ -45,7 +45,7 @@ struct virtio_fs_vq { >>>>> struct work_struct done_work; >>>>> struct list_head queued_reqs; >>>>> struct list_head end_reqs; /* End these requests */ >>>>> - struct delayed_work dispatch_work; >>>>> + struct work_struct dispatch_work; >>>>> struct fuse_dev *fud; >>&g...

...ote: [..] > > static void virtio_fs_hiprio_dispatch_work(struct work_struct *work) > > @@ -502,6 +522,7 @@ static int virtio_fs_setup_vqs(struct virtio_device *vdev, > > names[VQ_HIPRIO] = fs->vqs[VQ_HIPRIO].name; > > INIT_WORK(&fs->vqs[VQ_HIPRIO].done_work, virtio_fs_hiprio_done_work); > > INIT_LIST_HEAD(&fs->vqs[VQ_HIPRIO].queued_reqs); > > + INIT_LIST_HEAD(&fs->vqs[VQ_HIPRIO].end_reqs); > > INIT_DELAYED_WORK(&fs->vqs[VQ_HIPRIO].dispatch_work, > > virtio_fs_hi...

Hi, We have couple of places which can result in deadlock. This patch series fixes these. We can be called with fc->bg_lock (for background requests) while submitting a request. This leads to two constraints. - We can't end requests in submitter's context and call fuse_end_request() as it tries to take fc->bg_lock as well. So queue these requests on a list and use a worker to

...> are improvements which we can do over a period of time. > > [..] > > > +/* Per-virtqueue state */ > > > +struct virtio_fs_vq { > > > + spinlock_t lock; > > > + struct virtqueue *vq; /* protected by ->lock */ > > > + struct work_struct done_work; > > > + struct list_head queued_reqs; > > > + struct delayed_work dispatch_work; > > > + struct fuse_dev *fud; > > > + bool connected; > > > + long in_flight; > > > + char name[24]; > > > > I'd keep names somewhere separate as...

...fs_mutex); > +static LIST_HEAD(virtio_fs_instances); > + > +enum { > + VQ_HIPRIO, > + VQ_REQUEST > +}; > + > +/* Per-virtqueue state */ > +struct virtio_fs_vq { > + spinlock_t lock; > + struct virtqueue *vq; /* protected by ->lock */ > + struct work_struct done_work; > + struct list_head queued_reqs; > + struct delayed_work dispatch_work; > + struct fuse_dev *fud; > + bool connected; > + long in_flight; > + char name[24]; I'd keep names somewhere separate as they are not used on data path. > +} ____cacheline_aligned_in_smp; > + &g...

...fs_mutex); > +static LIST_HEAD(virtio_fs_instances); > + > +enum { > + VQ_HIPRIO, > + VQ_REQUEST > +}; > + > +/* Per-virtqueue state */ > +struct virtio_fs_vq { > + spinlock_t lock; > + struct virtqueue *vq; /* protected by ->lock */ > + struct work_struct done_work; > + struct list_head queued_reqs; > + struct delayed_work dispatch_work; > + struct fuse_dev *fud; > + bool connected; > + long in_flight; > + char name[24]; I'd keep names somewhere separate as they are not used on data path. > +} ____cacheline_aligned_in_smp; > + &g...

...fs_mutex); > +static LIST_HEAD(virtio_fs_instances); > + > +enum { > + VQ_HIPRIO, > + VQ_REQUEST > +}; > + > +/* Per-virtqueue state */ > +struct virtio_fs_vq { > + spinlock_t lock; > + struct virtqueue *vq; /* protected by ->lock */ > + struct work_struct done_work; > + struct list_head queued_reqs; > + struct delayed_work dispatch_work; > + struct fuse_dev *fud; > + bool connected; > + long in_flight; > + char name[24]; > +} ____cacheline_aligned_in_smp; > + > +/* A virtio-fs device instance */ > +struct virtio_fs { > + struc...

...>in_flight < 0); + + /* Wait for in flight requests to finish.*/ + while (1) { + spin_lock(&fsvq->lock); + if (!fsvq->in_flight) { + spin_unlock(&fsvq->lock); + break; + } + spin_unlock(&fsvq->lock); + usleep_range(1000, 2000); + } + + flush_work(&fsvq->done_work); + flush_delayed_work(&fsvq->dispatch_work); +} + +static inline void drain_hiprio_queued_reqs(struct virtio_fs_vq *fsvq) +{ + struct virtio_fs_forget *forget; + + spin_lock(&fsvq->lock); + while (1) { + forget = list_first_entry_or_null(&fsvq->queued_reqs, + struct virt...

...eletions(-) diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c index 1ea0f889e804..a76bd5a04521 100644 --- a/fs/fuse/virtio_fs.c +++ b/fs/fuse/virtio_fs.c @@ -180,9 +180,6 @@ static void virtio_fs_free_devs(struct virtio_fs *fs) if (!fsvq->fud) continue; - flush_work(&fsvq->done_work); - flush_delayed_work(&fsvq->dispatch_work); - /* TODO need to quiesce/end_requests/decrement dev_count */ fuse_dev_free(fsvq->fud); fsvq->fud = NULL; @@ -994,6 +991,8 @@ static int virtio_fs_fill_super(struct super_block *sb) atomic_inc(&fc->dev_count); } + /*...

...+ reinit_completion(&fsvq->in_flight_zero); + spin_unlock(&fsvq->lock); + wait_for_completion(&fsvq->in_flight_zero); + } else { spin_unlock(&fsvq->lock); - /* TODO use completion instead of timeout */ - usleep_range(1000, 2000); } flush_work(&fsvq->done_work); flush_delayed_work(&fsvq->dispatch_work); } -static void virtio_fs_drain_all_queues(struct virtio_fs *fs) +static void virtio_fs_drain_all_queues_locked(struct virtio_fs *fs) { struct virtio_fs_vq *fsvq; int i; @@ -141,6 +145,19 @@ static void virtio_fs_drain_all_queues(struct vi...

...suspend/restore, multiqueue support etc are improvements which we can do over a period of time. [..] > > +/* Per-virtqueue state */ > > +struct virtio_fs_vq { > > + spinlock_t lock; > > + struct virtqueue *vq; /* protected by ->lock */ > > + struct work_struct done_work; > > + struct list_head queued_reqs; > > + struct delayed_work dispatch_work; > > + struct fuse_dev *fud; > > + bool connected; > > + long in_flight; > > + char name[24]; > > I'd keep names somewhere separate as they are not used on data path. Ok, thi...

Hi Miklos, Here are few small cleanups for virtiofs for 5.5. I had received some comments from Michael Tsirkin on original virtiofs patches and these cleanups are result of these comments. Thanks Vivek Vivek Goyal (3): virtiofs: Use a common function to send forget virtiofs: Do not send forget request "struct list_head" element virtiofs: Use completions while waiting for queue

Hi, Michael Tsirkin pointed out issues w.r.t various locking related TODO items and races w.r.t device removal. In this first round of cleanups, I have taken care of most pressing issues. These patches apply on top of following. git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v4 I have tested these patches with mount/umount and device removal using qemu monitor. For