FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Device-mapper Development

 
 
LinkBack Thread Tools
 
Old 03-15-2010, 05:04 AM
Mikulas Patocka
 
Default Test chunk size against both origin and snapshot sector size

Test chunk size against both origin and snapshot sector size

Don't allow chunk size smaller than either origin or snapshot logical
sector size. Reading or writing data unaligned to sector size is not allowed
and causes immediate errors.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
drivers/md/dm-exception-store.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

Index: linux-2.6.34-rc1-devel/drivers/md/dm-exception-store.c
================================================== =================
--- linux-2.6.34-rc1-devel.orig/drivers/md/dm-exception-store.c 2010-03-12 14:38:31.000000000 +0100
+++ linux-2.6.34-rc1-devel/drivers/md/dm-exception-store.c 2010-03-12 14:39:56.000000000 +0100
@@ -173,7 +173,9 @@ int dm_exception_store_set_chunk_size(st

/* Validate the chunk size against the device block size */
if (chunk_size %
- (bdev_logical_block_size(dm_snap_cow(store->snap)->bdev) >> 9)) {
+ (bdev_logical_block_size(dm_snap_cow(store->snap)->bdev) >> 9) ||
+ chunk_size %
+ (bdev_logical_block_size(dm_snap_origin(store->snap)->bdev) >> 9)) {
*error = "Chunk size is not a multiple of device blocksize";
return -EINVAL;
}

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 03-15-2010, 01:52 PM
Mike Snitzer
 
Default Test chunk size against both origin and snapshot sector size

On Mon, Mar 15 2010 at 2:04am -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> Test chunk size against both origin and snapshot sector size
>
> Don't allow chunk size smaller than either origin or snapshot logical
> sector size. Reading or writing data unaligned to sector size is not allowed
> and causes immediate errors.
>
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
>
> ---
> drivers/md/dm-exception-store.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> Index: linux-2.6.34-rc1-devel/drivers/md/dm-exception-store.c
> ================================================== =================
> --- linux-2.6.34-rc1-devel.orig/drivers/md/dm-exception-store.c 2010-03-12 14:38:31.000000000 +0100
> +++ linux-2.6.34-rc1-devel/drivers/md/dm-exception-store.c 2010-03-12 14:39:56.000000000 +0100
> @@ -173,7 +173,9 @@ int dm_exception_store_set_chunk_size(st
>
> /* Validate the chunk size against the device block size */
> if (chunk_size %
> - (bdev_logical_block_size(dm_snap_cow(store->snap)->bdev) >> 9)) {
> + (bdev_logical_block_size(dm_snap_cow(store->snap)->bdev) >> 9) ||
> + chunk_size %
> + (bdev_logical_block_size(dm_snap_origin(store->snap)->bdev) >> 9)) {
> *error = "Chunk size is not a multiple of device blocksize";
> return -EINVAL;
> }

Shouldn't we split these checks out so that we can have more precise
error reporting? Ideally we'd share that chunk_size was not a multiple
of the "origin" or "snapshot" device's blocksize.

I was also thinking that we should avoid using %, e.g.:
(chunk_size & (bdev_logical_block_size(...) - 1))

but AFAIK bdev_logical_block_size() may not be a power of 2 (MD allows
for obscure non-power of 2 blocksizes doesn't it? Or is that just for
MD chunk and stripe size?).

Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 03-15-2010, 02:10 PM
Mikulas Patocka
 
Default Test chunk size against both origin and snapshot sector size

On Mon, 15 Mar 2010, Mike Snitzer wrote:

> On Mon, Mar 15 2010 at 2:04am -0400,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
>
> > Test chunk size against both origin and snapshot sector size
> >
> > Don't allow chunk size smaller than either origin or snapshot logical
> > sector size. Reading or writing data unaligned to sector size is not allowed
> > and causes immediate errors.
> >
> > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> >
> > ---
> > drivers/md/dm-exception-store.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > Index: linux-2.6.34-rc1-devel/drivers/md/dm-exception-store.c
> > ================================================== =================
> > --- linux-2.6.34-rc1-devel.orig/drivers/md/dm-exception-store.c 2010-03-12 14:38:31.000000000 +0100
> > +++ linux-2.6.34-rc1-devel/drivers/md/dm-exception-store.c 2010-03-12 14:39:56.000000000 +0100
> > @@ -173,7 +173,9 @@ int dm_exception_store_set_chunk_size(st
> >
> > /* Validate the chunk size against the device block size */
> > if (chunk_size %
> > - (bdev_logical_block_size(dm_snap_cow(store->snap)->bdev) >> 9)) {
> > + (bdev_logical_block_size(dm_snap_cow(store->snap)->bdev) >> 9) ||
> > + chunk_size %
> > + (bdev_logical_block_size(dm_snap_origin(store->snap)->bdev) >> 9)) {
> > *error = "Chunk size is not a multiple of device blocksize";
> > return -EINVAL;
> > }
>
> Shouldn't we split these checks out so that we can have more precise
> error reporting? Ideally we'd share that chunk_size was not a multiple
> of the "origin" or "snapshot" device's blocksize.

You can split it to three messages ("not multiple of origin ... snapshot
... both devices' blocksize"), but I think it's not so important to be
worth code size increase.

> I was also thinking that we should avoid using %, e.g.:
> (chunk_size & (bdev_logical_block_size(...) - 1))
>
> but AFAIK bdev_logical_block_size() may not be a power of 2 (MD allows
> for obscure non-power of 2 blocksizes doesn't it? Or is that just for
> MD chunk and stripe size?).
>
> Mike

The Linux bio stack and page cache require that bdev_logical_block_size()
is power of two. But the disks can be reformatted to other block sizes.
I'm wondering, what happens then ... I suppose it wouldn't even allow to
use the disk. I will try.

Mikulas

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 03-15-2010, 02:30 PM
Mike Snitzer
 
Default Test chunk size against both origin and snapshot sector size

On Mon, Mar 15 2010 at 11:10am -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

>
>
> On Mon, 15 Mar 2010, Mike Snitzer wrote:
>
> > On Mon, Mar 15 2010 at 2:04am -0400,
> > Mikulas Patocka <mpatocka@redhat.com> wrote:
> >
> > > Test chunk size against both origin and snapshot sector size
> > >
> > > Don't allow chunk size smaller than either origin or snapshot logical
> > > sector size. Reading or writing data unaligned to sector size is not allowed
> > > and causes immediate errors.
> > >
> > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> > >
> > > ---
> > > drivers/md/dm-exception-store.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > Index: linux-2.6.34-rc1-devel/drivers/md/dm-exception-store.c
> > > ================================================== =================
> > > --- linux-2.6.34-rc1-devel.orig/drivers/md/dm-exception-store.c 2010-03-12 14:38:31.000000000 +0100
> > > +++ linux-2.6.34-rc1-devel/drivers/md/dm-exception-store.c 2010-03-12 14:39:56.000000000 +0100
> > > @@ -173,7 +173,9 @@ int dm_exception_store_set_chunk_size(st
> > >
> > > /* Validate the chunk size against the device block size */
> > > if (chunk_size %
> > > - (bdev_logical_block_size(dm_snap_cow(store->snap)->bdev) >> 9)) {
> > > + (bdev_logical_block_size(dm_snap_cow(store->snap)->bdev) >> 9) ||
> > > + chunk_size %
> > > + (bdev_logical_block_size(dm_snap_origin(store->snap)->bdev) >> 9)) {
> > > *error = "Chunk size is not a multiple of device blocksize";
> > > return -EINVAL;
> > > }
> >
> > Shouldn't we split these checks out so that we can have more precise
> > error reporting? Ideally we'd share that chunk_size was not a multiple
> > of the "origin" or "snapshot" device's blocksize.
>
> You can split it to three messages ("not multiple of origin ... snapshot
> ... both devices' blocksize"), but I think it's not so important to be
> worth code size increase.
>
> > I was also thinking that we should avoid using %, e.g.:
> > (chunk_size & (bdev_logical_block_size(...) - 1))
> >
> > but AFAIK bdev_logical_block_size() may not be a power of 2 (MD allows
> > for obscure non-power of 2 blocksizes doesn't it? Or is that just for
> > MD chunk and stripe size?).
> >
> > Mike
>
> The Linux bio stack and page cache require that bdev_logical_block_size()
> is power of two.

OK, I'll have a look, but it sounds like we could use:
(chunk_size & (bdev_logical_block_size(...) - 1))

> But the disks can be reformatted to other block sizes.
> I'm wondering, what happens then ... I suppose it wouldn't even allow to
> use the disk. I will try.

Do you mean something like 512b logical and 4K physical? Such devices
must perform the appropriate r-m-w. A 4K formatted device will report
4K for both logical and physical (unless the device and format tool
allows for physical != logical).

Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 03-15-2010, 02:52 PM
Mikulas Patocka
 
Default Test chunk size against both origin and snapshot sector size

On Mon, 15 Mar 2010, Mike Snitzer wrote:

> On Mon, Mar 15 2010 at 11:10am -0400,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
>
> >
> >
> > On Mon, 15 Mar 2010, Mike Snitzer wrote:
> >
> > > On Mon, Mar 15 2010 at 2:04am -0400,
> > > Mikulas Patocka <mpatocka@redhat.com> wrote:
> > >
> > > > Test chunk size against both origin and snapshot sector size
> > > >
> > > > Don't allow chunk size smaller than either origin or snapshot logical
> > > > sector size. Reading or writing data unaligned to sector size is not allowed
> > > > and causes immediate errors.
> > > >
> > > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> > > >
> > > > ---
> > > > drivers/md/dm-exception-store.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > Index: linux-2.6.34-rc1-devel/drivers/md/dm-exception-store.c
> > > > ================================================== =================
> > > > --- linux-2.6.34-rc1-devel.orig/drivers/md/dm-exception-store.c 2010-03-12 14:38:31.000000000 +0100
> > > > +++ linux-2.6.34-rc1-devel/drivers/md/dm-exception-store.c 2010-03-12 14:39:56.000000000 +0100
> > > > @@ -173,7 +173,9 @@ int dm_exception_store_set_chunk_size(st
> > > >
> > > > /* Validate the chunk size against the device block size */
> > > > if (chunk_size %
> > > > - (bdev_logical_block_size(dm_snap_cow(store->snap)->bdev) >> 9)) {
> > > > + (bdev_logical_block_size(dm_snap_cow(store->snap)->bdev) >> 9) ||
> > > > + chunk_size %
> > > > + (bdev_logical_block_size(dm_snap_origin(store->snap)->bdev) >> 9)) {
> > > > *error = "Chunk size is not a multiple of device blocksize";
> > > > return -EINVAL;
> > > > }
> > >
> > > Shouldn't we split these checks out so that we can have more precise
> > > error reporting? Ideally we'd share that chunk_size was not a multiple
> > > of the "origin" or "snapshot" device's blocksize.
> >
> > You can split it to three messages ("not multiple of origin ... snapshot
> > ... both devices' blocksize"), but I think it's not so important to be
> > worth code size increase.
> >
> > > I was also thinking that we should avoid using %, e.g.:
> > > (chunk_size & (bdev_logical_block_size(...) - 1))
> > >
> > > but AFAIK bdev_logical_block_size() may not be a power of 2 (MD allows
> > > for obscure non-power of 2 blocksizes doesn't it? Or is that just for
> > > MD chunk and stripe size?).
> > >
> > > Mike
> >
> > The Linux bio stack and page cache require that bdev_logical_block_size()
> > is power of two.
>
> OK, I'll have a look, but it sounds like we could use:
> (chunk_size & (bdev_logical_block_size(...) - 1))
>
> > But the disks can be reformatted to other block sizes.
> > I'm wondering, what happens then ... I suppose it wouldn't even allow to
> > use the disk. I will try.
>
> Do you mean something like 512b logical and 4K physical? Such devices
> must perform the appropriate r-m-w. A 4K formatted device will report
> 4K for both logical and physical (unless the device and format tool
> allows for physical != logical).
>
> Mike

No, I meant what happens if you format it with 514, 516, etc physical
blocksize. the Linux clearly doesn't support it, so what will it do with
it? I'll try when I finish tests on 4K.

Mikulas

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 03-15-2010, 02:54 PM
Mikulas Patocka
 
Default Test chunk size against both origin and snapshot sector size

> > The Linux bio stack and page cache require that bdev_logical_block_size()
> > is power of two.
>
> OK, I'll have a look, but it sounds like we could use:
> (chunk_size & (bdev_logical_block_size(...) - 1))

We could, but it doesn't matter anyway because it's just initialization
and it's not performance critical.

Mikulas

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 

Thread Tools




All times are GMT. The time now is 10:52 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org