Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752901AbdGNHqq (ORCPT ); Fri, 14 Jul 2017 03:46:46 -0400 Received: from szxga05-in.huawei.com ([45.249.212.191]:2102 "EHLO szxga05-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750917AbdGNHqo (ORCPT ); Fri, 14 Jul 2017 03:46:44 -0400 Subject: Re: [PATCH v3 4/7] libsas: add sas event wait-complete support To: Hannes Reinecke , , References: <1499670369-44143-1-git-send-email-wangyijing@huawei.com> <1499670369-44143-5-git-send-email-wangyijing@huawei.com> CC: , , , , , , , , , , , , , , , , , , , Johannes Thumshirn From: wangyijing Message-ID: <596876CA.2080001@huawei.com> Date: Fri, 14 Jul 2017 15:46:18 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.177.23.4] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A0B0207.596876DE.01A6,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 44b925326c2779cd2d3bdea5e7986992 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6187 Lines: 187 在 2017/7/14 14:51, Hannes Reinecke 写道: > On 07/10/2017 09:06 AM, Yijing Wang wrote: >> Introduce wait-complete for libsas sas event processing, >> execute sas port create/destruct in sync. >> >> Signed-off-by: Yijing Wang >> CC: John Garry >> CC: Johannes Thumshirn >> CC: Ewan Milne >> CC: Christoph Hellwig >> CC: Tomas Henzl >> CC: Dan Williams >> --- >> drivers/scsi/libsas/sas_discover.c | 41 ++++++++++++++++++++++++++++---------- >> drivers/scsi/libsas/sas_internal.h | 34 +++++++++++++++++++++++++++++++ >> drivers/scsi/libsas/sas_port.c | 4 ++++ >> include/scsi/libsas.h | 5 ++++- >> 4 files changed, 72 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c >> index 60de662..5d4a3a8 100644 >> --- a/drivers/scsi/libsas/sas_discover.c >> +++ b/drivers/scsi/libsas/sas_discover.c >> @@ -525,16 +525,43 @@ static void sas_revalidate_domain(struct work_struct *work) >> mutex_unlock(&ha->disco_mutex); >> } >> >> +static const work_func_t sas_event_fns[DISC_NUM_EVENTS] = { >> + [DISCE_DISCOVER_DOMAIN] = sas_discover_domain, >> + [DISCE_REVALIDATE_DOMAIN] = sas_revalidate_domain, >> + [DISCE_PROBE] = sas_probe_devices, >> + [DISCE_SUSPEND] = sas_suspend_devices, >> + [DISCE_RESUME] = sas_resume_devices, >> + [DISCE_DESTRUCT] = sas_destruct_devices, >> +}; >> + >> +/* a simple wrapper for sas discover event funtions */ >> +static void sas_discover_common_fn(struct work_struct *work) >> +{ >> + struct sas_discovery_event *ev = to_sas_discovery_event(work); >> + struct asd_sas_port *port = ev->port; >> + >> + sas_event_fns[ev->type](work); >> + sas_port_put(port); >> +} >> + >> + >> /* ---------- Events ---------- */ >> >> static void sas_chain_work(struct sas_ha_struct *ha, struct sas_work *sw) >> { >> + int ret; >> + struct sas_discovery_event *ev = to_sas_discovery_event(&sw->work); >> + struct asd_sas_port *port = ev->port; >> + >> /* chained work is not subject to SA_HA_DRAINING or >> * SAS_HA_REGISTERED, because it is either submitted in the >> * workqueue, or known to be submitted from a context that is >> * not racing against draining >> */ >> - scsi_queue_work(ha->core.shost, &sw->work); >> + sas_port_get(port); >> + ret = scsi_queue_work(ha->core.shost, &sw->work); >> + if (ret != 1) >> + sas_port_put(port); >> } >> >> static void sas_chain_event(int event, unsigned long *pending, >> @@ -575,18 +602,10 @@ void sas_init_disc(struct sas_discovery *disc, struct asd_sas_port *port) >> { >> int i; >> >> - static const work_func_t sas_event_fns[DISC_NUM_EVENTS] = { >> - [DISCE_DISCOVER_DOMAIN] = sas_discover_domain, >> - [DISCE_REVALIDATE_DOMAIN] = sas_revalidate_domain, >> - [DISCE_PROBE] = sas_probe_devices, >> - [DISCE_SUSPEND] = sas_suspend_devices, >> - [DISCE_RESUME] = sas_resume_devices, >> - [DISCE_DESTRUCT] = sas_destruct_devices, >> - }; >> - >> disc->pending = 0; >> for (i = 0; i < DISC_NUM_EVENTS; i++) { >> - INIT_SAS_WORK(&disc->disc_work[i].work, sas_event_fns[i]); >> + INIT_SAS_WORK(&disc->disc_work[i].work, sas_discover_common_fn); >> disc->disc_work[i].port = port; >> + disc->disc_work[i].type = i; >> } >> } >> diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h >> index f03ce64..890b5d26 100644 >> --- a/drivers/scsi/libsas/sas_internal.h >> +++ b/drivers/scsi/libsas/sas_internal.h >> @@ -100,6 +100,40 @@ void sas_free_device(struct kref *kref); >> extern const work_func_t sas_phy_event_fns[PHY_NUM_EVENTS]; >> extern const work_func_t sas_port_event_fns[PORT_NUM_EVENTS]; >> >> +static void sas_complete_event(struct kref *kref) >> +{ >> + struct asd_sas_port *port = container_of(kref, struct asd_sas_port, ref); >> + >> + complete_all(&port->completion); >> +} >> + >> +static inline void sas_port_put(struct asd_sas_port *port) >> +{ >> + if (port->is_sync) >> + kref_put(&port->ref, sas_complete_event); >> +} >> + >> +static inline void sas_port_wait_init(struct asd_sas_port *port) >> +{ >> + init_completion(&port->completion); >> + kref_init(&port->ref); >> + port->is_sync = true; >> +} >> + >> +static inline void sas_port_wait_completion( >> + struct asd_sas_port *port) >> +{ >> + sas_port_put(port); >> + wait_for_completion(&port->completion); >> + port->is_sync = false; >> +} >> + >> +static inline void sas_port_get(struct asd_sas_port *port) >> +{ >> + if (port && port->is_sync) >> + kref_get(&port->ref); >> +} >> + >> #ifdef CONFIG_SCSI_SAS_HOST_SMP >> extern int sas_smp_host_handler(struct Scsi_Host *shost, struct request *req, >> struct request *rsp); >> diff --git a/drivers/scsi/libsas/sas_port.c b/drivers/scsi/libsas/sas_port.c >> index 9326628..d589adb 100644 >> --- a/drivers/scsi/libsas/sas_port.c >> +++ b/drivers/scsi/libsas/sas_port.c >> @@ -191,7 +191,9 @@ static void sas_form_port(struct asd_sas_phy *phy) >> if (si->dft->lldd_port_formed) >> si->dft->lldd_port_formed(phy); >> >> + sas_port_wait_init(port); >> sas_discover_event(phy->port, DISCE_DISCOVER_DOMAIN); >> + sas_port_wait_completion(port); >> } >> >> /** >> @@ -218,7 +220,9 @@ void sas_deform_port(struct asd_sas_phy *phy, int gone) >> dev->pathways--; >> >> if (port->num_phys == 1) { >> + sas_port_wait_init(port); >> sas_unregister_domain_devices(port, gone); >> + sas_port_wait_completion(port); >> sas_port_delete(port->port); >> port->port = NULL; >> } else { > > I would rather use the standard on-stack completion here; > like this: > > DECLARE_COMPLETION_ONSTACK(complete); > port->completion = &complete; > sas_unregister_domain_devices(port, gone); > wait_for_completion(&complete); > sas_port_delete(port->port); > > which would simplify the above helpers to: > > static inline void sas_port_put(struct asd_sas_port *port) > { > if (port->completion) > kref_put(&port->ref, sas_complete_event); > } > > and you could do away with the 'is_sync' helper. It looks better, thanks! > > Cheers, > > Hannes >