Btrfs devel - Sep 2012 - [PATCH] Btrfs: limit thread pool size when remounting

For some asynchronous threads, such as submit worker and cache worker, we
limit their thread pool size when mounting.

So we also need to do such things when remounting.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 fs/btrfs/super.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 83d6f9f..a58e834 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1158,17 +1158,20 @@ static void btrfs_resize_thread_pool(struct
btrfs_fs_info *fs_info,
 	printk(KERN_INFO "btrfs: resize thread pool %d -> %d\n",
 	       old_pool_size, new_pool_size);
 
-	btrfs_set_max_workers(&fs_info->generic_worker, new_pool_size);
+	btrfs_set_max_workers(&fs_info->generic_worker, min(1, new_pool_size));
 	btrfs_set_max_workers(&fs_info->workers, new_pool_size);
 	btrfs_set_max_workers(&fs_info->delalloc_workers, new_pool_size);
-	btrfs_set_max_workers(&fs_info->submit_workers, new_pool_size);
-	btrfs_set_max_workers(&fs_info->caching_workers, new_pool_size);
-	btrfs_set_max_workers(&fs_info->fixup_workers, new_pool_size);
+	btrfs_set_max_workers(&fs_info->submit_workers,
+			      min_t(u64, fs_info->fs_devices->num_devices,
+			      new_pool_size));
+	btrfs_set_max_workers(&fs_info->caching_workers, min(2,
new_pool_size));
+	btrfs_set_max_workers(&fs_info->fixup_workers, min(1, new_pool_size));
 	btrfs_set_max_workers(&fs_info->endio_workers, new_pool_size);
 	btrfs_set_max_workers(&fs_info->endio_meta_workers, new_pool_size);
 	btrfs_set_max_workers(&fs_info->endio_meta_write_workers,
new_pool_size);
 	btrfs_set_max_workers(&fs_info->endio_write_workers, new_pool_size);
-	btrfs_set_max_workers(&fs_info->endio_freespace_worker, new_pool_size);
+	btrfs_set_max_workers(&fs_info->endio_freespace_worker,
+			      min(1, new_pool_size));
 	btrfs_set_max_workers(&fs_info->delayed_workers, new_pool_size);
 	btrfs_set_max_workers(&fs_info->readahead_workers, new_pool_size);
 	btrfs_set_max_workers(&fs_info->scrub_workers, new_pool_size);
-- 
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, Sep 25, 2012 at 02:48:33PM +0800, Liu Bo wrote:
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1158,17 +1158,20 @@ static void btrfs_resize_thread_pool(struct
btrfs_fs_info *fs_info,
>  	printk(KERN_INFO "btrfs: resize thread pool %d -> %d\n",
>  	       old_pool_size, new_pool_size);
>  
> -	btrfs_set_max_workers(&fs_info->generic_worker, new_pool_size);
> +	btrfs_set_max_workers(&fs_info->generic_worker, min(1,
new_pool_size));
How could new_pool_size be < 1 ?

There''s a check in super.c to pick only values > 0
>  	btrfs_set_max_workers(&fs_info->workers, new_pool_size);
>  	btrfs_set_max_workers(&fs_info->delalloc_workers, new_pool_size);
> -	btrfs_set_max_workers(&fs_info->submit_workers, new_pool_size);
> -	btrfs_set_max_workers(&fs_info->caching_workers, new_pool_size);
> -	btrfs_set_max_workers(&fs_info->fixup_workers, new_pool_size);
> +	btrfs_set_max_workers(&fs_info->submit_workers,
> +			      min_t(u64, fs_info->fs_devices->num_devices,
> +			      new_pool_size));
This ask for update also when a new device is added/removed.
> +	btrfs_set_max_workers(&fs_info->caching_workers, min(2,
new_pool_size));
> +	btrfs_set_max_workers(&fs_info->fixup_workers, min(1,
new_pool_size));
Same as above, is it expected to be < 1 ?
>  	btrfs_set_max_workers(&fs_info->endio_workers, new_pool_size);
>  	btrfs_set_max_workers(&fs_info->endio_meta_workers,
new_pool_size);
>  	btrfs_set_max_workers(&fs_info->endio_meta_write_workers,
new_pool_size);
>  	btrfs_set_max_workers(&fs_info->endio_write_workers,
new_pool_size);
> -	btrfs_set_max_workers(&fs_info->endio_freespace_worker,
new_pool_size);
> +	btrfs_set_max_workers(&fs_info->endio_freespace_worker,
> +			      min(1, new_pool_size));
Not sure, do we actually need more than 1 free space worker?
>  	btrfs_set_max_workers(&fs_info->delayed_workers, new_pool_size);
>  	btrfs_set_max_workers(&fs_info->readahead_workers, new_pool_size);
>  	btrfs_set_max_workers(&fs_info->scrub_workers, new_pool_size);
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 09/25/2012 07:39 PM, David Sterba wrote:
> On Tue, Sep 25, 2012 at 02:48:33PM +0800, Liu Bo wrote:
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -1158,17 +1158,20 @@ static void btrfs_resize_thread_pool(struct
btrfs_fs_info *fs_info,
>>  	printk(KERN_INFO "btrfs: resize thread pool %d -> %d\n",
>>  	       old_pool_size, new_pool_size);
>>  
>> -	btrfs_set_max_workers(&fs_info->generic_worker,
new_pool_size);
>> +	btrfs_set_max_workers(&fs_info->generic_worker, min(1,
new_pool_size));
> 
> How could new_pool_size be < 1 ?
> 
> There''s a check in super.c to pick only values > 0
> 
I think we just need only 1 generic_worker
>>  	btrfs_set_max_workers(&fs_info->workers, new_pool_size);
>>  	btrfs_set_max_workers(&fs_info->delalloc_workers,
new_pool_size);
>> -	btrfs_set_max_workers(&fs_info->submit_workers,
new_pool_size);
>> -	btrfs_set_max_workers(&fs_info->caching_workers,
new_pool_size);
>> -	btrfs_set_max_workers(&fs_info->fixup_workers, new_pool_size);
>> +	btrfs_set_max_workers(&fs_info->submit_workers,
>> +			      min_t(u64, fs_info->fs_devices->num_devices,
>> +			      new_pool_size));
> 
> This ask for update also when a new device is added/removed.
> 
Oh, yes, but we should do it in another new patch instead.
>> +	btrfs_set_max_workers(&fs_info->caching_workers, min(2,
new_pool_size));
>> +	btrfs_set_max_workers(&fs_info->fixup_workers, min(1,
new_pool_size));
> 
> Same as above, is it expected to be < 1 ?
> 
>>  	btrfs_set_max_workers(&fs_info->endio_workers, new_pool_size);
>>  	btrfs_set_max_workers(&fs_info->endio_meta_workers,
new_pool_size);
>>  	btrfs_set_max_workers(&fs_info->endio_meta_write_workers,
new_pool_size);
>>  	btrfs_set_max_workers(&fs_info->endio_write_workers,
new_pool_size);
>> -	btrfs_set_max_workers(&fs_info->endio_freespace_worker,
new_pool_size);
>> +	btrfs_set_max_workers(&fs_info->endio_freespace_worker,
>> +			      min(1, new_pool_size));
> 
> Not sure, do we actually need more than 1 free space worker?
> 
Same as generic_worker and fixup_workers, I think only one is enough,
that'' why I make
the minimum limitation, or we can set it as 1 directly.


thanks,
liubo
>>  	btrfs_set_max_workers(&fs_info->delayed_workers,
new_pool_size);
>>  	btrfs_set_max_workers(&fs_info->readahead_workers,
new_pool_size);
>>  	btrfs_set_max_workers(&fs_info->scrub_workers, new_pool_size);
> --
> To unsubscribe from this list: send the line "unsubscribe
linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, Sep 25, 2012 at 08:07:00PM +0800, Liu Bo wrote:
> On 09/25/2012 07:39 PM, David Sterba wrote:
> > On Tue, Sep 25, 2012 at 02:48:33PM +0800, Liu Bo wrote:
> >> --- a/fs/btrfs/super.c
> >> +++ b/fs/btrfs/super.c
> >> @@ -1158,17 +1158,20 @@ static void
btrfs_resize_thread_pool(struct btrfs_fs_info *fs_info,
> >>  	printk(KERN_INFO "btrfs: resize thread pool %d ->
%d\n",
> >>  	       old_pool_size, new_pool_size);
> >>  
> >> -	btrfs_set_max_workers(&fs_info->generic_worker,
new_pool_size);
> >> +	btrfs_set_max_workers(&fs_info->generic_worker, min(1,
new_pool_size));
> > 
> > How could new_pool_size be < 1 ?
> > 
> > There''s a check in super.c to pick only values > 0
> 
> I think we just need only 1 generic_worker
I got it reversed, sorry for the noise.
> >>  	btrfs_set_max_workers(&fs_info->workers, new_pool_size);
> >>  	btrfs_set_max_workers(&fs_info->delalloc_workers,
new_pool_size);
> >> -	btrfs_set_max_workers(&fs_info->submit_workers,
new_pool_size);
> >> -	btrfs_set_max_workers(&fs_info->caching_workers,
new_pool_size);
> >> -	btrfs_set_max_workers(&fs_info->fixup_workers,
new_pool_size);
> >> +	btrfs_set_max_workers(&fs_info->submit_workers,
> >> +			      min_t(u64, fs_info->fs_devices->num_devices,
> >> +			      new_pool_size));
> > 
> > This ask for update also when a new device is added/removed.
> > 
> 
> Oh, yes, but we should do it in another new patch instead.
Yes, this was just a hint to pair the remount/thread_pool capabilities.

david
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html