linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Nishanth Aravamudan <nacc@us.ibm.com>
To: Brian King <brking@linux.vnet.ibm.com>
Cc: jgarzik@pobox.com, wayneb@linux.vnet.ibm.com,
	linux-kernel@vger.kernel.org, linux-ide@vger.kernel.org,
	Tejun Heo <tj@kernel.org>,
	mbizon@freebox.fr, linuxppc-dev@lists.ozlabs.org
Subject: [PATCH] libata/sas: only set FROZEN flag if new EH is supported
Date: Thu, 16 Jun 2011 08:28:36 -0700	[thread overview]
Message-ID: <20110616152836.GA15268@us.ibm.com> (raw)
In-Reply-To: <4DFA0507.3090609@linux.vnet.ibm.com>

On 16.06.2011 [08:28:39 -0500], Brian King wrote:
> On 06/16/2011 02:51 AM, Tejun Heo wrote:
> > On Wed, Jun 15, 2011 at 04:34:17PM -0700, Nishanth Aravamudan wrote:
> >>> That looks like the right thing to do. For ipr's usage of
> >>> libata, we don't have the concept of a port frozen state, so this flag
> >>> should really never get set. The alternate way to fix this would be to
> >>> only set ATA_PFLAG_FROZEN in ata_port_alloc if ap->ops->error_handler
> >>> is not NULL.
> >>
> >> It seemed like ipr is as you say, but I wasn't sure if it was
> >> appropriate to make the change above in the common libata-scis code or
> >> not. I don't want to break some other device on accident.
> >>
> >> Also, I tried your suggestion, but I don't think that can happen in
> >> ata_port_alloc? ata_port_alloc is allocated ap itself, and it seems like
> >> ap->ops typically gets set only after ata_port_alloc returns?
> > 
> > Maybe we can test error_handler in ata_sas_port_start()?
> 
> Good point. Since libsas is converted to the new eh now, we would need to have
> this test.

Commit 7b3a24c57d2eeda8dba9c205342b12689c4679f9 ("ahci: don't enable
port irq before handler is registered") caused a regression for CD-ROMs
attached to the IPR SATA bus on Power machines:

  ata_port_alloc: ENTER
  ata_port_probe: ata1: bus probe begin
  ata1.00: ata_dev_read_id: ENTER
  ata1.00: failed to IDENTIFY (I/O error, err_mask=0x40)
  ata1.00: ata_dev_read_id: ENTER
  ata1.00: failed to IDENTIFY (I/O error, err_mask=0x40)
  ata1.00: limiting speed to UDMA7:PIO5
  ata1.00: ata_dev_read_id: ENTER
  ata1.00: failed to IDENTIFY (I/O error, err_mask=0x40)
  ata1.00: disabled
  ata_port_probe: ata1: bus probe end
  scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured

The FROZEN flag added in that commit is only cleared by the new EH code,
which is not used by ipr. Clear this flag in the SAS code if we don't
support new EH.

Reported-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index d51f979..ebe1685 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3797,6 +3797,12 @@ EXPORT_SYMBOL_GPL(ata_sas_port_alloc);
  */
 int ata_sas_port_start(struct ata_port *ap)
 {
+	/*
+	 * the port is marked as frozen at allocation time, but if we don't
+	 * have new eh, we won't thaw it
+	 */
+	if (!ap->ops->error_handler)
+		ap->pflags &= ~ATA_PFLAG_FROZEN;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(ata_sas_port_start);


-- 
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center

  reply	other threads:[~2011-06-16 15:28 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-15 19:17 libata/ipr/powerpc: regression between 2.6.39-rc4 and 2.6.39-rc5 Nishanth Aravamudan
2011-06-15 20:02 ` Brian King
2011-06-15 23:34   ` Nishanth Aravamudan
2011-06-16  7:51     ` Tejun Heo
2011-06-16 13:28       ` Brian King
2011-06-16 15:28         ` Nishanth Aravamudan [this message]
2011-06-21 16:07           ` [PATCH] libata/sas: only set FROZEN flag if new EH is supported Nishanth Aravamudan
2011-06-21 20:30             ` Brian King
2011-06-21 20:39               ` Jeff Garzik
2011-06-23  4:31               ` Benjamin Herrenschmidt
2011-06-23  4:42                 ` Benjamin Herrenschmidt
2011-06-23  5:10                   ` Benjamin Herrenschmidt
2011-06-23 17:15                   ` Nishanth Aravamudan
2011-06-23 20:05                     ` Brian King
2011-06-23 21:43                       ` Benjamin Herrenschmidt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110616152836.GA15268@us.ibm.com \
    --to=nacc@us.ibm.com \
    --cc=brking@linux.vnet.ibm.com \
    --cc=jgarzik@pobox.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mbizon@freebox.fr \
    --cc=tj@kernel.org \
    --cc=wayneb@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).