Messages in this thread | | | Subject | Re: [PATCH] mm: filemap: clear idle flag for writes | From | Yang Shi <> | Date | Wed, 24 Jun 2020 12:18:02 -0700 |
| |
On 6/24/20 11:53 AM, Andrew Morton wrote: > On Thu, 25 Jun 2020 01:43:32 +0800 Yang Shi <yang.shi@linux.alibaba.com> wrote: > >> Since commit bbddabe2e436aa7869b3ac5248df5c14ddde0cbf ("mm: filemap: >> only do access activations on reads"), mark_page_accessed() is called >> for reads only. But the idle flag is cleared by mark_page_accessed() so >> the idle flag won't get cleared if the page is write accessed only. >> >> Basically idle page tracking is used to estimate workingset size of >> workload, noticeable size of workingset might be missed if the idle flag >> is not maintained correctly. >> >> It seems good enough to just clear idle flag for write operations. >> >> ... >> >> --- a/mm/filemap.c >> +++ b/mm/filemap.c >> @@ -41,6 +41,7 @@ >> #include <linux/delayacct.h> >> #include <linux/psi.h> >> #include <linux/ramfs.h> >> +#include <linux/page_idle.h> >> #include "internal.h" >> >> #define CREATE_TRACE_POINTS >> @@ -1630,6 +1631,11 @@ struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index, >> >> if (fgp_flags & FGP_ACCESSED) >> mark_page_accessed(page); >> + else if (fgp_flags & FGP_WRITE) { >> + /* Clear idle flag for buffer write */ >> + if (page_is_idle(page)) >> + clear_page_idle(page); >> + } >> >> no_page: >> if (!page && (fgp_flags & FGP_CREAT)) { > The kerneldoc comment for pagecache_get_page() could do with some > updating - it fails to mention FGP_WRITE, FGP_NOFS and FGP_NOWAIT.
Yes, will propose a separate patch later on.
> > This change seems correct but also will have runtime effects. What are > they?
Other than a couple of extra cycles when idle page tracking is enabled, I didn't think of other effects. It should be negligible. The idle flag doesn't play a role in page reclaim algorithm, so it won't have impact on that.
| |