On 2018年01月16日 21:47, Nikolay Borisov wrote: > > > On 15.01.2018 08:13, Qu Wenruo wrote: >> Enospc_debug makes extent allocator to print more debug messages, >> however for chunk allocation, there is no debug message for enospc_debug >> at all. >> >> This patch will add message for the following parts of chunk allocator: >> >> 1) No rw device at all >> Quite rare, but at least output one message for this case. >> >> 2) No enough space for some device >> This debug message is quite handy for unbalanced disks with stripe >> based profiles (RAID0/10/5/6). >> >> 3) Not enough free devices >> This debug message should tell us if current chunk allocator is >> working correctly on minimal device requirement. >> >> Although under most case, we will hit other ENOSPC before we even hit a >> chunk allocator ENOSPC, but such debug info won't help. >> >> Signed-off-by: Qu Wenruo <[email protected]> >> --- >> fs/btrfs/volumes.c | 19 +++++++++++++++++-- >> 1 file changed, 17 insertions(+), 2 deletions(-) >> >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >> index a25684287501..664d8a1b90b3 100644 >> --- a/fs/btrfs/volumes.c >> +++ b/fs/btrfs/volumes.c >> @@ -4622,8 +4622,11 @@ static int __btrfs_alloc_chunk(struct >> btrfs_trans_handle *trans, >> >> BUG_ON(!alloc_profile_is_valid(type, 0)); >> >> - if (list_empty(&fs_devices->alloc_list)) >> + if (list_empty(&fs_devices->alloc_list)) { >> + if (btrfs_test_opt(info, ENOSPC_DEBUG)) >> + btrfs_warn(info, "%s: No writable device", __func__); > > perhaps this shouldn't be gated on ENOSPC_DEBUG if it's a warning, or if > it's to be gated then make it a DEBUG.
Because the case of no writeable device is rare.
But change it to debug seems good.
>
>> return -ENOSPC;
>> + }
>>
>> index = __get_raid_index(type);
>>
>> @@ -4705,8 +4708,14 @@ static int __btrfs_alloc_chunk(struct
>> btrfs_trans_handle *trans,
>> if (ret == 0)
>> max_avail = max_stripe_size * dev_stripes;
>>
>> - if (max_avail < BTRFS_STRIPE_LEN * dev_stripes)
>> + if (max_avail < BTRFS_STRIPE_LEN * dev_stripes) {
>> + if (btrfs_test_opt(info, ENOSPC_DEBUG))
>> + btrfs_debug(info,
>> + "%s: devid %llu has no free space, have=%llu want=%u",
>> + __func__, device->devid, max_avail,
>> + BTRFS_STRIPE_LEN * dev_stripes);
>
> Here we have a debug output gated on ENOSCP_DEBUG so let's be consistent
> (hence my previous comment)
>> continue;
>> + }
>>
>> if (ndevs == fs_devices->rw_devices) {
>> WARN(1, "%s: found more than %llu devices\n",
>> @@ -4731,6 +4740,12 @@ static int __btrfs_alloc_chunk(struct
>> btrfs_trans_handle *trans,
>>
>> if (ndevs < devs_increment * sub_stripes || ndevs < devs_min) {
>> ret = -ENOSPC;
>> + if (btrfs_test_opt(info, ENOSPC_DEBUG)) {
>> + btrfs_debug(info,
>> + "%s: not enough devices with free space: have=%d minimal=%d
>> increment=%d",
>> + __func__, ndevs, devs_min,
>> + devs_increment * sub_stripes);
>
> Without looking at the code it's not really obvious what increment is.
> Perhaps you can use a more descriptive word?
"increment" is indeed less meaningful.
I'll change it to only output "minimal" just min(minimal, devs_min).
Thanks,
Qu
>
>> + }
>> goto error;
>> }
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
signature.asc
Description: OpenPGP digital signature
