| Message ID | 1499670369-44143-6-git-send-email-wangyijing@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
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@huawei.com> > CC: John Garry <john.garry@huawei.com> > CC: Johannes Thumshirn <jthumshirn@suse.de> > CC: Ewan Milne <emilne@redhat.com> > CC: Christoph Hellwig <hch@lst.de> > CC: Tomas Henzl <thenzl@redhat.com> > CC: Dan Williams <dan.j.williams@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? > + */ > + 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() > 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]; >
在 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@huawei.com> >> CC: John Garry <john.garry@huawei.com> >> CC: Johannes Thumshirn <jthumshirn@suse.de> >> CC: Ewan Milne <emilne@redhat.com> >> CC: Christoph Hellwig <hch@lst.de> >> CC: Tomas Henzl <thenzl@redhat.com> >> CC: Dan Williams <dan.j.williams@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]; >> > > > > . >
On 07/10/2017 09:06 AM, 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@huawei.com> > CC: John Garry <john.garry@huawei.com> > CC: Johannes Thumshirn <jthumshirn@suse.de> > CC: Ewan Milne <emilne@redhat.com> > CC: Christoph Hellwig <hch@lst.de> > CC: Tomas Henzl <thenzl@redhat.com> > CC: Dan Williams <dan.j.williams@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(-) > Reviewed-by: Hannes Reinecke <hare@suse.com> Cheers, Hannes
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. + */ + 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) 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];
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@huawei.com> CC: John Garry <john.garry@huawei.com> CC: Johannes Thumshirn <jthumshirn@suse.de> CC: Ewan Milne <emilne@redhat.com> CC: Christoph Hellwig <hch@lst.de> CC: Tomas Henzl <thenzl@redhat.com> CC: Dan Williams <dan.j.williams@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(-)