Re: [PATCH] mremap: Avoid TLB flushing anonymous pages that are not in swap cache
From: Dave Hansen
Date: Tue Jun 05 2018 - 15:49:31 EST
On 06/05/2018 12:12 PM, Mel Gorman wrote:
> That's fair enough. I updated part of the changelog to read
> This patch special cases anonymous pages to only flush ranges under the
> page table lock if the page is in swap cache and can be potentially queued
> for IO. Note that the full flush of the range being mremapped is still
> flushed so TLB flushes are not eliminated entirely.
> Does that work for you?
Looks good, thanks.
>> I usually try to make the non-pte-modifying functions take a pte_t
>> instead of 'pte_t *' to make it obvious that there no modification going
>> on. Any reason not to do that here?
> No, it was just a minor saving on stack usage.
We're just splitting hairs now :) but, realistically, this little helper
will get inlined anyway, so it probably all generates the same code.
>> BTW, do you want to add a tiny comment about why we do the
>> trylock_page()? I assume it's because we don't want to wait on finding
>> an exact answer: we just assume it is in the swap cache if the page is
>> locked and flush regardless.
> It's really because calling lock_page while holding a spinlock is
> eventually going to ruin your day.
Hah, yeah, that'll do it. Could you comment this, too?