Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1687162

Re: [PATCH v3 4/7] libsas: add sas event wait-complete support

From John Garry <john.garry@huawei.com>
Newsgroups linux.kernel
Subject Re: [PATCH v3 4/7] libsas: add sas event wait-complete support
Date 2017-07-14 10:50 +0200
Message-ID <u34sp-T0-7@gated-at.bofh.it> (permalink)
References <u1AZr-1Sl-3@gated-at.bofh.it> <u1AZs-1Sl-33@gated-at.bofh.it> <u32JY-88H-23@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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
> --

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v3 0/7] Enhance libsas hotplug feature Yijing Wang <wangyijing@huawei.com> - 2017-07-10 09:10 +0200
  [PATCH v3 4/7] libsas: add sas event wait-complete support Yijing Wang <wangyijing@huawei.com> - 2017-07-10 09:10 +0200
    Re: [PATCH v3 4/7] libsas: add sas event wait-complete support Hannes Reinecke <hare@suse.de> - 2017-07-14 09:00 +0200
      Re: [PATCH v3 4/7] libsas: add sas event wait-complete support wangyijing <wangyijing@huawei.com> - 2017-07-14 09:50 +0200
      Re: [PATCH v3 4/7] libsas: add sas event wait-complete support John Garry <john.garry@huawei.com> - 2017-07-14 10:50 +0200
  [PATCH v3 6/7] libsas: add wait-complete support to sync discovery event Yijing Wang <wangyijing@huawei.com> - 2017-07-10 09:10 +0200
    Re: [PATCH v3 6/7] libsas: add wait-complete support to sync  discovery event John Garry <john.garry@huawei.com> - 2017-07-12 16:00 +0200
      Re: [PATCH v3 6/7] libsas: add wait-complete support to sync  discovery event wangyijing <wangyijing@huawei.com> - 2017-07-13 04:30 +0200
    Re: [PATCH v3 6/7] libsas: add wait-complete support to sync  discovery event Hannes Reinecke <hare@suse.de> - 2017-07-14 09:00 +0200
  [PATCH v3 3/7] libsas: Use new workqueue to run sas event Yijing Wang <wangyijing@huawei.com> - 2017-07-10 09:10 +0200
    Re: [PATCH v3 3/7] libsas: Use new workqueue to run sas event Hannes Reinecke <hare@suse.de> - 2017-07-14 08:50 +0200
  Re: [PATCH v3 0/7] Enhance libsas hotplug feature John Garry <john.garry@huawei.com> - 2017-07-12 12:10 +0200
    Re: [PATCH v3 0/7] Enhance libsas hotplug feature Johannes Thumshirn <jthumshirn@suse.de> - 2017-07-12 14:00 +0200
    Re: [PATCH v3 0/7] Enhance libsas hotplug feature wangyijing <wangyijing@huawei.com> - 2017-07-13 03:30 +0200
    Re: [PATCH v3 0/7] Enhance libsas hotplug feature wangyijing <wangyijing@huawei.com> - 2017-07-13 03:40 +0200
      Re: [PATCH v3 0/7] Enhance libsas hotplug feature John Garry <john.garry@huawei.com> - 2017-07-13 10:10 +0200
        Re: [PATCH v3 0/7] Enhance libsas hotplug feature wangyijing <wangyijing@huawei.com> - 2017-07-13 10:50 +0200
  Re: [PATCH v3 0/7] Enhance libsas hotplug feature wangyijing <wangyijing@huawei.com> - 2017-07-14 10:30 +0200

csiph-web