| Message ID | 20141007112638.20944.qmail@ns.horizon.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On 7 October 2014 12:26, George Spelvin <linux@horizon.com> wrote: > lib/glob.c provides a new glob_match() function, with arguments in > (pattern, string) order. It replaced a private function with arguments > in (string, pattern) order, but I didn't swap the call site... > > The result was the entire ATA blacklist was effectively disabled. Oops :) > The lesson for today is "I f***ed up *how* badly *how* many months ago?", > er, I mean "Nobody Tests RC Kernels On Legacy Hardware". > > This was not a subtle break, but it made it through an entire RC > cycle unreported, presumably because all the people doing testing > have full-featured hardware. Everyone makes mistakes - although I'd be very surprised there are no testers using the same SSD as mine. It's the most common mSATA drive around here in the UK at the moment (even if it has a slightly dodgy firmware!). > I'd like to wait to add a Tested-by, but it's also important enough > I want to publicly post a fix ASAP Pesky timezone differences! I was at work when you posted this patch, but just got home and recompiled. I can confirm this works: [ 3.845109] ata5.00: disabling queued TRIM support [ 3.845112] ata5.00: ATA-9: Crucial_CT240M500SSD3, MU05, max UDMA/133 ...although the patch didn't apply as a quick copy-paste job from Gmail ...but that's an unrelated matter :) Thanks again, Steven. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 07, 2014 at 07:26:38AM -0400, George Spelvin wrote: > lib/glob.c provides a new glob_match() function, with arguments in > (pattern, string) order. It replaced a private function with arguments > in (string, pattern) order, but I didn't swap the call site... > > The result was the entire ATA blacklist was effectively disabled. > > The lesson for today is "I f***ed up *how* badly *how* many months ago?", > er, I mean "Nobody Tests RC Kernels On Legacy Hardware". > > This was not a subtle break, but it made it through an entire RC > cycle unreported, presumably because all the people doing testing > have full-featured hardware. > > (FWIW, the reason for the argument swap was because fnmatch() does it that > way, and for a while implementing a full fnmatch() was being considered.) > > Fixes: 428ac5fc056e0 (libata: Use glob_match from lib/glob.c) > Reported-by: Steven Honeyman <stevenhoneyman@gmail.com> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=71371#c21 > Signed-off-by: George Spelvin <linux@horizon.com> > Cc: <stable@vger.kernel.org> # 3.17 Oops, lol. Appiled to libata/for-3.17-fixes. Thanks!
Thanks for the ack. > Everyone makes mistakes - although I'd be very surprised there are no > testers using the same SSD as mine. It's the most common mSATA drive > around here in the UK at the moment (even if it has a slightly dodgy > firmware!). Yeah, but this one is particularly simple *and* particularly public. I'm feeling a lot of sympathy for the Mars Climate Orbiter people ATM. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Oops, lol. Appiled to libata/for-3.17-fixes.
I'm delighted that you can laugh; I was expecting a more
fulminating response.
The most amusing part is my oscillation between "how the
heck did I miss that?" and "how the heck did the *entire
rest of the world* miss that?"
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index dbdc5d3..8729a2c 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -4261,10 +4261,10 @@ static unsigned long ata_dev_blacklisted(const struct ata_device *dev) ata_id_c_string(dev->id, model_rev, ATA_ID_FW_REV, sizeof(model_rev)); while (ad->model_num) { - if (glob_match(model_num, ad->model_num)) { + if (glob_match(ad->model_num, model_num)) { if (ad->model_rev == NULL) return ad->horkage; - if (glob_match(model_rev, ad->model_rev)) + if (glob_match(ad->model_rev, model_rev)) return ad->horkage; } ad++;
lib/glob.c provides a new glob_match() function, with arguments in (pattern, string) order. It replaced a private function with arguments in (string, pattern) order, but I didn't swap the call site... The result was the entire ATA blacklist was effectively disabled. The lesson for today is "I f***ed up *how* badly *how* many months ago?", er, I mean "Nobody Tests RC Kernels On Legacy Hardware". This was not a subtle break, but it made it through an entire RC cycle unreported, presumably because all the people doing testing have full-featured hardware. (FWIW, the reason for the argument swap was because fnmatch() does it that way, and for a while implementing a full fnmatch() was being considered.) Fixes: 428ac5fc056e0 (libata: Use glob_match from lib/glob.c) Reported-by: Steven Honeyman <stevenhoneyman@gmail.com> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=71371#c21 Signed-off-by: George Spelvin <linux@horizon.com> Cc: <stable@vger.kernel.org> # 3.17 --- I'd like to wait to add a Tested-by, but it's also important enough I want to publicly post a fix ASAP. Mea maxima culpa. drivers/ata/libata-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html