On 07/05/2019 01:29 PM, Oscar Salvador wrote:
> On Fri, Jul 05, 2019 at 11:42:41AM +0530, Anshuman Khandual wrote:
>> unset_migratetype_isolate() already validates under zone lock that a given
>> page has already been isolated as MIGRATE_ISOLATE. There is no need for
>> another check before. Hence just drop this redundant validation.
>>
>> Cc: Oscar Salvador <osalva...@suse.de>
>> Cc: Michal Hocko <mho...@suse.com>
>> Cc: Qian Cai <c...@lca.pw>
>> Cc: Andrew Morton <a...@linux-foundation.org>
>> Cc: linux...@kvack.org
>> Cc: linux-kernel@vger.kernel.org
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khand...@arm.com>
>> ---
>> Is there any particular reason to do this migratetype pre-check without zone
>> lock before calling unsert_migrate_isolate() ? If not this should be removed.
>
> I have seen this kinda behavior-checks all over the kernel.
> I guess that one of the main goals is to avoid lock contention, so we check
> if the page has the right migratetype, and then we check it again under the
> lock
> to see whether that has changed.
So the worst case when it becomes redundant might not affect the performance
much ?
>
> e.g: simultaneous calls to undo_isolate_page_range
Right.
>
> But I am not sure if the motivation behind was something else, as the
> changelog
> that added this code was quite modest.
Agreed.
>
> Anyway, how did you come across with this?
> Do things get speed up without this check? Or what was the motivation to
> remove it?
Detected this during a code audit. I figured it can help save some cycles. The
other
call site start_isolate_page_range() does not check migrate type because the
page
block is guaranteed to be MIGRATE_ISOLATE ? I am not sure if a non-lock check
first
in this case is actually improving performance. In which case should we just
leave
the check as is ?
