From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.9 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1, USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2EF6AC33CB6 for ; Sun, 19 Jan 2020 22:06:23 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id D8B63206B7 for ; Sun, 19 Jan 2020 22:06:22 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="vkuRsCH1" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D8B63206B7 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 7A1106B0588; Sun, 19 Jan 2020 17:06:22 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 751086B0589; Sun, 19 Jan 2020 17:06:22 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 667756B058A; Sun, 19 Jan 2020 17:06:22 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0074.hostedemail.com [216.40.44.74]) by kanga.kvack.org (Postfix) with ESMTP id 52BBE6B0588 for ; Sun, 19 Jan 2020 17:06:22 -0500 (EST) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with SMTP id 06C2B8248047 for ; Sun, 19 Jan 2020 22:06:22 +0000 (UTC) X-FDA: 76395768204.03.swim34_549ff5851a144 X-HE-Tag: swim34_549ff5851a144 X-Filterd-Recvd-Size: 6154 Received: from mail-pg1-f194.google.com (mail-pg1-f194.google.com [209.85.215.194]) by imf27.hostedemail.com (Postfix) with ESMTP for ; Sun, 19 Jan 2020 22:06:21 +0000 (UTC) Received: by mail-pg1-f194.google.com with SMTP id b9so14478691pgk.12 for ; Sun, 19 Jan 2020 14:06:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:in-reply-to:message-id:references :user-agent:mime-version; bh=68HQTriyud3hSemdZ3Q5jhIfxQrwWHJ5pRMS4dq0wH4=; b=vkuRsCH1P3C+DUNAUhzEgCBECKKeqKhCK/U8NoCXNlAe8O/VJP0308w1PmiKQCdHtf Fadeo2zJOmWsGV8wFcFP60rHrCO7h9Y40+6hboeoewf/L54XRNXwhiC7Ho3i8/FEZ3MO QhTKyo81adpCj40y8XFNUXPMD3GR2zC+R1YZqcrB/wo9SUpdUC6HEkLPzI+fhly21zq1 6yEhow4DUHXrj980YdSOWTzSD2llmjzz1H+JwjZeJwWkpXcOfY/yD+sD2vAwvFBETwku Yslteojgp6Ed6MHgZj2K1XBm0909AUsO4+STO9hrd5OZAJs+x/yCLtCHmXo27B75A5/b 39Nw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version; bh=68HQTriyud3hSemdZ3Q5jhIfxQrwWHJ5pRMS4dq0wH4=; b=qlrPA+awMwo08kvpZBy8ZpIOCR6OhsbPmF8FeOOsYCx9F1qc8pJCGfYCJwrV0dqBRZ GyXDO3W+VbwsQOon8Mzd0DmBBavtIxlI3aCs4y9CFSZXGm9UEOLP/aQtU+SgeHyxadIb geVItFbfGFJVJMbNX9/vUUxxcmKHouS45Nf8oXXGp06zkcpuSeGfDqyEkFo+p0wRqTZo vAhALoDj+GNqVdyy9Eej0FZztJXfyumlIYicgxbsT3/fG2kwyErLGacBDe6tUY/kkFeZ HmLNtqr1ZvbZo6/7oVDbAz+ybsAnuSrSqDSs3uJMLxgnY/WxHFDGhOPFof4TV7taYJ9U rjBw== X-Gm-Message-State: APjAAAUX+yENFbuNM45n7d305NIBecrYPZYM0tLYh/OgZBJM9w/udldn 2eS+ieAsAAFPKkIGbbFKP5OhKg== X-Google-Smtp-Source: APXvYqy3T98OD/R+OoQiqnE1AoxgXq5IgcVJqBU+5qmWJWnDGcqCuNoDfQANOX9bvmYkBEF260DSBQ== X-Received: by 2002:a63:a84a:: with SMTP id i10mr55969557pgp.6.1579471580087; Sun, 19 Jan 2020 14:06:20 -0800 (PST) Received: from [2620:15c:17:3:3a5:23a7:5e32:4598] ([2620:15c:17:3:3a5:23a7:5e32:4598]) by smtp.gmail.com with ESMTPSA id b12sm36019062pfi.157.2020.01.19.14.06.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 19 Jan 2020 14:06:19 -0800 (PST) Date: Sun, 19 Jan 2020 14:06:18 -0800 (PST) From: David Rientjes X-X-Sender: rientjes@chino.kir.corp.google.com To: Wei Yang cc: akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/4] mm/page_alloc.c: extract commom part to check page In-Reply-To: <20200119131408.23247-2-richardw.yang@linux.intel.com> Message-ID: References: <20200119131408.23247-1-richardw.yang@linux.intel.com> <20200119131408.23247-2-richardw.yang@linux.intel.com> User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sun, 19 Jan 2020, Wei Yang wrote: > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index d047bf7d8fd4..8cd06729169f 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -1025,13 +1025,9 @@ static inline bool page_expected_state(struct page *page, > return true; > } > > -static void free_pages_check_bad(struct page *page) > +static inline const char *__check_page(struct page *page) > { > - const char *bad_reason; > - unsigned long bad_flags; > - > - bad_reason = NULL; > - bad_flags = 0; > + const char *bad_reason = NULL; > > if (unlikely(atomic_read(&page->_mapcount) != -1)) > bad_reason = "nonzero mapcount"; > @@ -1039,14 +1035,23 @@ static void free_pages_check_bad(struct page *page) > bad_reason = "non-NULL mapping"; > if (unlikely(page_ref_count(page) != 0)) > bad_reason = "nonzero _refcount"; > - if (unlikely(page->flags & PAGE_FLAGS_CHECK_AT_FREE)) { > - bad_reason = "PAGE_FLAGS_CHECK_AT_FREE flag(s) set"; > - bad_flags = PAGE_FLAGS_CHECK_AT_FREE; > - } > #ifdef CONFIG_MEMCG > if (unlikely(page->mem_cgroup)) > bad_reason = "page still charged to cgroup"; > #endif > + return bad_reason; > +} > + > +static void free_pages_check_bad(struct page *page) > +{ > + const char *bad_reason = NULL; > + unsigned long bad_flags = 0; > + > + bad_reason = __check_page(page); > + if (unlikely(page->flags & PAGE_FLAGS_CHECK_AT_FREE)) { > + bad_reason = "PAGE_FLAGS_CHECK_AT_FREE flag(s) set"; > + bad_flags = PAGE_FLAGS_CHECK_AT_FREE; > + } > bad_page(page, bad_reason, bad_flags); > } > > @@ -2044,12 +2049,7 @@ static void check_new_page_bad(struct page *page) > const char *bad_reason = NULL; > unsigned long bad_flags = 0; > > - if (unlikely(atomic_read(&page->_mapcount) != -1)) > - bad_reason = "nonzero mapcount"; > - if (unlikely(page->mapping != NULL)) > - bad_reason = "non-NULL mapping"; > - if (unlikely(page_ref_count(page) != 0)) > - bad_reason = "nonzero _refcount"; > + bad_reason = __check_page(page); > if (unlikely(page->flags & __PG_HWPOISON)) { > bad_reason = "HWPoisoned (hardware-corrupted)"; > bad_flags = __PG_HWPOISON; > @@ -2061,10 +2061,6 @@ static void check_new_page_bad(struct page *page) > bad_reason = "PAGE_FLAGS_CHECK_AT_PREP flag set"; > bad_flags = PAGE_FLAGS_CHECK_AT_PREP; > } > -#ifdef CONFIG_MEMCG > - if (unlikely(page->mem_cgroup)) > - bad_reason = "page still charged to cgroup"; > -#endif > bad_page(page, bad_reason, bad_flags); > } > I think this is compounding a previous problem in these functions: these are all "if" clauses, not "else if" clauses so they are presumably ordered based on least significant to most significant (we only see the last bad_reason that we find). For the page->mem_cgroup check, this leaves bad_flags set but it doesn't match bad_reason. Could you instead fix the problem with these functions so that we actually list *all* the problems with the page rather than only the last conditional that is true?