On Mon, Sep 02, 2024 at 06:44:09PM +0800, Kefeng Wang wrote: > On 2024/8/31 22:04, Matthew Wilcox wrote: > > On Thu, Aug 29, 2024 at 10:54:52PM +0800, Kefeng Wang wrote: > > > Non-LRU movable folio isolation will fail if it can't grab a reference > > > in isolate_movable_page(), so folio_get_nontail_page() could be called > > > ahead to unify the handling of non-LRU movable/LRU folio isolation a bit, > > > this is also prepare to convert isolate_movable_page() to take a folio. > > > Since the reference count of the non-LRU movable folio is increased, > > > a folio_put() is needed whether the folio is isolated or not. > > > > There's a reason I stopped where I did when converting this function > > to use folios. Usually I would explain, but I think it would do you > > good to think about why for a bit. > > Hm, I don't find the reason, > > The major change is that we move folio_get_nontail_page ahead, so we > may try add a reference for each page, it always fails to isolate > with/without this changes, so I suppose that there is no issue here, You haven't considered the effect on others. Taking the refcount on a page will necessarily dirty the cacheline. This is compaction code, so someone else may have this page allocated. The check is done without a refcount in order to minimise the effect if this page cannot be migrated. Try doing this on a NUMA system to really see the effects. More broadly, the problem is that you're sending patches faster than I can review them, and Andrew is picking them up. I don't know what to do about that.
