[PATCH] mkfs.btrfs: round all device sizes to sectorsize

[PATCH] mkfs.btrfs: round all device sizes to sectorsize

[Eric Sandeen]

make_btrfs() rounds down the first device size to a multiple of sectorsize:

	num_bytes = (num_bytes / sectorsize) * sectorsize;

but subsequent device adds don't.

This seems a bit odd & inconsistent, and it makes xfstest btrfs/011
_notrun(), because it explicitly checks that devices are the same size.

I don't know that there is anything inherently wrong with having
a few device bytes extend past the last block, but to be consistent,
it seems like btrfs_add_to_fsid() should round the size in the same
way.

And now btrfs/011 runs more consistently; the test devices don't
have to be sectorsize multiples in order for all mkfs'd device
sizes to match.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

ideally this might go into btrfs_device_size(), but we don't have
the chosen sector size anywhere near there...

diff --git a/utils.c b/utils.c
index e130849..4d7ee35 100644
--- a/utils.c
+++ b/utils.c
@@ -554,7 +554,7 @@ int btrfs_add_to_fsid(struct btrfs_trans_handle *trans,
 	device->sector_size = sectorsize;
 	device->fd = fd;
 	device->writeable = 1;
-	device->total_bytes = block_count;
+	device->total_bytes = (block_count / sectorsize) * sectorsize;
 	device->bytes_used = 0;
 	device->total_ios = 0;
 	device->dev_root = root->fs_info->dev_root;

Re: [PATCH] mkfs.btrfs: round all device sizes to sectorsize

[Anand Jain]

On 07/25/2014 12:27 PM, Eric Sandeen wrote:
> make_btrfs() rounds down the first device size to a multiple of sectorsize:
>
> 	num_bytes = (num_bytes / sectorsize) * sectorsize;
>
> but subsequent device adds don't.
>
> This seems a bit odd & inconsistent, and it makes xfstest btrfs/011
> _notrun(), because it explicitly checks that devices are the same size.
>
> I don't know that there is anything inherently wrong with having
> a few device bytes extend past the last block, but to be consistent,
> it seems like btrfs_add_to_fsid() should round the size in the same
> way.
>
> And now btrfs/011 runs more consistently; the test devices don't
> have to be sectorsize multiples in order for all mkfs'd device
> sizes to match.

> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> ideally this might go into btrfs_device_size(), but we don't have
> the chosen sector size anywhere near there...
>
> diff --git a/utils.c b/utils.c
> index e130849..4d7ee35 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -554,7 +554,7 @@ int btrfs_add_to_fsid(struct btrfs_trans_handle *trans,
>   	device->sector_size = sectorsize;
>   	device->fd = fd;
>   	device->writeable = 1;
> -	device->total_bytes = block_count;
> +	device->total_bytes = (block_count / sectorsize) * sectorsize;
>   	device->bytes_used = 0;
>   	device->total_ios = 0;
>   	device->dev_root = root->fs_info->dev_root;


its better to do this at the function btrfs_prepare_device() itself,
so that it would include first and the rest added.
or
we need to apply this on the block_count instead, as we do add
that to the total_bytes.

Thanks, Anand

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

Re: [PATCH] mkfs.btrfs: round all device sizes to sectorsize

[Zach Brown]

On Thu, Jul 24, 2014 at 11:27:32PM -0500, Eric Sandeen wrote:
> make_btrfs() rounds down the first device size to a multiple of sectorsize:

               ^^^^^^^^^^^

> -	device->total_bytes = block_count;
> +	device->total_bytes = (block_count / sectorsize) * sectorsize;

kerncompat.h:#define round_down(x, y) ((x) & ~__round_mask(x, y))

- z

Re: [PATCH] mkfs.btrfs: round all device sizes to sectorsize

[Eric Sandeen]

On 7/25/14, 12:12 PM, Zach Brown wrote:
> On Thu, Jul 24, 2014 at 11:27:32PM -0500, Eric Sandeen wrote:
>> make_btrfs() rounds down the first device size to a multiple of sectorsize:
> 
>                ^^^^^^^^^^^
> 
>> -	device->total_bytes = block_count;
>> +	device->total_bytes = (block_count / sectorsize) * sectorsize;
> 
> kerncompat.h:#define round_down(x, y) ((x) & ~__round_mask(x, y))
> 
> - z
> 

yeah yeah ;) this isn't copied kernel code but sure, that'd be better.

I'm trying to clean up this whole "we say blocks when we mean bytes!"
thing, and I'll include round_down() as well.

Thanks,
-Eric

Re: [PATCH] mkfs.btrfs: round all device sizes to sectorsize

[Eric Sandeen]

On 7/25/14, 12:25 PM, Eric Sandeen wrote:
> On 7/25/14, 12:12 PM, Zach Brown wrote:
>> On Thu, Jul 24, 2014 at 11:27:32PM -0500, Eric Sandeen wrote:
>>> make_btrfs() rounds down the first device size to a multiple of sectorsize:
>>
>>                ^^^^^^^^^^^
>>
>>> -	device->total_bytes = block_count;
>>> +	device->total_bytes = (block_count / sectorsize) * sectorsize;
>>
>> kerncompat.h:#define round_down(x, y) ((x) & ~__round_mask(x, y))
>>
>> - z
>>
> 
> yeah yeah ;) this isn't copied kernel code but sure, that'd be better.
> 
> I'm trying to clean up this whole "we say blocks when we mean bytes!"
> thing, and I'll include round_down() as well.

meh.  And half this is done in kernelspace for device add/replace
(device size setting etc) so TBH I'm increasingly inclined to
just back away slowly here.  :(

(IOWs device_add calls btrfs_prepare_device(), but the size
it finds is never used; the kernel does:

	device->total_bytes = i_size_read(bdev->bd_inode);

so changing prepare_device doesn't catch add/replace cases...)

Perhaps the simpler option is to remove the rounding which is
only done on the first device added in userspace, but I honestly
don't know what the design plan is, or what the ramifications of
that might be ...

-Eric