mpt2sas: don't handle broadcast primitives
PATCH 1/1 mpt2sas: don't handle broadcast primitives
The handling of broadcast primitives involves _scsih_block_io_all_device(), which does what the name implies. I have observed cases with >60s of blocking io on all devices, caused by a single bad device. The downsides of this code are obvious, while the upsides are more elusive.
--- drivers/scsi/mpt2sas/mpt2sas_base.c | 4 - drivers/scsi/mpt2sas/mpt2sas_scsih.c | 238 ---------------------------------- 2 files changed, 242 deletions(-)
drivers/scsi/mpt2sas/mpt2sas_base.c +0 -4
@@ -586,9 +586,6 @@ _base_display_event_data(struct MPT2SAS_ADAPTER *ioc,printk ("\n" );return ; } -case MPI2_EVENT_SAS_BROADCAST_PRIMITIVE : - desc ="SAS Broadcast Primitive" ; -break ;case MPI2_EVENT_SAS_INIT_DEVICE_STATUS_CHANGE : desc ="SAS Init Device Status Change" ;break ;
@@ -4370,7 +4367,6 @@ mpt2sas_base_attach(struct MPT2SAS_ADAPTER *ioc)/* here we enable the events we care about */ _base_unmask_events (ioc,MPI2_EVENT_SAS_DISCOVERY ); -_base_unmask_events (ioc,MPI2_EVENT_SAS_BROADCAST_PRIMITIVE );_base_unmask_events (ioc,MPI2_EVENT_SAS_TOPOLOGY_CHANGE_LIST );_base_unmask_events (ioc,MPI2_EVENT_SAS_DEVICE_STATUS_CHANGE );_base_unmask_events (ioc,MPI2_EVENT_SAS_ENCL_DEVICE_STATUS_CHANGE );
drivers/scsi/mpt2sas/mpt2sas_scsih.c +0 -238
274 lines not on this page · raw message
Signature · 1 line
1.7.10.4
Quoted 6 lines · Jörn Engel
The handling of broadcast primitives involves
_scsih_block_io_all_device(), which does what the name implies. I have observed cases with >60s of blocking io on all devices, caused by a single bad device. The downsides of this code are obvious, while the upsides are more elusive.
And since this patch looks more like an April fools joke: I have gathered a few machine-months of testing, including tortures that specifically stress the removed codepaths. This is a serious submission and unless someone can show me a _very_ good reason for keeping the deleted code, I would like to get it merged.
Jörn
--
Computer system analysis is like child-rearing; you can do grievous damage, but you cannot ensure success."
-- Tom DeMarco
Quoted 23 lines · Baruch Even
Quoted 8 lines · Jörn Engel
Quoted 6 lines · Jörn Engel
The handling of broadcast primitives involves
_scsih_block_io_all_device(), which does what the name implies. I have observed cases with >60s of blocking io on all devices, caused by a single bad device. The downsides of this code are obvious, while the upsides are more elusive.Quoted 5 lines
And since this patch looks more like an April fools joke: I have gathered a few machine-months of testing, including tortures that specifically stress the removed codepaths. This is a serious submission and unless someone can show me a _very_ good reason for keeping the deleted code, I would like to get it merged.
This would seem to cause an IO pause through the host whenever there is a disk removal/insertion or SES (SAS expander) change which seems like a bad proposition indeed. The part of the work that this code seems to handle is that when such a change happens something needs to detect the dead IOs (f.ex. surprise disk removal) but I believe that the SAS HBA firmware will do that internally already so I do think this code is needless.
Quoted 3 lines
The only thing I'd like not to lose is the actual notification and ability to log the fact that there was a broadcast notification on the SAS network.
I agree logging would be nice. However my attempts to keep logging and remove the IO pause were unsuccessful. Apparently something inside _scsih_sas_broadcast_primitive_event() is required to get future events. If someone from LSI with data sheets and understanding of the firmware can do a better patch, I would be happy.
Jörn
--
The story so far:
In the beginning the Universe was created. This has made a lot of people very angry and been widely regarded as a bad move. -- Douglas Adams
Quoted 8 lines · Jörn Engel
Quoted 6 lines · Jörn Engel
The handling of broadcast primitives involves
_scsih_block_io_all_device(), which does what the name implies. I have observed cases with >60s of blocking io on all devices, caused by a single bad device. The downsides of this code are obvious, while the upsides are more elusive.
Quoted 5 lines
And since this patch looks more like an April fools joke: I have gathered a few machine-months of testing, including tortures that specifically stress the removed codepaths. This is a serious submission and unless someone can show me a _very_ good reason for keeping the deleted code, I would like to get it merged.
This would seem to cause an IO pause through the host whenever there is a disk removal/insertion or SES (SAS expander) change which seems like a bad proposition indeed. The part of the work that this code seems to handle is that when such a change happens something needs to detect the dead IOs (f.ex. surprise disk removal) but I believe that the SAS HBA firmware will do that internally already so I do think this code is needless.
The only thing I'd like not to lose is the actual notification and ability to log the fact that there was a broadcast notification on the SAS network.
Baruch
--
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