Bug 154442

Summary: kernel dm-multipath: multiple pg_inits can be issued in parallel
Product: Red Hat Enterprise Linux 4 Reporter: Alasdair Kergon <agk>
Component: device-mapper-multipathAssignee: Alasdair Kergon <agk>
Status: CLOSED ERRATA QA Contact:
Severity: medium Docs Contact:
Priority: medium    
Version: 4.1CC: agk, christophe.varoqui, dmo, egoggin, lmb, poelstra, tranlan
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: RHSA-2005-514 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2005-10-05 12:58:39 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 156322    
Attachments:
Description Flags
serialize pg_init calls via pg_init_in_progress
none
Don't trigger process_queued_ios when there's no queue. none

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