Kairui Song <ryncsn@xxxxxxxxx> writes: > Huang, Ying <ying.huang@xxxxxxxxx> 于2024年1月8日周一 16:28写道： >> >> Kairui Song <ryncsn@xxxxxxxxx> writes: >> >> > From: Kairui Song <kasong@xxxxxxxxxxx> >> > >> > Since all callers of swapin_entry need to check the swap cache first, we >> > can merge this common routine into swapin_entry, so it can be shared and >> > optimized later. >> > >> > Also introduce a enum to better represent possible swap cache usage, and >> > add some comments about it, make the usage of swap cache easier to >> > understand. >> >> I don't find any benefit to do this. The code line number isn't >> reduced. The concept of swap cache isn't hided either. > > Hi Ying > > Thanks for the comments. > > Maybe I should squash this with the following commit? The following > commit want to do cache lookup in swapin_entry to avoid a redundant > shadow lookup, it can help to improve the performance by about 4% for > swapin path. It's good to improve performance. But I don't think we must put swap_cache_get_folio() in swapin_entry() to do that. We can just get "shadow" from swap_cache_get_folio() and pass it to swapin_entry(). > So it need to return a enum to represent cache status. I don't think we are talking about the same thing here. > Further more, note the comments added here: > > +/* > + * Caller of swapin_entry may need to know the cache lookup result: > + * > + * SWAP_CACHE_HIT: cache hit, cached folio is retured. > + * SWAP_CACHE_MISS: cache miss, folio is allocated, read from swap device > + * and adde to swap cache, but still may return a cached > + * folio if raced (check __read_swap_cache_async). > + * SWAP_CACHE_BYPASS: cache miss, folio is new allocated and read > + * from swap device bypassing the cache. > + */ > > SWAP_CACHE_MISS might be inaccurate, this is not an issue introduced > by this commit, but better exposed. May worth a fix later. So far I > can see two benefits fixing it: > - More accurate maj/min page fault count. > - Note the PageHWPoison check in do_swap_page, it ignored the race > case, if a page getting poisoned is raced with swapcache then it may > not work as expected. > > These are all minor issue indeed, some other optimization might also > be doable, but at least a comment might be helpful. > -- Best Regards, Huang, Ying
