Re: [PATCH 03/12] libata, libsas: introduce sched_eh and end_eh port ops

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On Sun, Apr 22, 2012 at 10:30 AM, James Bottomley
<James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, 2012-04-13 at 16:37 -0700, Dan Williams wrote:
>> When managing shost->host_eh_scheduled libata assumes that there is a
>> 1:1 shost-to-ata_port relationship.  libsas creates a 1:N relationship
>> so it needs to manage host_eh_scheduled cumulatively at the host level.
>> The sched_eh and end_eh port port ops allow libsas to track when domain
>> devices enter/leave the "eh-pending" state under ha->lock (previously
>> named ha->state_lock, but it is no longer just a lock for ha->state
>> changes).
>>
>> Since host_eh_scheduled indicates eh without backing commands pinning
>> the device it can be deallocated at any time.  Move the taking of the
>> domain_device reference under the port_lock to guarantee that the
>> ata_port stays around for the duration of eh.
>
>> Cc: Tejun Heo <tj@xxxxxxxxxx>
>> Acked-by: Jacek Danecki <jacek.danecki@xxxxxxxxx>
>
> Could we standardise on Acked-by, please.  In my book it means the
> maintainer of a piece of code agrees with the change and lets me take it
> through my tree.  I'm aware that not everyone uses this definition, so
> we can use a different standard from my current one, but what does it
> mean in this case?
>
>> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
>> ---
>>  drivers/ata/libata-core.c           |    4 ++
>>  drivers/ata/libata-eh.c             |   57 ++++++++++++++++++++++++++++-------
>>  drivers/scsi/libsas/sas_ata.c       |   38 +++++++++++++++++++++--
>>  drivers/scsi/libsas/sas_discover.c  |    6 ++--
>>  drivers/scsi/libsas/sas_event.c     |   12 ++++---
>>  drivers/scsi/libsas/sas_init.c      |   14 ++++-----
>>  drivers/scsi/libsas/sas_scsi_host.c |   27 +++++++++++++----
>>  include/linux/libata.h              |    4 ++
>>  include/scsi/libsas.h               |    4 ++
>>  include/scsi/sas_ata.h              |    5 +++
>>  10 files changed, 134 insertions(+), 37 deletions(-)
>
> This is a pretty big change for rc fixes.  None of the other changes in
> the series seem to be dependent on it, what bug is it actually fixing?

It fixes two bugs (which in hindsight could potentially be split into
two patches).  The major one is guarantees about when
host_eh_scheduled is cleared.  Prior to this patch when at least one
ata_port in a domain has completed eh the flag for host is cleared.
This patch plus "scsi: fix eh wakeup (scsi_schedule_eh vs
scsi_restart_operations)" fixes up deadlocks (waiting indefinitely for
eh to complete) and cases where eh terminates too early
(host_eh_scheduled cleared with work pending) in our hot plug tests.

The other bug this uncovered is the fact that libsas makes use of the
port and rphy object after libata has completed it's work, so this
patch also moved the taking of the domain_device reference to be under
spin_lock_irq(&sas_ha->phy_port_lock) and
spin_lock(&port->dev_list_lock).  Otherwise,  if no commands are
pending for the device libsas can proceed directly to freeing the
domain_device before the requested eh runs.

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Photos]     [Yosemite]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

Add to Google Powered by Linux