在 2017/7/13 0:50, John Garry 写道:
> On 10/07/2017 08:06, Yijing Wang wrote:
>> Sometimes, we want sync libsas probe or destruct in sas discovery work,
>> like when libsas revalidate domain. We need to split probe and destruct
>> work from the scsi host workqueue.
>>
>> Signed-off-by: Yijing Wang <wangyijing@xxxxxxxxxx>
>> CC: John Garry <john.garry@xxxxxxxxxx>
>> CC: Johannes Thumshirn <jthumshirn@xxxxxxx>
>> CC: Ewan Milne <emilne@xxxxxxxxxx>
>> CC: Christoph Hellwig <hch@xxxxxx>
>> CC: Tomas Henzl <thenzl@xxxxxxxxxx>
>> CC: Dan Williams <dan.j.williams@xxxxxxxxx>
>> ---
>> drivers/scsi/libsas/sas_discover.c | 13 ++++++++++++-
>> drivers/scsi/libsas/sas_init.c | 8 ++++++++
>> include/scsi/libsas.h | 1 +
>> 3 files changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
>> index 5d4a3a8..a25d648 100644
>> --- a/drivers/scsi/libsas/sas_discover.c
>> +++ b/drivers/scsi/libsas/sas_discover.c
>> @@ -559,7 +559,18 @@ static void sas_chain_work(struct sas_ha_struct *ha, struct sas_work *sw)
>> * not racing against draining
>> */
>> sas_port_get(port);
>> - ret = scsi_queue_work(ha->core.shost, &sw->work);
>> + /*
>> + * discovery event probe and destruct would be called in other
>> + * discovery event like discover domain and revalidate domain
>> + * events, in some cases, we need to sync execute probe and destruct
>> + * events, so run probe and destruct discover events except in a new
>> + * workqueue.
>
> Can we just use ha->disc_q for all discovery events for simplicity? Would this solve the disco mutex you try to solve in patch 7/7?
No, since we could queue sas discovery probe/destruct event in sas discovery work(like sas_revalidate_domain)
sas_revalidate_domain
sas_ex_revalidate_domain
sas_rediscover
sas_rediscover_dev
sas_unregister_devs_sas_addr
sas_unregister_dev
sas_discover_event(dev->port, DISCE_DESTRUCT)
sas_discover_new
sas_ex_discover_devices
sas_ex_discover_dev
sas_ex_discover_end_dev
sas_discover_end_dev
sas_discover_event(dev->port, DISCE_PROBE)
So we could not sync probe or destruct sas discovery event in sas_revalidate_domain if they are queued in a same workqueue,
or it would block for ever.
>
>> + */
>> + if (ev->type == DISCE_PROBE || ev->type == DISCE_DESTRUCT)
>> + ret = queue_work(ha->disc_q, &sw->work);
>> + else
>> + ret = scsi_queue_work(ha->core.shost, &sw->work);
>> +
>> if (ret != 1)
>
> Uhh, we are mixing bool and int here... but we're forced to by scsi_queue_work()
Eagle eye :), I could check the return value separately.
Thanks!
Yijing.
>
>> sas_port_put(port);
>> }
>> diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
>> index 2f3b736..df1d78b 100644
>> --- a/drivers/scsi/libsas/sas_init.c
>> +++ b/drivers/scsi/libsas/sas_init.c
>> @@ -152,6 +152,13 @@ int sas_register_ha(struct sas_ha_struct *sas_ha)
>> if (!sas_ha->event_q)
>> goto Undo_ports;
>>
>> + snprintf(name, 64, "%s_disc_q", dev_name(sas_ha->dev));
>> + sas_ha->disc_q = create_singlethread_workqueue(name);
>> + if(!sas_ha->disc_q) {
>> + destroy_workqueue(sas_ha->event_q);
>> + goto Undo_ports;
>> + }
>> +
>> INIT_LIST_HEAD(&sas_ha->eh_done_q);
>> INIT_LIST_HEAD(&sas_ha->eh_ata_q);
>>
>> @@ -187,6 +194,7 @@ int sas_unregister_ha(struct sas_ha_struct *sas_ha)
>> __sas_drain_work(sas_ha);
>> mutex_unlock(&sas_ha->drain_mutex);
>> destroy_workqueue(sas_ha->event_q);
>> + destroy_workqueue(sas_ha->disc_q);
>>
>> return 0;
>> }
>> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
>> index c2ef05e..4bcb9fe 100644
>> --- a/include/scsi/libsas.h
>> +++ b/include/scsi/libsas.h
>> @@ -406,6 +406,7 @@ struct sas_ha_struct {
>> struct device *dev; /* should be set */
>> struct module *lldd_module; /* should be set */
>> struct workqueue_struct *event_q;
>> + struct workqueue_struct *disc_q;
>>
>> u8 *sas_addr; /* must be set */
>> u8 hashed_sas_addr[HASHED_SAS_ADDR_SIZE];
>>
>
>
>
> .
>
