diff mbox

Btrfs: limit thread pool size when remounting

Message ID 1348555713-7518-1-git-send-email-bo.li.liu@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Liu Bo Sept. 25, 2012, 6:48 a.m. UTC
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(-)

Comments

David Sterba Sept. 25, 2012, 11:39 a.m. UTC | #1
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
Liu Bo Sept. 25, 2012, 12:07 p.m. UTC | #2
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
David Sterba Sept. 25, 2012, 12:50 p.m. UTC | #3
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
diff mbox

Patch

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);