From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Shi Subject: Re: [PATCH v16 15/22] mm/compaction: do page isolation first in compaction Date: Sun, 19 Jul 2020 11:59:36 +0800 Message-ID: References: <1594429136-20002-1-git-send-email-alex.shi@linux.alibaba.com> <1594429136-20002-16-git-send-email-alex.shi@linux.alibaba.com> Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: Sender: cgroups-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Content-Type: text/plain; charset="utf-8" To: Alexander Duyck Cc: Andrew Morton , Mel Gorman , Tejun Heo , Hugh Dickins , Konstantin Khlebnikov , Daniel Jordan , Yang Shi , Matthew Wilcox , Johannes Weiner , kbuild test robot , linux-mm , LKML , cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Shakeel Butt , Joonsoo Kim , Wei Yang , "Kirill A. Shutemov" 在 2020/7/18 上午12:09, Alexander Duyck 写道: >>> I wonder if it wouldn't make sense to combine these two atomic ops >>> with tests and the put_page into a single inline function? Then it >>> could be possible to just do one check and if succeeds you do the >>> block of code below, otherwise you just fall-through into the -EBUSY >>> case. >>> >> Uh, since get_page changes page->_refcount, TestClearPageLRU changes page->flags, >> So I don't know how to combine them, could you make it more clear with code? > Actually it is pretty straight forward. Something like this: > static inline bool get_page_unless_zero_or_nonlru(struct page *page) > { > if (get_page_unless_zero(page)) { > if (TestClearPageLRU(page)) > return true; > put_page(page); > } > return false; > } > > You can then add comments as necessary. The general idea is you are > having to do this in two different spots anyway so why not combine the > logic? Although it does assume you can change the ordering of the > other test above. It doesn't look different with original code, does it? Thanks Alex