lkml.org 
[lkml]   [2017]   [Jul]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 4/7] libsas: add sas event wait-complete support
From
Date
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
> --


\
 
 \ /
  Last update: 2017-07-14 10:43    [W:0.099 / U:1.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site