* Re: [PATCH V2 2/2] Add casts for the addr arg of connect and bind
From: NeilBrown @ 2016-01-14 21:37 UTC (permalink / raw)
To: Khem Raj, linux-raid
In-Reply-To: <1452753159-1527-2-git-send-email-raj.khem@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 367 bytes --]
On Thu, Jan 14 2016, Khem Raj wrote:
> glibc allows the addr arg to connect and socket to be any of a number
> of 'sockaddr_*' types, but musl requires 'const struct sockaddr *'
> which is in line with open group specs. So add casts to allow
> compilation with musl.
>
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
Thanks a lot! Both patches applied.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: mdadm Oops on a small ARM system
From: NeilBrown @ 2016-01-14 21:35 UTC (permalink / raw)
To: Roman Mamedov, linux-raid
In-Reply-To: <20160114125216.1286288f@natsu>
[-- Attachment #1: Type: text/plain, Size: 6981 bytes --]
On Thu, Jan 14 2016, Roman Mamedov wrote:
> Hello,
>
> I am trying to run mdadm on an ARM system with 64 MB of RAM.
>
> mdadm --assemble --scan results in the following error.
> mdadm --detail /dev/md4 locks up indefinitely.
>
> It is a 2-member degraded RAID1 of 1.5 TB disks, previously created on amd64 PC.
>
> I remember mdadm working on this configuration before, maybe not with this array.
> In fact the stock OS of the device uses mdadm to provide RAID functionality too.
>
> Currently I am using Debian Wheezy with kernel 3.2.68 and mdadm 3.2.5.
>
> Any ideas?
It is the kernel crashing, not the "mdadm" program itself.
It looks like some weird data corruption.
The sync_page_io function allocates a bio, uses it to submit and wait
for a read to read the superblock off a device. Then when it all
bio_put() to free the bio, the bio turns out to be partly corrupted.
All I can suggest is to try a different/newer kernel.
NeilBrown
>
> [Thu Jan 14 12:37:38 2016] md: md4 stopped.
> [Thu Jan 14 12:37:38 2016] md: bind<sda3>
> [Thu Jan 14 12:37:38 2016] md: raid1 personality registered for level 1
> [Thu Jan 14 12:37:38 2016] bio: create slab <bio-1> at 1
> [Thu Jan 14 12:37:38 2016] md/raid1:md4: active with 1 out of 2 mirrors
> [Thu Jan 14 12:37:38 2016] Unable to handle kernel paging request at virtual address 01314289
> [Thu Jan 14 12:37:38 2016] pgd = c2ea8000
> [Thu Jan 14 12:37:38 2016] [01314289] *pgd=00000000
> [Thu Jan 14 12:37:38 2016] Internal error: Oops: 1 [#1]
> [Thu Jan 14 12:37:38 2016] Modules linked in: raid1 md_mod fuse tcp_illinois ipv6 evdev mv643xx_eth inet_lro lm75 g760a gpio_keys usb_storage ext4 crc16 mbcache jbd2 sg sd_mod crc_t10dif sata_mv libata ehci_hcd usbcore scsi_mod usb_common
> [Thu Jan 14 12:37:38 2016] CPU: 0 Not tainted (3.2.0-4-orion5x #1 Debian 3.2.68-1+deb7u6)
> [Thu Jan 14 12:37:38 2016] PC is at mddev_bio_destructor+0x4/0xc [md_mod]
> [Thu Jan 14 12:37:38 2016] LR is at bio_put+0x44/0x48
> [Thu Jan 14 12:37:38 2016] pc : [<bf23993c>] lr : [<c00f0c68>] psr: 60000013
> [Thu Jan 14 12:37:38 2016] sp : c38dfd10 ip : c38e2044 fp : 00000000
> [Thu Jan 14 12:37:38 2016] r10: 00000200 r9 : 00000000 r8 : 00000008
> [Thu Jan 14 12:37:38 2016] r7 : 00000001 r6 : 00000010 r5 : f0000009 r4 : c2f7c724
> [Thu Jan 14 12:37:38 2016] r3 : 01314011 r2 : 00000000 r1 : 20000013 r0 : c2f7c724
> [Thu Jan 14 12:37:38 2016] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [Thu Jan 14 12:37:38 2016] Control: a005317f Table: 02ea8000 DAC: 00000015
> [Thu Jan 14 12:37:38 2016] Process mdadm (pid: 1777, stack limit = 0xc38de270)
> [Thu Jan 14 12:37:38 2016] Stack: (0xc38dfd10 to 0xc38e0000)
> [Thu Jan 14 12:37:38 2016] fd00: 20000013 bf239a50 60000013 00000000
> [Thu Jan 14 12:37:38 2016] fd20: c38dfd20 c38dfd20 00000000 c04c1cc0 c326baa0 c39db810 00000001 00000000
> [Thu Jan 14 12:37:38 2016] fd40: 00000001 bf24232c 00000200 c04c1cc0 00000000 00000001 00000008 00000000
> [Thu Jan 14 12:37:38 2016] fd60: bf248298 00000000 00000000 c321e7a0 c2d6a0e0 c39db800 bf23fd90 ad7175da
> [Thu Jan 14 12:37:38 2016] fd80: 00000008 bf245150 00000000 00000000 00000100 c39db898 becfe538 c0290f84
> [Thu Jan 14 12:37:38 2016] fda0: bf25c2fc c38dfdb4 ad7175da 00000000 c321ee20 c39db800 00000000 c39db810
> [Thu Jan 14 12:37:38 2016] fdc0: 00000000 bf25c514 bf23fd90 c39db898 becfe538 bf240060 00000000 c38dfdfc
> [Thu Jan 14 12:37:38 2016] fde0: c3402c80 c39db810 c2f5e320 c00f35d0 c39db800 c39db800 c39db810 c326baa0
> [Thu Jan 14 12:37:38 2016] fe00: c3496ac0 bf2458b0 0000000f 00000000 00000000 00000000 00000000 00000000
> [Thu Jan 14 12:37:38 2016] fe20: 00000000 00000000 00000000 c39db800 c39db800 00000000 c3496ac0 400c0930
> [Thu Jan 14 12:37:38 2016] fe40: c38de000 00000000 becfe538 bf2402c4 400c0930 c39db800 00000000 bf241878
> [Thu Jan 14 12:37:38 2016] fe60: 00000002 00000008 00000003 00000000 00000006 c00f9330 c2e79a40 c3869508
> [Thu Jan 14 12:37:38 2016] fe80: c397b428 c397b428 c2cd0f00 c2e79a40 c397b428 c2cd0f00 c3869508 00000000
> [Thu Jan 14 12:37:38 2016] fea0: c397b428 c00fab20 00000000 c3496ac0 c2fbf400 400c0930 0000009f c38de000
> [Thu Jan 14 12:37:38 2016] fec0: 00000000 c015ebc0 00000000 c015f514 000000d0 c38dff78 bf0b8a70 c2e27800
> [Thu Jan 14 12:37:38 2016] fee0: c2e27800 c3496ac0 c3402c90 bf000fbc 00000007 bf0c94c8 c2d1de00 00000000
> [Thu Jan 14 12:37:38 2016] ff00: c3267e38 00000004 00000004 00000000 c38de000 00000000 becfe538 c00d609c
> [Thu Jan 14 12:37:38 2016] ff20: c3402740 00000000 0000009f c2e7ac00 c3402750 bf0c96f8 c3402c80 c00f2e88
> [Thu Jan 14 12:37:38 2016] ff40: c2f5e320 c2e8fe58 c2f5e320 c2e8fe58 20000013 c2f5e320 c2e8fe58 c00c8bc4
> [Thu Jan 14 12:37:38 2016] ff60: 00000000 c32da1c0 00000000 400c0930 00000004 00000000 c38de000 00000000
> [Thu Jan 14 12:37:38 2016] ff80: becfe538 c00d616c 00000004 00000000 016d1908 00000001 00000000 00000036
> [Thu Jan 14 12:37:38 2016] ffa0: c0012388 c00121e0 016d1908 00000001 00000004 400c0930 00000000 00000000
> [Thu Jan 14 12:37:38 2016] ffc0: 016d1908 00000001 00000000 00000036 00000001 0000000a 016d27c0 becfe538
> [Thu Jan 14 12:37:38 2016] ffe0: 00000004 becfe0c8 0001cd44 b6f157ac 60000010 00000004 00000000 00000000
> [Thu Jan 14 12:37:38 2016] [<bf23993c>] (mddev_bio_destructor+0x4/0xc [md_mod]) from [<c00f0c68>] (bio_put+0x44/0x48)
> [Thu Jan 14 12:37:38 2016] [<c00f0c68>] (bio_put+0x44/0x48) from [<bf239a50>] (sync_page_io+0xc0/0xd4 [md_mod])
> [Thu Jan 14 12:37:38 2016] [<bf239a50>] (sync_page_io+0xc0/0xd4 [md_mod]) from [<bf24232c>] (read_sb_page+0xd0/0x17c [md_mod])
> [Thu Jan 14 12:37:38 2016] [<bf24232c>] (read_sb_page+0xd0/0x17c [md_mod]) from [<bf245150>] (bitmap_create+0x31c/0x6d4 [md_mod])
> [Thu Jan 14 12:37:38 2016] [<bf245150>] (bitmap_create+0x31c/0x6d4 [md_mod]) from [<bf240060>] (md_run+0x658/0x8b0 [md_mod])
> [Thu Jan 14 12:37:38 2016] [<bf240060>] (md_run+0x658/0x8b0 [md_mod]) from [<bf2402c4>] (do_md_run+0xc/0x74 [md_mod])
> [Thu Jan 14 12:37:38 2016] [<bf2402c4>] (do_md_run+0xc/0x74 [md_mod]) from [<bf241878>] (md_ioctl+0xd94/0x104c [md_mod])
> [Thu Jan 14 12:37:38 2016] [<bf241878>] (md_ioctl+0xd94/0x104c [md_mod]) from [<c015ebc0>] (__blkdev_driver_ioctl+0x1c/0x28)
> [Thu Jan 14 12:37:38 2016] [<c015ebc0>] (__blkdev_driver_ioctl+0x1c/0x28) from [<c015f514>] (blkdev_ioctl+0x6ac/0x720)
> [Thu Jan 14 12:37:38 2016] [<c015f514>] (blkdev_ioctl+0x6ac/0x720) from [<c00d609c>] (do_vfs_ioctl+0x504/0x588)
> [Thu Jan 14 12:37:38 2016] [<c00d609c>] (do_vfs_ioctl+0x504/0x588) from [<c00d616c>] (sys_ioctl+0x4c/0x6c)
> [Thu Jan 14 12:37:38 2016] [<c00d616c>] (sys_ioctl+0x4c/0x6c) from [<c00121e0>] (ret_fast_syscall+0x0/0x2c)
> [Thu Jan 14 12:37:38 2016] Code: e1a00004 e8bd80f8 bf239938 e5103004 (e5931278)
> [Thu Jan 14 12:37:38 2016] ---[ end trace f3c05bbfe27f5503 ]---
>
> --
> With respect,
> Roman
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* [PATCH RESEND] md:Check if call to thaw_bdev fails in unlock_fs
From: Nicholas Krause @ 2016-01-14 21:05 UTC (permalink / raw)
To: agk; +Cc: linux-raid, dm-devel, neilb, snitzer, linux-kernel
This adds a check to the function unlock_fs to check if the call
to thaw_bdev failed by returning a return code and if so print
on the log level of KERN_CRIT that the thaw has failed on the
passed device structure pointer's name before returnin immediately
to this function's caller.
Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
---
drivers/md/dm.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index ab37ae1..20ccc36 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -3152,7 +3152,11 @@ static void unlock_fs(struct mapped_device *md)
if (!test_bit(DMF_FROZEN, &md->flags))
return;
- thaw_bdev(md->bdev, md->frozen_sb);
+ if (!thaw_bdev(md->bdev, md->frozen_sb)) {
+ pr_crit("Thaw failed on the file device %s\n", md->name);
+ return;
+ }
+
md->frozen_sb = NULL;
clear_bit(DMF_FROZEN, &md->flags);
}
--
2.1.4
^ permalink raw reply related
* Re: [dm-devel] [PATCH v2 0/2] Introduce the bulk IV mode for improving the crypto engine efficiency
From: Mark Brown @ 2016-01-14 11:35 UTC (permalink / raw)
To: Mikulas Patocka
Cc: device-mapper development, Milan Broz, Jens Axboe, keith.busch,
linux-raid, martin.petersen, Mike Snitzer, Baolin Wang,
linux-block, neilb, LKML, sagig, Arnd Bergmann, tj,
dan.j.williams, Kent Overstreet, Alasdair G Kergon
In-Reply-To: <alpine.LRH.2.02.1601122111040.24452@file01.intranet.prod.int.rdu2.redhat.com>
[-- Attachment #1: Type: text/plain, Size: 998 bytes --]
On Tue, Jan 12, 2016 at 09:13:19PM -0500, Mikulas Patocka wrote:
> On Tue, 12 Jan 2016, Mark Brown wrote:
> > This isn't targeted at a specific driver or system, it's trying to make
> > dm-crypt better able to make use of hardware acceleration in general.
> If the hardware acceleration doesn't allow to set arbitrary XTS tweak,
> then this "large block" optimization on XTS can't be done at all.
> So, we need to know which driver(s) you want to optimize for and how do
> those driver(s) handle tweak generation.
Unfortunately the reality is just as I described it - we're looking for
general improvements, not at specific devices (well, Linaro is mainly
interested in ARM based SoCs but the range of ARM SoCs is such that that
that doesn't really narrow things down). It's probably better to ask if
there exists any hardware which could use this usefully, software only
implementations (or hardware that only does AES) at least give us
control over supplying the tweak.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* mdadm Oops on a small ARM system
From: Roman Mamedov @ 2016-01-14 7:52 UTC (permalink / raw)
To: linux-raid
[-- Attachment #1: Type: text/plain, Size: 6395 bytes --]
Hello,
I am trying to run mdadm on an ARM system with 64 MB of RAM.
mdadm --assemble --scan results in the following error.
mdadm --detail /dev/md4 locks up indefinitely.
It is a 2-member degraded RAID1 of 1.5 TB disks, previously created on amd64 PC.
I remember mdadm working on this configuration before, maybe not with this array.
In fact the stock OS of the device uses mdadm to provide RAID functionality too.
Currently I am using Debian Wheezy with kernel 3.2.68 and mdadm 3.2.5.
Any ideas?
[Thu Jan 14 12:37:38 2016] md: md4 stopped.
[Thu Jan 14 12:37:38 2016] md: bind<sda3>
[Thu Jan 14 12:37:38 2016] md: raid1 personality registered for level 1
[Thu Jan 14 12:37:38 2016] bio: create slab <bio-1> at 1
[Thu Jan 14 12:37:38 2016] md/raid1:md4: active with 1 out of 2 mirrors
[Thu Jan 14 12:37:38 2016] Unable to handle kernel paging request at virtual address 01314289
[Thu Jan 14 12:37:38 2016] pgd = c2ea8000
[Thu Jan 14 12:37:38 2016] [01314289] *pgd=00000000
[Thu Jan 14 12:37:38 2016] Internal error: Oops: 1 [#1]
[Thu Jan 14 12:37:38 2016] Modules linked in: raid1 md_mod fuse tcp_illinois ipv6 evdev mv643xx_eth inet_lro lm75 g760a gpio_keys usb_storage ext4 crc16 mbcache jbd2 sg sd_mod crc_t10dif sata_mv libata ehci_hcd usbcore scsi_mod usb_common
[Thu Jan 14 12:37:38 2016] CPU: 0 Not tainted (3.2.0-4-orion5x #1 Debian 3.2.68-1+deb7u6)
[Thu Jan 14 12:37:38 2016] PC is at mddev_bio_destructor+0x4/0xc [md_mod]
[Thu Jan 14 12:37:38 2016] LR is at bio_put+0x44/0x48
[Thu Jan 14 12:37:38 2016] pc : [<bf23993c>] lr : [<c00f0c68>] psr: 60000013
[Thu Jan 14 12:37:38 2016] sp : c38dfd10 ip : c38e2044 fp : 00000000
[Thu Jan 14 12:37:38 2016] r10: 00000200 r9 : 00000000 r8 : 00000008
[Thu Jan 14 12:37:38 2016] r7 : 00000001 r6 : 00000010 r5 : f0000009 r4 : c2f7c724
[Thu Jan 14 12:37:38 2016] r3 : 01314011 r2 : 00000000 r1 : 20000013 r0 : c2f7c724
[Thu Jan 14 12:37:38 2016] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[Thu Jan 14 12:37:38 2016] Control: a005317f Table: 02ea8000 DAC: 00000015
[Thu Jan 14 12:37:38 2016] Process mdadm (pid: 1777, stack limit = 0xc38de270)
[Thu Jan 14 12:37:38 2016] Stack: (0xc38dfd10 to 0xc38e0000)
[Thu Jan 14 12:37:38 2016] fd00: 20000013 bf239a50 60000013 00000000
[Thu Jan 14 12:37:38 2016] fd20: c38dfd20 c38dfd20 00000000 c04c1cc0 c326baa0 c39db810 00000001 00000000
[Thu Jan 14 12:37:38 2016] fd40: 00000001 bf24232c 00000200 c04c1cc0 00000000 00000001 00000008 00000000
[Thu Jan 14 12:37:38 2016] fd60: bf248298 00000000 00000000 c321e7a0 c2d6a0e0 c39db800 bf23fd90 ad7175da
[Thu Jan 14 12:37:38 2016] fd80: 00000008 bf245150 00000000 00000000 00000100 c39db898 becfe538 c0290f84
[Thu Jan 14 12:37:38 2016] fda0: bf25c2fc c38dfdb4 ad7175da 00000000 c321ee20 c39db800 00000000 c39db810
[Thu Jan 14 12:37:38 2016] fdc0: 00000000 bf25c514 bf23fd90 c39db898 becfe538 bf240060 00000000 c38dfdfc
[Thu Jan 14 12:37:38 2016] fde0: c3402c80 c39db810 c2f5e320 c00f35d0 c39db800 c39db800 c39db810 c326baa0
[Thu Jan 14 12:37:38 2016] fe00: c3496ac0 bf2458b0 0000000f 00000000 00000000 00000000 00000000 00000000
[Thu Jan 14 12:37:38 2016] fe20: 00000000 00000000 00000000 c39db800 c39db800 00000000 c3496ac0 400c0930
[Thu Jan 14 12:37:38 2016] fe40: c38de000 00000000 becfe538 bf2402c4 400c0930 c39db800 00000000 bf241878
[Thu Jan 14 12:37:38 2016] fe60: 00000002 00000008 00000003 00000000 00000006 c00f9330 c2e79a40 c3869508
[Thu Jan 14 12:37:38 2016] fe80: c397b428 c397b428 c2cd0f00 c2e79a40 c397b428 c2cd0f00 c3869508 00000000
[Thu Jan 14 12:37:38 2016] fea0: c397b428 c00fab20 00000000 c3496ac0 c2fbf400 400c0930 0000009f c38de000
[Thu Jan 14 12:37:38 2016] fec0: 00000000 c015ebc0 00000000 c015f514 000000d0 c38dff78 bf0b8a70 c2e27800
[Thu Jan 14 12:37:38 2016] fee0: c2e27800 c3496ac0 c3402c90 bf000fbc 00000007 bf0c94c8 c2d1de00 00000000
[Thu Jan 14 12:37:38 2016] ff00: c3267e38 00000004 00000004 00000000 c38de000 00000000 becfe538 c00d609c
[Thu Jan 14 12:37:38 2016] ff20: c3402740 00000000 0000009f c2e7ac00 c3402750 bf0c96f8 c3402c80 c00f2e88
[Thu Jan 14 12:37:38 2016] ff40: c2f5e320 c2e8fe58 c2f5e320 c2e8fe58 20000013 c2f5e320 c2e8fe58 c00c8bc4
[Thu Jan 14 12:37:38 2016] ff60: 00000000 c32da1c0 00000000 400c0930 00000004 00000000 c38de000 00000000
[Thu Jan 14 12:37:38 2016] ff80: becfe538 c00d616c 00000004 00000000 016d1908 00000001 00000000 00000036
[Thu Jan 14 12:37:38 2016] ffa0: c0012388 c00121e0 016d1908 00000001 00000004 400c0930 00000000 00000000
[Thu Jan 14 12:37:38 2016] ffc0: 016d1908 00000001 00000000 00000036 00000001 0000000a 016d27c0 becfe538
[Thu Jan 14 12:37:38 2016] ffe0: 00000004 becfe0c8 0001cd44 b6f157ac 60000010 00000004 00000000 00000000
[Thu Jan 14 12:37:38 2016] [<bf23993c>] (mddev_bio_destructor+0x4/0xc [md_mod]) from [<c00f0c68>] (bio_put+0x44/0x48)
[Thu Jan 14 12:37:38 2016] [<c00f0c68>] (bio_put+0x44/0x48) from [<bf239a50>] (sync_page_io+0xc0/0xd4 [md_mod])
[Thu Jan 14 12:37:38 2016] [<bf239a50>] (sync_page_io+0xc0/0xd4 [md_mod]) from [<bf24232c>] (read_sb_page+0xd0/0x17c [md_mod])
[Thu Jan 14 12:37:38 2016] [<bf24232c>] (read_sb_page+0xd0/0x17c [md_mod]) from [<bf245150>] (bitmap_create+0x31c/0x6d4 [md_mod])
[Thu Jan 14 12:37:38 2016] [<bf245150>] (bitmap_create+0x31c/0x6d4 [md_mod]) from [<bf240060>] (md_run+0x658/0x8b0 [md_mod])
[Thu Jan 14 12:37:38 2016] [<bf240060>] (md_run+0x658/0x8b0 [md_mod]) from [<bf2402c4>] (do_md_run+0xc/0x74 [md_mod])
[Thu Jan 14 12:37:38 2016] [<bf2402c4>] (do_md_run+0xc/0x74 [md_mod]) from [<bf241878>] (md_ioctl+0xd94/0x104c [md_mod])
[Thu Jan 14 12:37:38 2016] [<bf241878>] (md_ioctl+0xd94/0x104c [md_mod]) from [<c015ebc0>] (__blkdev_driver_ioctl+0x1c/0x28)
[Thu Jan 14 12:37:38 2016] [<c015ebc0>] (__blkdev_driver_ioctl+0x1c/0x28) from [<c015f514>] (blkdev_ioctl+0x6ac/0x720)
[Thu Jan 14 12:37:38 2016] [<c015f514>] (blkdev_ioctl+0x6ac/0x720) from [<c00d609c>] (do_vfs_ioctl+0x504/0x588)
[Thu Jan 14 12:37:38 2016] [<c00d609c>] (do_vfs_ioctl+0x504/0x588) from [<c00d616c>] (sys_ioctl+0x4c/0x6c)
[Thu Jan 14 12:37:38 2016] [<c00d616c>] (sys_ioctl+0x4c/0x6c) from [<c00121e0>] (ret_fast_syscall+0x0/0x2c)
[Thu Jan 14 12:37:38 2016] Code: e1a00004 e8bd80f8 bf239938 e5103004 (e5931278)
[Thu Jan 14 12:37:38 2016] ---[ end trace f3c05bbfe27f5503 ]---
--
With respect,
Roman
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* [PATCH V2 2/2] Add casts for the addr arg of connect and bind
From: Khem Raj @ 2016-01-14 6:32 UTC (permalink / raw)
To: linux-raid; +Cc: Khem Raj
In-Reply-To: <1452753159-1527-1-git-send-email-raj.khem@gmail.com>
glibc allows the addr arg to connect and socket to be any of a number
of 'sockaddr_*' types, but musl requires 'const struct sockaddr *'
which is in line with open group specs. So add casts to allow
compilation with musl.
Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
Changes since v1:
- Improve the commit meesage
- Divide into separate commit
mdmon.c | 2 +-
msg.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/mdmon.c b/mdmon.c
index ee12b7c..e4b73d9 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -235,7 +235,7 @@ static int make_control_sock(char *devname)
addr.sun_family = PF_LOCAL;
strcpy(addr.sun_path, path);
umask(077); /* ensure no world write access */
- if (bind(sfd, &addr, sizeof(addr)) < 0) {
+ if (bind(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
close(sfd);
return -1;
}
diff --git a/msg.c b/msg.c
index 754630b..45cd450 100644
--- a/msg.c
+++ b/msg.c
@@ -170,7 +170,7 @@ int connect_monitor(char *devname)
addr.sun_family = PF_LOCAL;
strcpy(addr.sun_path, path);
- if (connect(sfd, &addr, sizeof(addr)) < 0) {
+ if (connect(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
close(sfd);
return -1;
}
--
2.7.0
^ permalink raw reply related
* [PATCH V2 1/2] Define _POSIX_C_SOURCE if undefined
From: Khem Raj @ 2016-01-14 6:32 UTC (permalink / raw)
To: linux-raid; +Cc: Khem Raj
config.c uses _POSIX_C_SOURCE which is defined in features.h when
glibc/uclibc is used, but isn't defined when musl is used.
So provide a reasonable default.
Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
Changes since v1:
- Improve the commit meesage
- Divide into separate commit
config.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/config.c b/config.c
index c58c8fe..b308b6c 100644
--- a/config.c
+++ b/config.c
@@ -63,6 +63,9 @@
* but may not wrap over lines
*
*/
+#ifndef _POSIX_C_SOURCE
+#define _POSIX_C_SOURCE 200809L
+#endif
#ifndef CONFFILE
#define CONFFILE "/etc/mdadm.conf"
--
2.7.0
^ permalink raw reply related
* Re: [PATCH] Define _POSIX_C_SOURCE if undefined
From: NeilBrown @ 2016-01-14 5:36 UTC (permalink / raw)
To: Khem Raj; +Cc: linux-raid
In-Reply-To: <12D2DAF6-2702-442A-B655-E7D540BC1B7A@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3678 bytes --]
On Thu, Jan 14 2016, Khem Raj wrote:
> Hi NeilBrown
>
>> On Jan 13, 2016, at 4:40 PM, NeilBrown <nfbrown@novell.com> wrote:
>>
>> On Wed, Jan 13 2016, Khem Raj wrote:
>>
>>> typecast second argument of connect() API to use struct sockaddr*
>>>
>>
>> Hi,
>> You have told us what this patch does, but not why anyone should care.
>> Just a sentence or two is probably enough. Are you getting compiler
>> warnings (if so, what are they). Are we violating some standard (which
>> one).
>>
>
> No there is no violation of standard. It helps to port it to work with musl
> There is code in config.c which
> is conditionalized like
>
>
> #if _XOPEN_SOURCE >= 700 || _POSIX_C_SOURCE >= 200809L
> …
> #endif
>
> but we do not define _POSIX_C_SOURCE, glibc defines it in features.h so
> it gets in implicitly with glibc however when we use another libc
> implementation e.g. musl this define is not defined in libc and open
> group documentation says application should ensure that the feature test
> macro _POSIX_C_SOURCE is defined. So this adds a fallback and lets it
> port to musl.
Excellent - thanks.
So the first patch would just have the conditional #define, would have
the same subject as your original patch, and would say something like:
config.c uses _POSIX_C_SOURCE which is defined in features.h when glibc
is used, but isn't defined when musl is used. So provide a reasonable
default.
>
>
>> Is there a connection between defining _POSIX_C_SOURCE (as described in
>> the subject) and the second argument to connect (as mentioned in the
>> comment above) and the second argument to bind (as not mentioned until
>> the code).
>
> No, they are not connected. This is giving compiler diagnostics about type
> mismatches when using musl
> since definitions of sockaddr_un and sockaddr are different.
>
> musl defines the connect signature as
>
> int connect (int, const struct sockaddr *, socklen_t);
>
> which is inline with open group specs.
>
> It doesnt warn with glibc
> because the signature of connect() uses a union of struct types for second
> argument which is a GNU extention. Here are some part from
> /usr/include/sys/socket.h
>
>
> # define __SOCKADDR_ALLTYPES \
> __SOCKADDR_ONETYPE (sockaddr) \
> __SOCKADDR_ONETYPE (sockaddr_at) \
> __SOCKADDR_ONETYPE (sockaddr_ax25) \
> __SOCKADDR_ONETYPE (sockaddr_dl) \
> __SOCKADDR_ONETYPE (sockaddr_eon) \
> __SOCKADDR_ONETYPE (sockaddr_in) \
> __SOCKADDR_ONETYPE (sockaddr_in6) \
> __SOCKADDR_ONETYPE (sockaddr_inarp) \
> __SOCKADDR_ONETYPE (sockaddr_ipx) \
> __SOCKADDR_ONETYPE (sockaddr_iso) \
> __SOCKADDR_ONETYPE (sockaddr_ns) \
> __SOCKADDR_ONETYPE (sockaddr_un) \
> __SOCKADDR_ONETYPE (sockaddr_x25)
>
> typedef union { __SOCKADDR_ALLTYPES
> } __CONST_SOCKADDR_ARG __attribute__ ((__transparent_union__));
>
>
> extern int connect (int __fd, __CONST_SOCKADDR_ARG __addr, socklen_t __len);
>
Ok, so adding the casts should happen in a second patch with a subject
like
Subject: add casts for the addr arg of connect and bind
and then the comment above the patch would say something like:
glibc allows the addr arg to connect and socket to be any of a number
of 'sockaddr_*' types, but musl requires 'const struct sockaddr *'
which is in line with open group specs. So add casts to allow
compilation with musl.
If you could resend as two separate patches with appropriate
descriptions (use the above or alter to suit your preference) then it
will be obvious what the purpose of the patches is, and I'll apply them.
Thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: [PATCH] Define _POSIX_C_SOURCE if undefined
From: Khem Raj @ 2016-01-14 4:02 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
In-Reply-To: <87mvs96ljh.fsf@notabene.neil.brown.name>
[-- Attachment #1: Type: text/plain, Size: 4146 bytes --]
Hi NeilBrown
> On Jan 13, 2016, at 4:40 PM, NeilBrown <nfbrown@novell.com> wrote:
>
> On Wed, Jan 13 2016, Khem Raj wrote:
>
>> typecast second argument of connect() API to use struct sockaddr*
>>
>
> Hi,
> You have told us what this patch does, but not why anyone should care.
> Just a sentence or two is probably enough. Are you getting compiler
> warnings (if so, what are they). Are we violating some standard (which
> one).
>
No there is no violation of standard. It helps to port it to work with musl
There is code in config.c which
is conditionalized like
#if _XOPEN_SOURCE >= 700 || _POSIX_C_SOURCE >= 200809L
…
#endif
but we do not define _POSIX_C_SOURCE, glibc defines it in features.h so
it gets in implicitly with glibc however when we use another libc
implementation e.g. musl this define is not defined in libc and open
group documentation says application should ensure that the feature test
macro _POSIX_C_SOURCE is defined. So this adds a fallback and lets it
port to musl.
> Is there a connection between defining _POSIX_C_SOURCE (as described in
> the subject) and the second argument to connect (as mentioned in the
> comment above) and the second argument to bind (as not mentioned until
> the code).
No, they are not connected. This is giving compiler diagnostics about type
mismatches when using musl
since definitions of sockaddr_un and sockaddr are different.
musl defines the connect signature as
int connect (int, const struct sockaddr *, socklen_t);
which is inline with open group specs.
It doesnt warn with glibc
because the signature of connect() uses a union of struct types for second
argument which is a GNU extention. Here are some part from
/usr/include/sys/socket.h
# define __SOCKADDR_ALLTYPES \
__SOCKADDR_ONETYPE (sockaddr) \
__SOCKADDR_ONETYPE (sockaddr_at) \
__SOCKADDR_ONETYPE (sockaddr_ax25) \
__SOCKADDR_ONETYPE (sockaddr_dl) \
__SOCKADDR_ONETYPE (sockaddr_eon) \
__SOCKADDR_ONETYPE (sockaddr_in) \
__SOCKADDR_ONETYPE (sockaddr_in6) \
__SOCKADDR_ONETYPE (sockaddr_inarp) \
__SOCKADDR_ONETYPE (sockaddr_ipx) \
__SOCKADDR_ONETYPE (sockaddr_iso) \
__SOCKADDR_ONETYPE (sockaddr_ns) \
__SOCKADDR_ONETYPE (sockaddr_un) \
__SOCKADDR_ONETYPE (sockaddr_x25)
typedef union { __SOCKADDR_ALLTYPES
} __CONST_SOCKADDR_ARG __attribute__ ((__transparent_union__));
extern int connect (int __fd, __CONST_SOCKADDR_ARG __addr, socklen_t __len);
>
> Please explain.
>
> Thanks,
> NeilBrown
>
>
>> Signed-off-by: Khem Raj <raj.khem@gmail.com>
>> ---
>> config.c | 3 +++
>> mdmon.c | 2 +-
>> msg.c | 2 +-
>> 3 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/config.c b/config.c
>> index c58c8fe..b308b6c 100644
>> --- a/config.c
>> +++ b/config.c
>> @@ -63,6 +63,9 @@
>> * but may not wrap over lines
>> *
>> */
>> +#ifndef _POSIX_C_SOURCE
>> +#define _POSIX_C_SOURCE 200809L
>> +#endif
>>
>> #ifndef CONFFILE
>> #define CONFFILE "/etc/mdadm.conf"
>> diff --git a/mdmon.c b/mdmon.c
>> index ee12b7c..e4b73d9 100644
>> --- a/mdmon.c
>> +++ b/mdmon.c
>> @@ -235,7 +235,7 @@ static int make_control_sock(char *devname)
>> addr.sun_family = PF_LOCAL;
>> strcpy(addr.sun_path, path);
>> umask(077); /* ensure no world write access */
>> - if (bind(sfd, &addr, sizeof(addr)) < 0) {
>> + if (bind(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
>> close(sfd);
>> return -1;
>> }
>> diff --git a/msg.c b/msg.c
>> index 754630b..45cd450 100644
>> --- a/msg.c
>> +++ b/msg.c
>> @@ -170,7 +170,7 @@ int connect_monitor(char *devname)
>>
>> addr.sun_family = PF_LOCAL;
>> strcpy(addr.sun_path, path);
>> - if (connect(sfd, &addr, sizeof(addr)) < 0) {
>> + if (connect(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
>> close(sfd);
>> return -1;
>> }
>> --
>> 2.7.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 204 bytes --]
^ permalink raw reply
* Re: [PATCH] md/raid: only permit hot-add of compatible integrity profiles
From: NeilBrown @ 2016-01-14 0:56 UTC (permalink / raw)
To: Dan Williams
Cc: axboe, Mike Snitzer, martin.petersen, linux-kernel, stable,
linux-raid, linux-nvme, keith.busch, hch
In-Reply-To: <20160114000007.14556.8837.stgit@dwillia2-desk3.amr.corp.intel.com>
[-- Attachment #1: Type: text/plain, Size: 7176 bytes --]
On Thu, Jan 14 2016, Dan Williams wrote:
> It is not safe for an integrity profile to be changed while i/o is
> in-flight in the queue. Prevent adding new disks or otherwise online
> spares to an array if the device has an incompatible integrity profile.
>
> The original change to the blk_integrity_unregister implementation in
> md, commmit c7bfced9a671 "md: suspend i/o during runtime
> blk_integrity_unregister" introduced an immediate hang regression.
>
> This policy of disallowing changes the integrity profile once one has
> been established is shared with DM.
Thanks Dan. That looks like it should address the issues and seems to
make sense.
If it passes my smoke-testing I'll include it in my merge-window pull
request tomorrow.
NeilBrown
>
> Here is an abbreviated log from a test run that:
> 1/ Creates a degraded raid1 with an integrity-enabled device (pmem0s) [ 59.076127]
> 2/ Tries to add an integrity-disabled device (pmem1m) [ 90.489209]
> 3/ Retries with an integrity-enabled device (pmem1s) [ 205.671277]
>
> [ 59.076127] md/raid1:md0: active with 1 out of 2 mirrors
> [ 59.078302] md: data integrity enabled on md0
> [..]
> [ 90.489209] md0: incompatible integrity profile for pmem1m
> [..]
> [ 205.671277] md: super_written gets error=-5
> [ 205.677386] md/raid1:md0: Disk failure on pmem1m, disabling device.
> [ 205.677386] md/raid1:md0: Operation continuing on 1 devices.
> [ 205.683037] RAID1 conf printout:
> [ 205.684699] --- wd:1 rd:2
> [ 205.685972] disk 0, wo:0, o:1, dev:pmem0s
> [ 205.687562] disk 1, wo:1, o:1, dev:pmem1s
> [ 205.691717] md: recovery of RAID array md0
>
> Fixes: c7bfced9a671 ("md: suspend i/o during runtime blk_integrity_unregister")
> Cc: <stable@vger.kernel.org>
> Cc: Mike Snitzer <snitzer@redhat.com>
> Reported-by: NeilBrown <neilb@suse.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> drivers/md/md.c | 28 ++++++++++++++++------------
> drivers/md/md.h | 2 +-
> drivers/md/multipath.c | 6 +++---
> drivers/md/raid1.c | 6 +++---
> drivers/md/raid10.c | 6 +++---
> 5 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 61aacab424cf..b1e1f6b95782 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -2017,28 +2017,32 @@ int md_integrity_register(struct mddev *mddev)
> }
> EXPORT_SYMBOL(md_integrity_register);
>
> -/* Disable data integrity if non-capable/non-matching disk is being added */
> -void md_integrity_add_rdev(struct md_rdev *rdev, struct mddev *mddev)
> +/*
> + * Attempt to add an rdev, but only if it is consistent with the current
> + * integrity profile
> + */
> +int md_integrity_add_rdev(struct md_rdev *rdev, struct mddev *mddev)
> {
> struct blk_integrity *bi_rdev;
> struct blk_integrity *bi_mddev;
> + char name[BDEVNAME_SIZE];
>
> if (!mddev->gendisk)
> - return;
> + return 0;
>
> bi_rdev = bdev_get_integrity(rdev->bdev);
> bi_mddev = blk_get_integrity(mddev->gendisk);
>
> if (!bi_mddev) /* nothing to do */
> - return;
> - if (rdev->raid_disk < 0) /* skip spares */
> - return;
> - if (bi_rdev && blk_integrity_compare(mddev->gendisk,
> - rdev->bdev->bd_disk) >= 0)
> - return;
> - WARN_ON_ONCE(!mddev->suspended);
> - printk(KERN_NOTICE "disabling data integrity on %s\n", mdname(mddev));
> - blk_integrity_unregister(mddev->gendisk);
> + return 0;
> +
> + if (blk_integrity_compare(mddev->gendisk, rdev->bdev->bd_disk) != 0) {
> + printk(KERN_NOTICE "%s: incompatible integrity profile for %s\n",
> + mdname(mddev), bdevname(rdev->bdev, name));
> + return -ENXIO;
> + }
> +
> + return 0;
> }
> EXPORT_SYMBOL(md_integrity_add_rdev);
>
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index ca0b643fe3c1..dfa57b41541b 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -657,7 +657,7 @@ extern void md_wait_for_blocked_rdev(struct md_rdev *rdev, struct mddev *mddev);
> extern void md_set_array_sectors(struct mddev *mddev, sector_t array_sectors);
> extern int md_check_no_bitmap(struct mddev *mddev);
> extern int md_integrity_register(struct mddev *mddev);
> -extern void md_integrity_add_rdev(struct md_rdev *rdev, struct mddev *mddev);
> +extern int md_integrity_add_rdev(struct md_rdev *rdev, struct mddev *mddev);
> extern int strict_strtoul_scaled(const char *cp, unsigned long *res, int scale);
>
> extern void mddev_init(struct mddev *mddev);
> diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c
> index 7331a80d89f1..0a72ab6e6c20 100644
> --- a/drivers/md/multipath.c
> +++ b/drivers/md/multipath.c
> @@ -257,6 +257,9 @@ static int multipath_add_disk(struct mddev *mddev, struct md_rdev *rdev)
> disk_stack_limits(mddev->gendisk, rdev->bdev,
> rdev->data_offset << 9);
>
> + err = md_integrity_add_rdev(rdev, mddev);
> + if (err)
> + break;
> spin_lock_irq(&conf->device_lock);
> mddev->degraded--;
> rdev->raid_disk = path;
> @@ -264,9 +267,6 @@ static int multipath_add_disk(struct mddev *mddev, struct md_rdev *rdev)
> spin_unlock_irq(&conf->device_lock);
> rcu_assign_pointer(p->rdev, rdev);
> err = 0;
> - mddev_suspend(mddev);
> - md_integrity_add_rdev(rdev, mddev);
> - mddev_resume(mddev);
> break;
> }
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index e2169ff6e0f0..c4b913409226 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1589,6 +1589,9 @@ static int raid1_add_disk(struct mddev *mddev, struct md_rdev *rdev)
> if (mddev->recovery_disabled == conf->recovery_disabled)
> return -EBUSY;
>
> + if (md_integrity_add_rdev(rdev, mddev))
> + return -ENXIO;
> +
> if (rdev->raid_disk >= 0)
> first = last = rdev->raid_disk;
>
> @@ -1632,9 +1635,6 @@ static int raid1_add_disk(struct mddev *mddev, struct md_rdev *rdev)
> break;
> }
> }
> - mddev_suspend(mddev);
> - md_integrity_add_rdev(rdev, mddev);
> - mddev_resume(mddev);
> if (mddev->queue && blk_queue_discard(bdev_get_queue(rdev->bdev)))
> queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, mddev->queue);
> print_conf(conf);
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 84e597e1c489..ce959b4ae4df 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1698,6 +1698,9 @@ static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev)
> if (rdev->saved_raid_disk < 0 && !_enough(conf, 1, -1))
> return -EINVAL;
>
> + if (md_integrity_add_rdev(rdev, mddev))
> + return -ENXIO;
> +
> if (rdev->raid_disk >= 0)
> first = last = rdev->raid_disk;
>
> @@ -1739,9 +1742,6 @@ static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev)
> rcu_assign_pointer(p->rdev, rdev);
> break;
> }
> - mddev_suspend(mddev);
> - md_integrity_add_rdev(rdev, mddev);
> - mddev_resume(mddev);
> if (mddev->queue && blk_queue_discard(bdev_get_queue(rdev->bdev)))
> queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, mddev->queue);
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: [PATCH] Define _POSIX_C_SOURCE if undefined
From: NeilBrown @ 2016-01-14 0:40 UTC (permalink / raw)
To: linux-raid; +Cc: Khem Raj
In-Reply-To: <1452671964-35006-1-git-send-email-raj.khem@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2161 bytes --]
On Wed, Jan 13 2016, Khem Raj wrote:
> typecast second argument of connect() API to use struct sockaddr*
>
Hi,
You have told us what this patch does, but not why anyone should care.
Just a sentence or two is probably enough. Are you getting compiler
warnings (if so, what are they). Are we violating some standard (which
one).
Is there a connection between defining _POSIX_C_SOURCE (as described in
the subject) and the second argument to connect (as mentioned in the
comment above) and the second argument to bind (as not mentioned until
the code).
Please explain.
Thanks,
NeilBrown
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---
> config.c | 3 +++
> mdmon.c | 2 +-
> msg.c | 2 +-
> 3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/config.c b/config.c
> index c58c8fe..b308b6c 100644
> --- a/config.c
> +++ b/config.c
> @@ -63,6 +63,9 @@
> * but may not wrap over lines
> *
> */
> +#ifndef _POSIX_C_SOURCE
> +#define _POSIX_C_SOURCE 200809L
> +#endif
>
> #ifndef CONFFILE
> #define CONFFILE "/etc/mdadm.conf"
> diff --git a/mdmon.c b/mdmon.c
> index ee12b7c..e4b73d9 100644
> --- a/mdmon.c
> +++ b/mdmon.c
> @@ -235,7 +235,7 @@ static int make_control_sock(char *devname)
> addr.sun_family = PF_LOCAL;
> strcpy(addr.sun_path, path);
> umask(077); /* ensure no world write access */
> - if (bind(sfd, &addr, sizeof(addr)) < 0) {
> + if (bind(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
> close(sfd);
> return -1;
> }
> diff --git a/msg.c b/msg.c
> index 754630b..45cd450 100644
> --- a/msg.c
> +++ b/msg.c
> @@ -170,7 +170,7 @@ int connect_monitor(char *devname)
>
> addr.sun_family = PF_LOCAL;
> strcpy(addr.sun_path, path);
> - if (connect(sfd, &addr, sizeof(addr)) < 0) {
> + if (connect(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
> close(sfd);
> return -1;
> }
> --
> 2.7.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* [PATCH] md/raid: only permit hot-add of compatible integrity profiles
From: Dan Williams @ 2016-01-14 0:00 UTC (permalink / raw)
To: neilb
Cc: axboe, Mike Snitzer, martin.petersen, linux-kernel, stable,
NeilBrown, linux-raid, linux-nvme, keith.busch, hch
In-Reply-To: <874mei8bu8.fsf@notabene.neil.brown.name>
It is not safe for an integrity profile to be changed while i/o is
in-flight in the queue. Prevent adding new disks or otherwise online
spares to an array if the device has an incompatible integrity profile.
The original change to the blk_integrity_unregister implementation in
md, commmit c7bfced9a671 "md: suspend i/o during runtime
blk_integrity_unregister" introduced an immediate hang regression.
This policy of disallowing changes the integrity profile once one has
been established is shared with DM.
Here is an abbreviated log from a test run that:
1/ Creates a degraded raid1 with an integrity-enabled device (pmem0s) [ 59.076127]
2/ Tries to add an integrity-disabled device (pmem1m) [ 90.489209]
3/ Retries with an integrity-enabled device (pmem1s) [ 205.671277]
[ 59.076127] md/raid1:md0: active with 1 out of 2 mirrors
[ 59.078302] md: data integrity enabled on md0
[..]
[ 90.489209] md0: incompatible integrity profile for pmem1m
[..]
[ 205.671277] md: super_written gets error=-5
[ 205.677386] md/raid1:md0: Disk failure on pmem1m, disabling device.
[ 205.677386] md/raid1:md0: Operation continuing on 1 devices.
[ 205.683037] RAID1 conf printout:
[ 205.684699] --- wd:1 rd:2
[ 205.685972] disk 0, wo:0, o:1, dev:pmem0s
[ 205.687562] disk 1, wo:1, o:1, dev:pmem1s
[ 205.691717] md: recovery of RAID array md0
Fixes: c7bfced9a671 ("md: suspend i/o during runtime blk_integrity_unregister")
Cc: <stable@vger.kernel.org>
Cc: Mike Snitzer <snitzer@redhat.com>
Reported-by: NeilBrown <neilb@suse.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/md/md.c | 28 ++++++++++++++++------------
drivers/md/md.h | 2 +-
drivers/md/multipath.c | 6 +++---
drivers/md/raid1.c | 6 +++---
drivers/md/raid10.c | 6 +++---
5 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 61aacab424cf..b1e1f6b95782 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2017,28 +2017,32 @@ int md_integrity_register(struct mddev *mddev)
}
EXPORT_SYMBOL(md_integrity_register);
-/* Disable data integrity if non-capable/non-matching disk is being added */
-void md_integrity_add_rdev(struct md_rdev *rdev, struct mddev *mddev)
+/*
+ * Attempt to add an rdev, but only if it is consistent with the current
+ * integrity profile
+ */
+int md_integrity_add_rdev(struct md_rdev *rdev, struct mddev *mddev)
{
struct blk_integrity *bi_rdev;
struct blk_integrity *bi_mddev;
+ char name[BDEVNAME_SIZE];
if (!mddev->gendisk)
- return;
+ return 0;
bi_rdev = bdev_get_integrity(rdev->bdev);
bi_mddev = blk_get_integrity(mddev->gendisk);
if (!bi_mddev) /* nothing to do */
- return;
- if (rdev->raid_disk < 0) /* skip spares */
- return;
- if (bi_rdev && blk_integrity_compare(mddev->gendisk,
- rdev->bdev->bd_disk) >= 0)
- return;
- WARN_ON_ONCE(!mddev->suspended);
- printk(KERN_NOTICE "disabling data integrity on %s\n", mdname(mddev));
- blk_integrity_unregister(mddev->gendisk);
+ return 0;
+
+ if (blk_integrity_compare(mddev->gendisk, rdev->bdev->bd_disk) != 0) {
+ printk(KERN_NOTICE "%s: incompatible integrity profile for %s\n",
+ mdname(mddev), bdevname(rdev->bdev, name));
+ return -ENXIO;
+ }
+
+ return 0;
}
EXPORT_SYMBOL(md_integrity_add_rdev);
diff --git a/drivers/md/md.h b/drivers/md/md.h
index ca0b643fe3c1..dfa57b41541b 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -657,7 +657,7 @@ extern void md_wait_for_blocked_rdev(struct md_rdev *rdev, struct mddev *mddev);
extern void md_set_array_sectors(struct mddev *mddev, sector_t array_sectors);
extern int md_check_no_bitmap(struct mddev *mddev);
extern int md_integrity_register(struct mddev *mddev);
-extern void md_integrity_add_rdev(struct md_rdev *rdev, struct mddev *mddev);
+extern int md_integrity_add_rdev(struct md_rdev *rdev, struct mddev *mddev);
extern int strict_strtoul_scaled(const char *cp, unsigned long *res, int scale);
extern void mddev_init(struct mddev *mddev);
diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c
index 7331a80d89f1..0a72ab6e6c20 100644
--- a/drivers/md/multipath.c
+++ b/drivers/md/multipath.c
@@ -257,6 +257,9 @@ static int multipath_add_disk(struct mddev *mddev, struct md_rdev *rdev)
disk_stack_limits(mddev->gendisk, rdev->bdev,
rdev->data_offset << 9);
+ err = md_integrity_add_rdev(rdev, mddev);
+ if (err)
+ break;
spin_lock_irq(&conf->device_lock);
mddev->degraded--;
rdev->raid_disk = path;
@@ -264,9 +267,6 @@ static int multipath_add_disk(struct mddev *mddev, struct md_rdev *rdev)
spin_unlock_irq(&conf->device_lock);
rcu_assign_pointer(p->rdev, rdev);
err = 0;
- mddev_suspend(mddev);
- md_integrity_add_rdev(rdev, mddev);
- mddev_resume(mddev);
break;
}
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index e2169ff6e0f0..c4b913409226 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1589,6 +1589,9 @@ static int raid1_add_disk(struct mddev *mddev, struct md_rdev *rdev)
if (mddev->recovery_disabled == conf->recovery_disabled)
return -EBUSY;
+ if (md_integrity_add_rdev(rdev, mddev))
+ return -ENXIO;
+
if (rdev->raid_disk >= 0)
first = last = rdev->raid_disk;
@@ -1632,9 +1635,6 @@ static int raid1_add_disk(struct mddev *mddev, struct md_rdev *rdev)
break;
}
}
- mddev_suspend(mddev);
- md_integrity_add_rdev(rdev, mddev);
- mddev_resume(mddev);
if (mddev->queue && blk_queue_discard(bdev_get_queue(rdev->bdev)))
queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, mddev->queue);
print_conf(conf);
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 84e597e1c489..ce959b4ae4df 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1698,6 +1698,9 @@ static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev)
if (rdev->saved_raid_disk < 0 && !_enough(conf, 1, -1))
return -EINVAL;
+ if (md_integrity_add_rdev(rdev, mddev))
+ return -ENXIO;
+
if (rdev->raid_disk >= 0)
first = last = rdev->raid_disk;
@@ -1739,9 +1742,6 @@ static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev)
rcu_assign_pointer(p->rdev, rdev);
break;
}
- mddev_suspend(mddev);
- md_integrity_add_rdev(rdev, mddev);
- mddev_resume(mddev);
if (mddev->queue && blk_queue_discard(bdev_get_queue(rdev->bdev)))
queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, mddev->queue);
^ permalink raw reply related
* Re: [dm-devel] [PATCH v2 0/2] Introduce the bulk IV mode for improving the crypto engine efficiency
From: Mikulas Patocka @ 2016-01-13 15:00 UTC (permalink / raw)
To: Arnd Bergmann
Cc: device-mapper development, Mark Brown, Milan Broz, Jens Axboe,
keith.busch, linux-raid, martin.petersen, Mike Snitzer,
Baolin Wang, linux-block, neilb, LKML, sagig, tj, dan.j.williams,
Kent Overstreet, Alasdair G Kergon
In-Reply-To: <49336038.lhIkHu2mkF@wuerfel>
On Wed, 13 Jan 2016, Arnd Bergmann wrote:
> On Tuesday 12 January 2016 21:18:12 Mikulas Patocka wrote:
> > On Wed, 13 Jan 2016, Arnd Bergmann wrote:
> >
> > > On Tuesday 12 January 2016 18:31:19 Mikulas Patocka wrote:
> > > >
> > > > Another possibility is to use dm-crypt block size 4k and use a filesystem
> > > > with 4k blocksize on it (it will never send requests not aligned on 4k
> > > > boundary, so we could reject such requests with an error).
> > >
> > > Is there ever a reason to use something other than 4K block size on
> > > dm-crypt?
> > >
> > > Arnd
> >
> > You can't use 4k block on CBC (and most other encryption modes). If only a
> > part of 4k block is written (and then system crash happens), CBC would
> > corrupt the block completely.
> >
> > For example, suppose that EXT2 directory block is updated, the first
> > 512-byte sector is written and the rest of the sectors is not written
> > because of a crash. CBC would corrupt all sectors except the first one in
> > this case.
> >
> > You could use 4k block on XTS and ECB.
>
> Ah, I did not know that ext2 was doing sub-block writes. This may be
Ext2 is not doing sub-block writes.
Generally, disks and SSDs do not guarantee 4k write atomicity (only disks
with hardware 4k sector guarantee it).
For example, ext2 writes a full 4k block, only part of the block is
written to the disk and then power failure happens. On the next reboot CBC
will corrupt the unwritten part of the 4k block.
Mikulas
> something to address in the ext4 code (and other file systems), as
> a lot of flash storage devices (SD cards and eMMC) get really slow
> when you do writes smaller than 4K because of the internal
> read-modify-write cycle. Ideally you want to always drive those
> using 64K writes (for reads, it doesn't matter much).
>
> For hard drives, there are still a couple of older models that have
> native 512 byte sectors, but the majority of new drivers also
> prefers 4K writes. SSDs are typically optimized for 4K writes because
> that is what they expect software to do, but they use larger writes
> internally.
>
> Arnd
>
^ permalink raw reply
* Re: [dm-devel] [PATCH v2 0/2] Introduce the bulk IV mode for improving the crypto engine efficiency
From: Arnd Bergmann @ 2016-01-13 10:17 UTC (permalink / raw)
To: Mikulas Patocka
Cc: device-mapper development, Mark Brown, Milan Broz, Jens Axboe,
keith.busch, linux-raid, martin.petersen, Mike Snitzer,
Baolin Wang, linux-block, neilb, LKML, sagig, tj, dan.j.williams,
Kent Overstreet, Alasdair G Kergon
In-Reply-To: <alpine.LRH.2.02.1601122113240.24452@file01.intranet.prod.int.rdu2.redhat.com>
On Tuesday 12 January 2016 21:18:12 Mikulas Patocka wrote:
> On Wed, 13 Jan 2016, Arnd Bergmann wrote:
>
> > On Tuesday 12 January 2016 18:31:19 Mikulas Patocka wrote:
> > >
> > > Another possibility is to use dm-crypt block size 4k and use a filesystem
> > > with 4k blocksize on it (it will never send requests not aligned on 4k
> > > boundary, so we could reject such requests with an error).
> >
> > Is there ever a reason to use something other than 4K block size on
> > dm-crypt?
> >
> > Arnd
>
> You can't use 4k block on CBC (and most other encryption modes). If only a
> part of 4k block is written (and then system crash happens), CBC would
> corrupt the block completely.
>
> For example, suppose that EXT2 directory block is updated, the first
> 512-byte sector is written and the rest of the sectors is not written
> because of a crash. CBC would corrupt all sectors except the first one in
> this case.
>
> You could use 4k block on XTS and ECB.
Ah, I did not know that ext2 was doing sub-block writes. This may be
something to address in the ext4 code (and other file systems), as
a lot of flash storage devices (SD cards and eMMC) get really slow
when you do writes smaller than 4K because of the internal
read-modify-write cycle. Ideally you want to always drive those
using 64K writes (for reads, it doesn't matter much).
For hard drives, there are still a couple of older models that have
native 512 byte sectors, but the majority of new drivers also
prefers 4K writes. SSDs are typically optimized for 4K writes because
that is what they expect software to do, but they use larger writes
internally.
Arnd
^ permalink raw reply
* [PATCH] Define _POSIX_C_SOURCE if undefined
From: Khem Raj @ 2016-01-13 8:03 UTC (permalink / raw)
To: linux-raid; +Cc: Khem Raj
typecast second argument of connect() API to use struct sockaddr*
Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
config.c | 3 +++
mdmon.c | 2 +-
msg.c | 2 +-
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c
index c58c8fe..b308b6c 100644
--- a/config.c
+++ b/config.c
@@ -63,6 +63,9 @@
* but may not wrap over lines
*
*/
+#ifndef _POSIX_C_SOURCE
+#define _POSIX_C_SOURCE 200809L
+#endif
#ifndef CONFFILE
#define CONFFILE "/etc/mdadm.conf"
diff --git a/mdmon.c b/mdmon.c
index ee12b7c..e4b73d9 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -235,7 +235,7 @@ static int make_control_sock(char *devname)
addr.sun_family = PF_LOCAL;
strcpy(addr.sun_path, path);
umask(077); /* ensure no world write access */
- if (bind(sfd, &addr, sizeof(addr)) < 0) {
+ if (bind(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
close(sfd);
return -1;
}
diff --git a/msg.c b/msg.c
index 754630b..45cd450 100644
--- a/msg.c
+++ b/msg.c
@@ -170,7 +170,7 @@ int connect_monitor(char *devname)
addr.sun_family = PF_LOCAL;
strcpy(addr.sun_path, path);
- if (connect(sfd, &addr, sizeof(addr)) < 0) {
+ if (connect(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
close(sfd);
return -1;
}
--
2.7.0
^ permalink raw reply related
* [PATCH] Define _POSIX_C_SOURCE if undefined
From: Khem Raj @ 2016-01-13 7:59 UTC (permalink / raw)
To: linux-raid; +Cc: Khem Raj
typecast second argument of connect() API to use struct sockaddr*
Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
config.c | 3 +++
mdmon.c | 2 +-
msg.c | 2 +-
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c
index c58c8fe..b308b6c 100644
--- a/config.c
+++ b/config.c
@@ -63,6 +63,9 @@
* but may not wrap over lines
*
*/
+#ifndef _POSIX_C_SOURCE
+#define _POSIX_C_SOURCE 200809L
+#endif
#ifndef CONFFILE
#define CONFFILE "/etc/mdadm.conf"
diff --git a/mdmon.c b/mdmon.c
index ee12b7c..e4b73d9 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -235,7 +235,7 @@ static int make_control_sock(char *devname)
addr.sun_family = PF_LOCAL;
strcpy(addr.sun_path, path);
umask(077); /* ensure no world write access */
- if (bind(sfd, &addr, sizeof(addr)) < 0) {
+ if (bind(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
close(sfd);
return -1;
}
diff --git a/msg.c b/msg.c
index 754630b..45cd450 100644
--- a/msg.c
+++ b/msg.c
@@ -170,7 +170,7 @@ int connect_monitor(char *devname)
addr.sun_family = PF_LOCAL;
strcpy(addr.sun_path, path);
- if (connect(sfd, &addr, sizeof(addr)) < 0) {
+ if (connect(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
close(sfd);
return -1;
}
--
2.7.0
^ permalink raw reply related
* [PATCH] Define _POSIX_C_SOURCE if undefined
From: Khem Raj @ 2016-01-13 7:51 UTC (permalink / raw)
To: linux-raid; +Cc: Khem Raj
typecast second argument of connect() API to use struct sockaddr*
Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
config.c | 3 +++
mdmon.c | 2 +-
msg.c | 2 +-
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c
index c58c8fe..b308b6c 100644
--- a/config.c
+++ b/config.c
@@ -63,6 +63,9 @@
* but may not wrap over lines
*
*/
+#ifndef _POSIX_C_SOURCE
+#define _POSIX_C_SOURCE 200809L
+#endif
#ifndef CONFFILE
#define CONFFILE "/etc/mdadm.conf"
diff --git a/mdmon.c b/mdmon.c
index ee12b7c..e4b73d9 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -235,7 +235,7 @@ static int make_control_sock(char *devname)
addr.sun_family = PF_LOCAL;
strcpy(addr.sun_path, path);
umask(077); /* ensure no world write access */
- if (bind(sfd, &addr, sizeof(addr)) < 0) {
+ if (bind(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
close(sfd);
return -1;
}
diff --git a/msg.c b/msg.c
index 754630b..45cd450 100644
--- a/msg.c
+++ b/msg.c
@@ -170,7 +170,7 @@ int connect_monitor(char *devname)
addr.sun_family = PF_LOCAL;
strcpy(addr.sun_path, path);
- if (connect(sfd, &addr, sizeof(addr)) < 0) {
+ if (connect(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
close(sfd);
return -1;
}
--
2.7.0
^ permalink raw reply related
* [PATCH] Define _POSIX_C_SOURCE if undefined
From: Khem Raj @ 2016-01-13 7:40 UTC (permalink / raw)
To: linux-raid; +Cc: Khem Raj
typecast second argument of connect() API to use struct sockaddr*
Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
config.c | 3 +++
mdmon.c | 2 +-
msg.c | 2 +-
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c
index c58c8fe..b308b6c 100644
--- a/config.c
+++ b/config.c
@@ -63,6 +63,9 @@
* but may not wrap over lines
*
*/
+#ifndef _POSIX_C_SOURCE
+#define _POSIX_C_SOURCE 200809L
+#endif
#ifndef CONFFILE
#define CONFFILE "/etc/mdadm.conf"
diff --git a/mdmon.c b/mdmon.c
index ee12b7c..e4b73d9 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -235,7 +235,7 @@ static int make_control_sock(char *devname)
addr.sun_family = PF_LOCAL;
strcpy(addr.sun_path, path);
umask(077); /* ensure no world write access */
- if (bind(sfd, &addr, sizeof(addr)) < 0) {
+ if (bind(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
close(sfd);
return -1;
}
diff --git a/msg.c b/msg.c
index 754630b..45cd450 100644
--- a/msg.c
+++ b/msg.c
@@ -170,7 +170,7 @@ int connect_monitor(char *devname)
addr.sun_family = PF_LOCAL;
strcpy(addr.sun_path, path);
- if (connect(sfd, &addr, sizeof(addr)) < 0) {
+ if (connect(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
close(sfd);
return -1;
}
--
2.7.0
^ permalink raw reply related
* Re: [dm-devel] [PATCH v2 0/2] Introduce the bulk IV mode for improving the crypto engine efficiency
From: Milan Broz @ 2016-01-13 7:01 UTC (permalink / raw)
To: Arnd Bergmann, Mikulas Patocka
Cc: device-mapper development, Mark Brown, Milan Broz, Jens Axboe,
keith.busch, linux-raid, martin.petersen, Mike Snitzer,
Baolin Wang, linux-block, neilb, LKML, sagig, tj, dan.j.williams,
Kent Overstreet, Alasdair G Kergon
In-Reply-To: <5514385.nEhTK7fEcU@wuerfel>
On 01/13/2016 12:38 AM, Arnd Bergmann wrote:
> On Tuesday 12 January 2016 18:31:19 Mikulas Patocka wrote:
>>
>> Another possibility is to use dm-crypt block size 4k and use a filesystem
>> with 4k blocksize on it (it will never send requests not aligned on 4k
>> boundary, so we could reject such requests with an error).
>
> Is there ever a reason to use something other than 4K block size on
> dm-crypt?
Most existing sw FDE systems use 512bytes blocks. I would like to see
configurable block size (at least up to 4k) but as Mikulas pointed out
it opens several new problems.
Anyway, I do not see reason why crypto accelerators should not process
these small sectors better - just hw must be designed for it.
Milan
^ permalink raw reply
* Re: [dm-devel] [PATCH v2 0/2] Introduce the bulk IV mode for improving the crypto engine efficiency
From: Mikulas Patocka @ 2016-01-13 2:18 UTC (permalink / raw)
To: Arnd Bergmann
Cc: device-mapper development, Mark Brown, Milan Broz, Jens Axboe,
keith.busch, linux-raid, martin.petersen, Mike Snitzer,
Baolin Wang, linux-block, neilb, LKML, sagig, tj, dan.j.williams,
Kent Overstreet, Alasdair G Kergon
In-Reply-To: <5514385.nEhTK7fEcU@wuerfel>
On Wed, 13 Jan 2016, Arnd Bergmann wrote:
> On Tuesday 12 January 2016 18:31:19 Mikulas Patocka wrote:
> >
> > Another possibility is to use dm-crypt block size 4k and use a filesystem
> > with 4k blocksize on it (it will never send requests not aligned on 4k
> > boundary, so we could reject such requests with an error).
>
> Is there ever a reason to use something other than 4K block size on
> dm-crypt?
>
> Arnd
You can't use 4k block on CBC (and most other encryption modes). If only a
part of 4k block is written (and then system crash happens), CBC would
corrupt the block completely.
For example, suppose that EXT2 directory block is updated, the first
512-byte sector is written and the rest of the sectors is not written
because of a crash. CBC would corrupt all sectors except the first one in
this case.
You could use 4k block on XTS and ECB.
Mikulas
^ permalink raw reply
* Re: [dm-devel] [PATCH v2 0/2] Introduce the bulk IV mode for improving the crypto engine efficiency
From: Mikulas Patocka @ 2016-01-13 2:13 UTC (permalink / raw)
To: Mark Brown
Cc: device-mapper development, Milan Broz, Jens Axboe, keith.busch,
linux-raid, martin.petersen, Mike Snitzer, Baolin Wang,
linux-block, neilb, LKML, sagig, Arnd Bergmann, tj,
dan.j.williams, Kent Overstreet, Alasdair G Kergon
In-Reply-To: <20160112234025.GF6588@sirena.org.uk>
On Tue, 12 Jan 2016, Mark Brown wrote:
> On Tue, Jan 12, 2016 at 06:31:19PM -0500, Mikulas Patocka wrote:
> > On Mon, 4 Jan 2016, Mark Brown wrote:
>
> > > The main thing the out of tree req-dm-crypt code is doing was using a
> > > larger block size which does seem like a reasonable thing to allow
> > > people to tune for performance tradeofffs but I undertand that's a lot
> > > harder to achieve in a good way than one might hope.
>
> > But as Milan pointed out, that larger block size doesn't work if you
> > process requests with different sizes - the data encrypted with one
> > request size won't match if you decrypt them with a different request
> > size.
>
> Sure, you need to fix that block size.
>
> > Does the hardware encryption you are optimizing for allow setting
> > arbitrary tweaks in XTS mode? What is the specific driver you are
> > optimizing for?
>
> This isn't targeted at a specific driver or system, it's trying to make
> dm-crypt better able to make use of hardware acceleration in general.
If the hardware acceleration doesn't allow to set arbitrary XTS tweak,
then this "large block" optimization on XTS can't be done at all.
So, we need to know which driver(s) you want to optimize for and how do
those driver(s) handle tweak generation.
Mikulas
^ permalink raw reply
* Re: [dm-devel] [PATCH v2 0/2] Introduce the bulk IV mode for improving the crypto engine efficiency
From: Mark Brown @ 2016-01-12 23:40 UTC (permalink / raw)
To: Mikulas Patocka
Cc: device-mapper development, Milan Broz, Jens Axboe, keith.busch,
linux-raid, martin.petersen, Mike Snitzer, Baolin Wang,
linux-block, neilb, LKML, sagig, Arnd Bergmann, tj,
dan.j.williams, Kent Overstreet, Alasdair G Kergon
In-Reply-To: <alpine.LRH.2.02.1601121744400.18228@file01.intranet.prod.int.rdu2.redhat.com>
[-- Attachment #1: Type: text/plain, Size: 969 bytes --]
On Tue, Jan 12, 2016 at 06:31:19PM -0500, Mikulas Patocka wrote:
> On Mon, 4 Jan 2016, Mark Brown wrote:
> > The main thing the out of tree req-dm-crypt code is doing was using a
> > larger block size which does seem like a reasonable thing to allow
> > people to tune for performance tradeofffs but I undertand that's a lot
> > harder to achieve in a good way than one might hope.
> But as Milan pointed out, that larger block size doesn't work if you
> process requests with different sizes - the data encrypted with one
> request size won't match if you decrypt them with a different request
> size.
Sure, you need to fix that block size.
> Does the hardware encryption you are optimizing for allow setting
> arbitrary tweaks in XTS mode? What is the specific driver you are
> optimizing for?
This isn't targeted at a specific driver or system, it's trying to make
dm-crypt better able to make use of hardware acceleration in general.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [dm-devel] [PATCH v2 0/2] Introduce the bulk IV mode for improving the crypto engine efficiency
From: Arnd Bergmann @ 2016-01-12 23:38 UTC (permalink / raw)
To: Mikulas Patocka
Cc: device-mapper development, Mark Brown, Milan Broz, Jens Axboe,
keith.busch, linux-raid, martin.petersen, Mike Snitzer,
Baolin Wang, linux-block, neilb, LKML, sagig, tj, dan.j.williams,
Kent Overstreet, Alasdair G Kergon
In-Reply-To: <alpine.LRH.2.02.1601121744400.18228@file01.intranet.prod.int.rdu2.redhat.com>
On Tuesday 12 January 2016 18:31:19 Mikulas Patocka wrote:
>
> Another possibility is to use dm-crypt block size 4k and use a filesystem
> with 4k blocksize on it (it will never send requests not aligned on 4k
> boundary, so we could reject such requests with an error).
Is there ever a reason to use something other than 4K block size on
dm-crypt?
Arnd
^ permalink raw reply
* Re: [dm-devel] [PATCH v2 0/2] Introduce the bulk IV mode for improving the crypto engine efficiency
From: Mikulas Patocka @ 2016-01-12 23:31 UTC (permalink / raw)
To: device-mapper development, Mark Brown
Cc: Milan Broz, Jens Axboe, keith.busch, linux-raid, martin.petersen,
Mike Snitzer, Baolin Wang, linux-block, neilb, LKML, sagig,
Arnd Bergmann, tj, dan.j.williams, Kent Overstreet,
Alasdair G Kergon
In-Reply-To: <20160104201343.GQ16023@sirena.org.uk>
On Mon, 4 Jan 2016, Mark Brown wrote:
> On Sat, Jan 02, 2016 at 11:46:08PM +0100, Milan Broz wrote:
>
> > Anyway, I think that you should optimize driver, not add strange hw-dependent
> > crypto modes to dmcrypt. This is not the first crypto accelerator that is just not
> > suited for this kind of use.
>
> > (If it can process batch of chunks of data each with own IV, then it can work
> > with dmcrypt, but I think such optimized code should be inside crypto API,
> > not in dmcrypt.)
>
> The flip side of this is there is an awful lot of hardware out there
> that has basically this pattern and if we can make the difference
> between people being able to encrypt or not encrypt their storage due to
> performance then that seems like a win. Getting hardware changes isn't
> going to be a fast process. From a brief look at the crypto layer it
> does look there may be things we can do there, if only in terms of
> factoring out the common patterns for driving the queue of operations
> into the hardware so it's easy for drivers to do the best thing.
>
> One thing that occurs to me for the IV programming that has been
> proposed for SPI by Martin Sparl (and seen good results on Raspberry PI)
> is to insert transfers programming the crypto engine into the stream of
> DMA operations so we can keep the hardware busy. It won't work with
> every SoC out there but it will work with a lot of them, it's what
> hardware that explicitly supports this will be doing internally. It's
> the sort of thing that would benefit from factoring out, it's a lot of
> hassle to implement per driver.
>
> The main thing the out of tree req-dm-crypt code is doing was using a
> larger block size which does seem like a reasonable thing to allow
> people to tune for performance tradeofffs but I undertand that's a lot
> harder to achieve in a good way than one might hope.
But as Milan pointed out, that larger block size doesn't work if you
process requests with different sizes - the data encrypted with one
request size won't match if you decrypt them with a different request
size.
XTS with larger block could work if it were possible to use arbitrary
initial tweak - the function crypt() in crypto/xts.c calculates the
initial sector tweak by encrypting the iv:
tw(crypto_cipher_tfm(ctx->tweak), w->iv, w->iv);
and then calculates each cipher block's tweak by multiplying the tweak by
a constant polynomial (alpha):
gf128mul_x_ble(s.t, s.t); (s.t is the same as w->iv)
If we could supply the tweak directly, we could use larger sectors in
dm-crypt.
For example, we could use 64k XTS sectors and if the user is accessing 1k
offset in the sector, we could calculate initial sector tweak
tw(crypto_cipher_tfm(ctx->tweak), w->iv, w->iv);
and then multiply it by alpha^(1024/16) (because we are 1024 bytes into
the sector and xts block size is 16). That would make it possible to use
larger encryption requests and the data would match regardless of request
size.
But the Linux crypto API doesn't allow this - the code that would multiply
the tweak after initial encryption isn't there (maybe we could get this
behavior by modifying ctx->tweak to point to a null cipher, but it is
dirty hack to poke into private crypto structures).
Does the hardware encryption you are optimizing for allow setting
arbitrary tweaks in XTS mode? What is the specific driver you are
optimizing for?
Another possibility is to use dm-crypt block size 4k and use a filesystem
with 4k blocksize on it (it will never send requests not aligned on 4k
boundary, so we could reject such requests with an error).
Mikulas
^ permalink raw reply
* Re: [PATCH] Fix wrong description in manpage
From: NeilBrown @ 2016-01-12 21:26 UTC (permalink / raw)
Cc: linux-raid, Guoqing Jiang
In-Reply-To: <1452611304-14648-1-git-send-email-gqjiang@suse.com>
[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]
On Wed, Jan 13 2016, Guoqing Jiang wrote:
> The careless change was introduce by 'commit 7e6e839a2651
> (mdadm: change the num of cluster node)'. Which should be
> revert to avoid misunderstanding.
>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
> mdadm.8.in | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/mdadm.8.in b/mdadm.8.in
> index 2844039..c2f2c95 100644
> --- a/mdadm.8.in
> +++ b/mdadm.8.in
> @@ -1162,9 +1162,7 @@ is given, a random UUID is chosen.
> The
> .B name
> option will change the
> -.I name
> -of the array as stored in the superblock and bitmap. This option only
> -works for clustered environment.
> +.I name.
>
> The
> .B nodes
Thanks, that patch was messed up.
However your patch only fixes part of the mess that was made.
I've changed it to the following.
Thanks,
NeilBrown
From: Guoqing Jiang <gqjiang@suse.com>
Date: Tue, 12 Jan 2016 18:08:24 +0300
Subject: [PATCH] Fix wrong description in manpage
The careless change was introduce by 'commit 7e6e839a2651
(mdadm: change the num of cluster node)'. Which should be
revert to avoid misunderstanding.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
Signed-off-by: NeilBrown <neilb@suse.com>
diff --git a/mdadm.8.in b/mdadm.8.in
index 2844039584ac..97ce8dc8219b 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -1163,15 +1163,15 @@ The
.B name
option will change the
.I name
-of the array as stored in the superblock and bitmap. This option only
-works for clustered environment.
+of the array as stored in the superblock. This is only supported for
+version-1 superblocks.
The
.B nodes
option will change the
.I nodes
-of the array as stored in the superblock. This is only supported for
-version-1 superblocks.
+of the array as stored in the bitmap superblock. This option only
+works for a clustered environment.
The
.B homehost
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox