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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 91010C3DA7A for ; Thu, 5 Jan 2023 15:36:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E6F588E0002; Thu, 5 Jan 2023 10:36:07 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E1EBC8E0001; Thu, 5 Jan 2023 10:36:07 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CE66F8E0002; Thu, 5 Jan 2023 10:36:07 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id C06078E0001 for ; Thu, 5 Jan 2023 10:36:07 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 7C963AB120 for ; Thu, 5 Jan 2023 15:36:07 +0000 (UTC) X-FDA: 80321146374.08.813A55E Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf29.hostedemail.com (Postfix) with ESMTP id 1C3EF120014 for ; Thu, 5 Jan 2023 15:36:04 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=nVxl5SVx; dmarc=none; spf=none (imf29.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1672932965; h=from:from:sender: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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=gQUVnq/KRRYcYlYb/NUnN+HgkAGfxeQMSvNtoyFPIfA=; b=IvIF/xtA0ri8GbkjSet7V0KdCkDQIcFfhLw/+yu6MqLeDMNiIcDao3yZCaXwx7vOPwNoC8 to714nwRYMEzitALpGjwgvVIpKyvPG3eaNtjSVMe3SPeO5LBdvCvv0I1U0Mh0pcThxYYdN R/MPInMFMPNGPqu1DQBG9JEJam2sBZ4= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=nVxl5SVx; dmarc=none; spf=none (imf29.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1672932966; a=rsa-sha256; cv=none; b=j7qTlJeg9uVPY1XfEgCr3f0rpf/0LD1aa8zdeuJacgNQSETVM2z/nmojYNui5x1VfRjIft wrqpIatyPaRhuvBjQ0zJF01o3g/tX+pqkYculwAx82ZfbO3sHBTbDYu6y5BW83S7S+nhHm fwz3T33qss5fsD83PwW707bPIWj/ono= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=gQUVnq/KRRYcYlYb/NUnN+HgkAGfxeQMSvNtoyFPIfA=; b=nVxl5SVxoL53J3crMeqVtxMmpU NePoexvwH/aFyYAe2DpV5UNcUTKzQe1KqL7XHfWZu09jGQ9sv/h/jeC7UgIquWWRzin7QNnuff3l/ UxWaHhnXB5YYYOK1vq24S8u+DpanFGp2A5vHUz+D3S8LEESH2nXzH5mEH+Trpw6Dk9QflurylSbfK mpfl0wdSrT0dqKO2ANuixUeWxM4bPYIYTiy9F3r4NoWDTEgIGj5gXgHoScp0cKgSMS0r3HRd9doQ0 +yfePNNkASURsf1Auhh/0G1fx0QeVHW8wvccBYK3cHXrDIgt2+Y8IFHQyIEvMO7nQ8pleWpiK+9uc 4AUb13xg==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1pDSHb-00GERU-DZ; Thu, 05 Jan 2023 15:35:55 +0000 Date: Thu, 5 Jan 2023 15:35:55 +0000 From: Matthew Wilcox To: Vlastimil Babka Cc: David Rientjes , kernel test robot , Hyeonggon Yoo <42.hyeyoo@gmail.com>, oe-lkp@lists.linux.dev, lkp@intel.com, Mike Rapoport , Christoph Lameter , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: A better dump_page() Message-ID: References: <202212312021.bc1efe86-oliver.sang@intel.com> <41276905-b8a5-76ae-8a17-a8ec6558e988@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 1C3EF120014 X-Stat-Signature: iu75n3zaugyz9ojo1876bgcyzmb1ddim X-HE-Tag: 1672932964-810964 X-HE-Meta: U2FsdGVkX1+lK8c7u7kwqYRe/ZRKiXdzNEVM+s1V9dWFmXBSk3cMqA5ur1o++QctTO+cODj7XYCCUTpVy9zuO/ukIsXPNxA5Y+H2gYgpeqGsqTTn1Y2j8XTyNCVqyhe6+sAvShJfSo9kQ3STiQZ2+tIkYiXeqW7fxSKqXtb92UhWkOoTK/Je8TZe4EKThWuDY076pDvrXJ1s0UDkSzpdUx9r8oOskTQARI7govRYjMbNLlUjGxvFH5FZbajSc72ofaeKkdoUQpn01FDlC0Tknc+gVnJiHxDEC9no1oCRkQHG7D4L3ocZ1qVjxjfTG1bnZRrw0oKVVt+qpwUF9U1DlWAwEgZa3OPZc7JaofozRv0lq5Hs7yeRKaS/lf0jQIYVnGEchgb0ZuHVJg+3jj8TG5tF4Jv/PuJjZ0oz7TrFTxNQ11BkrPPjP6TkUMEBcIohcigJPOrkMmLKVRHp1jAKqG5iJbcZrG7bYaVkLkOlBx22qLVBBTxo36ndVc+fH3rkTYW5w97LvD/au7XfiELgXYTD6B3IrplSnwHVDG5MHVvQI4yf/GZwTmXCXLhsusD8HonGkysYeF/ks5rbNhyNlNUTu3MMFh35ROyEENgrTbEwiDdChD9WvnMHClJXifVHKBeuSzAlOR+GjgFxCU8JSYQS+gTnAdALyG/rAlx123NULr/CByhuHznLOf0As7olCFmsSYPYsGuliqq+cWpt3Ve9gExtf2iAypTbF3T3FNS2YqiZRVJyZVA+6gWaaKHz7HmTl7UIU5N8MrphMUOreUhcQLarxi9WQI/tCnKAxBubLvu7yXA+rl7SfJJDCpEhhkoiHmAAWucuOVJdwsvVlxN55Wc3oA5TgNn7T5g/W1V/ut2GNFi6dWhFxcerVNbb+gCJpJGwjSEtsoGl0XTahq5GwYoDkAXP2gpu+Fm+CZDZsEup1d0GfZO/xHGJlQSQNZBgASDp/ou9tu5K0Gh xbi/9oS/ alcRR 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 Thu, Jan 05, 2023 at 04:19:06PM +0100, Vlastimil Babka wrote: > On 1/4/23 00:29, Matthew Wilcox wrote: > > On Tue, Jan 03, 2023 at 03:07:12PM -0800, David Rientjes wrote: > >> On Tue, 3 Jan 2023, Matthew Wilcox wrote: > >> > >> > On Tue, Jan 03, 2023 at 11:42:11AM +0100, Vlastimil Babka wrote: > >> > > Separately we should also make the __dump_page() more resilient. > >> > > >> > Right. It's not ideal when one of our best debugging tools obfuscates > >> > the problem we're trying to debug. I've seen probems like this before, > >> > and the problem is that somebody calls dump_page() on a page that they > >> > don't own a refcount on. That lets the page mutate under us in some > >> > fairly awkward ways (as you've seen here, it seems to be part of several > >> > different compound allocations at various points during the dump > >> > process). > >> > > >> > One possibility I thought about was taking our own refcount on the > >> > page at the start of dump_page(). That would kill off the possibility > >> > of ever passing in a const struct page, and it would confuse people. > >> > Also, what if somebody passes in a pointer to something that's not a > >> > struct page? Then we've (tried to) modify memory that's not a refcount. > >> > > >> > I think the best we can do is to snapshot the struct page and the folio > >> > it appears to belong to at the start of dump_page(). It'll take a > >> > little care (for example, folio_pfn() must be passed the original > >> > folio, and not the snapshot), but I think it's doable. > >> > > >> > >> By snapshot do you mean memcpy() of the metadata to the stack? I assume > >> this still leaves the opportunity for the underlying mutation of the page > >> but makes the window more narrow. > > > > Right. We'd need to memcpy() both the page and the folio, so around 192 > > bytes. It doesn't make it consistent since it could be mutated during > > the memcpy(), but it will be consistent throughout the execution of the > > function, so we won't get calls like folio_entire_mapcount() aborting > > due to the folio having become a tail page halfway through. > > I'm afraid this problem can still happen if the snapshot is inconsistent in > the first place and you e.g. snapshot the tail page as tail page, and the > supposed folio head page as not head page. Oh, there would have to be an explicit check that the head page pointed to is actually a head page, and further that it really is the head page of this page. But the advantage is that we can check that we have a consistent snapshot _once_ rather than in every helper that we use in this function. The question is what to do if we have an inconsistent snapshot. The answer we usually use (eg in filemap, gup, etc) is to try again until we get a consistent answer. But I think that's the wrong answer here; if we get an inconsistent snapshot, we should give up. Whatever information might have been gleaned from the head page is now gone, because we don't have the original head page any more. So we should just dump the page as a non-compound page. > The local copying still makes a lot of sense though, as anything checked on > the copy that determines its further evaluation can be trusted to remain > consistent without complicated piecemeal READ_ONCE()'s etc. And as you > mentioned it will allow us to proceed with constification, where the > possibility of dump_page() through VM_BUG_ON_PAGE() is IIRC a big blocker > for constifying the various flags checking helpers etc. Yes, that's a nice side-benefit ;-)