Re: [PATCH v2 2/2] libsas: Enhance libsas hotplug

Johannes Thumshirn
2017-06-14T08:57:28Z

On 06/14/2017 09:33 AM, Yijing Wang wrote:
> Libsas complete a hotplug event notified by LLDD in several works,
> for example, if libsas receive a PHYE_LOSS_OF_SIGNAL, we process it
> in following steps:
>
> notify_phy_event [interrupt context]
> sas_queue_event [queue work on shost->work_q]
> sas_phye_loss_of_signal [running in shost->work_q]
> sas_deform_port [remove sas port]
> sas_unregister_dev
> sas_discover_event [queue destruct work on shost->work_q tail]
>
> In above case, complete whole hotplug in two works, remove sas port first, then
> put the destruction of device in another work and queue it on in the tail of
> workqueue, since sas port is the parent of the children rphy device, so if remove
> sas port first, the children rphy device would also be deleted, when the destruction
> work coming, it would find the target has been removed already, and report a
> sysfs warning calltrace.
>
> queue tail queue head
> DISCE_DESTRUCT----> PORTE_BYTES_DMAED event ----->PHYE_LOSS_OF_SIGNAL[running]
>
> There are other hotplug issues in current framework, in above case, if there is
> hotadd sas event queued between hotremove works, the hotplug order would be broken
> and unexpected issues would happen.
>
> In this patch, we try to solve these issues in following steps:
> 1. create a new workqueue used to run sas event work, instead of scsi host workqueue,
> because we may block sas event work, we cannot block the normal scsi works.
> When libsas receive a phy down event, sas_deform_port would be called, and now we
> block sas_deform_port and wait for destruction work finish, in sas_destruct_devices,
> we may wait ata error handler, it would take a long time, so if do all stuff in scsi
> host workq, libsas may block other scsi works too long.
> 2. create a new workqueue used to run sas discovery events work, instead of scsi host
> workqueue, because in some cases, eg. in revalidate domain event, we may unregister
> a sas device and discover new one, we must sync the execution, wait the remove process
> finish, then start a new discovery. So we must put the probe and destruct discovery
> events in a new workqueue to avoid deadlock.
> 3. introudce a asd_sas_port level wait-complete and a sas_discovery level wait-complete
> we use former wait-complete to achieve a sas event atomic process and use latter to
> make a sas discovery sync.
> 4. remove disco_mutex in sas_revalidate_domain, since now sas_revalidate_domain sync
> the destruct discovery event execution, it's no need to lock disco mutex there.

The way you've written the changelog suggests this patch should be split
into 4 patches, each one taking care of one of your change items.

--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850