On 2024/9/5 3:02, Matthew Wilcox wrote:
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.
Indeed, I asked whether the taking the refcount on a page
unconditionally is accepted or not in v1, but I should wait for more
time to see if there are more comments, thanks for your explanation,
will consider not only the current code modification, but also other
impacts of changes as much as possible.
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.
OK, I will slowdown for the new version to collect more comments.
