在 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 <wangyij...@huawei.com>
>> CC: John Garry <john.ga...@huawei.com>
>> CC: Johannes Thumshirn <jthumsh...@suse.de>
>> CC: Ewan Milne <emi...@redhat.com>
>> CC: Christoph Hellwig <h...@lst.de>
>> CC: Tomas Henzl <the...@redhat.com>
>> CC: Dan Williams <dan.j.willi...@intel.com>
>> ---
>>  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];
>>
> 
> 
> 
> .
> 

Reply via email to