[PATCH 3/3 v6] mm/vmalloc: Cache the vmalloc memory info

From: Ingo Molnar
Date: Tue Aug 25 2015 - 05:56:54 EST



* George Spelvin <linux@xxxxxxxxxxx> wrote:

> (I hope I'm not annoying you by bikeshedding this too much, although I
> think this is improving.)

[ I don't mind, although I wish other, more critical parts of the kernel got this
much attention as well ;-) ]

> Anyway, suggested changes for v6 (sigh...):
>
> First: you do a second read of vmap_info_gen to optimize out the copy
> of vmalloc_info if it's easily seen as pointless, but given how small
> vmalloc_info is (two words!), i'd be inclined to omit that optimization.
>
> Copy always, *then* see if it's worth keeping. Smaller code, faster
> fast path, and is barely noticeable on the slow path.

Ok, done.

> Second, and this is up to you, I'd be inclined to go fully non-blocking and
> only spin_trylock(). If that fails, just skip the cache update.

So I'm not sure about this one: we have no guarantee of the order every updater
reaches the spinlock, and we want the 'freshest' updater to do the update. The
trylock might cause us to drop the 'freshest' update erroneously - so this change
would introduce a 'stale data' bug I think.

> Third, ANSI C rules allow a compiler to assume that signed integer
> overflow does not occur. That means that gcc is allowed to optimize
> "if (x - y > 0)" to "if (x > y)".

That's annoying ...

> Given that gcc has annoyed us by using this optimization in other
> contexts, It might be safer to make them unsigned (which is required to
> wrap properly) and cast to integer after subtraction.

Ok, done.

> Basically, the following (untested, but pretty damn simple):

I've attached v6 which applies your first and last suggestion, but not the trylock
one.

I also removed _ONCE() accesses from the places that didn't need them.

I added your Reviewed-by optimistically, saving a v7 submission hopefully ;-)

Lightly tested.

Thanks,

Ingo

==============================>