* [PATCH] mkfs.btrfs: round all device sizes to sectorsize
@ 2014-07-25 4:27 Eric Sandeen
2014-07-25 6:52 ` Anand Jain
2014-07-25 17:12 ` Zach Brown
0 siblings, 2 replies; 5+ messages in thread
From: Eric Sandeen @ 2014-07-25 4:27 UTC (permalink / raw)
To: linux-btrfs
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;
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mkfs.btrfs: round all device sizes to sectorsize
2014-07-25 4:27 [PATCH] mkfs.btrfs: round all device sizes to sectorsize Eric Sandeen
@ 2014-07-25 6:52 ` Anand Jain
2014-07-25 17:12 ` Zach Brown
1 sibling, 0 replies; 5+ messages in thread
From: Anand Jain @ 2014-07-25 6:52 UTC (permalink / raw)
To: Eric Sandeen, linux-btrfs
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mkfs.btrfs: round all device sizes to sectorsize
2014-07-25 4:27 [PATCH] mkfs.btrfs: round all device sizes to sectorsize Eric Sandeen
2014-07-25 6:52 ` Anand Jain
@ 2014-07-25 17:12 ` Zach Brown
2014-07-25 17:25 ` Eric Sandeen
1 sibling, 1 reply; 5+ messages in thread
From: Zach Brown @ 2014-07-25 17:12 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-btrfs
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mkfs.btrfs: round all device sizes to sectorsize
2014-07-25 17:12 ` Zach Brown
@ 2014-07-25 17:25 ` Eric Sandeen
2014-07-25 18:04 ` Eric Sandeen
0 siblings, 1 reply; 5+ messages in thread
From: Eric Sandeen @ 2014-07-25 17:25 UTC (permalink / raw)
To: Zach Brown; +Cc: linux-btrfs
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mkfs.btrfs: round all device sizes to sectorsize
2014-07-25 17:25 ` Eric Sandeen
@ 2014-07-25 18:04 ` Eric Sandeen
0 siblings, 0 replies; 5+ messages in thread
From: Eric Sandeen @ 2014-07-25 18:04 UTC (permalink / raw)
To: Zach Brown; +Cc: linux-btrfs
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-07-25 18:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-25 4:27 [PATCH] mkfs.btrfs: round all device sizes to sectorsize Eric Sandeen
2014-07-25 6:52 ` Anand Jain
2014-07-25 17:12 ` Zach Brown
2014-07-25 17:25 ` Eric Sandeen
2014-07-25 18:04 ` Eric Sandeen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.