linux-kernel - Re: [PATCH 2/3] pci: Clamp pcie_set_readrq() when using "performance" settings

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: <1317749439.29415.251.camel@pasglop>
Date:	Tue, 04 Oct 2011 19:30:39 +0200
From:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Jon Mason <mason@...i.com>, Greg Kroah-Hartman <gregkh@...e.de>,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
	Benjamin LaHaise <bcrl@...ck.org>
Subject: Re: [PATCH 2/3] pci: Clamp pcie_set_readrq() when using
 "performance" settings

On Tue, 2011-10-04 at 09:51 -0700, Linus Torvalds wrote:
> On Tue, Oct 4, 2011 at 9:08 AM, Benjamin Herrenschmidt
> <benh@...nel.crashing.org> wrote:
> >>
> >> Well, bcrl argues that patches 1-2 of 3 are actively wrong.
> >
> > This is an argument that isn't finished :-)
> 
> Oh, I absolutely *hope* it isn't finished, but there's no way I'm
> applying a patch for -rc9 that people are still actively arguing
> whether it's at all valid or not.

Well, thing is, you -already- have the whole "performance" option which
is what we are 'arguing' about upstream. Except that the implementation
of it that you have in your tree now has very nasty bugs (ie it doesn't
do what it's supposed to and really doesn't work).

Patches 1 and 2 fix it to do what it's supposed to.

Whether that's a useful scheme or not is what Ben and I are discussing
and frankly the jury is still out as to whether it's beneficial or not
for the generic case. (It's basically an algorithm used today by pHyp on
power).

I wouldn't mind much if we just ripped it out, and left the other
options only at this stage.

> Which is why I'm planning on applying 3/3 just to make the whole issue
> irrelevant for 3.1, and then the people who want to test things out
> can apply whatever patches they want and play with the kernel command
> line options to actually enable whatever behavior they are testing.

As you like but I still think you should apply 1 and 2. As I said above,
the option is there already but the implementation is buggy. Let's at
least make it do what it's supposed to. It's still optional and will
still need testing and benchmarking but heh ...

Or the other option is to rip out all the "performance" case code.

I don't think it makes any sense to keep that option in it's current
non-working form. IE. The code right now does utterly wrong things and
generates something that cannot work if you chose that mode on your
kernel command line.

Cheers,
Ben.

--
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/