Messages in this thread | | | Subject | Re: [PATCH] mm/isolate: Drop pre-validating migrate type in undo_isolate_page_range() | From | Anshuman Khandual <> | Date | Fri, 5 Jul 2019 14:00:22 +0530 |
| |
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 <osalvador@suse.de> >> Cc: Michal Hocko <mhocko@suse.com> >> Cc: Qian Cai <cai@lca.pw> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: linux-mm@kvack.org >> Cc: linux-kernel@vger.kernel.org >> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@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 ?
| |