Re: [PATCH v12 7/9] libata: scsi: no poll when ODD is powered off

2013-01-14 Thread Aaron Lu
On Mon, Jan 14, 2013 at 01:01:47PM -0500, Jeff Garzik wrote:
> On 01/11/2013 01:44 PM, Tejun Heo wrote:
> >Hello,
> >
> >On Fri, Jan 11, 2013 at 11:16:26AM +0800, Aaron Lu wrote:
> >>OK, will make it atomic in next version, thanks for the advice.
> >>
> >>Perhaps I can add two scsi helper functions in scsi_lib.c like:
> >>
> >>void sdev_disable_disk_events(struct scsi_device *sdev)
> >>{
> >>atomic_inc(&sdev->disk_events_disable_depth);
> >>}
> >>
> >>void sdev_enable_disk_events(struct scsi_device *sdev)
> >>{
> >>if (WARN_ON_ONCE(atomic_read(&sdev->disk_events_disable_depth) <= 0))
> >>return;
> >>atomic_dec(&sdev->disk_events_disable_depth);
> >>}
> >>
> >>And call them in ATA layer. Do you like this?
> >
> >Sounds good to me.  James, how does the series look to you?
> 
> Indeed.  Want James' Acked-by for patch #1.
> 
> I think it's ready.  It can go into libata-dev.git #upstream, and be
> reverted prior to Linus push if James NAKs.

Great! Thanks.

I'll incorporate Tejun's suggestions into v13 and send them out soon.

-Aaron

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


[Resend PATCH 3/3] scsi: sd: remove unnecessary initialization in sd_probe()

2013-01-14 Thread Guo Chao
We use kzalloc() to allocate scsi_disk, no need to reset ->openers.

Signed-off-by: Guo Chao 
---
 drivers/scsi/sd.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 9fd67be..4f4bc7e 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2907,7 +2907,6 @@ static int sd_probe(struct device *dev)
sdkp->driver = &sd_template;
sdkp->disk = gd;
sdkp->index = index;
-   atomic_set(&sdkp->openers, 0);
atomic_set(&sdkp->device->ioerr_cnt, 0);
 
if (!sdp->request_queue->rq_timeout) {
-- 
1.7.9.5

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


[Resend PATCH 2/3] scsi: sd: no need to set gendisk->minors in sd_probe_async()

2013-01-14 Thread Guo Chao
We already call alloc_disk() with SD_MINORS and ->minors will
be always set there.

Signed-off-by: Guo Chao 
---
 drivers/scsi/sd.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 181cd87..9fd67be 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2801,7 +2801,6 @@ static void sd_probe_async(void *data, async_cookie_t 
cookie)
 
gd->major = sd_major((index & 0xf0) >> 4);
gd->first_minor = ((index & 0xf) << 4) | (index & 0xfff00);
-   gd->minors = SD_MINORS;
 
gd->fops = &sd_fops;
gd->private_data = &sdkp->driver;
-- 
1.7.9.5

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


[Resend PATCH 1/3] scsi: sd: set valid return value in failed path

2013-01-14 Thread Guo Chao
Set valid return value to avoid returning 0 in a
failed path of sd_init().

Signed-off-by: Guo Chao 
---
 drivers/scsi/sd.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 7992635..181cd87 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3126,6 +3126,7 @@ static int __init init_sd(void)
if (err)
goto err_out;
 
+   err = -ENOMEM;
sd_cdb_cache = kmem_cache_create("sd_ext_cdb", SD_EXT_CDB_SIZE,
 0, 0, NULL);
if (!sd_cdb_cache) {
-- 
1.7.9.5

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


Re: [PATCH 0/8] V2 Add support for new IBM SAS controllers

2013-01-14 Thread Brian King
On 01/11/2013 05:43 PM, wenxi...@linux.vnet.ibm.com wrote:
> This is version 2 of ipr patches to support new IBM SAS controllers. 
> In V2, we have fixed the following suggestions/warning/sparse errors:
> 
> 1.Changed simple_strtoul() to kstrtoul() in ipr_restore_iopoll_weight routine.
> 2.Removed the __dev annotations.
> 3.Fixed unlock bugs which are caused by my previous patches(reported by 
> sparse).
> 4.BUG_ON gcc 4.7 warning.
> 5.Fixed sparse error in original ipr driver.

James,

I've pulled down the series, made sure it builds on x86 with a recent
gcc, as well as ran them through sparse and smatch. This should
address the issues previously encountered with the patch set. Additionally,
it should ensure the series is bisectable.

The only thing I noticed was that the subject line has the V2
outside of []. Would you like Wendy to resend the series to fix this
in order to make it easier to apply?

Acked-by: Brian King 

Thanks,

Brian

-- 
Brian King
Power Linux I/O
IBM Linux Technology Center


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


Re: xhci problem

2013-01-14 Thread Sarah Sharp
On Sun, Jan 13, 2013 at 09:34:58AM -0500, Allan Dennis wrote:
> I left this problem for awhile, then finally got back to it. I upgraded
> the gentoo kernel to 3.6.11 and was partially successful: the 3TB drive
> mounted ok, but then had some serious trouble transferring files and I
> believe that linux force-unmounted it as a result.
> Then upgrading to 3.7.1 fixed the problem completely. Here is the dmesg
> output now:

Good to hear that upgrading helped.

It would be nice to know which commit fixed your issue, so that we can
make sure any Linux distributions include it in their stable releases.
Do you have time to git bisect the changes between 3.6 and 3.7?

Sarah Sharp

> [4.787051] scsi 7:0:0:0: Direct-Access ST330006 51AS
> CC45 PQ: 0 ANSI: 0
> [4.787176] sd 7:0:0:0: Attached scsi generic sg2 type 0
> [4.787416] sd 7:0:0:0: [sdc] Very big device. Trying to use READ
> CAPACITY(16).
> [4.787998] sd 7:0:0:0: [sdc] 5860533168 512-byte logical blocks:
> (3.00 TB/2.72 TiB)
> [4.788405] sd 7:0:0:0: [sdc] Write Protect is off
> [4.788408] sd 7:0:0:0: [sdc] Mode Sense: 03 00 00 00
> [4.76] sd 7:0:0:0: [sdc] No Caching mode page present
> [4.78] sd 7:0:0:0: [sdc] Assuming drive cache: write through
> [4.789246] sd 7:0:0:0: [sdc] Very big device. Trying to use READ
> CAPACITY(16).
> [4.790420] sd 7:0:0:0: [sdc] No Caching mode page present
> [4.790424] sd 7:0:0:0: [sdc] Assuming drive cache: write through
> [4.841399]  sdc: sdc1
> [4.841883] sd 7:0:0:0: [sdc] Very big device. Trying to use READ
> CAPACITY(16).
> [4.843331] sd 7:0:0:0: [sdc] No Caching mode page present
> [4.843334] sd 7:0:0:0: [sdc] Assuming drive cache: write through
> [4.843336] sd 7:0:0:0: [sdc] Attached SCSI disk
> 
> Thanks for your help!
> 
> On Wed, 2012-12-05 at 09:24 -0800, Sarah Sharp wrote:
> > On Tue, Dec 04, 2012 at 08:28:30PM -0500, Allan Dennis wrote:
> > > I hope you won't be annoyed by my question... here goes:
> > 
> > It's my job to answer questions, so fire away. :)
> > 
> > > I have 2TB and 3TB SATA drives, both connected to my Intense PC (Ivy
> > > Bridge tiny PC) on separate external drive enclosures on USB 3 cables. I
> > > have gentoo linux, and was running kernel 3.4.9 where both drives
> > > weren't recognized properly. Now I updated to 3.5.7 and the 2TB is good
> > > to go (yay!) but the 3TB guy is still giving me problems. Here is the
> > > dmesg output as I plugged in the 3TB (sdc) after the 2TB (sdb) was
> > > successful, with debug enabled in the kernel config. Is this a known
> > > issue? Can I help you figure out what's going wrong here by providing
> > > logs or whatnot?
> > 
> > I can't see anything wrong on the xHCI side, so I suspect the problem
> > might be that the SCSI layer doesn't like some response the hard drive is
> > sending.  I would suggest you take a usbmon trace, following the
> > directions here:
> > 
> > http://lxr.linux.no/#linux/Documentation/usb/usbmon.txt
> > 
> > Please hit 'reply-all' when you send the trace, since I've Cced the
> > linux-usb and usb-storage mailing lists.
> > 
> > Sarah Sharp
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 3/4] block: implement runtime pm strategy

2013-01-14 Thread Alan Stern
On Mon, 14 Jan 2013, Aaron Lu wrote:

> On Tue, Jan 08, 2013 at 10:22:45AM -0500, Alan Stern wrote:
> > Just as importantly, all of the public routines added in patch 2/4 to
> > blk-core.c should have kerneldoc explaining how and where to use them.  
> > In particular, the kerneldoc for blk_pm_runtime_init() has to mention
> > that the block runtime PM implementation works only for drivers that
> > use request structures for their I/O; it doesn't work for drivers that
> > use bio's directly.
> 
> How about the following description for them?

Overall this is very good.

> /**
>  * blk_pm_runtime_init - Block layer runtime PM initialization routine
>  * @q: the queue of the device
>  * @dev: the device the queue belongs to
>  *
>  * Description:
>  *Initialize runtime PM related fields for @q and start auto suspend
>  *for @dev. Drivers that want to take advantage of request based runtime
>  *PM should call this function after @dev has been initialized, and its
>  *request queue @q has been allocated, and runtime PM for it is not
>  *ready yet(either disabled/forbidden or its usage count >= 0).
>  *
>  *The block layer runtime PM is request based, so only works for drivers
>  *that use request as their IO unit instead of those directly use bio's.
>  */
> 
> /**
>  * blk_pre_runtime_suspend - Pre runtime suspend check
>  * @q: the queue of the device
>  *
>  * Description:
>  *This function will check if runtime suspend is allowed for the device
>  *by examining if there are any requests pending in the queue. If there
>  *are requests pending, the device can not be runtime suspended; 
> otherwise,
>  *the queue's status will be updated to SUSPENDING and the driver can
>  *proceed to suspend the device.
>  *
>  *For the not allowed case, we mark last busy for the device so that
>  *runtime PM core will try to autosuspend it some time later.
>  *
>  *This function should be called in the device's runtime suspend callback,
>  *before its runtime suspend function is called.

This doesn't quite make sense, because the runtime_suspend callback 
_is_ the runtime-suspend function.  How about "... should be called 
near the start of the device's runtime_suspend callback."?

A similar comment applies to the other functions.

>  *
>  * Return:
>  *0   - OK to runtime suspend the device
>  *-EBUSY  - Device should not be runtime suspended
>  */
> 
> /**
>  * blk_post_runtime_suspend - Post runtime suspend processing
>  * @q: the queue of the device
>  * @err: return value of the device's runtime suspend function
>  *
>  * Description:
>  *Update the queue's runtime status according to the return value of the
>  *device's runtime suspend function.
>  *
>  *This function should be called in the device's runtime suspend callback,
>  *after its runtime suspend function is called.
>  */
> 
> /**
>  * blk_pre_runtime_resume - Pre runtime resume processing
>  * @q: the queue of the device
>  *
>  * Description:
>  *Update the queue's runtime status to RESUMING in preparation for the
>  *runtime resume of the device.
>  *
>  *This function should be called in the device's runtime resume callback,
>  *before its runtime resume function is called.
>  */
> 
> /**
>  * blk_post_runtime_resume - Post runtime resume processing
>  * @q: the queue of the device
>  * @err: return value of the device's runtime resume function
>  *
>  * Description:
>  *Update the queue's runtime status according to the return value of the
>  *device's runtime resume function. If it is successfully resumed, process
>  *the requests that are queued into the device's queue when it is resuming
>  *and then mark last busy and initiate autosuspend for it.
>  *
>  *This function should be called in the device's runtime resume callback,
>  *after its runtime resume function is called.
>  */
> 
> Please feel free to suggest, thanks.

I would hypenate some of these words, such as "runtime-PM-related
fields" or "request-based runtime PM".  Also, "runtime_suspend" and 
"runtime_resume" generally should have either a '_' or a '-'.

But that's a very minor point; your descriptions are quite good.

Alan Stern

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


Re: [PATCH v12 7/9] libata: scsi: no poll when ODD is powered off

2013-01-14 Thread Jeff Garzik

On 01/11/2013 01:44 PM, Tejun Heo wrote:

Hello,

On Fri, Jan 11, 2013 at 11:16:26AM +0800, Aaron Lu wrote:

OK, will make it atomic in next version, thanks for the advice.

Perhaps I can add two scsi helper functions in scsi_lib.c like:

void sdev_disable_disk_events(struct scsi_device *sdev)
{
atomic_inc(&sdev->disk_events_disable_depth);
}

void sdev_enable_disk_events(struct scsi_device *sdev)
{
if (WARN_ON_ONCE(atomic_read(&sdev->disk_events_disable_depth) <= 0))
return;
atomic_dec(&sdev->disk_events_disable_depth);
}

And call them in ATA layer. Do you like this?


Sounds good to me.  James, how does the series look to you?


Indeed.  Want James' Acked-by for patch #1.

I think it's ready.  It can go into libata-dev.git #upstream, and be 
reverted prior to Linus push if James NAKs.


Jeff



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


Re: [PATCH] Fix libata-eh don't retry command after reset succeeded.

2013-01-14 Thread Sergei Shtylyov
Hello.

On 01/14/2013 05:01 PM, Mark Lord wrote:

>> From 9cc9a85f17a8525e53caf430611d762c105d324c Mon Sep 17 00:00:00 2001
>> From: Bian Yu 
>> Date: Tue, 18 Dec 2012 05:58:34 -0500
>> Subject: [PATCH] Fix libata-eh don't retry command after reset succeeded.
>>  It's introduced by commit 8d899e70c1b3afff, When disk has a UNC error,

   Please also specify that commit's summary in parens.

>>  qc->err_mask will set AC_ERR_MEDIA flag.
>>  Signed-off-by: Bian Yu 

MBR, Sergei

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


Re: [PATCH 6/8] V2 ipr: Implement block iopoll

2013-01-14 Thread wenxiong


Quoting Asias He :


Hello Wen Xiong,

On Sat, Jan 12, 2013 at 7:43 AM,   wrote:

This patch implements blk iopoll in ipr driver for performance improvement.


Can you provide the performance numbers with/without the io polling?
It would be interesting to know.



we enable blk iopoll support in ipr driver when firmware supports  
multiple hrrq. With multiple hrrq support and cpu binding tuning, we  
can see 10%-20% performance improvement in some FIO cases.


Thanks,
Wendy


Signed-off-by: Wen Xiong 
---
 drivers/scsi/ipr.c |  221  
+

 drivers/scsi/ipr.h |6 +
 2 files changed, 178 insertions(+), 49 deletions(-)

Index: b/drivers/scsi/ipr.c
===
--- a/drivers/scsi/ipr.c2013-01-11 16:11:20.062476228 -0600
+++ b/drivers/scsi/ipr.c2013-01-11 16:11:50.275910225 -0600
@@ -108,6 +108,7 @@ static const struct ipr_chip_cfg_t ipr_c
.max_cmds = 100,
.cache_line_size = 0x20,
.clear_isr = 1,
+   .iopoll_weight = 0,
{
.set_interrupt_mask_reg = 0x0022C,
.clr_interrupt_mask_reg = 0x00230,
@@ -132,6 +133,7 @@ static const struct ipr_chip_cfg_t ipr_c
.max_cmds = 100,
.cache_line_size = 0x20,
.clear_isr = 1,
+   .iopoll_weight = 0,
{
.set_interrupt_mask_reg = 0x00288,
.clr_interrupt_mask_reg = 0x0028C,
@@ -156,6 +158,7 @@ static const struct ipr_chip_cfg_t ipr_c
.max_cmds = 1000,
.cache_line_size = 0x20,
.clear_isr = 0,
+   .iopoll_weight = 64,
{
.set_interrupt_mask_reg = 0x00010,
.clr_interrupt_mask_reg = 0x00018,
@@ -3560,6 +3563,95 @@ static struct device_attribute ipr_ioa_r
.store = ipr_store_reset_adapter
 };

+static int ipr_iopoll(struct blk_iopoll *iop, int budget);
+ /**
+ * ipr_show_iopoll_weight - Show ipr polling mode
+ * @dev:   class device struct
+ * @buf:   buffer
+ *
+ * Return value:
+ * number of bytes printed to buffer
+ **/
+static ssize_t ipr_show_iopoll_weight(struct device *dev,
+  struct device_attribute *attr, char *buf)
+{
+   struct Scsi_Host *shost = class_to_shost(dev);
+   struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)shost->hostdata;
+   unsigned long lock_flags = 0;
+   int len;
+
+   spin_lock_irqsave(shost->host_lock, lock_flags);
+   len = snprintf(buf, PAGE_SIZE, "%d\n", ioa_cfg->iopoll_weight);
+   spin_unlock_irqrestore(shost->host_lock, lock_flags);
+
+   return len;
+}
+
+/**
+ * ipr_store_iopoll_weight - Change the adapter's polling mode
+ * @dev:   class device struct
+ * @buf:   buffer
+ *
+ * Return value:
+ * number of bytes printed to buffer
+ **/
+static ssize_t ipr_store_iopoll_weight(struct device *dev,
+   struct device_attribute *attr,
+   const char *buf, size_t count)
+{
+   struct Scsi_Host *shost = class_to_shost(dev);
+   struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)shost->hostdata;
+   unsigned long user_iopoll_weight;
+   unsigned long lock_flags = 0;
+   int i;
+
+   if (!ioa_cfg->sis64) {
+   dev_info(&ioa_cfg->pdev->dev, "blk-iopoll not  
supported on this adapter\n");

+   return -EINVAL;
+   }
+   if (kstrtoul(buf, 10, &user_iopoll_weight))
+   return -EINVAL;
+
+   if (user_iopoll_weight > 256) {
+   dev_info(&ioa_cfg->pdev->dev, "Invalid blk-iopoll  
weight. It must be less than 256\n");

+   return -EINVAL;
+   }
+
+   if (user_iopoll_weight == ioa_cfg->iopoll_weight) {
+   dev_info(&ioa_cfg->pdev->dev, "Current blk-iopoll  
weight has the same weight\n");

+   return strlen(buf);
+   }
+
+   if (blk_iopoll_enabled && ioa_cfg->iopoll_weight &&
+   ioa_cfg->sis64 && ioa_cfg->nvectors > 1) {
+   for (i = 1; i < ioa_cfg->hrrq_num; i++)
+   blk_iopoll_disable(&ioa_cfg->hrrq[i].iopoll);
+   }
+
+   spin_lock_irqsave(shost->host_lock, lock_flags);
+   ioa_cfg->iopoll_weight = user_iopoll_weight;
+   if (blk_iopoll_enabled && ioa_cfg->iopoll_weight &&
+   ioa_cfg->sis64 && ioa_cfg->nvectors > 1) {
+   for (i = 1; i < ioa_cfg->hrrq_num; i++) {
+   blk_iopoll_init(&ioa_cfg->hrrq[i].iopoll,
+   ioa_cfg->iopoll_weight, ipr_iopoll);
+   blk_iopoll_enable(&ioa_cfg->hrrq[i].iopoll);
+   }
+   }
+   spin_unlock_irqrestore(shost->host_lock, lock_flags);
+
+   return 

Re: [PATCH v6 0/4] block layer runtime pm

2013-01-14 Thread Alan Stern
On Mon, 14 Jan 2013, Aaron Lu wrote:

> On Tue, Jan 08, 2013 at 10:27:54AM -0500, Alan Stern wrote:
> > On Tue, 8 Jan 2013, Aaron Lu wrote:
> > 
> > > So this also reminds me that as long as CONFIG_PM_RUNTIME is selected,
> > > the blk_pm_add/put/peek_request functions will be in the block IO path.
> > > Shall we introduce a new config option to selectively build block
> > > runtime PM functionality? something like CONFIG_BLK_PM_RUNTIME perhaps?
> > > 
> > > Just some condition checks in those functions, not sure if it is worth a
> > > new config though. Please suggest, thanks.
> > 
> > I don't think it is needed.  The new routines will be very quick when 
> > blk_pm_runtime_init() hasn't been called, once you add back the checks 
> > for the queue's device.
> 
> Is it necessary to also add the q->dev check in the following case?
> 
> static void blk_pm_requeue_request(struct request *rq)
> {
>   if (rq->q->dev && !(rq->cmd_flags & REQ_PM))
>   rq->q->nr_pending--;
> }
> 
> So that we do not have a meanlingless value of nr_pending for those
> drivers that don't use block runtime PM, and this also has the benefit
> of making those drivers return early.

It doesn't really matter.  Since nr_pending doesn't get used when 
q->dev isn't set, it doesn't hurt for it to have a meaningless value.

Alan Stern

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


Re: [PATCH] Fix libata-eh don't retry command after reset succeeded.

2013-01-14 Thread Mark Lord
On 13-01-14 09:01 AM, Mark Lord wrote:
>>
>> From 9cc9a85f17a8525e53caf430611d762c105d324c Mon Sep 17 00:00:00 2001
>> From: Bian Yu 
>> Date: Tue, 18 Dec 2012 05:58:34 -0500
>> Subject: [PATCH] Fix libata-eh don't retry command after reset succeeded.
>>  It's introduced by commit 8d899e70c1b3afff, When disk has a UNC error,
>>  qc->err_mask will set AC_ERR_MEDIA flag.
>>  Signed-off-by: Bian Yu 
..
> Yup, good catch.  The original patch to fix retries (from me) had it that way,
> but somewhere among the enforced revisions this error crept in unnoticed.
> 
> Jeff -- wanna pick this one up?
> Could be useful for -stable, too.

The original version of this correction patch had the "Signed-off-by" line.
See below.


>From 78c0fb104c9957db682518b97ce6a01ce1bc07b6 Mon Sep 17 00:00:00 2001
From: Bian Yu 
Date: Wed, 12 Dec 2012 22:26:58 -0500
Subject: [PATCH] It should be a mistake introduced by commit 8d899e70c1b3afff.
 because only qc->flags can't be set AC_ERR_*

Signed-off-by: Bian Yu 
---
 drivers/ata/libata-eh.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index bf039b0..bcf4437 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2094,7 +2094,7 @@ static unsigned int ata_eh_speed_down(struct
ata_device *dev,
  */
 static inline int ata_eh_worth_retry(struct ata_queued_cmd *qc)
 {
-   if (qc->flags & AC_ERR_MEDIA)
+   if (qc->err_mask & AC_ERR_MEDIA)
return 0;   /* don't retry media errors */
if (qc->flags & ATA_QCFLAG_IO)
return 1;   /* otherwise retry anything from fs stack */
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Fix libata-eh don't retry command after reset succeeded.

2013-01-14 Thread Mark Lord
> 
> From 9cc9a85f17a8525e53caf430611d762c105d324c Mon Sep 17 00:00:00 2001
> From: Bian Yu 
> Date: Tue, 18 Dec 2012 05:58:34 -0500
> Subject: [PATCH] Fix libata-eh don't retry command after reset succeeded.
>  It's introduced by commit 8d899e70c1b3afff, When disk has a UNC error,
>  qc->err_mask will set AC_ERR_MEDIA flag.
>  Signed-off-by: Bian Yu 
>  
> ---
>  drivers/ata/libata-eh.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>  
> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
> index bf039b0..bcf4437 100644
> --- a/drivers/ata/libata-eh.c
> +++ b/drivers/ata/libata-eh.c
> @@ -2094,7 +2094,7 @@ static unsigned int ata_eh_speed_down(struct ata_device 
> *dev,
>   */
>  static inline int ata_eh_worth_retry(struct ata_queued_cmd *qc)
>  {
> -   if (qc->flags & AC_ERR_MEDIA)
> +   if (qc->err_mask & AC_ERR_MEDIA)
> return 0;   /* don't retry media errors */
> if (qc->flags & ATA_QCFLAG_IO)
> return 1;   /* otherwise retry anything from fs stack */
> -- 
> 1.7.1

Yup, good catch.  The original patch to fix retries (from me) had it that way,
but somewhere among the enforced revisions this error crept in unnoticed.

Jeff -- wanna pick this one up?
Could be useful for -stable, too.

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