Re: regarding bug #5914 - fs corruption on SATA

[Jens Axboe <axboe@suse.de>]

On Fri, Jan 27 2006, Bartlomiej Zolnierkiewicz wrote:
> On 1/27/06, Jens Axboe <axboe@suse.de> wrote:
> > On Fri, Jan 27 2006, Jens Axboe wrote:
> > > On Fri, Jan 27 2006, Nicolas Mailhot wrote:
> > > >
> > > > Le Ven 27 janvier 2006 09:13, Jens Axboe a écrit :
> > > > > On Thu, Jan 26 2006, Nicolas Mailhot wrote:
> > > > >> Le jeudi 26 janvier 2006 à 11:01 +0100, Nicolas Mailhot a écrit :
> > > > >> > Le Jeu 26 janvier 2006 10:21, Jens Axboe a écrit :
> > > > >> > > On Thu, Jan 26 2006, Nicolas Mailhot wrote:
> > > > >> >
> > > > >> > >> What parts can be done one a pre-breakage kernel and what parts on
> > > > >> a
> > > > >> > >> problem kernel (I ask this because a problem kernel will corrupt
> > > > >> > >> basically
> > > > >> > >> any file it writes to, even in single login mode the damage is
> > > > >> > >> significant
> > > > >> > >> so I need to limit the corruption window to minimum).
> > > > >> > >
> > > > >> > > You need a new kernel (after the barrier rework), so 2.6.16-rc1 for
> > > > >> > > instance.
> > > > >> >
> > > > >> > Ok, I'll do the test this evening (CET) with the rawhide/davej
> > > > >> kernel-of
> > > > >> > the day.
> > > > >>
> > > > >> I applied the fua backout patch and the kernel booted beautifully.
> > > > >> Now I guess I need to see if Maxtor released a fixed firmware right ?
> > > > >> (is it possible to change the firmware on a running system ?)
> > > > >
> > > > > If you can get an update firmware, it is usually done by booting from
> > > > > DOS floppy and running a special flash utility from there. Can you send
> > > > > me the hdparm -I /dev/sdX output of the problem drive? I think we should
> > > > > just blacklist it for FUA. This bug is so obscure I think it's a better
> > > > > solution than adding a FUA disable module parameter at this point.
> > > >
> > > > There is already fairly complete smart info available in
> > > > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=177951
> > > > (https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=123604
> > > > https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=123605)
> > >
> > > I didn't notice the smart info, yes that holds enough information.
> > > Thanks!
> >
> > Can you try and boot a kernel with this patch applied (needs to be one
> > of the newer ones, of course) and see if you still see the "w/ FUA"
> > string next to your Maxtor drive(s)?
> >
> > diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
> > index cfbceb5..3feda07 100644
> > --- a/drivers/scsi/libata-scsi.c
> > +++ b/drivers/scsi/libata-scsi.c
> > @@ -1700,6 +1700,28 @@ static unsigned int ata_msense_rw_recove
> >         return sizeof(def_rw_recovery_mpage);
> >  }
> >
> > +/*
> > + * We can turn this into a real blacklist if it's needed, for now just
> > + * blacklist any Maxtor BANC1G10 revision firmware
> > + */
> > +static int ata_dev_supports_fua(u16 *id)
> > +{
> > +       unsigned char model[41], fw[9];
> > +
> > +       if (!ata_id_has_fua(id))
> > +               return 0;
> > +
> > +       ata_dev_id_string(id, model, ATA_ID_PROD_OFS, sizeof(model));
> > +       ata_dev_id_string(id, fw, ATA_ID_FW_REV_OFS, sizeof(fw));
> > +
> > +       if (strncmp(model, "Maxtor", 6))
> > +               return 1;
> > +       if (strncmp(model, "BANC1G10", 8))
> > +               return 1;
> 
> s/model/fw/

Of course, silly typo! Thanks for catching that. Update patch below.

diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index cfbceb5..3feda07 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -1700,6 +1700,28 @@ static unsigned int ata_msense_rw_recove
 	return sizeof(def_rw_recovery_mpage);
 }
 
+/*
+ * We can turn this into a real blacklist if it's needed, for now just
+ * blacklist any Maxtor BANC1G10 revision firmware
+ */
+static int ata_dev_supports_fua(u16 *id)
+{
+	unsigned char model[41], fw[9];
+
+	if (!ata_id_has_fua(id))
+		return 0;
+
+	ata_dev_id_string(id, model, ATA_ID_PROD_OFS, sizeof(model));
+	ata_dev_id_string(id, fw, ATA_ID_FW_REV_OFS, sizeof(fw));
+
+	if (strncmp(model, "Maxtor", 6))
+		return 1;
+	if (strncmp(fw, "BANC1G10", 8))
+		return 1;
+
+	return 0; /* blacklisted */
+}
+
 /**
  *	ata_scsiop_mode_sense - Simulate MODE SENSE 6, 10 commands
  *	@args: device IDENTIFY data / SCSI command of interest.
@@ -1797,7 +1819,7 @@ unsigned int ata_scsiop_mode_sense(struc
 		return 0;
 
 	dpofua = 0;
-	if (ata_id_has_fua(args->id) && dev->flags & ATA_DFLAG_LBA48 &&
+	if (ata_dev_supports_fua(args->id) && dev->flags & ATA_DFLAG_LBA48 &&
 	    (!(dev->flags & ATA_DFLAG_PIO) || dev->multi_count))
 		dpofua = 1 << 4;
 

-- 
Jens Axboe