megaraid_sas: add an i/o barrier
Subsystems (2)
A barrier should be added to ensure proper
ordering of memory mapped writes.
--- drivers/scsi/megaraid/megaraid_sas_fusion.c | 1 + 1 file changed, 1 insertion(+)
drivers/scsi/megaraid/megaraid_sas_fusion.c +1 -0
@@ -204,6 +204,7 @@ megasas_fire_cmd_fusion(struct megasas_instance *instance, &instance->reg_set->inbound_low_queue_port);writel (le32_to_cpu (req_desc->u.high), &instance->reg_set->inbound_high_queue_port); +mmiowb ();spin_unlock_irqrestore (&instance->hba_lock, flags);#endif }
Signature · 1 line
2.4.3
Quoted 21 lines
-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Friday, January 29, 2016 11:05 PM
To: '[email protected]'
Cc: [email protected]; Desai, Kashyap; Uday Lingala
Subject:megaraid_sas: add an i/o barrierA barrier should be added to ensure proper ordering of memory mapped writes.
Signed-off-by Tomas Henzl <[email protected]>--- drivers/scsi/megaraid/megaraid_sas_fusion.c | 1 + 1 file changed, 1 insertion(+)drivers/scsi/megaraid/megaraid_sas_fusion.c +0
-0
drivers/scsi/megaraid/megaraid_sas_fusion.c megasas_fire_cmd_fusion(struct · line 209
Quoted 6 lines
drivers/scsi/megaraid/megaraid_sas_fusion.c +1
-0@@ -204,6 +204,7 @@ megasas_fire_cmd_fusion(struct megasas_instance *instance, &instance->reg_set->inbound_low_queue_port);writel (le32_to_cpu (req_desc->u.high), &instance->reg_set->inbound_high_queue_port); +mmiowb ();spin_unlock_irqrestore (&instance->hba_lock, flags);#endif }
Tomas-
We may need similar changes around below Functions as well, because there is no associated readX or mmiowb() call.megasas_fire_cmd_xscale()megasas_fire_cmd_ppc()megasas_fire_cmd_skinny()megasas_fire_cmd_gen2()
Also, wrireq() routine in same function megasas_fire_cmd_fusion() need i/o barrier.
Quoted 2 lines
drivers/scsi/megaraid/megaraid_sas_fusion.c +0
-1--2.4.3
Quoted 36 lines · Kashyap Desai
Quoted 21 lines
-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Friday, January 29, 2016 11:05 PM
To: '[email protected]'
Cc: [email protected]; Desai, Kashyap; Uday Lingala
Subject:megaraid_sas: add an i/o barrierA barrier should be added to ensure proper ordering of memory mapped writes.
Signed-off-by Tomas Henzl <[email protected]>--- drivers/scsi/megaraid/megaraid_sas_fusion.c | 1 + 1 file changed, 1 insertion(+)drivers/scsi/megaraid/megaraid_sas_fusion.c +0
-0drivers/scsi/megaraid/megaraid_sas_fusion.c megasas_fire_cmd_fusion(struct · line 209
Quoted 6 lines
drivers/scsi/megaraid/megaraid_sas_fusion.c +1
-0@@ -204,6 +204,7 @@ megasas_fire_cmd_fusion(struct megasas_instance *instance, &instance->reg_set->inbound_low_queue_port);writel (le32_to_cpu (req_desc->u.high), &instance->reg_set->inbound_high_queue_port); +mmiowb ();spin_unlock_irqrestore (&instance->hba_lock, flags);#endif }Tomas-
We may need similar changes around below Functions as well, because there is no associated readX or
mmiowb()call.megasas_fire_cmd_xscale()megasas_fire_cmd_ppc()megasas_fire_cmd_skinny()megasas_fire_cmd_gen2()
Quoted 2 lines
Also,
wrireq()routine in same functionmegasas_fire_cmd_fusion()need i/o barrier.
I don't think so (with the exception of megasas_fire_cmd_skinny - I missed this one). When two threads try to use a fire_cmd there is no protection of certain ordering, that had to be done in a caller of fire_cmd (for example in megasas_build_and_issue_cmd_fusion) and it seems to me that there is nothing like that. Likely is, that this - a strict ordering of commands - is not needed.
The protection which I'm adding is needed when a command consist of a sequence of more than one write, see memory-barriers.txt.
(It looks to me thatmegasas_fire_cmd_ -xscale -ppc -skiny -gen2 do not need the hba_lock unless there is another i/o sequence protected with the same lock (note also that there is
no such lock in megasas_fire_cmd_fusion).)
I'll add the mmiowb to megasas_fire_cmd_skinny and send a new patch - agreed?
--tms
Quoted 7 lines
Quoted 3 lines
drivers/scsi/megaraid/megaraid_sas_fusion.c +0
-1--2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 59 lines
-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Monday, February 01, 2016 6:23 PM
To: Kashyap Desai; [email protected]
Cc: Sumit Saxena; Uday Lingala
Subject: Re:megaraid_sas: add an i/o barrierQuoted 15 lines · Kashyap Desai
Quoted 15 lines
-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Friday, January 29, 2016 11:05 PM
To: '[email protected]'
Cc: [email protected]; Desai, Kashyap; Uday Lingala
Subject:megaraid_sas: add an i/o barrierA barrier should be added to ensure proper ordering of memory mapped writes.
Signed-off-by Tomas Henzl <[email protected]>--- drivers/scsi/megaraid/megaraid_sas_fusion.c | 1 + 1 file changed, 1 insertion(+)Quoted 6 lines
drivers/scsi/megaraid/megaraid_sas_fusion.c megasas_fire_cmd_fusion(struct
Quoted 6 lines
drivers/scsi/megaraid/megaraid_sas_fusion.c +0
-0@@ -204,6 +204,7 @@ megasas_fire_cmd_fusion(struct
megasas_instanceQuoted 15 lines
drivers/scsi/megaraid/megaraid_sas_fusion.c megasas_fire_cmd_fusion(struct megasas_instance *instance,
Quoted 6 lines
drivers/scsi/megaraid/megaraid_sas_fusion.c +1
-0*instance, &instance->reg_set->inbound_low_queue_port);writel (le32_to_cpu (req_desc->u.high), &instance->reg_set->inbound_high_queue_port); +mmiowb ();spin_unlock_irqrestore (&instance->hba_lock, flags);#endif }Tomas-
We may need similar changes around below Functions as well, because there is no associated readX or
mmiowb()call.megasas_fire_cmd_xscale()megasas_fire_cmd_ppc()megasas_fire_cmd_skinny()megasas_fire_cmd_gen2()Quoted 2 lines
Also,
wrireq()routine in same functionmegasas_fire_cmd_fusion()need i/o barrier.I don't think so (with the exception of
megasas_fire_cmd_skinny- I missed this one).
When two threads try to use afire_cmdthere is no protection of certain ordering, that had to be done in a caller offire_cmd(for example inmegasas_build_and_issue_cmd_fusion)
and it seems to me that there is nothing like that. Likely is, that this - a
strict ordering of commands - is not needed.
The protection which I'm adding is needed when a command consist of a sequence of more than one write, see memory-barriers.txt.
Quoted 4 lines
(It looks to me that
megasas_fire_cmd_-xscale -ppc -skiny -gen2 do not need thehba_lockunless there is another i/o sequence protected with the same lock (note also that there is no such lock inmegasas_fire_cmd_fusion).)
Agree, we don't need hba lock in older fire_cmd wrapper. We updated fusion code because it was active shipment and product life cycle was active for fusion adapter, so code changes was tested by Avago. We did not clean code in MFI based controller since it is only in critical support cycle.
Quoted 4 lines
I'll add the mmiowb to
megasas_fire_cmd_skinnyand send a new patch - agreed?
Got your word. You are right. Yes additional changes in megasas_fire_cmd_skinny along with current patch will be good to go.
Quoted 10 lines
--tms
Quoted 7 lines
Quoted 3 lines
drivers/scsi/megaraid/megaraid_sas_fusion.c +0
-1--2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html