From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-173.mta1.migadu.com (out-173.mta1.migadu.com [95.215.58.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 37F71175A73 for ; Mon, 29 Jun 2026 02:20:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.173 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782699647; cv=none; b=oZ4y8QeBh4WuMZxr40tz1mc2NzYfhgVMTCh9EpY3bY0nd/tujTrz6cHJ010oYV1/nBD7K0iHr2OBBy+EB3q1/mGR18L48jqBtIeYSjChIQjuV2FQyZ4Cn+8RN8/TTz49i6b87ms3aXaA2hOX2tM770mgpGHdrdc+qOuRxZT6M7k= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782699647; c=relaxed/simple; bh=yBpMPWowO8l10K5LTOH95sF7yoJF4YTDVq2ZNDPrVxk=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=KFTcF/VLbgR6yNZkHl5ElUUy3lUM3yBjOIYtF09Fb1zy5QsPC0OkrjZMfoLfg6uny1avQ5+MvZj/bx0xPYIiZljGtn5410WthPoTME9GGY35dMWPvfiQdYqVC1MxQy8fOg9WpHT+2lQiP+ygEUfd905nSbanNzGbHQl7FHDyQug= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=fPRn39Wa; arc=none smtp.client-ip=95.215.58.173 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="fPRn39Wa" Message-ID: <407fa67f-3212-4c5a-ac8c-7c5d41479cc9@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1782699643; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8ZMBFIIHzKp4wEGH39HgLodCOcu46BE1L/+pEUVqads=; b=fPRn39Wa0GEM59aN7rtvgym8CNcEvbYf4MbBgPWlhNekwP3UtQZcfBsn2jq35lV17B8YO+ ohzFJHKhjmLPVVKFk6qouSh54NzX3Ev2QRzLdD1OtpCpNa71KtAeJy434X2Kx/kWpVPre0 5qud769RpwjuoLjWTx08XQF30LuXn6w= Date: Mon, 29 Jun 2026 10:20:24 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH v2 2/6] mm/page_owner: use MIGRATE_REASON_NONE instead of -1 for last_migrate_reason To: Zi Yan , Andrew Morton , Vlastimil Babka Cc: Suren Baghdasaryan , Michal Hocko , Brendan Jackman , Johannes Weiner , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20260626024550.25677-1-ye.liu@linux.dev> <20260626024550.25677-3-ye.liu@linux.dev> Content-Language: en-US X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Ye Liu In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT 在 2026/6/27 02:45, Zi Yan 写道: > On Thu Jun 25, 2026 at 10:45 PM EDT, Ye Liu wrote: >> The last_migrate_reason field uses -1 as a sentinel value to mean "no >> migration has happened". Replace the four bare -1 occurrences with a >> local MIGRATE_REASON_NONE define so the intent is explicit and the >> magic number is eliminated. >> >> No functional change. >> >> Signed-off-by: Ye Liu >> --- >> mm/page_owner.c | 15 +++++++++++---- >> 1 file changed, 11 insertions(+), 4 deletions(-) >> >> diff --git a/mm/page_owner.c b/mm/page_owner.c >> index 342549891a8d..ebafa9d7ff07 100644 >> --- a/mm/page_owner.c >> +++ b/mm/page_owner.c >> @@ -21,6 +21,13 @@ >> */ >> #define PAGE_OWNER_STACK_DEPTH (16) >> >> +/* >> + * Used to indicate that a page has never been migrated, as the valid >> + * migrate_reason values are non-negative enum members (MR_* in >> + * include/linux/migrate_mode.h). > > Why not change last_migrate_reason to enum migrate_reason and functions > pass migrate reasons? MR_TYPES probably can be used to indicate no > migration has happened if we do not want to add MR_NO_MIGRATION. > > folio_set_owner_migrate_reason(), > __folio_set_owner_migrate_reason(), > move_hugetlb_state(), and > __update_page_owner_handle() need to update their signatures for this. > Thanks for the review. Using MR_TYPES as a sentinel is problematic - it's used kernel-wide as an array-size marker (migrate_reason_names[MR_TYPES]). Overloading it to also mean "never migrated" conflates two unrelated semantics. It also has a subtle out-of-bounds risk: if anyone ever uses the field as an array index without the guard check, MR_TYPES silently points past the end of the array, while -1 faults immediately. Adding MR_NONE = -1 to the enum is the right long-term fix, but that needs proper enum definition changes and cascading signature updates across multiple headers — better done as a separate series. The current patch is a minimal improvement: it kills the magic number and makes intent explicit without touching any header. > >> + */ >> +#define MIGRATE_REASON_NONE (-1) >> + >> struct page_owner { >> unsigned short order; >> short last_migrate_reason; >> @@ -339,7 +346,7 @@ noinline void __set_page_owner(struct page *page, unsigned short order, >> depot_stack_handle_t handle; >> >> handle = save_stack(gfp_mask); >> - __update_page_owner_handle(page, handle, order, gfp_mask, -1, >> + __update_page_owner_handle(page, handle, order, gfp_mask, MIGRATE_REASON_NONE, >> ts_nsec, current->pid, current->tgid, >> current->comm); >> inc_stack_record_count(handle, gfp_mask, 1 << order); >> @@ -596,7 +603,7 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn, >> if (ret >= count) >> goto err; >> >> - if (page_owner->last_migrate_reason != -1) { >> + if (page_owner->last_migrate_reason != MIGRATE_REASON_NONE) { >> ret += scnprintf(kbuf + ret, count - ret, >> "Page has been migrated, last migrate reason: %s\n", >> migrate_reason_names[page_owner->last_migrate_reason]); >> @@ -667,7 +674,7 @@ void __dump_page_owner(const struct page *page) >> stack_depot_print(handle); >> } >> >> - if (page_owner->last_migrate_reason != -1) >> + if (page_owner->last_migrate_reason != MIGRATE_REASON_NONE) >> pr_alert("page has been migrated, last migrate reason: %s\n", >> migrate_reason_names[page_owner->last_migrate_reason]); >> page_ext_put(page_ext); >> @@ -826,7 +833,7 @@ static void init_pages_in_zone(struct zone *zone) >> >> /* Found early allocated page */ >> __update_page_owner_handle(page, early_handle, 0, 0, >> - -1, local_clock(), current->pid, >> + MIGRATE_REASON_NONE, local_clock(), current->pid, >> current->tgid, current->comm); >> count++; >> ext_put_continue: > > > > -- Thanks, Ye Liu