On Tuesday 05 August 2008 13:57, Andrew Morton wrote:> On Tue, 5 Aug 2008 13:41:53 +1000 Nick Piggin <nickpiggin@yahoo.com.au> wrote:> > On Tuesday 05 August 2008 13:28, Linus Torvalds wrote:> > > On Tue, 5 Aug 2008, Nick Piggin wrote:> > > > On Thursday 31 July 2008 17:26, Nick Piggin wrote:> > > > > Hi,> > > > >> > > > > I'm wondering if I could get a patch merged which changes all> > > > > TestSetPageLocked and replaces them with trylock_page?> > > >> > > > Yes? No?> > > >> > > > The alternative is try to merge it via -mm or -next, but that just> > > > wastes everybodies time with conflicts of having these differences> > > > between -mm and mainline.> > >> > > Heh. I had just been _assuming_ this would go through -mm, since it's> > > exactly the kind of thing that usually does go through there.> > >> > > So I hadn't even really considered it.> >> > OK... it just causes Andrew headaches I suspect. But if he prefers> > to hold onto it for an entire release cycle... Andrew?>> Yup. A mechanical rename-foo-to-bar can be prepared and merged late -> there's little payback for the pain of maintaining it for a couple of> months.>> Would prefer either that we hold off until after 2.6.27 is released or> just do it now.>> > > I don't mind the patch per se, but can you give some background on what> > > the pending optimization is that makes such a big difference?> >> > Using the lock semantics bitops is the first one. While it is true> > that we could just hack them into TestSetPageLocked, I really prefer> > callers to require at least a cursory glance to convert them, and> > understand that this is a lock lock, and not a test_and_set bitop> > with full barrier semantics.>> This patch wouldn't do anything to ensure that callers get the review> which you describe, would it?>> (and we don't need to patch the code to just read it!)

OK, not it doesn't really ensure that I guess. But it at least helps meattempt to review callsites and maintainers of external patches hopefullywill get at least some heads up.

Basically it is the best we can do I think. I haven't seen any problemsin this area and I don't really expect to (only in some areas does thelock implementation itself assume knowledge of barriers beyond regularlock semantics).

In case something assumes a store won't be passed by TestSetPageLocked,for example, it should be commented. Code that makes subtle use ofmemory ordering and is not commented I think we can say from experienceis almost broken by definition :)

... no, that's not my excuse for introducing breakage.. I have *attempted*to review callers, but it's not easy to detect uncommented use of barrier.