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 18669C433F5 for ; Fri, 3 Dec 2021 04:21:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0FE326B0072; Thu, 2 Dec 2021 23:21:38 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0ADF06B0074; Thu, 2 Dec 2021 23:21:38 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E6A456B0075; Thu, 2 Dec 2021 23:21:37 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0199.hostedemail.com [216.40.44.199]) by kanga.kvack.org (Postfix) with ESMTP id D664B6B0072 for ; Thu, 2 Dec 2021 23:21:37 -0500 (EST) Received: from smtpin18.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 8E0B1853EF for ; Fri, 3 Dec 2021 04:21:27 +0000 (UTC) X-FDA: 78875183814.18.D9AA935 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf24.hostedemail.com (Postfix) with ESMTP id E0882B0000A1 for ; Fri, 3 Dec 2021 04:21:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1638505286; 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=U0DvXToWkjNdJ/hviORYDrqU29CNpTdKjgszLzYXKNA=; b=T6TLeF8kbuuBZlEYmrD1PJMhqul/oOunuh4dt9/Q3J8wPi3klfKm8yNdMI76dHp5GyKy26 De8qnMTL1jlK2p242uSXJUK5WZM3Sff8GDUzOoi4Ut0N7wLQar5lPspx7+LC+uuDszwAu4 AgcGHr6EGj1+PsWvdwd8g3m5RdTCqU8= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-146-xp8dyl1sMbWWaAVMMCLi8g-1; Thu, 02 Dec 2021 23:21:23 -0500 X-MC-Unique: xp8dyl1sMbWWaAVMMCLi8g-1 Received: by mail-wm1-f72.google.com with SMTP id c8-20020a7bc848000000b0033bf856f0easo2794985wml.1 for ; Thu, 02 Dec 2021 20:21:22 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=U0DvXToWkjNdJ/hviORYDrqU29CNpTdKjgszLzYXKNA=; b=tnhl5MXpHAEMR8HX5TfqbjVzNTIHNSV9EP9V5/olsv1BjMztQ4yyBTi7ttcIswVULR uyc960QD+kmQCLtR/m0HWwIkGggbqO+fxSLhg0LNB7tNPvRYLhgJpbpTRL1dAjUk5XW0 gpMu+pxXXSpWv3+uXWltGnfKl4vM87STrshPOV9/rG0kK0c29Jg9j6bww0olGdlB4J8V Z7V+FkNlqaS+4rRFzP1kShR2uQEPhYog9FP9DC2R7tijb2qHmw39/87gvCK0AoT0q6VD U5YsthLgxsRIdnGLxfL1eRN1B/c4G3nDmud3SX+80VAFpnwENaVYmHv+jCp80j64o52v HFWA== X-Gm-Message-State: AOAM533V9u5M4rERnAXADmHQ26n4K/kPcWvNPgtX6c9l2BOoZSz011TY H7GPMRPYG1BeRSn73cOFgmnn/3TBkdwbILodrwVFuSaeKqRshApF7kwwCOolL++FPl8qALRRRrn 7ahixfW1EdQ4= X-Received: by 2002:a7b:c017:: with SMTP id c23mr11778194wmb.137.1638505281712; Thu, 02 Dec 2021 20:21:21 -0800 (PST) X-Google-Smtp-Source: ABdhPJylxWV4mpD2tDaOEiuslxoimzq4PYi2YQm8Z/wgHD2G7olVBGDTmTAVQuBSN9IENsBZdAI1cw== X-Received: by 2002:a7b:c017:: with SMTP id c23mr11778152wmb.137.1638505281489; Thu, 02 Dec 2021 20:21:21 -0800 (PST) Received: from xz-m1.local ([64.64.123.26]) by smtp.gmail.com with ESMTPSA id u13sm4241484wmq.14.2021.12.02.20.21.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Dec 2021 20:21:20 -0800 (PST) Date: Fri, 3 Dec 2021 12:21:12 +0800 From: Peter Xu To: Alistair Popple Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Axel Rasmussen , Nadav Amit , Mike Rapoport , Hugh Dickins , Mike Kravetz , "Kirill A . Shutemov" , Jerome Glisse , Matthew Wilcox , Andrew Morton , David Hildenbrand , Andrea Arcangeli Subject: Re: [PATCH v6 01/23] mm: Introduce PTE_MARKER swap entry Message-ID: References: <20211115075522.73795-1-peterx@redhat.com> <20211115075522.73795-2-peterx@redhat.com> <11462319.U46FXHIEPT@nvdebian> MIME-Version: 1.0 In-Reply-To: <11462319.U46FXHIEPT@nvdebian> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-Stat-Signature: jmjus5k4e6xujecx7bixqqbno4p1m95b Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=T6TLeF8k; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf24.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: rspam06 X-Rspamd-Queue-Id: E0882B0000A1 X-HE-Tag: 1638505286-178484 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 Fri, Dec 03, 2021 at 02:30:00PM +1100, Alistair Popple wrote: > On Monday, 15 November 2021 6:55:00 PM AEDT Peter Xu wrote: > > [...] > > > diff --git a/include/linux/swapops.h b/include/linux/swapops.h > > index d356ab4047f7..5103d2a4ae38 100644 > > --- a/include/linux/swapops.h > > +++ b/include/linux/swapops.h > > @@ -247,6 +247,84 @@ static inline int is_writable_migration_entry(swp_entry_t entry) > > > > #endif > > > > +typedef unsigned long pte_marker; > > + > > +#define PTE_MARKER_MASK (0) > > + > > +#ifdef CONFIG_PTE_MARKER > > + > > +static inline swp_entry_t make_pte_marker_entry(pte_marker marker) > > +{ > > + return swp_entry(SWP_PTE_MARKER, marker); > > +} > > + > > +static inline bool is_pte_marker_entry(swp_entry_t entry) > > +{ > > + return swp_type(entry) == SWP_PTE_MARKER; > > +} > > + > > +static inline pte_marker pte_marker_get(swp_entry_t entry) > > +{ > > + return swp_offset(entry) & PTE_MARKER_MASK; > > I'm not sure the PTE_MARKER_MASK adds much, especially as we only have one > user. I don't see a problem with open-coding these kind of checks (ie. It's more or less a safety belt to make sure anything pte_marker_get() returned will be pte_marker defined bits only. > swp_offset(entry) & PTE_MARKER_UFFD_WP) as you kind of end up doing that anyway. > Alternatively if you want helper functions I think it would be better to define > them for each marker. Eg: is_pte_marker_uffd_wp(). Yes we can have something like is_pte_marker_uffd_wp(), I didn't do that explicitly because I want us to be clear that pte_marker is a bitmask, so calling "is_*" will be slightly opaque - strictly speaking it should be "pte_marker_has_uffd_wp_bit()" if there will be more bits defined, but then the name of the helper will look a bit odd too. Hence I just keep the only interface to fetch the whole marker and use "&" in the call sites to check. > > > +} > > + > > +static inline bool is_pte_marker(pte_t pte) > > +{ > > + return is_swap_pte(pte) && is_pte_marker_entry(pte_to_swp_entry(pte)); > > +} > > + > > +#else /* CONFIG_PTE_MARKER */ > > + > > +static inline swp_entry_t make_pte_marker_entry(pte_marker marker) > > +{ > > + /* This should never be called if !CONFIG_PTE_MARKER */ > > Can we leave this function undefined then? That way we will get an obvious > build error. We can, but then we need more macros to cover the common code. E.g. currently in hugetlb_change_protection() we have: /* None pte */ if (unlikely(uffd_wp)) /* Safe to modify directly (none->non-present). */ set_huge_pte_at(mm, address, ptep, make_pte_marker(PTE_MARKER_UFFD_WP)); If we drop this definition, to let it compile with !PTE_MARKER, we'll need: +#ifdef PTE_MARKER /* None pte */ if (unlikely(uffd_wp)) /* Safe to modify directly (none->non-present). */ set_huge_pte_at(mm, address, ptep, make_pte_marker(PTE_MARKER_UFFD_WP)); +#endif Comparing to adding macro checks over a few other places, I figured maybe it's easier to define them in the header once then we proper WARN_ON_ONCE() if triggered (while they should just never). > > Overall I'm liking the swap entry approach a lot more than the special pte > approach, but maybe that's just because I'm more familiar with special swap > entries :-) Swap entry solution is definitely cleaner to me if not considering wasting it with one bit. Operating on pte directly is actually slightly more challenging, because we don't have the protection of is_swap_pte() anymore. It can help shield out quite some strange stuff due to the pte->swp level hierachy. Thanks, -- Peter Xu