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=-16.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham 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 5636BC4743C for ; Mon, 21 Jun 2021 16:16:59 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id D3CFE611BD for ; Mon, 21 Jun 2021 16:16:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D3CFE611BD Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 6E73A6B007D; Mon, 21 Jun 2021 12:16:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 696B76B007E; Mon, 21 Jun 2021 12:16:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4C2716B0080; Mon, 21 Jun 2021 12:16:58 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0150.hostedemail.com [216.40.44.150]) by kanga.kvack.org (Postfix) with ESMTP id 18B006B007D for ; Mon, 21 Jun 2021 12:16:58 -0400 (EDT) Received: from smtpin17.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id A919A181AEF1E for ; Mon, 21 Jun 2021 16:16:57 +0000 (UTC) X-FDA: 78278234874.17.A73A784 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf12.hostedemail.com (Postfix) with ESMTP id F016C140E for ; Mon, 21 Jun 2021 16:16:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1624292216; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=1lxNM0QUCMLo1E6bAwNt8D/Sfkwf585U1gKeMbqY/jk=; b=G0k6yHAcPcNWrnKD1yOfTBePJyko1XzU9cmrureKrWyrheShSahkecnAsuOKH8qcX6TDlC n45d1JGCiuThAEm0X+HDpFGDUrTx3PokgKtwsrsW7sSkCx+WiVi1YjV1DzkqMbI/qT5wkW xwnySK9rW55xNHGhpaFyjxSa253I9NU= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-190-xhRZRfHgP0CWIQO4Akibkg-1; Mon, 21 Jun 2021 12:16:53 -0400 X-MC-Unique: xhRZRfHgP0CWIQO4Akibkg-1 Received: by mail-qk1-f198.google.com with SMTP id b125-20020a3799830000b02903ad1e638ccaso14552035qke.4 for ; Mon, 21 Jun 2021 09:16:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=1lxNM0QUCMLo1E6bAwNt8D/Sfkwf585U1gKeMbqY/jk=; b=nApFNuArFYfOovL0koqnlZC35VzqCb4ID4h6s+2155vbGBAeqlda+qTGHH30vZ8Fox nDApmub77srjQsPJgetvzWvxDnzfyqm1s18KpafQHL7hopr/Hz7XOVcZH5dd/xttIJo0 QmAchcMeGabABbk0dA0JE8V2UpMT0kA8p18PqI5sfDeEPYjCbcwlbm8dn3ycYJbs0gG0 q58ST5uzvY3ZzdXshMKmbxU+wkngAn3akyVvK736x0GrlGufGUTCHIv+apikxiqGFwOH HvzAXWkSGI4FrEXfhbJhpUpi2hAvcibfY1s3azNUOdUfXnxKd7WTcvYQiq/1mGByLw0q t5mQ== X-Gm-Message-State: AOAM531WS4/YS9tbrtXt9LI2/e/xv0FCq/VO1TH2IpPNHxT1ZiG24pr1 0eiXuaI8TvhsJrmh38z0NBHbQrMIznaASEBDMXc670rFX0BYjpQfGkhr5z0pyR9see9kbotT0lW s5Ms+gHQMnqE= X-Received: by 2002:a05:6214:2a8a:: with SMTP id jr10mr21094084qvb.50.1624292212843; Mon, 21 Jun 2021 09:16:52 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyr8NyDm41sDPcWGbbW3nH7HmA5CPGjpBNBrdzq7EcUuinbByU3GhTpe5HQQ9rYId+u5U//BA== X-Received: by 2002:a05:6214:2a8a:: with SMTP id jr10mr21094050qvb.50.1624292212565; Mon, 21 Jun 2021 09:16:52 -0700 (PDT) Received: from t490s (bras-base-toroon474qw-grc-65-184-144-111-238.dsl.bell.ca. [184.144.111.238]) by smtp.gmail.com with ESMTPSA id f11sm10131487qka.55.2021.06.21.09.16.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Jun 2021 09:16:52 -0700 (PDT) Date: Mon, 21 Jun 2021 12:16:50 -0400 From: Peter Xu To: Alistair Popple Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrea Arcangeli , "Kirill A . Shutemov" , Axel Rasmussen , Nadav Amit , Hugh Dickins , Jerome Glisse , Jason Gunthorpe , Andrew Morton , Miaohe Lin , Mike Rapoport , Matthew Wilcox , Mike Kravetz Subject: Re: [PATCH v3 08/27] mm: Introduce zap_details.zap_flags Message-ID: References: <20210527201927.29586-1-peterx@redhat.com> <20210527202130.30840-1-peterx@redhat.com> <5845701.Ud2vPSPtVx@nvdebian> MIME-Version: 1.0 In-Reply-To: <5845701.Ud2vPSPtVx@nvdebian> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=G0k6yHAc; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf12.hostedemail.com: domain of peterx@redhat.com has no SPF policy when checking 170.10.133.124) smtp.mailfrom=peterx@redhat.com X-Rspamd-Server: rspam02 X-Stat-Signature: e3n3b9c6wmekok77xkaionhzy8w6c74d X-Rspamd-Queue-Id: F016C140E X-HE-Tag: 1624292212-992241 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, Jun 21, 2021 at 10:09:00PM +1000, Alistair Popple wrote: > On Friday, 28 May 2021 6:21:30 AM AEST Peter Xu wrote: > > Instead of trying to introduce one variable for every new zap_details fields, > > let's introduce a flag so that it can start to encode true/false informations. > > > > Let's start to use this flag first to clean up the only check_mapping variable. > > Firstly, the name "check_mapping" implies this is a "boolean", but actually it > > stores the mapping inside, just in a way that it won't be set if we don't want > > to check the mapping. > > > > To make things clearer, introduce the 1st zap flag ZAP_FLAG_CHECK_MAPPING, so > > that we only check against the mapping if this bit set. At the same time, we > > can rename check_mapping into zap_mapping and set it always. > > > > Since at it, introduce another helper zap_check_mapping_skip() and use it in > > zap_pte_range() properly. > > > > Some old comments have been removed in zap_pte_range() because they're > > duplicated, and since now we're with ZAP_FLAG_CHECK_MAPPING flag, it'll be very > > easy to grep this information by simply grepping the flag. > > > > It'll also make life easier when we want to e.g. pass in zap_flags into the > > callers like unmap_mapping_pages() (instead of adding new booleans besides the > > even_cows parameter). > > > > Signed-off-by: Peter Xu > > --- > > include/linux/mm.h | 19 ++++++++++++++++++- > > mm/memory.c | 31 ++++++++----------------------- > > 2 files changed, 26 insertions(+), 24 deletions(-) > > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > index db155be8e66c..52d3ef2ed753 100644 > > --- a/include/linux/mm.h > > +++ b/include/linux/mm.h > > @@ -1721,13 +1721,30 @@ static inline bool can_do_mlock(void) { return false; } > > extern int user_shm_lock(size_t, struct user_struct *); > > extern void user_shm_unlock(size_t, struct user_struct *); > > > > +/* Whether to check page->mapping when zapping */ > > +#define ZAP_FLAG_CHECK_MAPPING BIT(0) > > + > > /* > > * Parameter block passed down to zap_pte_range in exceptional cases. > > */ > > struct zap_details { > > - struct address_space *check_mapping; /* Check page->mapping if set */ > > + struct address_space *zap_mapping; > > + unsigned long zap_flags; > > }; > > > > +/* Return true if skip zapping this page, false otherwise */ > > +static inline bool > > +zap_check_mapping_skip(struct zap_details *details, struct page *page) > > +{ > > + if (!details || !page) > > + return false; > > + > > + if (!(details->zap_flags & ZAP_FLAG_CHECK_MAPPING)) > > + return false; [1] > > + > > + return details->zap_mapping != page_rmapping(page); > > I doubt this matters in practice, but there is a slight behaviour change > here that might be worth checking. Previously this check was equivalent > to: > > details->zap_mapping && details->zap_mapping != page_rmapping(page) Yes; IMHO "details->zap_mapping" is just replaced by the check at [1]. For example, there's only one real user of this mapping check, which is unmap_mapping_pages() below [2]. With the old code, we have: details.check_mapping = even_cows ? NULL : mapping; So "details->zap_mapping" is only true if "!even_cows". With the new code, we'll have: if (!even_cows) details.zap_flags |= ZAP_FLAG_CHECK_MAPPING; So ZAP_FLAG_CHECK_MAPPING is only set if "!even_cows", while that's what we check exactly at [1]. > > Otherwise I think this looks good. > > > +} > > + > > struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr, > > pte_t pte); > > struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr, > > diff --git a/mm/memory.c b/mm/memory.c > > index 27cf8a6375c6..c9dc4e9e05b5 100644 > > --- a/mm/memory.c > > +++ b/mm/memory.c > > @@ -1330,16 +1330,8 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb, > > struct page *page; > > > > page = vm_normal_page(vma, addr, ptent); > > - if (unlikely(details) && page) { > > - /* > > - * unmap_shared_mapping_pages() wants to > > - * invalidate cache without truncating: > > - * unmap shared but keep private pages. > > - */ > > - if (details->check_mapping && > > - details->check_mapping != page_rmapping(page)) > > - continue; > > - } > > + if (unlikely(zap_check_mapping_skip(details, page))) > > + continue; > > ptent = ptep_get_and_clear_full(mm, addr, pte, > > tlb->fullmm); > > tlb_remove_tlb_entry(tlb, pte, addr); > > @@ -1372,17 +1364,8 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb, > > is_device_exclusive_entry(entry)) { > > struct page *page = pfn_swap_entry_to_page(entry); > > > > - if (unlikely(details && details->check_mapping)) { > > - /* > > - * unmap_shared_mapping_pages() wants to > > - * invalidate cache without truncating: > > - * unmap shared but keep private pages. > > - */ > > - if (details->check_mapping != > > - page_rmapping(page)) > > - continue; > > - } > > - > > + if (unlikely(zap_check_mapping_skip(details, page))) > > + continue; > > pte_clear_not_present_full(mm, addr, pte, tlb->fullmm); > > rss[mm_counter(page)]--; > > > > @@ -3345,9 +3328,11 @@ void unmap_mapping_pages(struct address_space *mapping, pgoff_t start, > > pgoff_t nr, bool even_cows) > > { > > pgoff_t first_index = start, last_index = start + nr - 1; > > - struct zap_details details = { }; > > + struct zap_details details = { .zap_mapping = mapping }; > > + > > + if (!even_cows) > > + details.zap_flags |= ZAP_FLAG_CHECK_MAPPING; > > > > - details.check_mapping = even_cows ? NULL : mapping; [2] > > if (last_index < first_index) > > last_index = ULONG_MAX; Thanks, -- Peter Xu