On 04/12/2015 08:54 PM, James Bottomley wrote: > On Sun, 2015-04-12 at 20:11 -0400, Joe Lawrence wrote: >> On 12/30/2014 09:07 AM, Joe Lawrence wrote: >>> A colleague noticed that the mpt2 and mpt3sas drivers do not correctly >>> check the PCI master abort pattern in _base_wait_for_doorbell_ack. This >>> pattern should be checked *prior* to any valid bit patterns, which would >>> always return true since a PCI read on master abort sets all bits high. >>> >>> The second patch adds similar checking to _base_wait_for_doorbell_int and >>> _base_wait_for_doorbell_not_used to avoid potentially long loops around >>> PCI reads. >>> >>> Joe Lawrence (2): >>> mpt2sas,mpt3sas: correct master-abort checking in doorbell ack >>> mpt2sas,mpt3sas: additional master abort checks >>> >>> drivers/scsi/mpt2sas/mpt2sas_base.c | 17 ++++++++++++----- >>> drivers/scsi/mpt3sas/mpt3sas_base.c | 17 ++++++++++++----- >>> 2 files changed, 24 insertions(+), 10 deletions(-) >>> >> >> Avago ping? >> >> This one was pretty straightforward: check 0xFFFFFFFF *before* any >> individual bit(s), i.e. before reading the doorbell register. > > OK, Joe, explain why this patch is important: what problems could result > from it not being present? If you convince everyone then no more mpt2/3 > sas patches until this is at least commented on and a plan of action > proposed. Hi James, As currently coded: If the PCI read returns a master abort, _base_wait_for_doorbell_ack will loop until it exhausts its timeout (up to 15 seconds). Other parts of the driver, like the periodic watchdog or EEH, may detect a similar problem before such a long time and cleanup the mess. However, complete device removal may be stalled until whoever called _base_wait_for_doorbell_ack is satisfied that it has finished. This behavior is not really a bug, but feels like one in the making. Should additional code be introduced, copy/pasted, etc. it may not do what was intended. For future reference, would a repost have been more appropriate? This changeset was so small that I figured a status ping would have sufficed. Regards, -- Joe -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html