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 5C3CBC54EED for ; Mon, 30 Jan 2023 13:50:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D324B6B0072; Mon, 30 Jan 2023 08:50:49 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id CE27E6B0073; Mon, 30 Jan 2023 08:50:49 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BAA1C6B0074; Mon, 30 Jan 2023 08:50:49 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id A86966B0072 for ; Mon, 30 Jan 2023 08:50:49 -0500 (EST) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 7E1FCAADB6 for ; Mon, 30 Jan 2023 13:50:49 +0000 (UTC) X-FDA: 80411601018.02.78C1735 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf10.hostedemail.com (Postfix) with ESMTP id E116FC0007 for ; Mon, 30 Jan 2023 13:50:46 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=f569xcH+; spf=none (imf10.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1675086647; 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=FJJhSnS74wz+vMkzuMi0pDFQajUZrCNVRR6WH9otUtY=; b=WW1ZB2boL8ba2S22hzNo0zUHbvOop6SG0o22uaOTmDoJrW+FJ1xteZ6lhnAflaGDIobBti uN4RGSZOI4z1yA42wESSMg20pZulnPsDYgk99VsAJvVcumsQFjweMIPAyEThpX1vH6c3Ra HJtjAADZDeUjCrR14l08arddXPI5uWo= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=f569xcH+; spf=none (imf10.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1675086647; a=rsa-sha256; cv=none; b=Dh63jsN7iwF6U7IXK9MgvD1A63b9XbvqXxG4p8yeqWNzJFL5zdAUBJ4kM/Zv02Kc3k97up HwLJUsXLqTSUAri4cNdYQCT3rYyj5mx3+mM9J5my3ABOADsaeftV520RKAXh7iVrHV0Q2A VE/bW3av96rS+WkJdBju8+7ErHZjfrQ= 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=FJJhSnS74wz+vMkzuMi0pDFQajUZrCNVRR6WH9otUtY=; b=f569xcH+e4Um8PG9hy8OrfmrG5 1GRZ7GlpfGYZzgCU2JihwSLvRQQenEW+R21iGN4eeDQIJB3vQaEPETHklsJUaUuQQsrm0cjOoslkf 2mlL/2e0bEXHiO14aU6PA4IG9HcUQVpJdvW2YdviFGtfD6S3f9eqUi6ZKIihxGpJdZ2XAniyL0D9W lvmGd8BpIszV5EWaGSr6xzP1s9Aks9puvkmVPD5SL3qS1uV8gm42I2O29Kkz8a8CCT7TCqEs2sIbI MyuOybUBvLb6SsBCW3A5TvUtEjmdyJK3GFp1B6GMUPiTysvdg4r6xlP510IYNYO3ZJvE6uO8uH6SH 2L3xUqfQ==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1pMUYS-00AOl8-KJ; Mon, 30 Jan 2023 13:50:40 +0000 Date: Mon, 30 Jan 2023 13:50:40 +0000 From: Matthew Wilcox To: Yin Fengwei Cc: david@redhat.com, linux-mm@kvack.org, dave.hansen@intel.com, tim.c.chen@intel.com, ying.huang@intel.com Subject: Re: [RFC PATCH 3/5] rmap: add page_add_file_rmap_range() Message-ID: References: <20230130125504.2509710-1-fengwei.yin@intel.com> <20230130125504.2509710-4-fengwei.yin@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230130125504.2509710-4-fengwei.yin@intel.com> X-Rspamd-Queue-Id: E116FC0007 X-Stat-Signature: bnmzmn6qyaw36995d593r8t5fe8dh9qo X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1675086646-397608 X-HE-Meta: U2FsdGVkX18+CTHJ0yBL21t4DUpJ1VXTtugCvvUi0+pvNJ87m6/SBUEBi6SNS6WM8QyHnIO6VsddBHzvtk9EkMEyLzPfqpyD1D0aL+APiVxc7GCd3cAwT+07ZbRel68f8F0A/dr2dZiPin0Vdy/eLJ4wXwo+CSJZXa05GBa+9Estr3SIBCAF8x84c8HAJNkQqMgZfPYxonhlSZ6ZTQNyp3zSiSBuzzC0mY4K4HqFM0h0tuoR44VknPMjEoJRpijI1AnrzYjfLMldKa3CsirHr9rufeJHtVoUIKHGUk5QU+TP0yZQ6zu/662W25b1G5HCGnJgBwyxIz9dS3rWPpexWRcx58r2vg53x+j3AGv7vx5KjQGqxv4CLbEPh+aYqguwKowiOrIZME0sYmHUaUt+4M9bMPXKB/LriEK9REiD9ybN1NYqFwEhBP68gS3scyjILkq0HL/XQSJRFrCixXG/dRIIJM7SCdrNDowBv/RFRH7w+5cEc/KSnIWfjyMIy0g/XK/on7IudgUk9JJ8c0kuPgkiGO6/0/qay0s4lVpcCbetrhkCzsvrU3SQk0acq1TBlilY23Y998JcfnvYQXO/Mp+e1vg0k1ZyQy4s2OLm2Y961KWk9zkR55DpMUpOLf6Vy4q++EPYIz+nqI90zruM8mxQqgz4Zwq1sS8BtKFlkwIb9Xis+alCMgw7ejZfBRhs/ClKkbN1EG8I1TmHY64/A1i+CMRmJvOxXvVXHq6hHaDxfeBz7FQTF+pWrnyLrtnJFttiyovN3L8ZW8f817Fr5tB7Xq+Kw2AH61MaxEwjrY/BD8YuJ5Ex0lzMNipjFAgbEXl0fzAT9yTJXGGQP9M9kZqwqhI7z1B0/ftffNd2zs7E69B0484Eecaf5cDLbKlqHB4sE3r+87UAn0B+g24pglTpA1K0LXUKFdgGykzDAFIohNuGtNscdIF/jyR03hRr7lbJH7BFHY/VbRyx8qn qdngPSGM UfjreFrSs7GrIBhbPX5Zq04C0Opc3DHIMY89SHpmrd6IxZVCKpBXK8qhuwu8ed8UgbxljTcjcKJ1zsiEX4xDvHj3P8pAtEEQFgy1ogzf2m5NYf18JoE4qVtTtLH+TYKsN4p9hf8emkUUSvT2bgn5YyoSPK2ILfylVMoN18pVNmJx3CyfFq0GgqKVE3JLsXO8jfaY3Ic3k90iTGCg= 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 Mon, Jan 30, 2023 at 08:55:02PM +0800, Yin Fengwei wrote: > +++ b/include/linux/rmap.h > @@ -198,6 +198,8 @@ void folio_add_new_anon_rmap(struct folio *, struct vm_area_struct *, > unsigned long address); > void page_add_file_rmap(struct page *, struct vm_area_struct *, > bool compound); > +void page_add_file_rmap_range(struct folio *, struct page *, int len, > + struct vm_area_struct *, bool compound); Again, two tabs please. > -/** > - * page_add_file_rmap - add pte mapping to a file page > - * @page: the page to add the mapping to > - * @vma: the vm area in which the mapping is added > - * @compound: charge the page as compound or small page > - * > - * The caller needs to hold the pte lock. > - */ We really should have kernel-doc for this function. > -void page_add_file_rmap(struct page *page, struct vm_area_struct *vma, > - bool compound) > +void page_add_file_rmap_range(struct folio *folio, struct page *page, int len, > + struct vm_area_struct *vma, bool compound) Indentation > { > - struct folio *folio = page_folio(page); > atomic_t *mapped = &folio->_nr_pages_mapped; > - int nr = 0, nr_pmdmapped = 0; > + int nr = 0, nr_pmdmapped = 0, nr_pages = folio_nr_pages(folio); Should be unsigned. Also the 'len' parameter should be called nr_pages, and then do something like: if (nr_pages > folio_nr_pages(folio)) nr_pages = folio_nr_pages(folio); > bool first; > > - VM_BUG_ON_PAGE(compound && !PageTransHuge(page), page); > - > - /* Is page being mapped by PTE? Is this its first map to be added? */ > - if (likely(!compound)) { > - first = atomic_inc_and_test(&page->_mapcount); > - nr = first; > - if (first && folio_test_large(folio)) { > - nr = atomic_inc_return_relaxed(mapped); > - nr = (nr < COMPOUND_MAPPED); > - } > - } else if (folio_test_pmd_mappable(folio)) { > - /* That test is redundant: it's for safety or to optimize out */ > - > + if (compound) { You've dropped the 'unlikely' marker here. Also I'm not sure why you switched the order of these two cases around; makes it harder to review. > first = atomic_inc_and_test(&folio->_entire_mapcount); > if (first) { > nr = atomic_add_return_relaxed(COMPOUND_MAPPED, mapped); > if (likely(nr < COMPOUND_MAPPED + COMPOUND_MAPPED)) { > - nr_pmdmapped = folio_nr_pages(folio); > + nr_pmdmapped = nr_pages; > nr = nr_pmdmapped - (nr & FOLIO_PAGES_MAPPED); > /* Raced ahead of a remove and another add? */ > if (unlikely(nr < 0)) > @@ -1344,6 +1323,20 @@ void page_add_file_rmap(struct page *page, struct vm_area_struct *vma, > nr = 0; > } > } > + } else { > + int i = 0, new_mapped = 0, count; > + > + count = min_t(int, len, nr_pages - folio_page_idx(folio, page)); > + do { > + first = atomic_inc_and_test(&page->_mapcount); > + new_mapped = first; > + if (first && folio_test_large(folio)) { > + new_mapped = atomic_inc_return_relaxed(mapped); > + new_mapped = (new_mapped < COMPOUND_MAPPED); > + } > + if (new_mapped) Space between if and '('. I'm not quite sure why you want to pass 'page' in rather than the offset of the page within the folio, but perhaps that will become obvious as I read along.