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 374ECC30653 for ; Tue, 25 Jun 2024 08:54:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 99A8C6B02F3; Tue, 25 Jun 2024 04:54:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 94A2A6B02F4; Tue, 25 Jun 2024 04:54:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7C3036B02F5; Tue, 25 Jun 2024 04:54:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 5B2446B02F3 for ; Tue, 25 Jun 2024 04:54:17 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id EC0D080D1B for ; Tue, 25 Jun 2024 08:54:16 +0000 (UTC) X-FDA: 82268799312.05.8189518 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131]) by imf21.hostedemail.com (Postfix) with ESMTP id 963621C000C for ; Tue, 25 Jun 2024 08:54:13 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=v+9AZcbR; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=XlTDAOuP; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=v+9AZcbR; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=XlTDAOuP; dmarc=none; spf=pass (imf21.hostedemail.com: domain of jack@suse.cz designates 195.135.223.131 as permitted sender) smtp.mailfrom=jack@suse.cz ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719305643; a=rsa-sha256; cv=none; b=jZXcKchu3YCPCjO6jfGyzvbvDhRN+IOo+YiGH7IQt3pdumxf+R0sCnuawUQcI25kYSI6ty 1kPVa0dtzysNSALrvpkpO7nEVCfycMJ8apkbBtgLkiWuEt2Bz9nZUv1ObrfaJe6bhL+bPk SPV5+UMpZvF8eB09N7fhgvZpM5pRyHY= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=v+9AZcbR; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=XlTDAOuP; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=v+9AZcbR; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=XlTDAOuP; dmarc=none; spf=pass (imf21.hostedemail.com: domain of jack@suse.cz designates 195.135.223.131 as permitted sender) smtp.mailfrom=jack@suse.cz ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1719305643; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=dLtP8yR3zI88QJSRwPEXeVNKhmP5aSeQ9B2p9bauRMs=; b=6/sT77noRLDayLccDX66JadnpPN9yfTB2H9DUP/3vat0xIbPu6p+zdSOI0AbgNp18hhwhs bbHXrQ9j5uLQKh01W0dqgkn+qB8wr5l+ryWEswVQ/UuxhY4MIr0rttEvrbCm626NVqE+ob IdU0vMXJaYJZuySesIopo07SATj6KrY= Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id A4BF21F7D3; Tue, 25 Jun 2024 08:54:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1719305651; h=from:from:reply-to: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=dLtP8yR3zI88QJSRwPEXeVNKhmP5aSeQ9B2p9bauRMs=; b=v+9AZcbRcR8uiec0HN3k6Y0VXfau6dUarUZKm0eztY6ICrhDKEXvi0gFnhaOTj/kmhXand X0udk7xlTvoG+fspyLFnfyTtVds7gDnb/cCVyLsFsU6N0Y8lsQYFgFpWQgygQOs7W2odvs Kne8ZrFsttU/ibd1EI+/nvjaoq8/mxQ= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1719305651; h=from:from:reply-to: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=dLtP8yR3zI88QJSRwPEXeVNKhmP5aSeQ9B2p9bauRMs=; b=XlTDAOuP69AJePLAGJjIrATe2D+NxYp4uKvFLH162ooRJYjQwHCpzRsiegnHPmEdIwxqfz 8DhBObACrWJ0mSDw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1719305651; h=from:from:reply-to: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=dLtP8yR3zI88QJSRwPEXeVNKhmP5aSeQ9B2p9bauRMs=; b=v+9AZcbRcR8uiec0HN3k6Y0VXfau6dUarUZKm0eztY6ICrhDKEXvi0gFnhaOTj/kmhXand X0udk7xlTvoG+fspyLFnfyTtVds7gDnb/cCVyLsFsU6N0Y8lsQYFgFpWQgygQOs7W2odvs Kne8ZrFsttU/ibd1EI+/nvjaoq8/mxQ= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1719305651; h=from:from:reply-to: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=dLtP8yR3zI88QJSRwPEXeVNKhmP5aSeQ9B2p9bauRMs=; b=XlTDAOuP69AJePLAGJjIrATe2D+NxYp4uKvFLH162ooRJYjQwHCpzRsiegnHPmEdIwxqfz 8DhBObACrWJ0mSDw== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 9714F13AC1; Tue, 25 Jun 2024 08:54:11 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id FFjfJLOFemZTPAAAD6G6ig (envelope-from ); Tue, 25 Jun 2024 08:54:11 +0000 Received: by quack3.suse.cz (Postfix, from userid 1000) id 3DFACA08A4; Tue, 25 Jun 2024 10:54:11 +0200 (CEST) Date: Tue, 25 Jun 2024 10:54:11 +0200 From: Jan Kara To: Mateusz Guzik Cc: akpm@linux-foundation.org, brauner@kernel.org, viro@zeniv.linux.org.uk, jack@suse.cz, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v2] vfs: remove redundant smp_mb for thp handling in do_dentry_open Message-ID: <20240625085411.yqowxadalnha5t3f@quack3> References: <20240624085402.493630-1-mjguzik@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240624085402.493630-1-mjguzik@gmail.com> X-Rspamd-Action: no action X-Rspamd-Queue-Id: 963621C000C X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: 5pp8c8uec66cnjjrdqen1apn6uste95t X-HE-Tag: 1719305653-350945 X-HE-Meta: U2FsdGVkX19LY+Cc/T+vy8fY/0u+Zixl5uZ3xo699fTN5Ky+6CKhuhFLrXl352Mgs5bUPdDr58E256U328sYML7I/dO2fs2abzeg9kXV8/t0ncT+BMbXbzdWCuMYq5/fk0e6bB3DZSC7wV3AjkGXCCmqs/ZFqvDDGzI7Je1vz5cyGSm4FDXJAmyA/qB6E7BLgp0qNGXJ+sQ93lowrajd0/C6T0A5Pr4chrgNmNbdij1Bq4Sr+0qoLr7s/dYVEp66xRcakGkBUDETb4GKRKBwY7k4HX+ZO5eHJ5QMQMUu0gkBSd/txgwaCRQc38qwr3j8fLBNbVzsZhWuBcAZjQ8y+i3Y0VpeMYnluCt9veUSQeRp5W21Y/2LjFFTBa2g+rZ29qEBTfdkU9jYVZ3Dptc5q8cJsbMlnPlAaOYBmG5L8eJ+gG8Wga/dHaTTtEJCBYSeG9QdA4gb0Nm+15gUp5ycHe0wNabd4/kSaI7t+ppzXY2bVlfWZwiHjaFV6/KCvl13EUvANDKsJ/J+dwR/JPfKxLV5LbFp2eU+K73u2lmvIKv5R4qrNA0JgqvEFe+KbqbVOALoqHLsBAOVmrQXobe4oP5raqlnSUVObjvsvQk9rOTEQieQHqaWn29skP7rYu3md5mVhOvzJkErH7EsKP5n2i2l3BTh89p1FaurPE6hRtu/Qt02FMJtMCzkaHTn5/nAj0isd1z4rd6W7BeRsCbHky4CAgrhIwUimYGkQU8DkjHg7gVZdtD/VjSFuiT2mJayA8lkjKLSxuNLaQizboLB8SDY/cqfDNWgNw+/XrSAG5SYZFkzR7M00EBCJeycPoKW9nWoRHCveOpOaYUnGR97tDE2hguOUN8PxuoGP5DzV4OoOf+fIEW62Vgibm9NkvDZSYYBu+kQjPixiFpA1EmvycroWgfHbyXzHO7YlUuBexB7X/oqRryfUDsHfPde0fjHRI5SECT5NS7wXArz4LP 8PJravMB LdGG0El+IHvLfqAmNcyrc1D2rzfSis9QTyoAtG3pJ6CkMolDrrDsYVAvE98v962cjPhrf5LZV446t5FJvk4Vx6q0ez2gUPmRnJ4u3stblJA+O4Q4tgfQcfH/ln0y9JU1OKHXYzHyNlO0PSr6zbZiZr3lnZ87EEZ/8PUaouSy3nZ7C8Z9FRQ+ZegSGMDgpvHeN5FOOQSXrfPoiLsjmBMtifX+Hu5oVF8OTTEE0DTmJzkUSQDNfXgaNLgd1caI4kpzOSa58n1cN6T//S12nheRnU2v8eVc9kr9ta5dD6M61QlvOK6n80EDD42V6mNRuhWWTxb0E1hRRrnMl6GkuoSeXRXOZBe3en1gKh3DJsvXfLwTNTYo1QhqInuqxdYO5NLBRcVNK 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 Mon 24-06-24 10:54:02, Mateusz Guzik wrote: > opening for write performs: > > if (f->f_mode & FMODE_WRITE) { > [snip] > smp_mb(); > if (filemap_nr_thps(inode->i_mapping)) { > [snip] > } > } > > filemap_nr_thps on kernels built without CONFIG_READ_ONLY_THP_FOR > expands to 0, allowing the compiler to eliminate the entire thing, with > exception of the fence (and the branch leading there). > > So happens required synchronisation between i_writecount and nr_thps > changes is already provided by the full fence coming from > get_write_access -> atomic_inc_unless_negative, thus the smp_mb instance > above can be removed regardless of CONFIG_READ_ONLY_THP_FOR. > > While I updated commentary in places claiming to match the now-removed > fence, I did not try to patch them to act on the compile option. > > 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 Yeah, I didn't like the ifdef but this version looks good to me. Feel free to add: Reviewed-by: Jan Kara Honza > --- > > v2: > - just whack the fence instead of ifdefing > - change To recipient, the person who committed the original change is > no longer active > > fs/open.c | 9 ++++----- > mm/khugepaged.c | 10 +++++----- > 2 files changed, 9 insertions(+), 10 deletions(-) > > diff --git a/fs/open.c b/fs/open.c > index 28f2fcbebb1b..64976b6dc75f 100644 > --- a/fs/open.c > +++ b/fs/open.c > @@ -986,12 +986,11 @@ static int do_dentry_open(struct file *f, > */ > if (f->f_mode & FMODE_WRITE) { > /* > - * Paired with smp_mb() in collapse_file() to ensure nr_thps > - * is up to date and the update to i_writecount by > - * get_write_access() is visible. Ensures subsequent insertion > - * of THPs into the page cache will fail. > + * Depends on full fence from get_write_access() to synchronize > + * against collapse_file() regarding i_writecount and nr_thps > + * updates. Ensures subsequent insertion of THPs into the page > + * cache will fail. > */ > - smp_mb(); > if (filemap_nr_thps(inode->i_mapping)) { > struct address_space *mapping = inode->i_mapping; > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index 409f67a817f1..2e017585f813 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -1997,9 +1997,9 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr, > if (!is_shmem) { > filemap_nr_thps_inc(mapping); > /* > - * Paired with smp_mb() in do_dentry_open() to ensure > - * i_writecount is up to date and the update to nr_thps is > - * visible. Ensures the page cache will be truncated if the > + * Paired with the fence in do_dentry_open() -> get_write_access() > + * to ensure i_writecount is up to date and the update to nr_thps > + * is visible. Ensures the page cache will be truncated if the > * file is opened writable. > */ > smp_mb(); > @@ -2187,8 +2187,8 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr, > if (!is_shmem && result == SCAN_COPY_MC) { > filemap_nr_thps_dec(mapping); > /* > - * Paired with smp_mb() in do_dentry_open() to > - * ensure the update to nr_thps is visible. > + * Paired with the fence in do_dentry_open() -> get_write_access() > + * to ensure the update to nr_thps is visible. > */ > smp_mb(); > } > -- > 2.43.0 > -- Jan Kara SUSE Labs, CR