Messages in this thread | | | From | Muchun Song <> | Date | Tue, 20 Oct 2020 21:08:25 +0800 | Subject | Re: [External] Re: [PATCH] mm/memory.c: Introduce non-atomic __{Set,Clear}PageSwapCache |
| |
On Tue, Oct 20, 2020 at 7:51 PM Xu, Yanfei <yanfei.xu@windriver.com> wrote: > > > > On 10/19/20 10:58 PM, Muchun Song wrote: > > On Mon, Oct 19, 2020 at 8:31 PM Michal Hocko <mhocko@suse.com> wrote: > >> > >> On Mon 19-10-20 18:15:20, Muchun Song wrote: > >>> For the exclusive reference page, the non-atomic operations is enough, > >>> so replace them to non-atomic operations. > >> > >> I do expect you do not see any difference in runtime and this is mostly > >> driven by the code reading, right? Being explicit about this in the code > >> would be preferred. > > > > Yeah, just code reading.And the set_bit and __set_bit is actually different > > on some architectures. Thanks. > > > >> > >> No objection to the change. > >> > >>> Signed-off-by: Muchun Song <songmuchun@bytedance.com> > >> > >> With an improved changelog > >> Acked-by: Michal Hocko <mhocko@suse.com> > >> > >>> --- > >>> include/linux/page-flags.h | 2 ++ > >>> mm/memory.c | 4 ++-- > >>> 2 files changed, 4 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h > >>> index fbbb841a9346..ec039dde5e4b 100644 > >>> --- a/include/linux/page-flags.h > >>> +++ b/include/linux/page-flags.h > >>> @@ -401,6 +401,8 @@ static __always_inline int PageSwapCache(struct page *page) > >>> } > >>> SETPAGEFLAG(SwapCache, swapcache, PF_NO_TAIL) > >>> CLEARPAGEFLAG(SwapCache, swapcache, PF_NO_TAIL) > >>> +__SETPAGEFLAG(SwapCache, swapcache, PF_NO_TAIL) > >>> +__CLEARPAGEFLAG(SwapCache, swapcache, PF_NO_TAIL) > >>> #else > >>> PAGEFLAG_FALSE(SwapCache) > >>> #endif > >>> diff --git a/mm/memory.c b/mm/memory.c > >>> index 2d267ef6621a..02dd62da26e0 100644 > >>> --- a/mm/memory.c > >>> +++ b/mm/memory.c > >>> @@ -3128,10 +3128,10 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > >>> set_page_private(page, entry.val); > >>> > >>> /* Tell memcg to use swap ownership records */ > >>> - SetPageSwapCache(page); > >>> + __SetPageSwapCache(page); > > Good evening, Muchun. I found there are still some places could be > replaced with __SetPageSwapCache(). Such as shmem_replace_page(), why
Yeah, thanks for your suggestion.
> PG_locked has been set before SetPageSwapCache() is involved.
In this case, It doesn't matter whether PG_locked is set before SetPageSwapCache.
> > Would you please to check the rest places? :)
Ok, I'll take a look. Thanks.
> > Thanks > > Acked-by: Yanfei Xu <yanfei.xu@windriver.com> > > >>> err = mem_cgroup_charge(page, vma->vm_mm, > >>> GFP_KERNEL); > >>> - ClearPageSwapCache(page); > >>> + __ClearPageSwapCache(page); > >>> if (err) { > >>> ret = VM_FAULT_OOM; > >>> goto out_page; > >>> -- > >>> 2.20.1 > >>> > >> > >> -- > >> Michal Hocko > >> SUSE Labs > > > > > >
-- Yours, Muchun
| |