在 2020/8/16 下午12:17, Matthew Wilcox 写道:
> On Sun, Aug 16, 2020 at 11:47:57AM +0800, Alex Shi wrote:
>> Current pageblock_flags is only 4 bits, so it has to share a char size
>> in cmpxchg when get set, the false sharing cause perf drop.
>>
>> If we incrase the bits up to 8, false sharing would gone in cmpxchg. and
>> the only cost is half char per pageblock, which is half char per 128MB
>> on x86, 4 chars in 1 GB.
>
> I don't believe this patch has that effect, mostly because it still does
> cmpxchg() on words instead of bytes.
Hi Matthew,
Thank a lot for comments!
Sorry, I must overlook sth, would you like point out why the cmpxchg is still
on words after patch 1 applied?
>
> But which functions would benefit? It seems to me this cmpxchg() is
> only called from the set_pageblock_migratetype() morass of functions,
> none of which are called in hot paths as far as I can make out.
>
> So are you just reasoning by analogy with the previous patch where you
> have measured a performance improvement, or did you send the wrong patch,
> or did I overlook a hot path that calls one of the pageblock migration
> functions?
>
Uh, I am reading compaction.c and found the following commit introduced
test_and_set_skip under a lock. It looks like the pagelock_flags setting
has false sharing in cmpxchg. but I have no valid data on this yet.
Thanks
Alex
e380bebe4771548 mm, compaction: keep migration source private to a single compaction instance
if (!locked) {
locked = compact_trylock_irqsave(zone_lru_lock(zone),
&flags, cc);
- if (!locked)
+
+ /* Allow future scanning if the lock is contended */
+ if (!locked) {
+ clear_pageblock_skip(page);
break;
+ }
+
+ /* Try get exclusive access under lock */
+ if (!skip_updated) {
+ skip_updated = true;
+ if (test_and_set_skip(cc, page, low_pfn))
+ goto isolate_abort;
+ }
