From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on gnuweeb.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=ALL_TRUSTED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NO_DNS_FOR_FROM,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.6 Received: from biznet-home.integral.gnuweeb.org (unknown [182.2.36.16]) by gnuweeb.org (Postfix) with ESMTPSA id F29B8831DA; Tue, 28 Feb 2023 08:01:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gnuweeb.org; s=default; t=1677571274; bh=xUpQ0Xo2NE8oaRu79qOTI/d+TDpkm3C5IO6ZS1rJgEw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=dskrUgXVrYPdXEFvsHlrNEygCz/KmoC03D5Tb7TNWTqmySshek9lolSNCkO8xRedE 857OfpJoLbd++j7EhouXB0iQK9CTBZWFjHWapttlPz1F1Wp6mlchToG+9sIeNvrsuu JGj7oskiL3zEvKr4QGl/6GnuUckRZURfdGE9QmbDk1Xnjj6dIKcLdcYRjVVDVIfkWy R4TXtGFd6Ge01i9y7HtiSNzkW+0RN5QV5PHdVtffucBTsB3pB3xtouIOW411lajrLw QhRUn2vbIvR1WNFi7jtAkQ9D5t+5YdcBv+4ogD8ESiMCSL3MM3kf3dqVIagGrLqYXv XcZTWMflGaTqQ== Date: Tue, 28 Feb 2023 15:01:06 +0700 From: Ammar Faizi To: Dave Chinner Cc: Chris Mason , Josef Bacik , David Sterba , Tejun Heo , Lai Jiangshan , Filipe Manana , Linux Btrfs Mailing List , Linux Kernel Mailing List , Linux Fsdevel Mailing List , GNU/Weeb Mailing List Subject: Re: [RFC PATCH v1 0/6] Introducing `wq_cpu_set` mount option for btrfs Message-ID: References: <20230226160259.18354-1-ammarfaizi2@gnuweeb.org> <20230227221745.GI2825702@dread.disaster.area> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230227221745.GI2825702@dread.disaster.area> X-Bpl: hUx9VaHkTWcLO7S8CQCslj6OzqBx2hfLChRz45nPESx5VSB/xuJQVOKOB1zSXE3yc9ntP27bV1M1 List-Id: On Tue, Feb 28, 2023 at 09:17:45AM +1100, Dave Chinner wrote: > This seems like the wrong model for setting cpu locality for > internal filesystem threads. > > Users are used to controlling cpu sets and other locality behaviour > of a task with wrapper tools like numactl. Wrap th emount command > with a numactl command to limit the CPU set, then have the btrfs > fill_super() callback set the cpu mask for the work queues it > creates based on the cpu mask that has been set for the mount task. > > That is, I think the model should be "inherit cpu mask from parent > task" rather than adding mount options. This model allows anything > that numactl can control (e.g. memory locality) to also influence > the filesystem default behaviour without having to add yet more > mount options in the future.... Good idea on the tooling part. I like the idea of using numactl to determine a proper CPU set. But users may also use /etc/fstab to mount their btrfs storage. In that case, using mount option is still handy. Also, if we always inherit CPU mask from the parent task who calls the mount, it will be breaking the CPU affinity for old users who inadvertently call their mount cmd with random CPU mask. We should keep the old behavior and allow user to opt in if they want to. Maybe something like: numactl -N 0 mount -t btrfs -o rw,wq_cpu_set=inherit /dev/bla bla -- Ammar Faizi