Messages in this thread | | | Date | 23 Aug 2015 21:04:03 -0400 | From | "George Spelvin" <> | Subject | Re: [PATCH 3/3 v3] mm/vmalloc: Cache the vmalloc memory info |
| |
First, an actual, albeit minor, bug: initializing both vmap_info_gen and vmap_info_cache_gen to 0 marks the cache as valid, which it's not.
vmap_info_gen should be initialized to 1 to force an initial cache update.
Second, I don't see why you need a 64-bit counter. Seqlocks consider 32 bits (31 bits, actually, the lsbit means "update in progress") quite a strong enough guarantee.
Third, it seems as though vmap_info_cache_gen is basically a duplicate of vmap_info_lock.sequence. It should be possible to make one variable serve both purposes.
You just need a kludge to handle the case of multiple vamp_info updates between cache updates.
There are two simple ones:
1) Avoid bumping vmap_info_gen unnecessarily. In vmap_unlock(), do vmap_info_gen = (vmap_info_lock.sequence | 1) + 1; 2) - Make vmap_info_gen a seqcount_t - In vmap_unlock(), do write_seqcount_barrier(&vmap_info_gen) - In get_vmalloc_info, inside the seqlock critical section, do vmap_info_lock.seqcount.sequence = vmap_info_gen.sequence - 1; (Using the vmap_info_gen.sequence read while validating the cache in the first place.)
I should try to write an actual patch illustrating this.
| |