public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* Support for data CRC
@ 2017-08-21 19:04 Sebastian Andrzej Siewior
  2017-08-21 19:52 ` Chris Murphy
  2017-08-21 23:02 ` Support for data CRC Dave Chinner
  0 siblings, 2 replies; 15+ messages in thread
From: Sebastian Andrzej Siewior @ 2017-08-21 19:04 UTC (permalink / raw)
  To: linux-xfs

Hi,

my understanding is that only metadata is protected by CRC in the new
on-disk format. Are there any plans to also protect data?

Sebastian

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Support for data CRC
  2017-08-21 19:04 Support for data CRC Sebastian Andrzej Siewior
@ 2017-08-21 19:52 ` Chris Murphy
  2017-08-22 11:10   ` Sebastian Andrzej Siewior
  2017-08-21 23:02 ` Support for data CRC Dave Chinner
  1 sibling, 1 reply; 15+ messages in thread
From: Chris Murphy @ 2017-08-21 19:52 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: xfs list

On Mon, Aug 21, 2017 at 1:04 PM, Sebastian Andrzej Siewior
<sebastian@breakpoint.cc> wrote:
> Hi,
>
> my understanding is that only metadata is protected by CRC in the new
> on-disk format. Are there any plans to also protect data?

See dm-integrity, merged as of 4.12 kernel. I haven't seen any
performance benchmarking so far.

-- 
Chris Murphy

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Support for data CRC
  2017-08-21 19:04 Support for data CRC Sebastian Andrzej Siewior
  2017-08-21 19:52 ` Chris Murphy
@ 2017-08-21 23:02 ` Dave Chinner
  2017-08-22  5:51   ` Christoph Hellwig
  1 sibling, 1 reply; 15+ messages in thread
From: Dave Chinner @ 2017-08-21 23:02 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: linux-xfs

On Mon, Aug 21, 2017 at 09:04:15PM +0200, Sebastian Andrzej Siewior wrote:
> Hi,
> 
> my understanding is that only metadata is protected by CRC in the new
> on-disk format. Are there any plans to also protect data?

Not in XFS.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Support for data CRC
  2017-08-21 23:02 ` Support for data CRC Dave Chinner
@ 2017-08-22  5:51   ` Christoph Hellwig
  2017-08-22 11:13     ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2017-08-22  5:51 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Sebastian Andrzej Siewior, linux-xfs

On Tue, Aug 22, 2017 at 09:02:32AM +1000, Dave Chinner wrote:
> On Mon, Aug 21, 2017 at 09:04:15PM +0200, Sebastian Andrzej Siewior wrote:
> > Hi,
> > 
> > my understanding is that only metadata is protected by CRC in the new
> > on-disk format. Are there any plans to also protect data?
> 
> Not in XFS.

I had at least some customer interest in it, although not in the short
term.  Now that we have reflink we can do the required copy on write
mechanisms, but I've not done any more detailed analysis of potential
implementations yet.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Support for data CRC
  2017-08-21 19:52 ` Chris Murphy
@ 2017-08-22 11:10   ` Sebastian Andrzej Siewior
  2017-08-22 22:37     ` Chris Murphy
  0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Andrzej Siewior @ 2017-08-22 11:10 UTC (permalink / raw)
  To: Chris Murphy; +Cc: xfs list

On 2017-08-21 13:52:48 [-0600], Chris Murphy wrote:
> On Mon, Aug 21, 2017 at 1:04 PM, Sebastian Andrzej Siewior
> <sebastian@breakpoint.cc> wrote:
> > Hi,
> >
> > my understanding is that only metadata is protected by CRC in the new
> > on-disk format. Are there any plans to also protect data?
> 
> See dm-integrity, merged as of 4.12 kernel. I haven't seen any
> performance benchmarking so far.

So this ensures integrity with the help of crypto algorithms. This is a
bit much as something like crc32c/crc64 would be sufficient. However
dm-integrity does not provide a way to recover (as far as I can tell).
My idea was that if the "normal" path is faulty (and noticed by the crc
check) it (xfs or the dm layer) would try to read the data via an
alternative path if possible (say on RAID1/5/6).

Sebastian

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Support for data CRC
  2017-08-22  5:51   ` Christoph Hellwig
@ 2017-08-22 11:13     ` Sebastian Andrzej Siewior
  2017-08-23  6:19       ` Christoph Hellwig
  0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Andrzej Siewior @ 2017-08-22 11:13 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Dave Chinner, linux-xfs

On 2017-08-21 22:51:36 [-0700], Christoph Hellwig wrote:
> On Tue, Aug 22, 2017 at 09:02:32AM +1000, Dave Chinner wrote:
> > On Mon, Aug 21, 2017 at 09:04:15PM +0200, Sebastian Andrzej Siewior wrote:
> > > Hi,
> > > 
> > > my understanding is that only metadata is protected by CRC in the new
> > > on-disk format. Are there any plans to also protect data?
> > 
> > Not in XFS.
> 
> I had at least some customer interest in it, although not in the short
> term.  Now that we have reflink we can do the required copy on write
> mechanisms, but I've not done any more detailed analysis of potential
> implementations yet.

copy on write. So the data crc is dead / not possible / not wanted or
part of copy on write?

Sebastian

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Support for data CRC
  2017-08-22 11:10   ` Sebastian Andrzej Siewior
@ 2017-08-22 22:37     ` Chris Murphy
  2017-09-10 19:43       ` Testing dm-integrity (WAS: Re: Support for data CRC) Sebastian Andrzej Siewior
  0 siblings, 1 reply; 15+ messages in thread
From: Chris Murphy @ 2017-08-22 22:37 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: Chris Murphy, xfs list

On Tue, Aug 22, 2017 at 5:10 AM, Sebastian Andrzej Siewior
<sebastian@breakpoint.cc> wrote:
> On 2017-08-21 13:52:48 [-0600], Chris Murphy wrote:
>> On Mon, Aug 21, 2017 at 1:04 PM, Sebastian Andrzej Siewior
>> <sebastian@breakpoint.cc> wrote:
>> > Hi,
>> >
>> > my understanding is that only metadata is protected by CRC in the new
>> > on-disk format. Are there any plans to also protect data?
>>
>> See dm-integrity, merged as of 4.12 kernel. I haven't seen any
>> performance benchmarking so far.
>
> So this ensures integrity with the help of crypto algorithms. This is a
> bit much as something like crc32c/crc64 would be sufficient. However
> dm-integrity does not provide a way to recover (as far as I can tell).

If a block fails integrity checking, there's a read error that goes to
the next layer which would need to have replication such as mdadm/lvm
RAID. And that layer would grab a good copy, and fix the bad copy. So
every physical drive would first have dm-integrity applied, which is a
1:1 mapping, and then each dm-integrity logical device is made a PV
for LVM, and then you create raid1/5/6 LV's or whatever you want, and
then format it whatever you want.

Documentation says either crc or hash is possible, including crc32.
https://www.kernel.org/doc/Documentation/device-mapper/dm-integrity.txt


> My idea was that if the "normal" path is faulty (and noticed by the crc
> check) it (xfs or the dm layer) would try to read the data via an
> alternative path if possible (say on RAID1/5/6).

Yes you have to build each layer you want separately with this method.
Otherwise you're looking at ZFS or Btrfs if you want it integrated.


-- 
Chris Murphy

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Support for data CRC
  2017-08-22 11:13     ` Sebastian Andrzej Siewior
@ 2017-08-23  6:19       ` Christoph Hellwig
  2017-09-10 19:50         ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2017-08-23  6:19 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: Christoph Hellwig, Dave Chinner, linux-xfs

On Tue, Aug 22, 2017 at 01:13:22PM +0200, Sebastian Andrzej Siewior wrote:
> copy on write. So the data crc is dead / not possible / not wanted or
> part of copy on write?

We support copy on write (or redirect on write to be correct) now
as part of the reflink feature.  These out of place writes are
a requirement to implement data checksums, as otherwise your data
and checksum can not be updated atomically.  Everything else is
beyond that at this point is vapourware.  We'd have to decided
first if we really want the feature, and then where to place the
checksum (extent structure, rmap, ?).

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Testing dm-integrity (WAS: Re: Support for data CRC)
  2017-08-22 22:37     ` Chris Murphy
@ 2017-09-10 19:43       ` Sebastian Andrzej Siewior
  2017-09-10 20:06         ` Milan Broz
  0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Andrzej Siewior @ 2017-09-10 19:43 UTC (permalink / raw)
  To: dm-devel
  Cc: Chris Murphy, xfs list, Mikulas Patocka, Milan Broz, Mike Snitzer

On 2017-08-22 16:37:07 [-0600], Chris Murphy wrote:
> Documentation says either crc or hash is possible, including crc32.
> https://www.kernel.org/doc/Documentation/device-mapper/dm-integrity.txt
> > My idea was that if the "normal" path is faulty (and noticed by the crc
> > check) it (xfs or the dm layer) would try to read the data via an
> > alternative path if possible (say on RAID1/5/6).
> 
> Yes you have to build each layer you want separately with this method.

So I tried this:

dmsetup create test-raid1 --table '0 4194304 raid raid1 3 0 region_size 1024 2 - /dev/vdc - /dev/vdd'

# creating the target (size 1)
provided_data_sectors=1
table="0 $provided_data_sectors integrity /dev/mapper/test-raid1 4096 - D 1 internal_hash:crc32c"
dmsetup create dm-int --table "$table"
dmsetup remove /dev/mapper/dm-int

# hexdumped. is there a tool for that?
provided_data_sectors=4125208
table="0 $provided_data_sectors integrity /dev/mapper/test-raid1 4096 - D 1 internal_hash:crc32c"
dmsetup create dm-int --table "$table"

# without that cat, I get checksum errors errors
cat < /dev/zero > /dev/mapper/dm-int
#
mkfs.xfs /dev/mapper/dm-int
mount /dev/mapper/dm-int
echo asdfghjkl > /mnt/1234567890
umount /mnt/
dmsetup remove /dev/mapper/dm-int
dmsetup remove /dev/mapper/dm-raid1

#looking for data
hexdump -C /dev/vdc |grep asdfghjkl
 01226000  61 73 64 66 67 68 6a 6b  6c 0a 00 00 00 00 00 00  |asdfghjkl.......|

echo A > A
dd if=A of=/dev/vdc bs=1 seek=19030016 count=1
 1+0 records in
 1+0 records out
 1 byte copied, 0.00152606 s, 0.7 kB/s

#check
hexdump -C /dev/vdc |grep ^01226000
 01226000  41 62 64 66 67 68 6a 6b  6c 0a 00 00 00 00 00 00  |Abdfghjkl.......|

#and now
dmsetup create test-raid1 --table '0 4194304 raid raid1 3 0 region_size 1024 2 - /dev/vdc - /dev/vdd'
table="0 $provided_data_sectors integrity /dev/mapper/test-raid1 4096 - D 1 internal_hash:crc32c"
dmsetup create dm-int --table "$table"

mount /dev/mapper/dm-int /mnt
cat /mnt/1234567890 
 cat: /mnt/1234567890: Input/output error

# and the kernel says
dmesg
 md/raid1:mdX: active with 2 out of 2 mirrors
 device-mapper: integrity: Checksum failed at sector 0x48
 XFS (dm-1): Mounting V5 Filesystem
 XFS (dm-1): Ending clean mount
 device-mapper: integrity: Checksum failed at sector 0x48
 device-mapper: integrity: Checksum failed at sector 0x48

at some point I expected dm-integrity to check the other disk in RAID.
Isn't this possible or is my setup wrong at some point? If it is not
possible, could it be made possible?

Sebastian

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Support for data CRC
  2017-08-23  6:19       ` Christoph Hellwig
@ 2017-09-10 19:50         ` Sebastian Andrzej Siewior
  2017-09-11  6:37           ` Christoph Hellwig
  0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Andrzej Siewior @ 2017-09-10 19:50 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Dave Chinner, linux-xfs

On 2017-08-22 23:19:31 [-0700], Christoph Hellwig wrote:
> as part of the reflink feature.  These out of place writes are
> a requirement to implement data checksums, as otherwise your data
> and checksum can not be updated atomically.  Everything else is
> beyond that at this point is vapourware.  We'd have to decided
So using the journal for atomic updates doesn't work then. Which means
fragmentation unavoidable.

Sebastian

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Testing dm-integrity (WAS: Re: Support for data CRC)
  2017-09-10 19:43       ` Testing dm-integrity (WAS: Re: Support for data CRC) Sebastian Andrzej Siewior
@ 2017-09-10 20:06         ` Milan Broz
  2017-09-11  6:38           ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 15+ messages in thread
From: Milan Broz @ 2017-09-10 20:06 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior, dm-devel
  Cc: Chris Murphy, xfs list, Mikulas Patocka, Mike Snitzer


On 09/10/2017 09:43 PM, Sebastian Andrzej Siewior wrote:
> On 2017-08-22 16:37:07 [-0600], Chris Murphy wrote:
>> Documentation says either crc or hash is possible, including crc32.
>> https://www.kernel.org/doc/Documentation/device-mapper/dm-integrity.txt
>>> My idea was that if the "normal" path is faulty (and noticed by the crc
>>> check) it (xfs or the dm layer) would try to read the data via an
>>> alternative path if possible (say on RAID1/5/6).
>>
>> Yes you have to build each layer you want separately with this method.
> 
> So I tried this:
> 
> dmsetup create test-raid1 --table '0 4194304 raid raid1 3 0 region_size 1024 2 - /dev/vdc - /dev/vdd'
> 
> # creating the target (size 1)
> provided_data_sectors=1
> table="0 $provided_data_sectors integrity /dev/mapper/test-raid1 4096 - D 1 internal_hash:crc32c"
> dmsetup create dm-int --table "$table"
> dmsetup remove /dev/mapper/dm-int
> 
> # hexdumped. is there a tool for that?

There is integritysetup tool (part of cryptsetup package) that automates format and mount of dm-integrity devices.
For now in devel git, I expect we will do some release package this month.
(So no need to use dmsetup here.)

...

> at some point I expected dm-integrity to check the other disk in RAID.

The dm-integrity has no connection to RAID, it will just return EILSEQ (instead of EIO error)
to inform that the error is an data integrity error, not a media (or other) IO error.

> Isn't this possible or is my setup wrong at some point? If it is not
> possible, could it be made possible?

It is up to RAID driver to handle integrity error properly (the same should happen
for hardware DIF integrity error I guess - you can try to simulate it with scsi_debug).

Milan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Support for data CRC
  2017-09-10 19:50         ` Sebastian Andrzej Siewior
@ 2017-09-11  6:37           ` Christoph Hellwig
  2017-09-11  6:52             ` Dave Chinner
  0 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2017-09-11  6:37 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: Christoph Hellwig, Dave Chinner, linux-xfs

On Sun, Sep 10, 2017 at 09:50:27PM +0200, Sebastian Andrzej Siewior wrote:
> On 2017-08-22 23:19:31 [-0700], Christoph Hellwig wrote:
> > as part of the reflink feature.  These out of place writes are
> > a requirement to implement data checksums, as otherwise your data
> > and checksum can not be updated atomically.  Everything else is
> > beyond that at this point is vapourware.  We'd have to decided
> So using the journal for atomic updates doesn't work then. Which means
> fragmentation unavoidable.

Well, in theory you could also do data journalling - but in the end
out of place writes are probably going to be more efficient, and at
least for XFS also area already implemented.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Testing dm-integrity (WAS: Re: Support for data CRC)
  2017-09-10 20:06         ` Milan Broz
@ 2017-09-11  6:38           ` Sebastian Andrzej Siewior
  2017-09-11 23:44             ` Mikulas Patocka
  0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Andrzej Siewior @ 2017-09-11  6:38 UTC (permalink / raw)
  To: Milan Broz
  Cc: dm-devel, Chris Murphy, xfs list, Mikulas Patocka, Mike Snitzer

On 2017-09-10 22:06:13 [+0200], Milan Broz wrote:
> There is integritysetup tool (part of cryptsetup package) that automates format and mount of dm-integrity devices.
> For now in devel git, I expect we will do some release package this month.
> (So no need to use dmsetup here.)
ah, good, thank. I read the documentation and it mentioned just those
steps not a pointer to tool.

> > at some point I expected dm-integrity to check the other disk in RAID.
> 
> The dm-integrity has no connection to RAID, it will just return EILSEQ (instead of EIO error)
> to inform that the error is an data integrity error, not a media (or other) IO error.

okay. Now I have a pointer where to look next.

> Milan

Sebastian

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Support for data CRC
  2017-09-11  6:37           ` Christoph Hellwig
@ 2017-09-11  6:52             ` Dave Chinner
  0 siblings, 0 replies; 15+ messages in thread
From: Dave Chinner @ 2017-09-11  6:52 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sebastian Andrzej Siewior, linux-xfs

On Sun, Sep 10, 2017 at 11:37:29PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 10, 2017 at 09:50:27PM +0200, Sebastian Andrzej Siewior wrote:
> > On 2017-08-22 23:19:31 [-0700], Christoph Hellwig wrote:
> > > as part of the reflink feature.  These out of place writes are
> > > a requirement to implement data checksums, as otherwise your data
> > > and checksum can not be updated atomically.  Everything else is
> > > beyond that at this point is vapourware.  We'd have to decided
> > So using the journal for atomic updates doesn't work then. Which means
> > fragmentation unavoidable.
> 
> Well, in theory you could also do data journalling - but in the end
> out of place writes are probably going to be more efficient, and at
> least for XFS also area already implemented.

>From a quick look at the dm-integrity code, it looks like it is just
a journalling block device. i.e. it logs both the data and the
calculated CRC in it's internal journal before writing the data and
CRC to their separate destinations so it can avoid inconsistent
data/CRCs if a crash occurs.

Cheers,

Dave.

-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Testing dm-integrity (WAS: Re: Support for data CRC)
  2017-09-11  6:38           ` Sebastian Andrzej Siewior
@ 2017-09-11 23:44             ` Mikulas Patocka
  0 siblings, 0 replies; 15+ messages in thread
From: Mikulas Patocka @ 2017-09-11 23:44 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Milan Broz, dm-devel, Chris Murphy, xfs list, Mike Snitzer



On Mon, 11 Sep 2017, Sebastian Andrzej Siewior wrote:

> On 2017-09-10 22:06:13 [+0200], Milan Broz wrote:
> > There is integritysetup tool (part of cryptsetup package) that automates format and mount of dm-integrity devices.
> > For now in devel git, I expect we will do some release package this month.
> > (So no need to use dmsetup here.)
> ah, good, thank. I read the documentation and it mentioned just those
> steps not a pointer to tool.
> 
> > > at some point I expected dm-integrity to check the other disk in RAID.

If you need to check the other disk in case of error, you need to put 
dm-integrity below the RAID driver - i.e. create two dm-integrity targets 
on two physical disks and put the RAID target above them.

Mikulas

> > The dm-integrity has no connection to RAID, it will just return EILSEQ (instead of EIO error)
> > to inform that the error is an data integrity error, not a media (or other) IO error.
> 
> okay. Now I have a pointer where to look next.
> 
> > Milan
> 
> Sebastian
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-09-11 23:45 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-21 19:04 Support for data CRC Sebastian Andrzej Siewior
2017-08-21 19:52 ` Chris Murphy
2017-08-22 11:10   ` Sebastian Andrzej Siewior
2017-08-22 22:37     ` Chris Murphy
2017-09-10 19:43       ` Testing dm-integrity (WAS: Re: Support for data CRC) Sebastian Andrzej Siewior
2017-09-10 20:06         ` Milan Broz
2017-09-11  6:38           ` Sebastian Andrzej Siewior
2017-09-11 23:44             ` Mikulas Patocka
2017-08-21 23:02 ` Support for data CRC Dave Chinner
2017-08-22  5:51   ` Christoph Hellwig
2017-08-22 11:13     ` Sebastian Andrzej Siewior
2017-08-23  6:19       ` Christoph Hellwig
2017-09-10 19:50         ` Sebastian Andrzej Siewior
2017-09-11  6:37           ` Christoph Hellwig
2017-09-11  6:52             ` Dave Chinner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox