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 AB27CC46CA2 for ; Tue, 19 Dec 2023 08:41:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 29E338D000F; Tue, 19 Dec 2023 03:41:03 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 24F5D8D0005; Tue, 19 Dec 2023 03:41:03 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 117A78D000F; Tue, 19 Dec 2023 03:41:03 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 023788D0005 for ; Tue, 19 Dec 2023 03:41:03 -0500 (EST) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id CD86D14015C for ; Tue, 19 Dec 2023 08:41:02 +0000 (UTC) X-FDA: 81582922764.23.4F0FBD2 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf10.hostedemail.com (Postfix) with ESMTP id C9714C0017 for ; Tue, 19 Dec 2023 08:41:00 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf10.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702975261; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=VSL+YtfVYtwkAkc5QGBBJM9PylwCr0wv5FYeit7R8Vg=; b=Y1XKkGc/t+aOhpFZBEiO10zu2W6mhZOHiolRpU0x14xTh6FAE3HW6gSt9OC6ZvOFpEYMUz GyE97Uu4W9aaUdhKkUyUlxZVhILs/K2AMKps6IVAtW/VQrpxxOgAyvaQQjV4Bc9T/FGZSK Z3CWja2HZ2suYDVp69Q68TbrU146IT0= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf10.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702975261; a=rsa-sha256; cv=none; b=ts2LqtpTOqX2YBB+N1KIiGFuwsYcG7szfAFgFDjxgxlaXdNFyBgywCRGXR5/g+vH9GgVER ZeJ/9zwwcattSGwW7XOJSOC4EzLGExwMuaWV1FvmYLMH36bZ8DrOUU4Fc4WkzrxWKbX0aF 1Kv3MY322ahPmTcgFTFmZutsJFBf3Jo= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id BA2AE1FB; Tue, 19 Dec 2023 00:41:43 -0800 (PST) Received: from [10.57.75.230] (unknown [10.57.75.230]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 117D13F738; Tue, 19 Dec 2023 00:40:57 -0800 (PST) Message-ID: <6ceb7a49-0039-4faf-a069-7d47d74790f6@arm.com> Date: Tue, 19 Dec 2023 08:40:56 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 13/39] mm/rmap: factor out adding folio mappings into __folio_add_rmap() Content-Language: en-GB To: David Hildenbrand , linux-kernel@vger.kernel.org Cc: linux-mm@kvack.org, Andrew Morton , "Matthew Wilcox (Oracle)" , Hugh Dickins , Yin Fengwei , Mike Kravetz , Muchun Song , Peter Xu References: <20231211155652.131054-1-david@redhat.com> <20231211155652.131054-14-david@redhat.com> From: Ryan Roberts In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: C9714C0017 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: y63yn145g5w9bgswmdu46u3ntjrarygt X-HE-Tag: 1702975260-292034 X-HE-Meta: U2FsdGVkX18E/4g3f8XtOQvdK2C6YiyZVm5BVr3GVFZdOvteS691ZcI8sSLSEsQOssuEchRsS2yvPLt4YcKw+tiCTq77R3R3ZN1yAPOUc8FGNTJ3mNSjQCgnVZSXdnS4dQLw6132EEy3iepkTV0+cLy/USJ3s4zp+KKY2MJdc9A2HVAypKqIvu0LY9XSTqnLyK64YpDc/Ad8FPE9xdjw7DVl9kIdStW6sIV5dbuNPuuA7jo1fswSeYfbXiVZO2o3CSAXuVqt39+5U3GwjgZ5vgT5H5Jy0kKvkJZ8DAkkHGNmD3Bg3nOy97v9zdAMJYeTJ3Agadm/Xsnd1iSXFvKFe8p9FaSjlNKU6z/du8ox5uTtlxWu6+FnGy3ROYQ2GBvRZ/2R8cXfaPrp7Yqdp9bgiH3OmI+VatCspOhJYAIGIXOi0V/CG+NauC9AyAWpfNNFUsFUf0ltim5n9Rcr5n5UWVKUarGsyX0DFZKObw/GVAe7KhEyRFIRnNCFmnRAezKs+tvp9vRsBpTcNz8t73rYysnBVp7Xkoc7g8EUqz/YUUjTcc2ionVPbZQqcpB72oVQf8LBVYnDD3qT1yc0u/qFLntD6TMUV8ZoK+qYxVRe8WWpQVGVeNGQxtm4Fbxa8G8yvCBoNLZNPJa7kH51VTC+oAwoqyuYbzpp5dMNL9KY6TKTVneZHzJESH86D0vXMOgkb53WMULSowrQIToQIYPEtL0PNZQzuiDTM241W29SedXr/z0D9mluINb6pPpXwcfbB380rii5iaSYreLKzlnRIFfd6h4G6fdhSE9nrT/B6swHiTMGA9sumD8JDW91Lep4zk8cUE0iP1OhsJYdaWONhwrkB0q05VUo5jmh1aHtF+DY7+irD3+W+bbNdpp+Z2w/VwOMggJrQlfTSKv9Bmt+DLo76QWYNcR/ZVxP/8Zcwqu2ros8H+Ff5X2JrgDvH+EC3gj7NNydGmysAvKCGDx MLmcmTXr E/QQTr2FacmykhhPd7ur4wYUR1BejjONaEOFOmre3VtCH3TTuCndKr+u8aglLunn5fbvip0bNUKSri1+a+mAWFcPPaGwKp3NsmotAFDZLw8277vslry/uzkhISQx4Ellb8UQjgNDU0Mb3FIbF1g3liF/2sFIrW3u4/4le+6/8joIy0j+HFkTuqOgaf09t9XVZ2yi67G/GHmmymu6UAAAVQPCch/xaDzbCNlyFIjY78o9UvuOoKt4jgJhMMQ== 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: List-Subscribe: List-Unsubscribe: On 18/12/2023 17:06, David Hildenbrand wrote: > On 18.12.23 17:07, Ryan Roberts wrote: >> On 11/12/2023 15:56, David Hildenbrand wrote: >>> Let's factor it out to prepare for reuse as we convert >>> page_add_anon_rmap() to folio_add_anon_rmap_[pte|ptes|pmd](). >>> >>> Make the compiler always special-case on the granularity by using >>> __always_inline. >>> >>> Reviewed-by: Yin Fengwei >>> Signed-off-by: David Hildenbrand >>> --- >>>   mm/rmap.c | 81 ++++++++++++++++++++++++++++++------------------------- >>>   1 file changed, 45 insertions(+), 36 deletions(-) >>> >>> diff --git a/mm/rmap.c b/mm/rmap.c >>> index 2ff2f11275e5..c5761986a411 100644 >>> --- a/mm/rmap.c >>> +++ b/mm/rmap.c >>> @@ -1157,6 +1157,49 @@ int folio_total_mapcount(struct folio *folio) >>>       return mapcount; >>>   } >>>   +static __always_inline unsigned int __folio_add_rmap(struct folio *folio, >>> +        struct page *page, int nr_pages, enum rmap_mode mode, >>> +        unsigned int *nr_pmdmapped) >>> +{ >>> +    atomic_t *mapped = &folio->_nr_pages_mapped; >>> +    int first, nr = 0; >>> + >>> +    __folio_rmap_sanity_checks(folio, page, nr_pages, mode); >>> + >>> +    /* Is page being mapped by PTE? Is this its first map to be added? */ >> >> I suspect this comment is left over from the old version? It sounds a bit odd in >> its new context. > > In this patch, I'm just moving the code, so it would have to be dropped in a > previous patch. > > I'm happy to drop all these comments in previous patches. Well it doesn't really mean much to me in this new context, so I would reword if there is still something you need to convey to the reader, else just remove. > >> >>> +    switch (mode) { >>> +    case RMAP_MODE_PTE: >>> +        do { >>> +            first = atomic_inc_and_test(&page->_mapcount); >>> +            if (first && folio_test_large(folio)) { >>> +                first = atomic_inc_return_relaxed(mapped); >>> +                first = (first < COMPOUND_MAPPED); >>> +            } >>> + >>> +            if (first) >>> +                nr++; >>> +        } while (page++, --nr_pages > 0); >>> +        break; >>> +    case RMAP_MODE_PMD: >>> +        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 = *nr_pmdmapped - (nr & FOLIO_PAGES_MAPPED); >>> +                /* Raced ahead of a remove and another add? */ >>> +                if (unlikely(nr < 0)) >>> +                    nr = 0; >>> +            } else { >>> +                /* Raced ahead of a remove of COMPOUND_MAPPED */ >>> +                nr = 0; >>> +            } >>> +        } >>> +        break; >>> +    } >>> +    return nr; >>> +} >>> + >>>   /** >>>    * folio_move_anon_rmap - move a folio to our anon_vma >>>    * @folio:    The folio to move to our anon_vma >>> @@ -1380,45 +1423,11 @@ static __always_inline void >>> __folio_add_file_rmap(struct folio *folio, >>>           struct page *page, int nr_pages, struct vm_area_struct *vma, >>>           enum rmap_mode mode) >>>   { >>> -    atomic_t *mapped = &folio->_nr_pages_mapped; >>> -    unsigned int nr_pmdmapped = 0, first; >>> -    int nr = 0; >>> +    unsigned int nr, nr_pmdmapped = 0; >> >> You're still being inconsistent with signed/unsigned here. Is there a reason >> these can't be signed like nr_pages in the interface? > > I can turn them into signed values. > > Personally, I think it's misleading to use "signed" for values that have > absolutely no meaning for negative meaning. But sure, we can be consistent, at > least in rmap code. > Well it's an easy way to detect overflow? But I know what you mean. There are lots of other APIs that accept signed/unsigned 32/64 bits; It's a mess. It would be a tiny step in the right direction if a series could at least be consistent with itself though, IMHO. :)