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 80542C61DA4 for ; Thu, 9 Mar 2023 22:40:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2B384280002; Thu, 9 Mar 2023 17:40:14 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 263F26B007B; Thu, 9 Mar 2023 17:40:14 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 12CD4280002; Thu, 9 Mar 2023 17:40:14 -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 047C76B0078 for ; Thu, 9 Mar 2023 17:40:14 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id CA324A0F71 for ; Thu, 9 Mar 2023 22:40:13 +0000 (UTC) X-FDA: 80550829506.26.73D0F5F Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com [209.85.167.44]) by imf28.hostedemail.com (Postfix) with ESMTP id E8EF8C0004 for ; Thu, 9 Mar 2023 22:40:11 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=DkAi09Pu; spf=pass (imf28.hostedemail.com: domain of axelrasmussen@google.com designates 209.85.167.44 as permitted sender) smtp.mailfrom=axelrasmussen@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1678401612; 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:dkim-signature; bh=QbAikeIdL+omb6y11NKt1vr7KucGsxizbqoLplhFCoo=; b=nw+qQLmnI5uym3MqHruI3jj43br1+SrLH1yipbskvevo79YzifNg3SUShWQp6Xwj77EljI H1OgtZVlSZbw4VZWlq+lllEadmx7yFCWI5PL86q0sKSK2zo4Jssr3qCE9TdIxvHy2R99c4 UFlk5ig5SUvCDLMidUrCS6B4SJAS0q0= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=DkAi09Pu; spf=pass (imf28.hostedemail.com: domain of axelrasmussen@google.com designates 209.85.167.44 as permitted sender) smtp.mailfrom=axelrasmussen@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1678401612; a=rsa-sha256; cv=none; b=ACKyndVyNvoS4V7Tvlgm38RIh9CC8HVltwGufDJfoK1EPdyIka1+QwTcEG8MVZUQ2lofml ZHSE1Q9qpv8Y2lLm68FAtlJh+4GvhuFb2nds7UYXSBuU8WYRPHU0Op3lhJKw1rXgJZWBVj PAQCKa5/WAZb1fedtwvLhpUtpPuw1FA= Received: by mail-lf1-f44.google.com with SMTP id g17so4290926lfv.4 for ; Thu, 09 Mar 2023 14:40:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1678401610; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=QbAikeIdL+omb6y11NKt1vr7KucGsxizbqoLplhFCoo=; b=DkAi09PuPVTp0hxHpLSz4t87TRBgGLDjZ9W4eKqEWOosoh7NEt2ji+IrV4y/BdgcY9 skLeabsKW4jeG2GzdbXYXwyO+MsKjGqXldrPeDdFRmTSd/uFUS8ec7Bn2+AjYHuCaESx weJLsbQ1qHUJ5RAtAOYTNaviRtR1X+0QPCY9OhbKxhB5nDmp5wOWw4UBRzy/bMgQNQBr dPgZGHl4IojX9goLytD6PJVC92GxsOh2TfNXEjmmQDB3qTrtDDjxUPQM2+DUAJiE1LLR YFdepkGF4YX3sbCfx6WYuiNAH/RneZ17WAP7RBd1anbeQX0C5MJ+HkeqWIRf3rAB6pZx /djw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678401610; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=QbAikeIdL+omb6y11NKt1vr7KucGsxizbqoLplhFCoo=; b=UlL5fmc0cYoBxCNMPWGFWyB50GKnGLgDqbWJgbVSOdGzCrnbLvqOX3rZhFhsk3EoBi a3NcBchJyEQaa5T07iRestSdE3YwoOGCu0mELp71RPO/zlBu7OxeyWfnqdnAh0+2H0Rq UmtsRBLNs9G5y1rlla8Wb9LjqXJSrYE/pLZZB5FevM7S3lfss9IDKUNxFowYQSbUqXV2 3sy7+frRQQkAtsMEEfjVnSm7vOFpPPxvuLVtc/nF3GOWVY6qQmGADuKhn2f+EsLd+L/z fXJ6QH2Sh0iDBjx1LqMyNtymqHu2ojockiAJj4fuuELUEAmfnBZxSuMJUD4MYuHLMxLa OpzA== X-Gm-Message-State: AO0yUKVWSOIwRsOHmKdng0upimtoMBA6u0sngLWft0pGqGkivSPeFck6 VwH5Yf0zRClFqQF9CDa5GHJ/XBc0e7XaEKBTHV+A3Q== X-Google-Smtp-Source: AK7set9wPQ7n+O+qozP441kYgMXEWRqd09XR/uf9pCcC+fLMi9lyfYb5jxqy2HTgbAi1e6AGJJ6CFWpaMh2S7PfTWlM= X-Received: by 2002:a05:6512:3c99:b0:4d8:86c2:75ea with SMTP id h25-20020a0565123c9900b004d886c275eamr149481lfv.3.1678401609928; Thu, 09 Mar 2023 14:40:09 -0800 (PST) MIME-Version: 1.0 References: <20230308221932.1548827-1-axelrasmussen@google.com> <20230308221932.1548827-4-axelrasmussen@google.com> In-Reply-To: From: Axel Rasmussen Date: Thu, 9 Mar 2023 14:39:33 -0800 Message-ID: Subject: Re: [PATCH v4 3/4] mm: userfaultfd: combine 'mode' and 'wp_copy' arguments To: Peter Xu Cc: Alexander Viro , Andrew Morton , Hugh Dickins , Jan Kara , "Liam R. Howlett" , Matthew Wilcox , Mike Kravetz , Mike Rapoport , Muchun Song , Nadav Amit , Shuah Khan , James Houghton , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: rk3xtxr4exhg38c4m667q15qj3qx14wc X-Rspam-User: X-Rspamd-Queue-Id: E8EF8C0004 X-Rspamd-Server: rspam06 X-HE-Tag: 1678401611-978087 X-HE-Meta: U2FsdGVkX1+dtcbFm4TQV6QG3ZkaQ3pzxaBPkNdj/5OJ2mDLaPDF87U2iEWgTK18b/bLdwEYRUgFHZidGioNBqvfG+hX2RbNwt6rzjywZ8X0R5M9aQuvfnZsLkXlJQSb37bfo2yIkHC809ZCT2XiGEATF/IxtZ7L4iEWETHklsQwcoXTBbthY+zh6YBpN2dEzb+ZVwwgMiIoRVyZcnJxljqk2Y9zTCSIPHqV6dXZogAoz31LOboXe79nz32EGj1H3khOdJZBsXswFLxphh3CspwqsfM9/lGUIOp0eEaDMwLrQTjbyQxqpRZn5KWL9DtMWBYqhnSfw84MirGEtKh1YgMuVKpLCs7HWwrO6efFYga+jaaRRw799+cN+cvDBTX8lToaWutqeOs1eel8Lu27YMgkt8vF/PI0FoZjO3E2laey537Tr2rjrg+guc0/M2A44QKK3lomMx3R7iJD2bQrBExQEI8uC1RiQLbVC9dDVJHx3APqqbfq6RtFJxL4sg/MenPXUppCH1iyesfk8zFvwjawv+zOruCsap78TPR+i3/K/Lll3BEMnQ2FJ8O9SV6oO7SaXURHWMKq1GPxbBv3fumgUNLAGyNS4JAtRsCaWmPJ3NGsItQHx72XhsWCTqbUbGXbwiLhqMxXhb4QD1ycokPawYzgm7fy5Y6Tzg+HbscP7iph7LemC/wVnbrlP6WoDEx48BSS4nbaF/DV9TMOu4zd1x1IJ4vd8Q506IguL1z5WxN/VZ40+mIQRXaUMuejibR8maiTlwklRKJWQpzZzNdVNy0TCYHUWhOTtlG1hu7QPoYNZybSqAhWqCO90XxEoYcnah9LySkjCPCdNbw8/rZkTgcule+v6mlQJg4Py+gyiea6FB8K1jURx56SePcztAJ6R10S3J5sdZv+U+0FvtDNUuIcgleg8q3UnSOc9ph3aNtWbb0YGVHgx6dE0QqVkAMw0f/b4rV57ivJmyy b3j10Q74 hbmLT/Kwez7EW9wpzqz9Lhz5ao6l84BRvOLRKHY8OjQTdKA1H6PE0zmVMTi3296XDfH2fK0kVPUZmOTU2BazhH113Ov2y4a0Vg2IqRbCwio7gdHx/K0m6q/PeW88edfD+d8qwOx0iEKmEdqh5EmviruekZfX3OImBC3W1Ok4ugTF0OSAClnToI3f6BaHC31UcI0K4tE6b9s7qo2Su6v+pv+w69t/ChZGnwQNOmilY1DQk2G2hiOYWcFAdF01+zVUClo2GLpRzzfYfJv8Z1VxhAEwadMLg6hmIOsDT8mT1JvEoB1IoMlhx+d6EomW2NH8xK0/A 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 Wed, Mar 8, 2023 at 2:43=E2=80=AFPM Peter Xu wrote: > > All nitpicks below. > > On Wed, Mar 08, 2023 at 02:19:31PM -0800, Axel Rasmussen wrote: > > +static inline bool uffd_flags_has_mode(uffd_flags_t flags, enum mfill_= atomic_mode expected) > > +{ > > + return (flags & MFILL_ATOMIC_MODE_MASK) =3D=3D ((__force uffd_fla= gs_t) expected); > > +} > > I would still call it uffd_flags_get_mode() or uffd_flags_mode(), "has" > sounds a bit like there can be >1 modes set but it's not. I want a helper which does the comparison, instead of just returning the mode, because it avoids all callers needing to do the __force cast themselves to appease sparse. How about uffd_flags_mode_is() ? > > > + > > +static inline uffd_flags_t uffd_flags_set_mode(uffd_flags_t flags, enu= m mfill_atomic_mode mode) > > +{ > > + return flags | ((__force uffd_flags_t) mode); > > +} > > IIUC this __force mostly won't work in any way because it protects > e.g. illegal math ops upon it (to only allow bitops, iiuc) but here it's = an > OR so it's always legal.. > > So I'd just drop it and also clear the mode mask to be very clear it sets > the mode right, rather than any chance of messing up when set twice: > > flags &=3D ~MFILL_ATOMIC_MODE_MASK; > return flags | mode; Without this __force, "make C=3D1" gives errors like this: ./include/linux/userfaultfd_k.h:66:16: warning: restricted uffd_flags_t degrades to integer ./include/linux/userfaultfd_k.h:66:22: warning: incorrect type in return expression (different base types) ./include/linux/userfaultfd_k.h:66:22: expected restricted uffd_flags_t ./include/linux/userfaultfd_k.h:66:22: got unsigned int This is because the mode being passed in is effectively an integer, so the | expression loses the restricted type. Casting the mode first like this appeases sparse. An alternative would be to do the cast in the definition of the mode values up-front; but as we noticed before, we can't really usefully do that with it still being an enum (so we'd have to hard-code things like the mode mask, etc.) I do completely agree about clearing the mask bits first, to avoid mistakes. I'll send an updated version with that change. If we're going to have an inline helper anyway to do that, for me it makes less sense to switch away from the num approach (basically the benefit of that would be to avoid needing this cast, and therefore the helper; but if we want the helper anyway for other reasons ...). > > But feel free to ignore this if there's no other reason to repost, I don'= t > think it matters a huge deal. > > Acked-by: Peter Xu > > Thanks, > > -- > Peter Xu >