Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 154442 - kernel dm-multipath: multiple pg_inits can be issued in parallel
Summary: kernel dm-multipath: multiple pg_inits can be issued in parallel
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 4
Classification: Red Hat
Component: device-mapper-multipath
Version: 4.1
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
: ---
Assignee: Alasdair Kergon
QA Contact:
URL:
Whiteboard:
: 154443 156576 (view as bug list)
Depends On:
Blocks: 156322
TreeView+ depends on / blocked
 
Reported: 2005-04-11 18:17 UTC by Alasdair Kergon
Modified: 2010-01-12 02:20 UTC (History)
7 users (show)

Fixed In Version: RHSA-2005-514
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-10-05 12:58:39 UTC


Attachments (Terms of Use)
serialize pg_init calls via pg_init_in_progress (deleted)
2005-04-22 10:30 UTC, Lars Marowsky-Bree
no flags Details | Diff
Don't trigger process_queued_ios when there's no queue. (deleted)
2005-07-02 22:16 UTC, Alasdair Kergon
no flags Details | Diff


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2005:514 qe-ready SHIPPED_LIVE Important: Updated kernel packages available for Red Hat Enterprise Linux 4 Update 2 2005-10-05 04:00:00 UTC

Description Alasdair Kergon 2005-04-11 18:17:42 UTC
Only one pg_init should happen on a device at once.

But it's possible for __switch_pg to get called before the last pg_init completed.

Comment 1 Lars Marowsky-Bree 2005-04-21 20:15:15 UTC
A possible fix would be to set a pg_init_in_progress flag in
process_queued_ios() together with clearing the pg_init_required flag, and not
issue new pg_init ops while this is set. This flag would get cleared in
dm_pg_init_complete().

That could be either done with an additional field or by converting
pg_init_required to a pg_init_flags and PG_FLAG_REQUIRED, _IN_PROGRESS masks
(which could potentially allow for future extensions; a _SUCCESS, _FAILED etc
comes to mind).

I can code a patch for either an additional field or the conversion to flags.


Comment 2 Lars Marowsky-Bree 2005-04-22 10:30:58 UTC
Created attachment 113521 [details]
serialize pg_init calls via pg_init_in_progress

Sketch of a patch. It compiles, I'll test it next ;)

Comment 3 Lars Marowsky-Bree 2005-04-22 11:08:00 UTC
"Works for me", please consider for udm inclusion...

Comment 4 Alasdair Kergon 2005-07-02 19:51:13 UTC
Added it to my 'editing' directory.


Comment 5 Alasdair Kergon 2005-07-02 19:55:21 UTC
*** Bug 154443 has been marked as a duplicate of this bug. ***

Comment 6 Alasdair Kergon 2005-07-02 19:57:10 UTC
*** Bug 156576 has been marked as a duplicate of this bug. ***

Comment 7 Alasdair Kergon 2005-07-02 22:03:17 UTC
Created attachment 116297 [details]
Don't trigger process_queued_ios when there's no queue.

Comment 8 Alasdair Kergon 2005-07-02 22:10:23 UTC
Comment on attachment 116297 [details]
Don't trigger process_queued_ios when there's no queue.

>Add to last patch:
>  Only reset queue_io in pg_init_complete if another pg_init isn't required.
>  Ensure process_queued_ios is never called if queue is empty.
>
>--- md-2.6.13-udm1-17/dm-mpath.c	Sat Jul  2 20:10:33 2005
>+++ md-2.6.13-udm1-18/dm-mpath.c	Sat Jul  2 22:53:48 2005
>@@ -337,7 +337,7 @@ static int queue_if_no_path(struct multi
> 
> 	m->saved_queue_if_no_path = m->queue_if_no_path;
> 	m->queue_if_no_path = queue_if_no_path;
>-	if (!m->queue_if_no_path)
>+	if (!m->queue_if_no_path && m->queue_size)
> 		queue_work(kmultipathd, &m->process_queued_ios);
> 
> 	spin_unlock_irqrestore(&m->lock, flags);
>@@ -856,7 +856,7 @@ static int reinstate_path(struct pgpath 
> 	pgpath->path.is_active = 1;
> 
> 	m->current_pgpath = NULL;
>-	if (!m->nr_valid_paths++)
>+	if (!m->nr_valid_paths++ && m->queue_size)
> 		queue_work(kmultipathd, &m->process_queued_ios);
> 
> 	queue_work(kmultipathd, &m->trigger_event);
>@@ -990,12 +990,12 @@ void dm_pg_init_complete(struct path *pa
> 	}
> 
> 	spin_lock_irqsave(&m->lock, flags);
>-	if (!err_flags)
>-		m->queue_io = 0;
>-	else {
>+	if (err_flags) {
> 		m->current_pgpath = NULL;
> 		m->current_pg = NULL;
>-	}
>+	} else if (!m->pg_init_required)
>+		m->queue_io = 0;
>+
> 	m->pg_init_in_progress = 0;
> 	queue_work(kmultipathd, &m->process_queued_ios);
> 	spin_unlock_irqrestore(&m->lock, flags);

Comment 10 Alasdair Kergon 2005-07-02 22:16:38 UTC
Created attachment 116298 [details]
Don't trigger process_queued_ios when there's no queue.

Second attempt at patch attachment...

Comment 11 Alasdair Kergon 2005-07-08 19:55:58 UTC
patches submitted to -mm

Comment 17 Red Hat Bugzilla 2005-10-05 12:58:40 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on the solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2005-514.html



Note You need to log in before you can comment on or make changes to this bug.