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=-2.4 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no 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 B48E8C352A4 for ; Mon, 10 Feb 2020 21:33:26 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 702A52070A for ; Mon, 10 Feb 2020 21:33:26 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=nvidia.com header.i=@nvidia.com header.b="cvotvSuM" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 702A52070A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=nvidia.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id F2CEC6B017B; Mon, 10 Feb 2020 16:33:25 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id EDCAF6B017D; Mon, 10 Feb 2020 16:33:25 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DCB526B017E; Mon, 10 Feb 2020 16:33:25 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0187.hostedemail.com [216.40.44.187]) by kanga.kvack.org (Postfix) with ESMTP id C54406B017B for ; Mon, 10 Feb 2020 16:33:25 -0500 (EST) Received: from smtpin28.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 46C673A92 for ; Mon, 10 Feb 2020 21:33:25 +0000 (UTC) X-FDA: 76475518770.28.goose27_3ae3a88feb94b X-HE-Tag: goose27_3ae3a88feb94b X-Filterd-Recvd-Size: 5215 Received: from hqnvemgate26.nvidia.com (hqnvemgate26.nvidia.com [216.228.121.65]) by imf47.hostedemail.com (Postfix) with ESMTP for ; Mon, 10 Feb 2020 21:33:24 +0000 (UTC) Received: from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by hqnvemgate26.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA) id ; Mon, 10 Feb 2020 13:33:09 -0800 Received: from hqmail.nvidia.com ([172.20.161.6]) by hqpgpgate101.nvidia.com (PGP Universal service); Mon, 10 Feb 2020 13:33:23 -0800 X-PGP-Universal: processed; by hqpgpgate101.nvidia.com on Mon, 10 Feb 2020 13:33:23 -0800 Received: from [10.110.48.28] (10.124.1.5) by HQMAIL107.nvidia.com (172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Mon, 10 Feb 2020 21:33:22 +0000 Subject: Re: [PATCH] mm: Improve dump_page() for compound pages To: "Kirill A. Shutemov" CC: Matthew Wilcox , , "Kirill A . Shutemov" References: <20200208044415.30012-1-willy@infradead.org> <20200210124225.56cjwblfa7njgp5o@box> <92d69132-6664-3fc3-4743-11109df0b079@nvidia.com> <20200210212125.f24n5javxkfv6uvi@box> From: John Hubbard X-Nvconfidentiality: public Message-ID: <85d46612-b5b1-f566-7026-e4fec9166c3b@nvidia.com> Date: Mon, 10 Feb 2020 13:33:22 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.2 MIME-Version: 1.0 In-Reply-To: <20200210212125.f24n5javxkfv6uvi@box> X-Originating-IP: [10.124.1.5] X-ClientProxiedBy: HQMAIL111.nvidia.com (172.20.187.18) To HQMAIL107.nvidia.com (172.20.187.13) Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1581370389; bh=hezEqD9CBRtXslBHVR37SWi8MwDK4vBAv/3fzjGygOQ=; h=X-PGP-Universal:Subject:To:CC:References:From:X-Nvconfidentiality: Message-ID:Date:User-Agent:MIME-Version:In-Reply-To: X-Originating-IP:X-ClientProxiedBy:Content-Type:Content-Language: Content-Transfer-Encoding; b=cvotvSuM+jaN5hwqAtROVchPl/OBkRspLDLs+UHc/5OZBTRcBmlxsijNGWt7F5i+I UwVYGpzqYYWYt0AcKCt/ir6Y2Y6MicHhxXcudiYcjnDXMMLRUdZVg/7GR/13Quj/Tt wNGvGb24fBTdRe59aI+RxDWnuTw5FRSpHota4DgENNUQyjWmAKozn7n66km5n4rU7r vLPLFbixTEeI+4fdzaUrTg9QWgcOtg9c/cUwWNET56C3iKEhea7Co8phAlGwl1nSqZ lFNSgFvE6dctyP/XZUB9otBswW9EUyN3cQnEknEVL4J+/iV0geW1SY4iRyo2KnOygF Db74VStAhWjMQ== 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 2/10/20 1:21 PM, Kirill A. Shutemov wrote: > On Mon, Feb 10, 2020 at 11:50:21AM -0800, John Hubbard wrote: >> On 2/10/20 4:42 AM, Kirill A. Shutemov wrote: >> ... >>>> @@ -66,25 +68,32 @@ void __dump_page(struct page *page, const char *reason) >>>> goto hex_only; >>>> } >>>> >>>> - mapping = page_mapping(page); >>>> + if (page < head || (page >= head + MAX_ORDER_NR_PAGES)) { >>>> + /* Corrupt page, cannot call page_mapping */ >>>> + mapping = page->mapping; >>>> + head = page; >>>> + compound = false; >>>> + } else { >>>> + mapping = page_mapping(page); >>>> + } >>>> >>>> /* >>>> * Avoid VM_BUG_ON() in page_mapcount(). >>>> * page->_mapcount space in struct page is used by sl[aou]b pages to >>>> * encode own info. >>>> */ >>>> - mapcount = PageSlab(page) ? 0 : page_mapcount(page); >>>> + mapcount = PageSlab(head) ? 0 : page_mapcount(head); >>> >>> This is wrong. We want to see mapcount for the tail page, not head. >>> >> >> I see what you mean: page_mapcount(page) sums up both the page's and the >> head page's mapcount in some cases. The function doesn't seem to work >> correctly unless it is fed the tail page. > > What makes you think this? It has to be called on the page user provided. > Head or tail. Actually, we are in total agreement there, but I meant "if it is a real tail page"--in other words, I should have written: "the function doesn't seem to work correctly unless it is fed the original page. If you feed it the head page unconditionally, it breaks page_mapcounts()'s assumptions". Sorry for the confusing response. > >> Here, even though the "head" variable's meaning is overloaded (=="head page, >> unless the tail page was corrupted, in which case, tail page"), it would still be >> accurate to change that line back to the original line, so that it once again >> reads: >> >> mapcount = PageSlab(page) ? 0 : page_mapcount(page); > > PageSlab() can be called for the head page. It's not clear to me whether it's safer to use the...user-provided page, or the head page, for checking PageSlab()--it's probably set on both head and non-head pages, right? I guess you're saying it should be like this: mapcount = PageSlab(head) ? 0 : page_mapcount(page); ...yes? > >> >> Matthew? >> >> (Also, I see that __page_mapcount is EXPORT-ed, which is odd: nothing uses it >> other than page_mapcount. > > ... and page_mapcount() can be inlined anywhere. > sigh, please ignore anything I say about EXPORTS--I've been reminded of this point before, I'm afraid. :) thanks, -- John Hubbard NVIDIA