public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Ingo Molnar <mingo@elte.hu>,
	linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Darren Hart <dvhltc@us.ibm.com>, Hugh Dickins <hughd@google.com>
Subject: Re: [GIT PULL] futex fixlet
Date: Fri, 30 Dec 2011 18:07:58 +0100	[thread overview]
Message-ID: <1325264878.30917.11.camel@twins> (raw)
In-Reply-To: <CA+55aFydsXqRg-QwfVBuUOSfpGQ24MjgWaK-z+3Qvgn-BHn6Tw@mail.gmail.com>

On Thu, 2011-12-29 at 17:26 -0800, Linus Torvalds wrote:
> Why do we even bother to check "page->mapping" at all? What's the
> *use* of that loop? My gut feeling is that *that* is the fundamental
> problem, and we should just get rid of it, rather than add all these
> totally random work-arounds for the problem.
> 
> Peter Z? That "if (!page->mapping) goto again" actually goes back to
> 38d47c1b7075, in 2008. Why does it exist in the first place? There's
> no comment nor explanation in the changelog.
> 
> Why don't we just unconditionally return -EFAULT? What is the retry
> actually supposed to *do*? If somebody races with a mmap/munmap, why
> the hell would we care? What is it doing? 

Vague memory seems to suggest it was to do with an unmap race, now the
only such race we care about is swapping, if someone has a futex and
does munmap+mmap under us we really don't care and you get to keep
whatever result that yields.

That said, the ->mapping test is wrong because ->mapping is not actually
cleared when the page is unmapped.

Also, I suspect the is_page_cache_freeable() test in pageout() avoids
the worst of it. It keeps the page around if we have a reference to it,
so a minor fault will then quickly re-instate the same page without loss
of data.

So I _think_ you're completely right and we can simply kill the whole
thing, but I've been trying very hard to forget everything kernel
related for a week, and I really shouldn't kick-start my brain until
somewhere next week.



  reply	other threads:[~2011-12-30 17:08 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-29 21:07 [GIT PULL] futex fixlet Ingo Molnar
2011-12-30  1:26 ` Linus Torvalds
2011-12-30 17:07   ` Peter Zijlstra [this message]
2011-12-30 20:06     ` Linus Torvalds
2011-12-30 21:01       ` Hugh Dickins

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1325264878.30917.11.camel@twins \
    --to=a.p.zijlstra@chello.nl \
    --cc=dvhltc@us.ibm.com \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox