linux-kernel - Re: [PATCH] mm: disable preemption in apply_to_pte

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: <200902140030.59027.nickpiggin@yahoo.com.au>
Date:	Sat, 14 Feb 2009 00:30:58 +1100
From:	Nick Piggin <nickpiggin@...oo.com.au>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Jeremy Fitzhardinge <jeremy@...p.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH] mm: disable preemption in apply_to_pte_range

On Friday 13 February 2009 22:48:30 Peter Zijlstra wrote:
> On Thu, 2009-02-12 at 17:39 -0800, Jeremy Fitzhardinge wrote:
> > In general the model for lazy updates is that you're batching the
> > updates in some queue somewhere, which is almost certainly a piece of
> > percpu state being maintained by someone.  Its therefore broken and/or
> > meaningless to have the code making the updates wandering between cpus
> > for the duration of the lazy updates.
> >
> > > If so, should we do the preempt_disable/enable within those functions?
> > > Probably not worth the cost, I guess.
> >
> > The specific rules are that
> > arch_enter_lazy_mmu_mode()/arch_leave_lazy_mmu_mode() require you to be
> > holding the appropriate pte locks for the ptes you're updating, so
> > preemption is naturally disabled in that case.
>
> Right, except on -rt where the pte lock is a mutex.
>
> > This all goes a bit strange with init_mm's non-requirement for taking
> > pte locks.  The caller has to arrange for some kind of serialization on
> > updating the range in question, and that could be a mutex.  Explicitly
> > disabling preemption in enter_lazy_mmu_mode would make sense for this
> > case, but it would be redundant for the common case of batched updates
> > to usermode ptes.
>
> I really utterly hate how you just plonk preempt_disable() in there
> unconditionally and without very clear comments on how and why.

And even on mainline kernels, builds without the lazy mmu mode stuff
don't need preemption disabled here either, so it is technically a
regression in those cases too.

> I'd rather we'd fix up the init_mm to also have a pte lock.

Well that wouldn't fix -rt; there would need to be a preempt_disable
within arch_enter_lazy_mmu_mode(), which I think is the cleanest
solution.

And, hmm... this makes me wonder what is being applied to what? Are
the callers of apply_to_pte_range on init_mm doing the correct
locking? I'd not be surprised if not. Jeremy did you notice any
particular backtraces?

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