Re: [PATCH 0/3] mm/vmalloc: Cache the /proc/meminfo vmalloc statistics

From: George Spelvin
Date: Sun Aug 23 2015 - 00:49:08 EST


Linus wrote:
> I don't think any of this can be called "correct", in that the
> unlocked accesses to the cached state are clearly racy, but I think
> it's very much "acceptable".

I'd think you could easily fix that with a seqlock-like system.

What makes it so simple is that you can always fall back to
calc_vmalloc_info if there's any problem, rather than looping or blocking.

The basic idea is that you have a seqlock counter, but if either of
the two lsbits are set, the cached information is stale.

Basically, you need a seqlock and a spinlock. The seqlock does
most of the work, and the spinlock ensures that there's only one
updater of the cache.

vmap_unlock() does set_bit(0, &seq->sequence). This marks the information
as stale.

get_vmalloc_info reads the seqlock. There are two case:
- If the two lsbits are 10, the cached information is valid.
Copy it out, re-check the seqlock, and loop if the sequence
number changes.
- In any other case, the cached information is
not valid.
- Try to obtain the spinlock. Do not block if it's unavailable.
- If unavailable, do not block.
- If the lock is acquired:
- Set the sequence to (sequence | 3) + 1 (we're the only writer)
- This bumps the sequence number and leaves the lsbits at 00 (invalid)
- Memory barrier TBD. Do the RCU ops in calc_vmalloc_info do it for us?
- Call calc_vmalloc_info
- If we obtained the spinlock earlier:
- Copy our vmi to cached_info
- smp_wmb()
- set_bit(1, &seq->sequence). This marks the information as valid,
as long as bit 0 is still clear.
- Release the spinlock.

Basically, bit 0 says "vmalloc info has changed", and bit 1 says
"vmalloc cache has been updated". This clears bit 0 before
starting the update so that an update during calc_vmalloc_info
will force a new update.

So the three case are basically:
00 - calc_vmalloc_info() in progress
01 - vmap_unlock() during calc_vmalloc_info()
10 - cached_info is valid
11 - vmap_unlock has invalidated cached_info, awaiting refresh

Logically, the sequence number should be initialized to ...01,
but the code above handles 00 okay.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/