| Message ID | 20150810135551.64d7dbac@noble (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
Hi On Mon, 10 Aug 2015, NeilBrown wrote: > > Hi Mikulas, > I have a customer hitting the deadlock you described over a year ago > in: > > Subject: [dm-devel] [PATCH] block: flush queued bios when the process > blocks Ask block layer maintainers to accept that patch. > I notice that patch never went upstream. > > Has anything else been done to fix this deadlock? > > My thought was that something like the below would be sufficient. Do > you see any problem with that? It avoids the deadlock by dropping the > lock while sleeping. The patch below introduces a bug - if the user is continuously reading the same chunk (for example, by issuing multiple direct i/o requests to the same chunk), the function pending_complete never finishes. We need to hold the lock while waiting to make sure that no new read requests are submitted. Mikulas > Thanks > NeilBrown > > diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c > index 7c82d3ccce87..d29bcd02f9cf 100644 > --- a/drivers/md/dm-snap.c > +++ b/drivers/md/dm-snap.c > @@ -1454,6 +1454,7 @@ static void pending_complete(struct dm_snap_pending_exception *pe, int success) > } > *e = pe->e; > > +retry: > down_write(&s->lock); > if (!s->valid) { > free_completed_exception(e); > @@ -1462,7 +1463,11 @@ static void pending_complete(struct dm_snap_pending_exception *pe, int success) > } > > /* Check for conflicting reads */ > - __check_for_conflicting_io(s, pe->e.old_chunk); > + if (__chunk_size_tracked(s, pe->e.old_chunk)) { > + up_write(&s->lock); > + msleep(1); > + goto retry; > + } > > /* > * Add a proper exception, and remove the > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c index 7c82d3ccce87..d29bcd02f9cf 100644 --- a/drivers/md/dm-snap.c +++ b/drivers/md/dm-snap.c @@ -1454,6 +1454,7 @@ static void pending_complete(struct dm_snap_pending_exception *pe, int success) } *e = pe->e; +retry: down_write(&s->lock); if (!s->valid) { free_completed_exception(e); @@ -1462,7 +1463,11 @@ static void pending_complete(struct dm_snap_pending_exception *pe, int success) } /* Check for conflicting reads */ - __check_for_conflicting_io(s, pe->e.old_chunk); + if (__chunk_size_tracked(s, pe->e.old_chunk)) { + up_write(&s->lock); + msleep(1); + goto retry; + } /* * Add a proper exception, and remove the
Hi Mikulas, I have a customer hitting the deadlock you described over a year ago in: Subject: [dm-devel] [PATCH] block: flush queued bios when the process blocks I notice that patch never went upstream. Has anything else been done to fix this deadlock? My thought was that something like the below would be sufficient. Do you see any problem with that? It avoids the deadlock by dropping the lock while sleeping. Thanks NeilBrown -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel