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 CF51EC2BD05 for ; Mon, 24 Jun 2024 08:17:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6038C6B040F; Mon, 24 Jun 2024 04:17:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 58B4C6B0411; Mon, 24 Jun 2024 04:17:24 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 406136B0412; Mon, 24 Jun 2024 04:17:24 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 1FA1A6B040F for ; Mon, 24 Jun 2024 04:17:24 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 938F7120CB1 for ; Mon, 24 Jun 2024 08:17:23 +0000 (UTC) X-FDA: 82265077566.04.C45A137 Received: from mail-ed1-f44.google.com (mail-ed1-f44.google.com [209.85.208.44]) by imf28.hostedemail.com (Postfix) with ESMTP id 9D064C0002 for ; Mon, 24 Jun 2024 08:17:21 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=WFVNXe6u; spf=pass (imf28.hostedemail.com: domain of mjguzik@gmail.com designates 209.85.208.44 as permitted sender) smtp.mailfrom=mjguzik@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1719217028; 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=fQqbHSkqEg8UIT4DFzvDECtddkt78IAjLyY6LQO60XA=; b=k0en6vsWVHzrDhl9Q3By4xFc+0ci9yDOAuZXaf/rlsbchVF4ks5U9BzynfhtYg+HSWz3KD tx08qnNdQ6vEqutLzKGzEYecfkSFz6G8of/a2puIxE1/UEGNH7ooWE0JD3NOv7v+M/3j2K zr/LkIvrUFSpleZ8vCHFXOLxOcb5p00= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719217028; a=rsa-sha256; cv=none; b=7Ag2osP0gnp1enR11YuIJIQcDOU2UD3rwvMzFXkzA59aOx1rYv5+qk6C97AxjuESmVfP9x zWe299qJxZG+zcT/N/7ErUoiIm2DIp9LC/EYuXsy/eyk1WMnrf3tM7AIAjP/o7kRCmf2Ks f3/EgXq7A8avP3GWGNJ4Fh9VfaPg1bU= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=WFVNXe6u; spf=pass (imf28.hostedemail.com: domain of mjguzik@gmail.com designates 209.85.208.44 as permitted sender) smtp.mailfrom=mjguzik@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-ed1-f44.google.com with SMTP id 4fb4d7f45d1cf-57cbc66a0a6so3318183a12.1 for ; Mon, 24 Jun 2024 01:17:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1719217040; x=1719821840; darn=kvack.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=fQqbHSkqEg8UIT4DFzvDECtddkt78IAjLyY6LQO60XA=; b=WFVNXe6uJGwpBWHU8u/WHjhS/07/6lHNQIkPgTd98mntojO4oBSFjx93R/F8a3CsEu jhcKDoeG7X2kzKnYhm0rUic9/zBA/P5yD2mrHFVPeCN75SGimXGAfDP8ylqGXPzYnPkF i5GXBxNTbpSFR8rp0vwQrhvxgcUzfjM3mT8DSzmCbAHb/f1AJVNT1G+rogG15BiWZIUj vQqrkpnJCBp7/ujJYY2MeX37fW7vlsQxt3uoY/RW6ve+DB8oxB5IE4Xx6eOIYvRDhBUp AmvcspfzB5TN/5yl8ieh50Hl7TWt00LaH+EqxaAj28JndzmkFbhA7+K8gIB45s6YL3HR mkyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719217040; x=1719821840; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=fQqbHSkqEg8UIT4DFzvDECtddkt78IAjLyY6LQO60XA=; b=dOttAx7veXwxW5YE4LCBIuEs1miuY+3T93cxztxAGYcD5asFCVi0vB6tiD6E2lRCC4 VhhAy5enUxM/DT+cpCGgShYMXGQKOAe3oOVxKLUgzAJRxySFQFXPVjYqYQg2O8sLfaj4 rWRhY7uUCUaECKrBEXGHv0fZ9mbvNyUHo0POLaGEpRYpyxtF/FX8smTYo9enEoIzmgwT CCt9QpGQwBEI/xaEtzyUzSApuSVhwuHrYsOmxHQYhQV6gBM98HbKzlwVUbHlPhR46UQC rIk0mm+v8rW2SzHbuLvxxn9csYPiwUEld0PdlRH94nCm5M9l8pNn/8Q/LNVpMMPOLDvj ZmiQ== X-Gm-Message-State: AOJu0YzNyoiXcwS575QpDfwdEbXouR7wUwgyjyUscP4odo7S5PuikAYl YbAZ3P2iWgSX9FmArjIVNBUaYZQA8jsk/GshW+e8ct7MciRULSs4xI4qtbSM X-Google-Smtp-Source: AGHT+IEgDXbhNOqfnHDw/RFFKZaSiw6LDw+WjOtm/3URZplVmEWC705lBJRJnwjprOQFPpv/eurX5g== X-Received: by 2002:a50:8d58:0:b0:57c:70b4:7ac with SMTP id 4fb4d7f45d1cf-57d44c41970mr3855799a12.15.1719217039737; Mon, 24 Jun 2024 01:17:19 -0700 (PDT) Received: from f (cst-prg-81-171.cust.vodafone.cz. [46.135.81.171]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-57d30427f8fsm4471791a12.34.2024.06.24.01.17.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Jun 2024 01:17:19 -0700 (PDT) Date: Mon, 24 Jun 2024 10:17:12 +0200 From: Mateusz Guzik To: linux-mm@kvack.org Cc: brauner@kernel.org, viro@zeniv.linux.org.uk, jack@suse.cz, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: [RFC PATCH] vfs: wrap CONFIG_READ_ONLY_THP_FOR_FS-related code with an ifdef Message-ID: References: <20240624074135.486845-1-mjguzik@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20240624074135.486845-1-mjguzik@gmail.com> X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 9D064C0002 X-Stat-Signature: ukpj7zkxbx6x1m3z6barzhys1u8hftbc X-HE-Tag: 1719217041-915680 X-HE-Meta: U2FsdGVkX18aCB4C4JpnXcNuLqoQj6cxh5dfhoLCH7h9erK0OC2qBKg8P1+D972Mr/lM2IO9jP4f76OD12cBxMlqFk3Xgoo4zpqC5ydx7auGWMZJxDUGUAsitgXYmstcgxQJ1DQm5hk/0HsPZGO2llErlWHBPAbZc8Q/1fIEoxxIBh2ZBIt26Ioslv129k7qyrZb9NUg99/UDuvoEHR4aDbtAhAmdsd6M9RTJCfh0wHIss/0ZlWmNWXF4UKt8B3TtQtn51zU/SQ5pM2G8EDm3LwFiNdpEMTlD3RTovmd5gQvm5sUW8k7FBI4EkYQR1q8CgCtQ7ujSGbGPmyhbIbei85YYGY1zEWitkK7Rn75mpVMd/lEULRGhKuWETrPi3a6iw8eoDjuIV2sUqkZVeLWJ8edGsAT9aq8OMDqGF/TPMoE7ipaZ3qNgcpiizTYoeZfUXKThdsFq9SYWzgKAm7+yuEVgq/TWtVEO/ODoqYG/mYEzvfW1o1FNw0f6Yxzez6M6DOBwBs76Chfisz8NCLqt/+puKU8njeJQNn0U3IU1I0l1XQfrRAm18YgASgEwi4YyFdpopScqbAcQs8RjxToTRv8o82CckiWkvKGX9gc8R1gTdgvMKLEXzXMxrN+Z78Boguo8RiNrxSI6g9VhiXn5slbmww1KlY/i3pIj6WOAEbY0z2c3ffQAucjfGytwSWN/DmAS6+TMTQOMMQlYF0haH2MQOhk0zF2TB38ZbxBs/BDXnAIosc1DmyWTZcqjery/8rqtzJzFXl6jTpSw7N+/ncoL0/cUqNY6D5NuGwqHlnF8Xsq6t0J9A47ljj39EWpyEMR02f6mlesFenZjFKfMMWMh+nvggDnB72qkqoOJQhgCLYeminJGVquiloiqTbmiiVUHhZktguCA9XFvTXzLp/nSuauyrjBUr2f6fTp0AomOxM/vW74Y0YNrFr3XZiCIJi6JBkiWdO4aas5FO0 O3kE1l2r ldpfC4vr3yRkDfRpzjqClOHnlrzTai2ljqGSVQ+kv/htgyOJa1v419BrTEV43m4D8WfCV8ageNQBQpDYg0UzV4N95aImIMYDA3MicbxbhEqfnCpr6kMViVBHOvY5sCopvJlgaXTA6Z+2Tju2mNxm2jo28LEP+5QRuPgPYT3JsonZHvXC7LDnaqmMGNzalwG9hP5GvrwBF0zzb31ttNXFcFa+NaAYb+Hjd0/E2Tnu1pv64X2pfp1hXprUMeKvTB4lBYq/JLPQ5ihUtRUyopNF8bzwh22fp5j9shzzBe0hf8E8AySqHt7ChYTUOL3f/r/dKy9r9hq5EwcGpvoMFetPunyhRrF0jCRypldRb+KLJuIBuhKtHpA/UtAtzQ1MSwv0RbHXe+HHuoetFYUNIs93h/bIFRzxjSaCXYXyEkrg2Op9qvna0vs1P8J32Vb1qwhPDkiGD2R+YJPIylpk57tvH+UfMcw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000005, 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 Mon, Jun 24, 2024 at 09:41:34AM +0200, Mateusz Guzik wrote: > On kernels compiled without this option (which is currently the default > state) filemap_nr_thps expands to 0. > > do_dentry_open has a big chunk dependent on it, most of which gets > optimized away, except for a branch and a full fence: > > if (f->f_mode & FMODE_WRITE) { > [snip] > smp_mb(); > if (filemap_nr_thps(inode->i_mapping)) { > [snip] > } > } > > While the branch is pretty minor the fence really does not need to be > there. > [the To: recipient bounces, thus got dropped] Now that I sent this I remembered that some of the atomic ops in Linux provide a full fence regardless of an architecture. get_write_access uses atomic_inc_unless_negative which qualifies? 1551 static __always_inline bool 1552 atomic_inc_unless_negative(atomic_t *v) 1553 { 1554 │ kcsan_mb(); 1555 │ instrument_atomic_read_write(v, sizeof(*v)); 1556 │ return raw_atomic_inc_unless_negative(v); 1557 } If so, the ifdefed-out smp_mb can be eliminated altogether if I got my fences right. On top of that mnt_get_write_access injects another smp_mb() anyway. > This is a bare-minimum patch which takes care of it until someone(tm) > cleans this up. Notably it does not conditionally compile other spots > which issue the matching fence. > > I did not bother benchmarking it, not issuing a spurious full fence in > the fast path does not warrant justification from perf standpoint. > > Signed-off-by: Mateusz Guzik > --- > > I am not particularly familiar with any of this, the smp_mb in the open > for write path was sticking out like a sore thumb on code read so I > figured there may be One Weird Trick to whack it. > > If the stock code is correct as is, then the ifdef as above is fine. > > The ifdefed chunk is big enough that it should probably be its own > routine. I don't want to bikeshed so I did not go for it. > > For a moment I considered adding filemap_nr_thps_mb which would expand > to 0 or issue the fence + do the read, but then I figured a routine > claiming to post a fence and only conditionally do it is misleading at > best. > > As per the commit message fences in collapse_file remain compiled in. > It is unclear to me if the code following them is doing anything useful > on kernels !CONFIG_READ_ONLY_THP_FOR_FS. > > All that said, if there is cosmetic touch ups you want done here, I can > do them. > > However, a nice full patch would take care of all of the above and I > have neither the information needed to do it nor the interest to get it, > so should someone insinst on a full version I'm going to suggest they > write it themselves. I repeat this is merely a damage control until > someone sorts thigs out. > > fs/open.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/open.c b/fs/open.c > index 28f2fcbebb1b..654c300b3c33 100644 > --- a/fs/open.c > +++ b/fs/open.c > @@ -980,6 +980,7 @@ static int do_dentry_open(struct file *f, > if ((f->f_flags & O_DIRECT) && !(f->f_mode & FMODE_CAN_ODIRECT)) > return -EINVAL; > > +#ifdef CONFIG_READ_ONLY_THP_FOR_FS > /* > * XXX: Huge page cache doesn't support writing yet. Drop all page > * cache for this file before processing writes. > @@ -1007,6 +1008,7 @@ static int do_dentry_open(struct file *f, > filemap_invalidate_unlock(inode->i_mapping); > } > } > +#endif > > return 0; > > -- > 2.43.0 >