I found a little bug (probably) in sys/dev/ata-all.c which somehow corrupts device parameters structure. When I first did "atacontrol list" device info about ad0 looked like this: Master: ad0 <Maxtor 6Y080P0/YAR41BW0> ATA/ATAPI revision 7 after I ran "atacontrol cap ad0" it printed somewhat messy output like having enabled SMART but not supported... then I did "atacontrol list" again and saw that the line about ad0 have changed to something like this: Master: ad0 <W0Maxtor 6Y080P0/YAR41BW0> ATA/ATAPI revision 0 or similar. After some digging and comparing the way "IOCATADEVICES" and "IOCATAGPARM" work I saw (probably) bogus ata_getparam() call. After removing this call to ata_getparam() everything work as expected (atleast that's what it looks like for ~30 min run). "atacontrol cap ad0" shows right results and doesn't screw the device parameters. I just hope that this doesn't break something else but I doubt it coz it just gets info and doesn't set anything. The "giant" patch is attached. It's agains today's -STABLE. regards -------------- next part -------------- --- ata-all.c.old Sat Jul 1 04:10:30 2006 +++ ata-all.c Sat Jul 1 04:40:26 2006 @@ -505,7 +505,6 @@ return error; case IOCATAGPARM: - ata_getparam(atadev, 0); bcopy(&atadev->param, params, sizeof(struct ata_params)); return 0;
On Friday 30 June 2006 22:01, tbyte@otel.net wrote:> I found a little bug (probably) in sys/dev/ata-all.c which somehow > corrupts device parameters structure. When I first did "atacontrol > list" device info about ad0 looked like this: > Master: ad0 <Maxtor 6Y080P0/YAR41BW0> ATA/ATAPI revision 7 > after I ran "atacontrol cap ad0" it printed somewhat messy output > like having enabled SMART but not supported... > then I did "atacontrol list" again and saw that the line about ad0 > have changed to something like this: > Master: ad0 <W0Maxtor 6Y080P0/YAR41BW0> ATA/ATAPI revision 0 > or similar. > > After some digging and comparing the way "IOCATADEVICES" and > "IOCATAGPARM" work I saw (probably) bogus ata_getparam() call. > After removing this call to ata_getparam() everything work as > expected (atleast that's what it looks like for ~30 min run). > "atacontrol cap ad0" shows right results and doesn't screw the > device parameters. I just hope that this doesn't break something > else but I doubt it coz it just gets info and doesn't set anything. > > The "giant" patch is attached. It's agains today's -STABLE.Don't forget to open a PR for this issue, so it doesn't get lost. -- Anish Mistry -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 187 bytes Desc: not available Url : http://lists.freebsd.org/pipermail/freebsd-stable/attachments/20060701/89a37edf/attachment.pgp
>Submitter-Id: stable-users >Originator: Iasen Kostoff >Organization: OTEL.net >Confidential: no >Synopsis: Bug in ata (ata-all.c) driver >Severity: serious >Priority: medium >Category: kern >Class: sw-bug >Release: FreeBSD 6.1-STABLE i386 >Environment:System: FreeBSD 6.1-STABLE #2: Sat Jul 1 13:01:14 EEST 2006 root@WarHeaD.OTEL.net:/usr/src6/sys/i386/compile/WARHEAD>Description:I found a little bug (probably) in sys/dev/ata-all.c which somehow corrupts device parameters structure. When I first did "atacontrol list" device info about ad0 looked like this: Master: ad0 <Maxtor 6Y080P0/YAR41BW0> ATA/ATAPI revision 7 after I ran "atacontrol cap ad0" it printed somewhat messy output like having enabled SMART but not supported... then I did "atacontrol list" again and saw that the line about ad0 have changed to something like this: Master: ad0 <W0Maxtor 6Y080P0/YAR41BW0> ATA/ATAPI revision 0 or similar. After some digging and comparing the way "IOCATADEVICES" and "IOCATAGPARM" work I saw (probably) bogus ata_getparam() call. After removing this call to ata_getparam() everything work as expected (atleast that's what it looks like for ~30 min run). "atacontrol cap ad0" shows right results and doesn't screw the device parameters. I just hope that this doesn't break something else but I doubt it coz it just gets info and doesn't set anything.>How-To-Repeat:In description.>Fix:--- ata-all.c.old Sat Jul 1 04:10:30 2006 +++ ata-all.c Sat Jul 1 04:40:26 2006 @@ -505,7 +505,6 @@ return error; case IOCATAGPARM: - ata_getparam(atadev, 0); bcopy(&atadev->param, params, sizeof(struct ata_params)); return 0;