* Commit 6856ebbf3e46c8184cec15f59d6436692f85d8ba causes libsas panic on SATA DVD
@ 2008-04-13 22:19 James Bottomley
2008-04-13 22:36 ` James Bottomley
0 siblings, 1 reply; 3+ messages in thread
From: James Bottomley @ 2008-04-13 22:19 UTC (permalink / raw
To: Tejun Heo; +Cc: linux-ide
This patch:
commit 6856ebbf3e46c8184cec15f59d6436692f85d8ba
Author: Tejun Heo <htejun@gmail.com>
Date: Mon Apr 7 22:47:20 2008 +0900
libata: replace tf_read with qc_fill_rtf for non-SFF drivers
Takes away our libsas port_ops->sff_tf_read
However, as soon as we get into atapi_request_sense() we hit
#ifdef CONFIG_ATA_SFF
ap->ops->sff_tf_read(ap, &qc->tf);
#endif
And panic.
atapi_request_sense seems to be unavoidable if we hit an AC_ERR_DEV in
atapi_qc_complete
How is this supposed to work?
James
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Commit 6856ebbf3e46c8184cec15f59d6436692f85d8ba causes libsas panic on SATA DVD
2008-04-13 22:19 Commit 6856ebbf3e46c8184cec15f59d6436692f85d8ba causes libsas panic on SATA DVD James Bottomley
@ 2008-04-13 22:36 ` James Bottomley
2008-04-14 0:35 ` Tejun Heo
0 siblings, 1 reply; 3+ messages in thread
From: James Bottomley @ 2008-04-13 22:36 UTC (permalink / raw
To: Tejun Heo; +Cc: linux-ide
On Sun, 2008-04-13 at 17:19 -0500, James Bottomley wrote:
> This patch:
>
> commit 6856ebbf3e46c8184cec15f59d6436692f85d8ba
> Author: Tejun Heo <htejun@gmail.com>
> Date: Mon Apr 7 22:47:20 2008 +0900
>
> libata: replace tf_read with qc_fill_rtf for non-SFF drivers
>
> Takes away our libsas port_ops->sff_tf_read
>
> However, as soon as we get into atapi_request_sense() we hit
>
> #ifdef CONFIG_ATA_SFF
> ap->ops->sff_tf_read(ap, &qc->tf);
> #endif
>
> And panic.
>
> atapi_request_sense seems to be unavoidable if we hit an AC_ERR_DEV in
> atapi_qc_complete
>
> How is this supposed to work?
This is the patch that fixes the panic for me. It looks like this was
just an oversight in the conversion to qc_fill_rtf.
James
---
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index b195d82..8b72c64 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2390,9 +2390,7 @@ static void atapi_request_sense(struct ata_queued_cmd *qc)
/* FIXME: is this needed? */
memset(cmd->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
-#ifdef CONFIG_ATA_SFF
- ap->ops->sff_tf_read(ap, &qc->tf);
-#endif
+ ap->ops->qc_fill_rtf(qc);
/* fill these in, for the case where they are -not- overwritten */
cmd->sense_buffer[0] = 0x70;
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: Commit 6856ebbf3e46c8184cec15f59d6436692f85d8ba causes libsas panic on SATA DVD
2008-04-13 22:36 ` James Bottomley
@ 2008-04-14 0:35 ` Tejun Heo
0 siblings, 0 replies; 3+ messages in thread
From: Tejun Heo @ 2008-04-14 0:35 UTC (permalink / raw
To: James Bottomley; +Cc: linux-ide
Hello, James.
On Sun, 2008-04-13 at 17:36 -0500, James Bottomley wrote:
> On Sun, 2008-04-13 at 17:19 -0500, James Bottomley wrote:
> > This patch:
> >
> > commit 6856ebbf3e46c8184cec15f59d6436692f85d8ba
> > Author: Tejun Heo <htejun@gmail.com>
> > Date: Mon Apr 7 22:47:20 2008 +0900
> >
> > libata: replace tf_read with qc_fill_rtf for non-SFF drivers
> >
> > Takes away our libsas port_ops->sff_tf_read
> >
> > However, as soon as we get into atapi_request_sense() we hit
> >
> > #ifdef CONFIG_ATA_SFF
> > ap->ops->sff_tf_read(ap, &qc->tf);
> > #endif
> >
> > And panic.
> >
> > atapi_request_sense seems to be unavoidable if we hit an AC_ERR_DEV in
> > atapi_qc_complete
> >
> > How is this supposed to work?
Ah.. Right. I forgot sas was non-SFF + old EH there.
> This is the patch that fixes the panic for me. It looks like this was
> just an oversight in the conversion to qc_fill_rtf.
>
> James
>
> ---
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index b195d82..8b72c64 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2390,9 +2390,7 @@ static void atapi_request_sense(struct ata_queued_cmd *qc)
> /* FIXME: is this needed? */
> memset(cmd->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
>
> -#ifdef CONFIG_ATA_SFF
> - ap->ops->sff_tf_read(ap, &qc->tf);
> -#endif
> + ap->ops->qc_fill_rtf(qc);
>
> /* fill these in, for the case where they are -not- overwritten */
> cmd->sense_buffer[0] = 0x70;
>
I think what's more appropriate is to just add
if (ap->ops->sff_tf_read)
on top of it or just kill it altogether. TF is already read when the
original command failed with CHECK SENSE. Will send a patch.
--
tejun
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-04-14 0:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-04-13 22:19 Commit 6856ebbf3e46c8184cec15f59d6436692f85d8ba causes libsas panic on SATA DVD James Bottomley
2008-04-13 22:36 ` James Bottomley
2008-04-14 0:35 ` Tejun Heo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.