linux-kernel - Re: 2.6.26-rc5-mm2

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: <Pine.LNX.4.64.0806101723070.23096@blonde.site>
Date:	Tue, 10 Jun 2008 17:50:14 +0100 (BST)
From:	Hugh Dickins <hugh@...itas.com>
To:	Lee Schermerhorn <Lee.Schermerhorn@...com>
cc:	Nick Piggin <nickpiggin@...oo.com.au>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, kernel-testers@...r.kernel.org,
	linux-mm@...ck.org
Subject: Re: 2.6.26-rc5-mm2

On Tue, 10 Jun 2008, Lee Schermerhorn wrote:
> On Tue, 2008-06-10 at 17:28 +1000, Nick Piggin wrote:
> > mm/memory.c:do_wp_page
> > //TODO:  is this safe?  do_anonymous_page() does it this way.
> > 
> > That's a bit disheartening. Surely a question like that has to
> > be answered definitively? (hopefully whatever is doing the
> > asking won't get merged until answered)
> 
> I put those C++ TODO comments in there specifically to raise their
> visibility in hopes that someone [like you :)] would notice and maybe
> have an answer to the question.  I noted the issue in the change log as
> well--i.e., that I had moved set_pte_at() to after the lru_cache_add and
> 'new_rmap.   The existing order may be that way for a reason, but it's
> not clear [to me] what that reason is.  As I noted, do_anonymous_page()
> sets the pte after the lru_add and new_rmap.
> 
> I agree, these questions need to be answered and the TODO's resolved
> before merging.   Any thoughts as to the ordering?

The ordering of lru_cache_add*, page_add_*_rmap and set_pte_at does
not matter (but update_mmu_cache must come after set_pte_at not before).

Even if the page table lock were not held across them (it is), I think
their ordering would not matter much (just benign races); though it's
always worth keeping in mind that once you've done the lru_cache_add,
that page is now visible to vmscan.c.

But I'm all in favour of you imposing consistency there (as part of
a wider patch? perhaps not; and do_swap_page does now look out of step).
It can sometimes help when inserting debug checks e.g. on page_mapcount.

I think you'll find the lru_cache_add_active_or_noreclaim could
actually be moved into page_add_new_rmap - I found that helpful when
working on eliminating the PageSwapCache flag (work now grown out of
date, I'm afraid), to know that the page was not publicly visible
until I did lru_cache_add_active at the end of page_add_new_rmap.

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