Messages in this thread | | | Date | Tue, 9 May 2023 13:47:20 -1000 | From | Tejun Heo <> | Subject | Re: [PATCH 08/13] btrfs: Use alloc_ordered_workqueue() to create ordered workqueues |
| |
Hello, David.
On Wed, May 10, 2023 at 01:36:20AM +0200, David Sterba wrote: ... > Yeah I think so but I'm not entierly sure. The ordering for all queues > that don't start with max_active > 1 should not be required, here the > parallelization and out of order processing is expected and serialized > or decided once the work is done. > > > > In btrfs_resize_thread_pool the workqueue_set_max_active is called > > > directly or indirectly so this can set the max_active to a user-defined > > > mount option. Could this be a problem or trigger a warning? This would > > > lead to max_active==1 + WQ_UNBOUND. > > > > That's not a problem. The only thing we need to make sure is that the > > workqueues which actually *must* be ordered use alloc_ordered_workqueue() as > > they won't be implicitly treated as ordered in the future. > > > > * The current patch converts two - fs_info->discard_ctl.discard_workers and > > scrub_workers when @is_dev_replace is set. Do they actually need to be > > ordered? > > > > * As you pointed out, fs_info->fixup_workers and > > fs_info->qgroup_rescan_workers are also currently implicitly ordered. Do > > they actually need to be ordered? > > I think all of them somehow implictly depend on the ordering. The > replace process sequentially goes over a block group and copies blocks. > > The fixup process is quite obscure and we should preserve the semantics > as much as possible. It has something to do with pages that get out of > sync with extent state without btrfs knowing and that there are more such > requests hapenning at the same time is low but once it happens it can > lead to corruptions. > > Quota rescan is in its nature also a sequential process but I think it > does not need to be ordered, it's started from higher level context like > enabling quotas or rescan but there are also calls at remount time so > this makes it less clear. > > In summary, if the ordered queue could be used then I'd recommend to do > it as the safe option.
I see. It seems rather error-prone to make workqueues implicitly ordered from btrfs_alloc_workqueue(). I'll see if I can make it explicit and keep all workqueues which are currently guaranteed to be ordered ordered.
Thanks.
-- tejun
| |