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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2ACDAC433F5 for ; Thu, 7 Oct 2021 02:47:35 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id B7EF96109F for ; Thu, 7 Oct 2021 02:47:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org B7EF96109F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 55362940007; Wed, 6 Oct 2021 22:47:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 501BA6B0071; Wed, 6 Oct 2021 22:47:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3F0C7940007; Wed, 6 Oct 2021 22:47:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0001.hostedemail.com [216.40.44.1]) by kanga.kvack.org (Postfix) with ESMTP id 326686B006C for ; Wed, 6 Oct 2021 22:47:34 -0400 (EDT) Received: from smtpin27.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id E8884348DA for ; Thu, 7 Oct 2021 02:47:33 +0000 (UTC) X-FDA: 78668105586.27.3A96B97 Received: from mail-ed1-f46.google.com (mail-ed1-f46.google.com [209.85.208.46]) by imf24.hostedemail.com (Postfix) with ESMTP id B3B92B00157F for ; Thu, 7 Oct 2021 02:47:33 +0000 (UTC) Received: by mail-ed1-f46.google.com with SMTP id dj4so17211941edb.5 for ; Wed, 06 Oct 2021 19:47:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=VyqxLGwIUlQqC1Zl70rnuaga2NhkHcEvoeQooiZvHpc=; b=KpYpF99/J1U8AB4Yg+f9FLyspi817QPhOlycUj58ufDQZjblYuPUQE03e57iuzjqLu 8oaXnl5GS4Fbr3eRlxseg91/g6YAgVck0VmK+0lUjc0ikXDnuB97yUYcv8KeQABV5wEo ZGYywMcMgm2paHxMYLqXK3/DIa2OoctbhkxgzarElrIoLgLczf+U1522/KHZCJStU0pD t6HugBhmK4OFi0WmAi8my9HfvvhgoDfqOSQdrAE34a8eIHGzCBlozTcqMUEc/6mOPoXy tr4dYRaVAcSCo4sDGCROFa6mNHYCxhTfMH/Y2mK/xemucQ6lnkcN6N181hChnYyYvVTB fkXw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=VyqxLGwIUlQqC1Zl70rnuaga2NhkHcEvoeQooiZvHpc=; b=5QymNlhj5yLQpj2IKvmwbuurMHdW90mS4S1sCf0Oc9KbMokNw/z60N+nBnhGbR1YXa kGL4BecngEScgJlzJ6j1sj4xR/1ZEeXVCGVnr0TbLABwzEI9B3biHwZJGyKakcnEDaar GTVsr3vmfRMEmSDbg4HBh9jPijbnISuQaKK+21cUPOHyKZyFj3cgpVaqmkXxsGdlz08y LcgJmvzOnotFkG1hOdHqAwKUcoLiUNPgPpSNVxFfDHTFbdiObp30w59nh4dDTmXpzSRk J4w9hAQE5a04NHP8eQwQR4r5Edn10Z4RVUqLzYZoNNePIbKP4voodmIyDrUQQNSPKo3G 2i5w== X-Gm-Message-State: AOAM531/fzXR8XeK1eQnZt7TPZyS9NatLHXRNqtCfiQWBfG7ZyN387Vr Uobe8iwUoIapb8q/sHrTeHpyWY7VydgpcpwJFLI= X-Google-Smtp-Source: ABdhPJw+gjwOXm4udeiroALI6GqG2qKHnZUWg6Wg2e0RBEhgFKnvwFQdmdDL021sW7ojKqbyvtYBBJf1jeD8T1fBP4I= X-Received: by 2002:a05:6402:1b11:: with SMTP id by17mr2549661edb.71.1633574852376; Wed, 06 Oct 2021 19:47:32 -0700 (PDT) MIME-Version: 1.0 References: <20210930215311.240774-1-shy828301@gmail.com> <20210930215311.240774-4-shy828301@gmail.com> In-Reply-To: From: Yang Shi Date: Wed, 6 Oct 2021 19:47:20 -0700 Message-ID: Subject: Re: [v3 PATCH 3/5] mm: hwpoison: refactor refcount check handling To: Peter Xu Cc: =?UTF-8?B?SE9SSUdVQ0hJIE5BT1lBKOWggOWPoyDnm7TkuZ8p?= , Hugh Dickins , "Kirill A. Shutemov" , Matthew Wilcox , Oscar Salvador , Andrew Morton , Linux MM , Linux FS-devel Mailing List , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: B3B92B00157F X-Stat-Signature: p1riw8icsw4z5xjxqctbfeccodze658u Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="KpYpF99/"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf24.hostedemail.com: domain of shy828301@gmail.com designates 209.85.208.46 as permitted sender) smtp.mailfrom=shy828301@gmail.com X-HE-Tag: 1633574853-762679 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 Wed, Oct 6, 2021 at 3:02 PM Peter Xu wrote: > > On Thu, Sep 30, 2021 at 02:53:09PM -0700, Yang Shi wrote: > > +/* > > + * Return true if page is still referenced by others, otherwise return > > + * false. > > + * > > + * The dec is true when one extra refcount is expected. > > + */ > > +static bool has_extra_refcount(struct page_state *ps, struct page *p, > > + bool dec) > > Nit: would it be nicer to keep using things like "extra_pins", so we pass in 1 > for swapcache dirty case and 0 for the rest? Then it'll also match with most > of the similar cases in e.g. huge_memory.c (please try grep "extra_pins" there). Thanks for the suggestion. Yes, it makes some sense to me. And the code comments in patch 4/5 does says (the suggested version by Naoya): /* * The shmem page is kept in page cache instead of truncating * so is expected to have an extra refcount after error-handling. */ Will rename it in the new version. > > > +{ > > + int count = page_count(p) - 1; > > + > > + if (dec) > > + count -= 1; > > + > > + if (count > 0) { > > + pr_err("Memory failure: %#lx: %s still referenced by %d users\n", > > + page_to_pfn(p), action_page_types[ps->type], count); > > + return true; > > + } > > + > > + return false; > > +} > > + > > /* > > * Error hit kernel page. > > * Do nothing, try to be lucky and not touch this instead. For a few cases we > > * could be more sophisticated. > > */ > > -static int me_kernel(struct page *p, unsigned long pfn) > > +static int me_kernel(struct page_state *ps, struct page *p) > > Not sure whether it's intended, but some of the action() hooks do not call the > refcount check now while in the past they'll all do. Just to double check > they're expected, like this one and me_unknown(). Yeah, it is intentional. Before this change all me_* handlers did check refcount even though it was not necessary, for example, me_kernel() and me_unknown(). > > > { > > unlock_page(p); > > return MF_IGNORED; > > @@ -820,9 +852,9 @@ static int me_kernel(struct page *p, unsigned long pfn) > > /* > > * Page in unknown state. Do nothing. > > */ > > -static int me_unknown(struct page *p, unsigned long pfn) > > +static int me_unknown(struct page_state *ps, struct page *p) > > { > > - pr_err("Memory failure: %#lx: Unknown page state\n", pfn); > > + pr_err("Memory failure: %#lx: Unknown page state\n", page_to_pfn(p)); > > unlock_page(p); > > return MF_FAILED; > > } > > Thanks, > > -- > Peter Xu >