Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751450AbdGNInY (ORCPT ); Fri, 14 Jul 2017 04:43:24 -0400 Received: from szxga01-in.huawei.com ([45.249.212.187]:9771 "EHLO szxga01-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750799AbdGNInT (ORCPT ); Fri, 14 Jul 2017 04:43:19 -0400 Subject: Re: [PATCH v3 4/7] libsas: add sas event wait-complete support To: Hannes Reinecke , Yijing Wang , , References: <1499670369-44143-1-git-send-email-wangyijing@huawei.com> <1499670369-44143-5-git-send-email-wangyijing@huawei.com> CC: , , , , , , , , , , , , , , , , , , Johannes Thumshirn From: John Garry Message-ID: <230b428c-c5b2-e14c-dbbd-2ebf3f295fe5@huawei.com> Date: Fri, 14 Jul 2017 09:42:43 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.203.181.152] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090201.59688422.0031,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: 83673bea9fa0f0322f699911f185b345 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2183 Lines: 69 On 14/07/2017 07:51, Hannes Reinecke wrote: >> #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--; >> > Hannes thanks for checking. >> > 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. > I did wonder if we could avoid using completion altogether and just flush the respective queue which the work item is being processed in. But, due to the intricacy of SCSI/ATA EH, and since we still use shost workqueue for the libsas hotplug processing, maybe it best to keep it straightforward and keep using completions. Anyway, The idea to declare the completion on the stack seems sound. And, for patch 6/7, I don't think the is_sync element is even required without any change to declaration of completion in struct sas_discovery_event. John > Cheers, > > Hannes > --