Date:   Tue, 28 Feb 2023 09:17:45 +1100
From:   Dave Chinner <david@fromorbit.com>
To:     Ammar Faizi <ammarfaizi2@gnuweeb.org>
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: <20230227221745.GI2825702@dread.disaster.area>
References: <20230226160259.18354-1-ammarfaizi2@gnuweeb.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230226160259.18354-1-ammarfaizi2@gnuweeb.org>
Precedence: bulk

On Sun, Feb 26, 2023 at 11:02:53PM +0700, Ammar Faizi wrote:
> ## Test wq_cpu_set
> sudo mount -t btrfs -o rw,compress-force=zstd:15,commit=1500,wq_cpu_set=0.4.1.5 /dev/sda2 hdd/a;
> cp -rf /path/folder_with_many_large_files/ hdd/a/test;
> sync; # See the CPU usage in htop.
> sudo umount hdd/a;

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....

-Dave.
-- 
Dave Chinner
david@fromorbit.com