Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 6 May 2021 09:02:50 +0200
From:   Michal Hocko <mhocko@suse.com>
To:     Aili Yao <yaoaili@kingsoft.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        David Hildenbrand <david@redhat.com>, linux-mm@kvack.org,
        LKML <linux-kernel@vger.kernel.org>, yaoaili126@gmail.com
Subject: Re: [PATCH] Revert "mm/gup: check page posion status for coredump."
Message-ID: <YJOUmh+HcDXBdyuS@dhcp22.suse.cz>
References: <20210505135407.31590-1-mhocko@kernel.org>
 <20210506134750.17d2f6eb@alex-virtual-machine>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210506134750.17d2f6eb@alex-virtual-machine>
Precedence: bulk

On Thu 06-05-21 13:47:50, Aili Yao wrote:
> On Wed, 5 May 2021 15:54:07 +0200
> Michal Hocko <mhocko@kernel.org> wrote:
> 
> > From: Michal Hocko <mhocko@suse.com>
> > 
> > While reviewing http://lkml.kernel.org/r/20210429122519.15183-4-david@redhat.com
> > I have crossed d3378e86d182 ("mm/gup: check page posion status for
> > coredump.") and noticed that this patch is broken in two ways. First it
> > doesn't really prevent hwpoison pages from being dumped because hwpoison
> > pages can be marked asynchornously at any time after the check.
> 
> I rethink this:
> There are two cases for this coredump panic issue.
> One is the scenario that the hwpoison flag is set correctly, and the previous patch
> will make it recoverable and avoid panic.
> 
> Another is the hwpoison flag not valid in the check, maybe race condition. I don't think
> this case is worth and reliazable to be covered. As the SRAR can happen freshly in the dump
> process and thus can't be detected.
> 
> And the previous patch doesn't make the Another case worse and unacceptable. just as it can't be
> covered.
> 
> So here is the patch:
> For most case in this topic, the patch will work. For the case hwpoison flag not valid, it will
> fallback to the original process before this patch --- just panic. 

Please propose a new fix which a) doesn't leak a page reference b)
evaluates how realistic is the scenario c) explain why any other gup
user doesn't really need to care - or in other words is the gup layer
really suitable for this issue?
-- 
Michal Hocko
SUSE Labs