linux-kernel - Re: [GIT PATCH] USB patches for 3.9-rc1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+55aFywBFO4F+gEmq5h05VBe4unEJLh4RckJwaWPsU67bw44w@mail.gmail.com>
Date:	Fri, 22 Feb 2013 17:10:43 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
Cc:	Fabio Baltieri <fabio.baltieri@...aro.org>,
	Dave Jones <davej@...hat.com>,
	Greg KH <gregkh@...uxfoundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	USB list <linux-usb@...r.kernel.org>, tianyu.lan@...el.com,
	Linux ACPI <linux-acpi@...r.kernel.org>
Subject: Re: [GIT PATCH] USB patches for 3.9-rc1

On Fri, Feb 22, 2013 at 4:48 PM, Rafael J. Wysocki <rjw@...k.pl> wrote:
>
> The problem is, though, that even if bisection turns up something, it doesn't
> automatically mean that this particular commit is the one that caused the
> problem to happen in the first place.

No, I agree. I just react *very* strongly when somebody starts arguing
against reverting.

The fact that the commit that was bisected fixes another bug in a
previous commit does in fact just imply that that *previous* commit
should perhaps be reverted. Of course, I see that that is the first in
the whole series, so I understand the pain, but even so..

And looking at that commit that makes power resources be treated
specially, that looks completely bogus. It's not what the old code did
either, and it seems to be a total hack for the breakage that just
happens to fix that one machine for purely random reasons, as far as I
can tell. The ACPI_BUS_TYPE_POWER case always had match=1 before, no?

The thing that looks to have been dropped somewhere is that

  .acpi_op_start = !!context,

which first became

  device->add_type = context ? ACPI_BUS_ADD_START : ACPI_BUS_ADD_MATCH;

and then somehow that wasn't needed any more, so it became just an unconditional

  device->add_type = ACPI_BUS_ADD_START;

for unexplained reasons (the commit message says that the argument
"context" wasn't used, and renames it not_used to reinforce the point,
but doesn't explain why it can remove the use that was there).

That unconditional "add_type = ACPI_BUS_ADD_START" then later becomes
"flags.match_driver = true", which is where the whole match_driver
thing comes in.

Anyway, I don't see how magically it became about ACPI_BUS_TYPE_POWER.
The code is not exactly obvious, though, so whatever. But that
"context is suddenly not used" commit (e3863094c6f9: "ACPI: Drop the
second argument of acpi_bus_scan()") looks odd.

Maybe "context" was effectively always non-NULL, but it *looks* like
that series of commits is intended to have no semantic changes, and
that's the one that looked very dubious to me.

                   Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/