Date:   Tue, 28 Feb 2023 15:01:06 +0700
From:   Ammar Faizi <ammarfaizi2@gnuweeb.org>
To:     Dave Chinner <david@fromorbit.com>
Cc:     Chris Mason <clm@fb.com>, Josef Bacik <josef@toxicpanda.com>,
        David Sterba <dsterba@suse.com>, Tejun Heo <tj@kernel.org>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        Filipe Manana <fdmanana@suse.com>,
        Linux Btrfs Mailing List <linux-btrfs@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux Fsdevel Mailing List <linux-fsdevel@vger.kernel.org>,
        GNU/Weeb Mailing List <gwml@vger.gnuweeb.org>
Subject: Re: [RFC PATCH v1 0/6] Introducing `wq_cpu_set` mount option for
 btrfs
Message-ID: <Y/20wsdwxx8OSw/+@biznet-home.integral.gnuweeb.org>
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>
Precedence: bulk

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