Arne Jansen <sensille@xxxxxxx> writes:
> + */
> + mutex_lock(&fs_info->scrub_lock);
> + atomic_inc(&fs_info->scrubs_running);
> + mutex_unlock(&fs_info->scrub_lock);
It seems odd to protect an atomic_inc with a mutex.
Is that done for some side effect? Otherwise you either
don't need atomic or don't need the lock.
That seems to be all over the source file.
> +int btrfs_scrub_pause(struct btrfs_root *root)
> +{
> + struct btrfs_fs_info *fs_info = root->fs_info;
> + mutex_lock(&fs_info->scrub_lock);
As I understand it you take that mutex on every transaction
commit, which is a fast path for normal IO.
For me that looks like a scalability problem with enough
cores. Did you do any performance testing of this on a system
with a reasonable number of cores?
btrfs already has enough scalability problems, please don't
add new ones.
-Andi
--
ak@xxxxxxxxxxxxxxx -- Speaking for myself only
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
