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;
Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.112.34 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.112.34; helo=mail.nvidia.com;
Subject: Re: [PATCH resend] mm/gup: fix try_grab_compound_head() race with
 split_huge_page()
To:     Jann Horn <jannh@google.com>,
        Andrew Morton <akpm@linux-foundation.org>
CC:     Linux-MM <linux-mm@kvack.org>,
        kernel list <linux-kernel@vger.kernel.org>,
        Matthew Wilcox <willy@infradead.org>,
        "Kirill A . Shutemov" <kirill@shutemov.name>,
        Jan Kara <jack@suse.cz>, stable <stable@vger.kernel.org>
References: <20210611161545.998858-1-jannh@google.com>
 <20210611153624.65badf761078f86f76365ab9@linux-foundation.org>
 <CAG48ez3mjsf41Z2vCjmkuBaHe9XgoXbWDGvM4OJdUjqiCQbN4A@mail.gmail.com>
From:   John Hubbard <jhubbard@nvidia.com>
Message-ID: <9041f85d-515d-576d-21a9-6f10b6e1279e@nvidia.com>
Date:   Sat, 12 Jun 2021 00:17:05 -1000
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <CAG48ez3mjsf41Z2vCjmkuBaHe9XgoXbWDGvM4OJdUjqiCQbN4A@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Jun 2021 10:17:08.2823
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: f3603c15-1a72-4468-0d06-08d92d8b3c3b
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.112.34];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource: CO1NAM11FT059.eop-nam11.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR12MB2997
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 6/11/21 3:49 PM, Jann Horn wrote:
> On Sat, Jun 12, 2021 at 12:36 AM Andrew Morton
> <akpm@linux-foundation.org> wrote:
>> On Fri, 11 Jun 2021 18:15:45 +0200 Jann Horn <jannh@google.com> wrote:
>>> +/* Equivalent to calling put_page() @refs times. */
>>> +static void put_page_refs(struct page *page, int refs)
>>> +{
>>> +     VM_BUG_ON_PAGE(page_ref_count(page) < refs, page);
>>
>> I don't think there's a need to nuke the whole kernel in this case.
>> Can we warn then simply leak the page?  That way we have a much better
>> chance of getting a good bug report.
> 
> Ah, yeah, I guess that makes sense. I had just copied this over from
> put_compound_head(), and figured it was fine to keep it as-is, but I
> guess changing it would be reasonable. I'm not quite sure what the
> best way to do that would be though.
> 
> I guess the check should go away in !DEBUG_VM builds?
> 
> Should I just explicitly put the check in an ifdef block? Like so:
> 
> #ifdef CONFIG_DEBUG_VM
> if (VM_WARN_ON_ONCE_PAGE(...))
>    return;
> #endif
> 
> Or, since inline ifdeffery looks ugly, get rid of the explicit ifdef,

Agreed: VM_WARN_ON_ONCE_PAGE(), at least at the API level, seems like
the best thing to use here. However, as you point out below, it needs a
little something first.

> and change the !DEBUG_VM definition of VM_WARN_ON_ONCE_PAGE() as
> follows so that the branch is compiled away?
> 
> #define VM_WARN_ON_ONCE_PAGE(cond, page)  (BUILD_BUG_ON_INVALID(cond), false)
> 
> That would look kinda neat, but it would be different from the
> behavior of WARN_ON(), which still returns the original condition even
> in !BUG builds, so that could be confusing...
> 

The VM_WARN_ON_ONCE_PAGE() is not implemented exactly right
in the !CONFIG_DEBUG_VM case. IMHO it should follow the WARN*()
behavior, and return the original condition and keep going
in that case.

Then you could use it directly here.


thanks,

-- 
John Hubbard
NVIDIA