diff mbox

[1/2] dm-multipath: cleanup IO queueing after table load

Message ID 1407956023-32372-2-git-send-email-bmarzins@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Mike Snitzer
Headers show

Commit Message

Benjamin Marzinski Aug. 13, 2014, 6:53 p.m. UTC
Commit e809917735ebf1b9a56c24e877ce0d320baee2ec changed the code in
multipath_busy that checked if a multipath device was initializing a path.
It returned busy if queue_io was set on the device. However queue_io is set
when the table is loaded, and as long as the device was busy, multipath_map
wouldn't run to clear it upon receiving requsts. multipath_ioctl and
reinstate_path were still able to clear queue_io. But if no paths were
reinstated and the multipath device received no ioctls after the table
reload, all requests to it would simply queue.

This patch switches the multipath_busy code to check for a pending or in
progress path initialization, without using queue_io.  This means incoming
requests will allow multipath_map to initialize paths and clear queue_io.

This patch also changes __choose_pgpath to make it clear queue_io if there
are no valid paths, since there are obviously no paths that can be
initialized.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 drivers/md/dm-mpath.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Mike Snitzer Aug. 14, 2014, 2:46 a.m. UTC | #1
On Wed, Aug 13, 2014 at 2:53 PM, Benjamin Marzinski <bmarzins@redhat.com> wrote:
> Commit e809917735ebf1b9a56c24e877ce0d320baee2ec changed the code in
> multipath_busy that checked if a multipath device was initializing a path.
> It returned busy if queue_io was set on the device. However queue_io is set
> when the table is loaded, and as long as the device was busy, multipath_map
> wouldn't run to clear it upon receiving requsts. multipath_ioctl and
> reinstate_path were still able to clear queue_io. But if no paths were
> reinstated and the multipath device received no ioctls after the table
> reload, all requests to it would simply queue.
>
> This patch switches the multipath_busy code to check for a pending or in
> progress path initialization, without using queue_io.  This means incoming
> requests will allow multipath_map to initialize paths and clear queue_io.

This was already addressed with commit 7a7a3b45fe ("dm mpath: fix IO
hang due to logic bug in multipath_busy").

> This patch also changes __choose_pgpath to make it clear queue_io if there
> are no valid paths, since there are obviously no paths that can be
> initialized.

Can you elaborate on why this is needed?  If it still is then it needs
to be rebased.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Benjamin Marzinski Aug. 15, 2014, 3:46 p.m. UTC | #2
On Wed, Aug 13, 2014 at 10:46:15PM -0400, Mike Snitzer wrote:
> On Wed, Aug 13, 2014 at 2:53 PM, Benjamin Marzinski <bmarzins@redhat.com> wrote:
> > Commit e809917735ebf1b9a56c24e877ce0d320baee2ec changed the code in
> > multipath_busy that checked if a multipath device was initializing a path.
> > It returned busy if queue_io was set on the device. However queue_io is set
> > when the table is loaded, and as long as the device was busy, multipath_map
> > wouldn't run to clear it upon receiving requsts. multipath_ioctl and
> > reinstate_path were still able to clear queue_io. But if no paths were
> > reinstated and the multipath device received no ioctls after the table
> > reload, all requests to it would simply queue.
> >
> > This patch switches the multipath_busy code to check for a pending or in
> > progress path initialization, without using queue_io.  This means incoming
> > requests will allow multipath_map to initialize paths and clear queue_io.
> 
> This was already addressed with commit 7a7a3b45fe ("dm mpath: fix IO
> hang due to logic bug in multipath_busy").
> 
> > This patch also changes __choose_pgpath to make it clear queue_io if there
> > are no valid paths, since there are obviously no paths that can be
> > initialized.
> 
> Can you elaborate on why this is needed?  If it still is then it needs
> to be rebased.

For a while now, it's been possible to create a multipath table with no
devices.  With this patch set, it will be even easier to reload a
multipath device with no valid paths (since multipath now supports
reloading failed paths). In these scenarios, we still need to disable
queue_io, so that IOs to the device will not just back up.

-Ben

> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
diff mbox

Patch

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 3f6fd9d..a0b83cb 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -317,8 +317,10 @@  static void __choose_pgpath(struct multipath *m, size_t nr_bytes)
 	struct priority_group *pg;
 	unsigned bypassed = 1;
 
-	if (!m->nr_valid_paths)
+	if (!m->nr_valid_paths) {
+		m->queue_io = 0;
 		goto failed;
+	}
 
 	/* Were we instructed to switch PG? */
 	if (m->next_pg) {
@@ -1612,7 +1614,7 @@  static int multipath_busy(struct dm_target *ti)
 	spin_lock_irqsave(&m->lock, flags);
 
 	/* pg_init in progress, requeue until done */
-	if (!pg_ready(m)) {
+	if (m->pg_init_required || m->pg_init_in_progress) {
 		busy = 1;
 		goto out;
 	}