* Re: [PATCH 08/22] crypto: chcr: Make use of the new sg_map helper function
From: Harsh Jain @ 2017-04-15 4:53 UTC (permalink / raw)
To: Logan Gunthorpe, harsh-ut6Up61K2wZBDgjK7y7TUQ
Cc: Steve Wise, linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
target-devel-u79uwXL29TY76Z2rM5mHXA, Sumit Semwal,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
linux-scsi-u79uwXL29TY76Z2rM5mHXA, Matthew Wilcox,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, Christoph Hellwig,
fcoe-devel-s9riP+hp16TNLxjTenLetw,
open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
linux-media-u79uwXL29TY76Z2rM5mHXA, Ming Lin,
intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
sparmaintainer-GLv8BlqOqDDQT0dZR+AlfA,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w, Jens Axboe,
linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw, Martin K. Petersen,
Greg Kroah-Hartman, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA, netdev@
In-Reply-To: <1492121135-4437-9-git-send-email-logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
On Fri, Apr 14, 2017 at 3:35 AM, Logan Gunthorpe <logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org> wrote:
> The get_page in this area looks *highly* suspect due to there being no
> corresponding put_page. However, I've left that as is to avoid breaking
> things.
chcr driver will post the request to LLD driver cxgb4 and put_page is
implemented there. it will no harm. Any how
we have removed the below code from driver.
http://www.mail-archive.com/linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg24561.html
After this merge we can ignore your patch. Thanks
>
> I've also removed the KMAP_ATOMIC_ARGS check as it appears to be dead
> code that dates back to when it was first committed...
>
> Signed-off-by: Logan Gunthorpe <logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
> ---
> drivers/crypto/chelsio/chcr_algo.c | 28 +++++++++++++++-------------
> 1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/crypto/chelsio/chcr_algo.c b/drivers/crypto/chelsio/chcr_algo.c
> index 41bc7f4..a993d1d 100644
> --- a/drivers/crypto/chelsio/chcr_algo.c
> +++ b/drivers/crypto/chelsio/chcr_algo.c
> @@ -1489,22 +1489,21 @@ static struct sk_buff *create_authenc_wr(struct aead_request *req,
> return ERR_PTR(-EINVAL);
> }
>
> -static void aes_gcm_empty_pld_pad(struct scatterlist *sg,
> - unsigned short offset)
> +static int aes_gcm_empty_pld_pad(struct scatterlist *sg,
> + unsigned short offset)
> {
> - struct page *spage;
> unsigned char *addr;
>
> - spage = sg_page(sg);
> - get_page(spage); /* so that it is not freed by NIC */
> -#ifdef KMAP_ATOMIC_ARGS
> - addr = kmap_atomic(spage, KM_SOFTIRQ0);
> -#else
> - addr = kmap_atomic(spage);
> -#endif
> - memset(addr + sg->offset, 0, offset + 1);
> + get_page(sg_page(sg)); /* so that it is not freed by NIC */
> +
> + addr = sg_map(sg, SG_KMAP_ATOMIC);
> + if (IS_ERR(addr))
> + return PTR_ERR(addr);
> +
> + memset(addr, 0, offset + 1);
> + sg_unmap(sg, addr, SG_KMAP_ATOMIC);
>
> - kunmap_atomic(addr);
> + return 0;
> }
>
> static int set_msg_len(u8 *block, unsigned int msglen, int csize)
> @@ -1940,7 +1939,10 @@ static struct sk_buff *create_gcm_wr(struct aead_request *req,
> if (req->cryptlen) {
> write_sg_to_skb(skb, &frags, src, req->cryptlen);
> } else {
> - aes_gcm_empty_pld_pad(req->dst, authsize - 1);
> + err = aes_gcm_empty_pld_pad(req->dst, authsize - 1);
> + if (err)
> + goto dstmap_fail;
> +
> write_sg_to_skb(skb, &frags, reqctx->dst, crypt_len);
>
> }
> --
> 2.1.4
>
^ permalink raw reply
* Re: [PATCH 09/22] dm-crypt: Make use of the new sg_map helper in 4 call sites
From: Milan Broz @ 2017-04-15 12:10 UTC (permalink / raw)
To: Logan Gunthorpe, Christoph Hellwig
Cc: Steve Wise, linux-nvme, Stephen Bates, target-devel, Sumit Semwal,
devel, rds-devel, Sagi Grimberg, linux-scsi, Matthew Wilcox,
linux-rdma, fcoe-devel, Ross Zwisler, open-iscsi, linux-media,
Ming Lin, intel-gfx, sparmaintainer, linux-raid, dri-devel,
Dan Williams, megaraidlinux.pdl, Jens Axboe, linaro-mm-sig@
In-Reply-To: <302ae5ab-d515-5427-2e54-d58a9cdb8241@deltatee.com>
On 04/14/2017 06:03 PM, Logan Gunthorpe wrote:
>
>
> On 14/04/17 02:39 AM, Christoph Hellwig wrote:
>> On Thu, Apr 13, 2017 at 04:05:22PM -0600, Logan Gunthorpe wrote:
>>> Very straightforward conversion to the new function in all four spots.
>>
>> I think the right fix here is to switch dm-crypt to the ahash API
>> that takes a scatterlist.
>
> Hmm, well I'm not sure I understand the code enough to make that
> conversion. But I was looking at it. One tricky bit seems to be that
> crypt_iv_lmk_one adds a seed, skips the first 16 bytes in the page and
> then hashes another 16 bytes from other data. What would you do
> construct a new sgl for it and pass it to the ahash api?
>
> The other thing is crypt_iv_lmk_post also seems to modify the page after
> the hash with a crypto_xor so you'd still need at least one kmap in there.
yes, it is in fact modification of CBC mode implemented this hacky way.
These IVs are only for compatibility with loopaes and very old trueCrypt formats.
I think your patch is ok (if it is just plain conversion), if it is
really needed, we can switch to ahash later in follow-up patch.
All common code in dmcrypt uses async API already.
p.s.
there is a lot of lists on cc, but for this patch is missing dm-devel, dmcrypt changes
need to go through Mike's tree (I added dm-devel to cc:)
Milan
^ permalink raw reply
* Re: [PATCH 09/22] dm-crypt: Make use of the new sg_map helper in 4 call sites
From: Logan Gunthorpe @ 2017-04-15 17:47 UTC (permalink / raw)
To: Milan Broz, Christoph Hellwig
Cc: Steve Wise, linux-nvme, Stephen Bates, target-devel, Sumit Semwal,
devel, rds-devel, Sagi Grimberg, linux-scsi, Matthew Wilcox,
linux-rdma, fcoe-devel, Ross Zwisler, open-iscsi, linux-media,
Ming Lin, intel-gfx, sparmaintainer, linux-raid, dri-devel,
Dan Williams, megaraidlinux.pdl, Jens Axboe, linaro-mm-sig@
In-Reply-To: <22703638-27a9-7dc5-5180-536f61661592@gmail.com>
Thanks for the information Milan.
On 15/04/17 06:10 AM, Milan Broz wrote:
> I think your patch is ok (if it is just plain conversion), if it is
> really needed, we can switch to ahash later in follow-up patch.
Sounds good to me.
> p.s.
> there is a lot of lists on cc, but for this patch is missing dm-devel, dmcrypt changes
> need to go through Mike's tree (I added dm-devel to cc:)
Oh, sorry, I thought I had included all the lists. My hope however would
be to get the first patch merged and then re-send the remaining patches
to their respective maintainers. So that would have happened later. It's
hard to manage patches otherwise with such large distribution lists.
Logan
^ permalink raw reply
* Re: [md PATCH] md: handle read-only member devices better.
From: Anthony Youngman @ 2017-04-15 20:22 UTC (permalink / raw)
To: NeilBrown, Shaohua Li; +Cc: Linux-RAID, Nanda Kishore Chinnaram
In-Reply-To: <877f2muvmc.fsf@notabene.neil.brown.name>
On 15/04/17 05:45, NeilBrown wrote:
> So at present, callers of blkdev_get_by_dev() need to do their own
> bdev_read_only() tests before writing.
> We could discuss where in md.c is the best place to put them, but unless
> you want to take on a largish project to 'fix' (or audit) all callers of
> blkdev_get_by_dev(), they need to go in md somewhere.
anthony@crappit:~/gitstuff/linux-stable> git grep -a blkdev_get_by_dev >
blkdev_get_by_dev.txt
anthony@crappit:~/gitstuff/linux-stable> cat blkdev_get_by_dev.txt
drivers/block/xen-blkback/xenbus.c: bdev =
blkdev_get_by_dev(vbd->pdevice, vbd->readonly ?
drivers/md/dm.c: bdev = blkdev_get_by_dev(dev, td->dm_dev.mode |
FMODE_EXCL, _claim_ptr);
drivers/md/md.c: bdev = blkdev_get_by_dev(dev,
FMODE_READ|FMODE_WRITE|FMODE_EXCL,
drivers/mtd/devices/block2mtd.c: bdev =
blkdev_get_by_dev(devt, mode, dev);
fs/block_dev.c: * blkdev_get_by_dev - open a block device by device number
fs/block_dev.c:struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t
mode, void *holder)
fs/block_dev.c:EXPORT_SYMBOL(blkdev_get_by_dev);
fs/ext4/super.c: bdev = blkdev_get_by_dev(dev,
FMODE_READ|FMODE_WRITE|FMODE_EXCL, sb);
fs/f2fs/super.c:
blkdev_get_by_dev(sbi->sb->s_bdev->bd_dev,
fs/jfs/jfs_logmgr.c: bdev = blkdev_get_by_dev(sbi->logdev,
FMODE_READ|FMODE_WRITE|FMODE_EXCL,
fs/nfs/blocklayout/dev.c: bdev = blkdev_get_by_dev(dev, FMODE_READ
| FMODE_WRITE, NULL);
fs/reiserfs/journal.c: journal->j_dev_bd =
blkdev_get_by_dev(jdev, blkdev_mode,
include/linux/fs.h:extern struct block_device *blkdev_get_by_dev(dev_t
dev, fmode_t mode,
kernel/power/swap.c: hib_resume_bdev =
blkdev_get_by_dev(swsusp_resume_device,
anthony@crappit:~/gitstuff/linux-stable>
So, if I'm mad enough to take this on, does that mean going through all
of these checking/fixing them to test for a return code of -EACCESS?
No promises that I will, I keep on wanting to do this sort of stuff and
then life gets in the way, but if I can't do it it'll go on the list of
projects on the raid wiki.
Cheers,
Wol
^ permalink raw reply
* [PATCH] mdadm: retire mdassemble in make everything
From: Coly Li @ 2017-04-16 7:01 UTC (permalink / raw)
To: linux-raid; +Cc: Coly Li, Jes Sorensen
make everything reports no rule to make mdassemble, because mdassemble
is removed from mdadm. This patch removes mdassemble from "everything"
in Makefile, now there is no failure when compiling a static mdadm binary.
Signed-off-by: Coly Li <colyli@suse.de>
Cc: Jes Sorensen <jes.sorensen@gmail.com>
---
Makefile | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile
index a5d2a0a..5655812 100644
--- a/Makefile
+++ b/Makefile
@@ -169,12 +169,10 @@ check_rundir:
fi
everything: all mdadm.static swap_super test_stripe raid6check \
- mdassemble mdassemble.auto mdassemble.static mdassemble.man \
mdadm.Os mdadm.O2 man
everything-test: all mdadm.static swap_super test_stripe \
- mdassemble.auto mdassemble.static mdassemble.man \
mdadm.Os mdadm.O2 man
-# mdadm.uclibc and mdassemble.uclibc don't work on x86-64
+# mdadm.uclibc doesn't work on x86-64
# mdadm.tcc doesn't work..
mdadm : $(OBJS) | check_rundir
@@ -226,9 +224,6 @@ md.man : md.4
mdadm.conf.man : mdadm.conf.5
man -l mdadm.conf.5 > mdadm.conf.man
-mdassemble.man : mdassemble.8
- man -l mdassemble.8 > mdassemble.man
-
raid6check.man : raid6check.8
man -l raid6check.8 > raid6check.man
@@ -293,9 +288,7 @@ test: mdadm mdmon test_stripe swap_super raid6check
clean :
rm -f mdadm mdmon $(OBJS) $(MON_OBJS) $(STATICOBJS) core *.man \
mdadm.tcc mdadm.uclibc mdadm.static *.orig *.porig *.rej *.alt .merge_file_* \
- mdadm.Os mdadm.O2 mdmon.O2 \
- mdassemble mdassemble.static mdassemble.auto mdassemble.uclibc \
- mdassemble.klibc swap_super \
+ mdadm.Os mdadm.O2 mdmon.O2 swap_super \
init.cpio.gz mdadm.uclibc.static test_stripe raid6check raid6check.o mdmon \
mdadm.8
--
2.10.2
^ permalink raw reply related
* Re: [PATCH v5 3/7] raid5-ppl: Partial Parity Log write logging implementation
From: Greg Thelen @ 2017-04-16 22:58 UTC (permalink / raw)
To: Artur Paszkiewicz; +Cc: Shaohua Li, linux-raid
In-Reply-To: <20170309090003.13298-4-artur.paszkiewicz@intel.com>
On Thu, Mar 09 2017, Artur Paszkiewicz wrote:
> Implement the calculation of partial parity for a stripe and PPL write
> logging functionality. The description of PPL is added to the
> documentation. More details can be found in the comments in raid5-ppl.c.
>
> Attach a page for holding the partial parity data to stripe_head.
> Allocate it only if mddev has the MD_HAS_PPL flag set.
>
> Partial parity is the xor of not modified data chunks of a stripe and is
> calculated as follows:
>
> - reconstruct-write case:
> xor data from all not updated disks in a stripe
>
> - read-modify-write case:
> xor old data and parity from all updated disks in a stripe
>
> Implement it using the async_tx API and integrate into raid_run_ops().
> It must be called when we still have access to old data, so do it when
> STRIPE_OP_BIODRAIN is set, but before ops_run_prexor5(). The result is
> stored into sh->ppl_page.
>
> Partial parity is not meaningful for full stripe write and is not stored
> in the log or used for recovery, so don't attempt to calculate it when
> stripe has STRIPE_FULL_WRITE.
>
> Put the PPL metadata structures to md_p.h because userspace tools
> (mdadm) will also need to read/write PPL.
...
> diff --git a/include/uapi/linux/raid/md_p.h b/include/uapi/linux/raid/md_p.h
> index fe2112810c43..d9a1ead867b9 100644
> --- a/include/uapi/linux/raid/md_p.h
> +++ b/include/uapi/linux/raid/md_p.h
> @@ -398,4 +398,31 @@ struct r5l_meta_block {
>
> #define R5LOG_VERSION 0x1
> #define R5LOG_MAGIC 0x6433c509
> +
> +struct ppl_header_entry {
> + __le64 data_sector; /* raid sector of the new data */
> + __le32 pp_size; /* length of partial parity */
> + __le32 data_size; /* length of data */
> + __le32 parity_disk; /* member disk containing parity */
> + __le32 checksum; /* checksum of partial parity data for this
> + * entry (~crc32c) */
> +} __attribute__ ((__packed__));
> +
> +#define PPL_HEADER_SIZE 4096
> +#define PPL_HDR_RESERVED 512
> +#define PPL_HDR_ENTRY_SPACE \
> + (PPL_HEADER_SIZE - PPL_HDR_RESERVED - 4 * sizeof(u32) - sizeof(u64))
PPL_HDR_ENTRY_SPACE is ostensibly a function of the ppl_header structure fields.
Can we use its field types: e.g. __le32 rather than u32.
This fixes klibc build error:
In file included from /klibc/usr/klibc/../include/sys/md.h:30:0,
from /klibc/usr/kinit/do_mounts_md.c:19:
/linux-next/usr/include/linux/raid/md_p.h:414:51: error: 'u32' undeclared here (not in a function)
(PPL_HEADER_SIZE - PPL_HDR_RESERVED - 4 * sizeof(u32) - sizeof(u64))
#define PPL_HDR_ENTRY_SPACE \
- (PPL_HEADER_SIZE - PPL_HDR_RESERVED - 4 * sizeof(u32) - sizeof(u64))
+ (PPL_HEADER_SIZE - PPL_HDR_RESERVED - 4 * sizeof(__le32) - sizeof(__le64))
> +#define PPL_HDR_MAX_ENTRIES \
> + (PPL_HDR_ENTRY_SPACE / sizeof(struct ppl_header_entry))
> +
> +struct ppl_header {
> + __u8 reserved[PPL_HDR_RESERVED];/* reserved space, fill with 0xff */
> + __le32 signature; /* signature (family number of volume) */
> + __le32 padding; /* zero pad */
> + __le64 generation; /* generation number of the header */
> + __le32 entries_count; /* number of entries in entry array */
> + __le32 checksum; /* checksum of the header (~crc32c) */
> + struct ppl_header_entry entries[PPL_HDR_MAX_ENTRIES];
> +} __attribute__ ((__packed__));
> +
> #endif
^ permalink raw reply
* Lost second disk during resync, can I recover?
From: Adam B @ 2017-04-17 0:30 UTC (permalink / raw)
To: linux-raid
I've got a synology ds1812+
Could have sworn I did raid 6, but md says raid5 (dont recall
personalities pre-failure)
Lost disk 2, was rebuilding and disk 6 crashed.
Booting from a fresh machine, and attempting to discover the arrays
(md125 is the target)
root@ds:~# mdadm --assemble --scan
mdadm: /dev/md/DiskStation:2 assembled from 6 drives - not enough to
start the array.
mdadm: /dev/md/1_1 has been started with 7 drives (out of 8).
mdadm: /dev/md/0_1 has been started with 7 drives (out of 8).
mdadm: /dev/md/DiskStation:2 exists - ignoring
mdadm: /dev/md125 assembled from 6 drives - not enough to start the array.
root@ds:~# cat /proc/mdstat
Personalities : [linear] [raid0] [raid1] [raid10] [raid6] [raid5] [raid4]
md126 : active raid1 sda1[0] sdh1[7] sdg1[6] sde1[4] sdd1[3] sdc1[2] sdb1[1]
2490176 blocks [8/7] [UUUUU_UU]
md127 : active raid1 sda2[0] sdb2[7] sdh2[6] sdg2[5] sde2[3] sdd2[2] sdc2[1]
2097088 blocks [8/7] [UUUU_UUU]
md2 : active raid5 sdf3[5]
20478809792 blocks super 1.2 level 5, 64k chunk, algorithm 2
[8/1] [_____U__]
md1 : active raid1 sdf2[0]
2097088 blocks [8/1] [U_______]
md0 : active raid1 sdf1[0]
2490176 blocks [8/1] [U_______]
Attempting to assemble gives the same error:
root@ds:~# mdadm --assemble --force /dev/md125 /dev/sda3 /dev/sdb3
/dev/sdc3 /dev/sdd3 /dev/sde3 /dev/sdg3 /dev/sdh3
mdadm: /dev/md125 assembled from 6 drives - not enough to start the array.
but also yields:
root@ds:~# cat /proc/mdstat
Personalities : [linear] [raid0] [raid1] [raid10] [raid6] [raid5] [raid4]
md125 : inactive sda3[8](S) sdh3[7](S) sdg3[6](S) sde3[4](S)
sdd3[3](S) sdc3[2](S) sdb3[9](S)
20478810915 blocks super 1.2
but:
root@ds:~# mdadm --detail /dev/md125
mdadm: md device /dev/md125 does not appear to be active.
I attempt to run:
root@ds:~# mdadm --manage --run /dev/md125
mdadm: started /dev/md125
root@ds:~# cat /proc/mdstat
Personalities : [linear] [raid0] [raid1] [raid10] [raid6] [raid5] [raid4]
md125 : active raid5 sda3[8] sdh3[7] sdg3[6] sde3[4] sdd3[3] sdc3[2]
20478809792 blocks super 1.2 level 5, 64k chunk, algorithm 2
[8/6] [U_UUU_UU]
Add back in the disk we were rebuilding (disk 2) before we lost disk 6:
root@ds:~# mdadm --manage /dev/md125 --add /dev/sdb3
mdadm: added /dev/sdb3
root@ds:~# cat /proc/mdstat
Personalities : [linear] [raid0] [raid1] [raid10] [raid6] [raid5] [raid4]
md125 : active raid5 sdb3[9](S) sda3[8] sdh3[7] sdg3[6] sde3[4] sdd3[3] sdc3[2]
20478809792 blocks super 1.2 level 5, 64k chunk, algorithm 2
[8/6] [U_UUU_UU]
But nothing is happening, recovery-wise:
root@ds:~# mdadm -D /dev/md125
/dev/md125:
Version : 1.2
Creation Time : Fri Oct 17 22:45:17 2014
Raid Level : raid5
Array Size : 20478809792 (19530.12 GiB 20970.30 GB)
Used Dev Size : 2925544256 (2790.02 GiB 2995.76 GB)
Raid Devices : 8
Total Devices : 7
Persistence : Superblock is persistent
Update Time : Sun Apr 16 16:22:16 2017
State : clean, FAILED
Active Devices : 6
Working Devices : 7
Failed Devices : 0
Spare Devices : 1
Layout : left-symmetric
Chunk Size : 64K
Name : DiskStation:2
UUID : e299685e:5842617d:6d90981b:8f7b91e4
Events : 351742
Number Major Minor RaidDevice State
8 8 3 0 active sync /dev/hda3
1 0 0 1 removed
2 8 35 2 active sync /dev/sdc3
3 8 51 3 active sync /dev/sdd3
4 8 67 4 active sync /dev/sde3
5 0 0 5 removed
6 8 99 6 active sync /dev/sdg3
7 8 115 7 active sync /dev/sdh3
9 8 19 - spare /dev/sdb3
Sooo.. how screwed am I? Can I recover? Detailed drive status from
--examine below:
root@ds:~# cat raid.status
/dev/sda3:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : e299685e:5842617d:6d90981b:8f7b91e4
Name : DiskStation:2
Creation Time : Fri Oct 17 22:45:17 2014
Raid Level : raid5
Raid Devices : 8
Avail Dev Size : 5851088833 (2790.02 GiB 2995.76 GB)
Array Size : 40957619584 (19530.12 GiB 20970.30 GB)
Used Dev Size : 5851088512 (2790.02 GiB 2995.76 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : clean
Device UUID : f6be170f:999d3a14:0911df81:519c5544
Update Time : Sun Apr 16 16:33:10 2017
Checksum : eef101aa - correct
Events : 351744
Layout : left-symmetric
Chunk Size : 64K
Device Role : Active device 0
Array State : A.AAA.AA ('A' == active, '.' == missing)
/dev/sdb3:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : e299685e:5842617d:6d90981b:8f7b91e4
Name : DiskStation:2
Creation Time : Fri Oct 17 22:45:17 2014
Raid Level : raid5
Raid Devices : 8
Avail Dev Size : 5851088833 (2790.02 GiB 2995.76 GB)
Array Size : 40957619584 (19530.12 GiB 20970.30 GB)
Used Dev Size : 5851088512 (2790.02 GiB 2995.76 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : clean
Device UUID : 0f426e67:9501ed59:df7ecf80:36f1bc62
Update Time : Sun Apr 16 16:33:10 2017
Checksum : aa51ab8c - correct
Events : 351744
Layout : left-symmetric
Chunk Size : 64K
Device Role : spare
Array State : A.AAA.AA ('A' == active, '.' == missing)
/dev/sdc3:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : e299685e:5842617d:6d90981b:8f7b91e4
Name : DiskStation:2
Creation Time : Fri Oct 17 22:45:17 2014
Raid Level : raid5
Raid Devices : 8
Avail Dev Size : 5851088833 (2790.02 GiB 2995.76 GB)
Array Size : 40957619584 (19530.12 GiB 20970.30 GB)
Used Dev Size : 5851088512 (2790.02 GiB 2995.76 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : clean
Device UUID : 10494bba:5db57d5f:bbabfc26:7792e5ad
Update Time : Sun Apr 16 16:33:10 2017
Checksum : f415346b - correct
Events : 351744
Layout : left-symmetric
Chunk Size : 64K
Device Role : Active device 2
Array State : A.AAA.AA ('A' == active, '.' == missing)
/dev/sdd3:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : e299685e:5842617d:6d90981b:8f7b91e4
Name : DiskStation:2
Creation Time : Fri Oct 17 22:45:17 2014
Raid Level : raid5
Raid Devices : 8
Avail Dev Size : 5851088833 (2790.02 GiB 2995.76 GB)
Array Size : 40957619584 (19530.12 GiB 20970.30 GB)
Used Dev Size : 5851088512 (2790.02 GiB 2995.76 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : clean
Device UUID : 253ec56c:8d4a1ffa:24abf930:a47b2c9e
Update Time : Sun Apr 16 16:33:10 2017
Checksum : 3b74a738 - correct
Events : 351744
Layout : left-symmetric
Chunk Size : 64K
Device Role : Active device 3
Array State : A.AAA.AA ('A' == active, '.' == missing)
/dev/sde3:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : e299685e:5842617d:6d90981b:8f7b91e4
Name : DiskStation:2
Creation Time : Fri Oct 17 22:45:17 2014
Raid Level : raid5
Raid Devices : 8
Avail Dev Size : 5851088833 (2790.02 GiB 2995.76 GB)
Array Size : 40957619584 (19530.12 GiB 20970.30 GB)
Used Dev Size : 5851088512 (2790.02 GiB 2995.76 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : clean
Device UUID : c3e94c4f:10a5ff16:05fd1967:2fd44d38
Update Time : Sun Apr 16 16:33:10 2017
Checksum : b1e57c5 - correct
Events : 351744
Layout : left-symmetric
Chunk Size : 64K
Device Role : Active device 4
Array State : A.AAA.AA ('A' == active, '.' == missing)
/dev/sdg3:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : e299685e:5842617d:6d90981b:8f7b91e4
Name : DiskStation:2
Creation Time : Fri Oct 17 22:45:17 2014
Raid Level : raid5
Raid Devices : 8
Avail Dev Size : 5851088833 (2790.02 GiB 2995.76 GB)
Array Size : 40957619584 (19530.12 GiB 20970.30 GB)
Used Dev Size : 5851088512 (2790.02 GiB 2995.76 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : clean
Device UUID : 8f0770d8:c3e6df4b:f2ab4b40:06c57ad1
Update Time : Sun Apr 16 16:33:10 2017
Checksum : 3b80570b - correct
Events : 351744
Layout : left-symmetric
Chunk Size : 64K
Device Role : Active device 6
Array State : A.AAA.AA ('A' == active, '.' == missing)
/dev/sdh3:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : e299685e:5842617d:6d90981b:8f7b91e4
Name : DiskStation:2
Creation Time : Fri Oct 17 22:45:17 2014
Raid Level : raid5
Raid Devices : 8
Avail Dev Size : 5851088833 (2790.02 GiB 2995.76 GB)
Array Size : 40957619584 (19530.12 GiB 20970.30 GB)
Used Dev Size : 5851088512 (2790.02 GiB 2995.76 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : clean
Device UUID : fba39acd:5df21885:b9a329c5:cffdf686
Update Time : Sun Apr 16 16:33:10 2017
Checksum : a43e2fa2 - correct
Events : 351744
Layout : left-symmetric
Chunk Size : 64K
Device Role : Active device 7
Array State : A.AAA.AA ('A' == active, '.' == missing)
^ permalink raw reply
* Re: [PATCH 0/4] integrated stat and fstat into utility functions
From: Zhilong Liu @ 2017-04-17 2:23 UTC (permalink / raw)
To: Paul Menzel, Jes.Sorensen; +Cc: linux-raid
In-Reply-To: <1492173114.8709.29.camel@googlemail.com>
On 04/14/2017 08:31 PM, Paul Menzel wrote:
> Dear Zhilong,
>
>
> Am Samstag, den 01.04.2017, 20:51 +0800 schrieb Zhilong Liu:
>
> […]
>
>> Zhilong Liu (4):
>> mdadm/util:integrate stat operations into one utility
>> mdadm/util:integrate fstat operations into one utility function
>> mdadm/Create:declaring an existing struct within same function
>> mdadm/Monitor:check the block device when use waitclean parameter
> Just a small wish, could you please add exactly one space between the
> “subsystem prefix” and the summary?
>
> Like *mdadm/util: Integrate stat …*. That’d be great.
>
> […]
Thanks very much for your suggestion. :-).
Best regards,
Zhilong
>
> Thanks,
>
> Paul
^ permalink raw reply
* Re: Lost second disk during resync, can I recover?
From: Reindl Harald @ 2017-04-17 6:40 UTC (permalink / raw)
To: Adam B, linux-raid
In-Reply-To: <CABM35JeXN_+coqggH4JDxCvvYAeqD4jt0FHHY=ykMMRofdQDJQ@mail.gmail.com>
Am 17.04.2017 um 02:30 schrieb Adam B:
> I've got a synology ds1812+
> Could have sworn I did raid 6, but md says raid5 (dont recall
> personalities pre-failure)
> Lost disk 2, was rebuilding and disk 6 crashed
> But nothing is happening, recovery-wise:
>
> root@ds:~# mdadm -D /dev/md125
> /dev/md125:
> Version : 1.2
> Creation Time : Fri Oct 17 22:45:17 2014
> Raid Level : raid5
if you lost 2 disks on RAID5 you are done
^ permalink raw reply
* Re: Lost second disk during resync, can I recover?
From: Eyal Lebedinsky @ 2017-04-17 6:52 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <CABM35JeXN_+coqggH4JDxCvvYAeqD4jt0FHHY=ykMMRofdQDJQ@mail.gmail.com>
On 17/04/17 10:30, Adam B wrote:
> I've got a synology ds1812+
> Could have sworn I did raid 6, but md says raid5 (dont recall
> personalities pre-failure)
> Lost disk 2, was rebuilding and disk 6 crashed.
>
> Booting from a fresh machine, and attempting to discover the arrays
> (md125 is the target)
>
> root@ds:~# mdadm --assemble --scan
> mdadm: /dev/md/DiskStation:2 assembled from 6 drives - not enough to
> start the array.
> mdadm: /dev/md/1_1 has been started with 7 drives (out of 8).
> mdadm: /dev/md/0_1 has been started with 7 drives (out of 8).
> mdadm: /dev/md/DiskStation:2 exists - ignoring
> mdadm: /dev/md125 assembled from 6 drives - not enough to start the array.
> root@ds:~# cat /proc/mdstat
> Personalities : [linear] [raid0] [raid1] [raid10] [raid6] [raid5] [raid4]
> md126 : active raid1 sda1[0] sdh1[7] sdg1[6] sde1[4] sdd1[3] sdc1[2] sdb1[1]
> 2490176 blocks [8/7] [UUUUU_UU]
>
> md127 : active raid1 sda2[0] sdb2[7] sdh2[6] sdg2[5] sde2[3] sdd2[2] sdc2[1]
> 2097088 blocks [8/7] [UUUU_UUU]
>
> md2 : active raid5 sdf3[5]
> 20478809792 blocks super 1.2 level 5, 64k chunk, algorithm 2
> [8/1] [_____U__]
>
> md1 : active raid1 sdf2[0]
> 2097088 blocks [8/1] [U_______]
>
> md0 : active raid1 sdf1[0]
> 2490176 blocks [8/1] [U_______]
>
> Attempting to assemble gives the same error:
> root@ds:~# mdadm --assemble --force /dev/md125 /dev/sda3 /dev/sdb3
> /dev/sdc3 /dev/sdd3 /dev/sde3 /dev/sdg3 /dev/sdh3
> mdadm: /dev/md125 assembled from 6 drives - not enough to start the array.
>
> but also yields:
> root@ds:~# cat /proc/mdstat
> Personalities : [linear] [raid0] [raid1] [raid10] [raid6] [raid5] [raid4]
> md125 : inactive sda3[8](S) sdh3[7](S) sdg3[6](S) sde3[4](S)
> sdd3[3](S) sdc3[2](S) sdb3[9](S)
> 20478810915 blocks super 1.2
>
> but:
>
> root@ds:~# mdadm --detail /dev/md125
> mdadm: md device /dev/md125 does not appear to be active.
>
> I attempt to run:
> root@ds:~# mdadm --manage --run /dev/md125
> mdadm: started /dev/md125
> root@ds:~# cat /proc/mdstat
> Personalities : [linear] [raid0] [raid1] [raid10] [raid6] [raid5] [raid4]
> md125 : active raid5 sda3[8] sdh3[7] sdg3[6] sde3[4] sdd3[3] sdc3[2]
> 20478809792 blocks super 1.2 level 5, 64k chunk, algorithm 2
> [8/6] [U_UUU_UU]
>
> Add back in the disk we were rebuilding (disk 2) before we lost disk 6:
> root@ds:~# mdadm --manage /dev/md125 --add /dev/sdb3
When I have a disk kicked out, after a plug out/in I reintroduce it with
# mdadm --re-add /dev/md125 /dev/sdb3
This triggers a recovery which is often very quick (assuming you have a bitmap).
This, of course, does not work when the disk is physically fully dead, a rare event
for me.
[trim]
--
Eyal Lebedinsky (eyal@eyal.emu.id.au)
^ permalink raw reply
* Re: [PATCH 1/4] mdadm/util:integrate stat operations into one utility
From: Liu Zhilong @ 2017-04-17 7:08 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid
In-Reply-To: <e41b85e9-53ff-5f94-f80d-02ebd4e36238@gmail.com>
On 04/14/2017 11:20 PM, Jes Sorensen wrote:
> On 04/14/2017 06:14 AM, Liu Zhilong wrote:
>>
>>
>> On 04/05/2017 11:42 PM, jes.sorensen@gmail.com wrote:
>>> Zhilong Liu <zlliu@suse.com> writes:
> [snip]
>>> For a function like this, lets name it better md_is_blkdev()
>>
>> How about return the devid after checking? Because always need the
>> stb.st_rdev to parse
>> the major and minor number. Although "util.c" has "devnm2devid" to
>> gather the devid via
>> to devnm, but it's convenient to return devid when check the blkdev with
>> absolute path.
>> would you mind the function like this?
>>
>> // returns dev-id when success, return 0 when failure
>> dev_t stat_md_is_blkdev(char *dev)
>> {
>> struct stat stb;
>>
>> if (stat(dev, &stb) != 0) {
>> pr_err("stat failed for %s: %s\n", dev, strerror(errno));
>> return 0;
>> }
>> if ((S_IFMT & stb.st_mode) != S_IFBLK) {
>> pr_err("%s is not a block device.\n", dev);
>> return 0;
>> }
>> return stb.st_rdev;
>> }
>
> I am generally wary of too many smart handlers, but I think it makes
> some sense here to de-duplicate the repeated code.
>
> That said, your function would ditch the error information, and the
> caller wouldn't know why it failed. If you made it more like this and
> return the error code, plus stick the major/minor number into rdev, if
> one is provided:
>
> int stat_md_is_blkdev(char *devname, *dev_t rdev)
> {
> }
>
I want to integrate two situations into one function.
1. only check the 'dev'(such as /dev/loop2) whether or not is block device.
2. optionally return the rdev-id after checking the block device.
this sample "int stat_md_is_blkdev(char *devname, *dev_t rdev)" is smart
for the situation 2. but for situation 1, the second parameter is a
little waste.
how about like this?
dev_t stat_md_is_blkdev(char *devname, bool require_rdev)
{
struct stat stb;
if (stat(devname, &stb) != 0) {
printf("stat failed for %s.\n", devname);
return 1;
}
if ((S_IFMT & stb.st_mode) != S_IFBLK) {
printf("%s is not a block device.\n", devname);
return 1;
}
if (require_rdev)
return stb.st_rdev;
return 0;
}
the caller would be like this,
situation 1:
char dev[20] = "/dev/loop2";
if (stat_md_is_blkdev(dev, false))
return 1;
situation 2:
dev_t st_rdev;
char dev[20] = "/dev/loop2";
st_rdev = stat_md_is_blkdev(dev, true);
if (st_rdev == 1)
return 1;
Thanks very much,
-Zhilong
> Jes
>
>
^ permalink raw reply
* Re: [PATCH 1/4] mdadm/util:integrate stat operations into one utility
From: Zhilong Liu @ 2017-04-17 7:18 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid
In-Reply-To: <db0ed456-77cb-2188-24f7-a970a5a6be80@suse.com>
On 04/17/2017 03:08 PM, Liu Zhilong wrote:
>
>
> On 04/14/2017 11:20 PM, Jes Sorensen wrote:
>> On 04/14/2017 06:14 AM, Liu Zhilong wrote:
>>>
>>>
>>> On 04/05/2017 11:42 PM, jes.sorensen@gmail.com wrote:
>>>> Zhilong Liu <zlliu@suse.com> writes:
>> [snip]
>>>> For a function like this, lets name it better md_is_blkdev()
>>>
>>> How about return the devid after checking? Because always need the
>>> stb.st_rdev to parse
>>> the major and minor number. Although "util.c" has "devnm2devid" to
>>> gather the devid via
>>> to devnm, but it's convenient to return devid when check the blkdev
>>> with
>>> absolute path.
>>> would you mind the function like this?
>>>
>>> // returns dev-id when success, return 0 when failure
>>> dev_t stat_md_is_blkdev(char *dev)
>>> {
>>> struct stat stb;
>>>
>>> if (stat(dev, &stb) != 0) {
>>> pr_err("stat failed for %s: %s\n", dev, strerror(errno));
>>> return 0;
>>> }
>>> if ((S_IFMT & stb.st_mode) != S_IFBLK) {
>>> pr_err("%s is not a block device.\n", dev);
>>> return 0;
>>> }
>>> return stb.st_rdev;
>>> }
>>
>> I am generally wary of too many smart handlers, but I think it makes
>> some sense here to de-duplicate the repeated code.
>>
>> That said, your function would ditch the error information, and the
>> caller wouldn't know why it failed. If you made it more like this and
>> return the error code, plus stick the major/minor number into rdev,
>> if one is provided:
>>
>> int stat_md_is_blkdev(char *devname, *dev_t rdev)
>> {
>> }
>>
>
> I want to integrate two situations into one function.
> 1. only check the 'dev'(such as /dev/loop2) whether or not is block
> device.
> 2. optionally return the rdev-id after checking the block device.
> this sample "int stat_md_is_blkdev(char *devname, *dev_t rdev)" is
> smart for the situation 2. but for situation 1, the second parameter
> is a little waste.
>
> how about like this?
> dev_t stat_md_is_blkdev(char *devname, bool require_rdev)
> {
> struct stat stb;
>
> if (stat(devname, &stb) != 0) {
> printf("stat failed for %s.\n", devname);
in addition: the errno wouldn't be missed in the final patch.
pr_err("stat failed for %s: %s\n", dev, strerror(errno));
Thanks,
-Zhilong
> return 1;
> }
> if ((S_IFMT & stb.st_mode) != S_IFBLK) {
> printf("%s is not a block device.\n", devname);
> return 1;
> }
> if (require_rdev)
> return stb.st_rdev;
> return 0;
> }
>
> the caller would be like this,
> situation 1:
> char dev[20] = "/dev/loop2";
> if (stat_md_is_blkdev(dev, false))
> return 1;
>
> situation 2:
> dev_t st_rdev;
> char dev[20] = "/dev/loop2";
> st_rdev = stat_md_is_blkdev(dev, true);
> if (st_rdev == 1)
> return 1;
>
>
> Thanks very much,
> -Zhilong
>
>> Jes
>>
>>
>
> --
> 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
>
^ permalink raw reply
* Re: Lost second disk during resync, can I recover?
From: Roman Mamedov @ 2017-04-17 9:08 UTC (permalink / raw)
To: Adam B; +Cc: linux-raid
In-Reply-To: <CABM35JeXN_+coqggH4JDxCvvYAeqD4jt0FHHY=ykMMRofdQDJQ@mail.gmail.com>
On Sun, 16 Apr 2017 17:30:26 -0700
Adam B <adam.backer@gmail.com> wrote:
> I've got a synology ds1812+
> Could have sworn I did raid 6, but md says raid5 (dont recall
> personalities pre-failure)
> Lost disk 2, was rebuilding and disk 6 crashed.
>
> Booting from a fresh machine, and attempting to discover the arrays
> (md125 is the target)
>
> root@ds:~# mdadm --assemble --scan
> mdadm: /dev/md/DiskStation:2 assembled from 6 drives - not enough to
> start the array.
> mdadm: /dev/md/1_1 has been started with 7 drives (out of 8).
> mdadm: /dev/md/0_1 has been started with 7 drives (out of 8).
> mdadm: /dev/md/DiskStation:2 exists - ignoring
> mdadm: /dev/md125 assembled from 6 drives - not enough to start the array.
> root@ds:~# cat /proc/mdstat
> Personalities : [linear] [raid0] [raid1] [raid10] [raid6] [raid5] [raid4]
> md126 : active raid1 sda1[0] sdh1[7] sdg1[6] sde1[4] sdd1[3] sdc1[2] sdb1[1]
> 2490176 blocks [8/7] [UUUUU_UU]
>
> md127 : active raid1 sda2[0] sdb2[7] sdh2[6] sdg2[5] sde2[3] sdd2[2] sdc2[1]
> 2097088 blocks [8/7] [UUUU_UUU]
>
> md2 : active raid5 sdf3[5]
> 20478809792 blocks super 1.2 level 5, 64k chunk, algorithm 2
> [8/1] [_____U__]
From a quick glance it's not clear what is sdf3, why it's got an array of its
own, and why later you don't try to do anything with it (didn't try re-add,
neither list in assemble, nor examine). Isn't it that one more drive you need
(even if partially bad) that would let you start the RAID5?
Also perhaps before being able to do anything with it you will need to stop
its "md2" array.
--
With respect,
Roman
^ permalink raw reply
* [PATCH] md/raid10: wait up frozen array in handle_write_completed
From: Guoqing Jiang @ 2017-04-17 9:11 UTC (permalink / raw)
To: linux-raid; +Cc: shli, neilb
Since nr_queued is changed, we need to call wake_up here
if the array is already frozen and waiting for condition
"nr_pending == nr_queued + extra" to be true.
And commit 824e47daddbf ("RAID1: avoid unnecessary spin
locks in I/O barrier code") which has already added the
wake_up for raid1.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
drivers/md/raid10.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 4167091..acb3f46 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -2704,6 +2704,11 @@ static void handle_write_completed(struct r10conf *conf, struct r10bio *r10_bio)
list_add(&r10_bio->retry_list, &conf->bio_end_io_list);
conf->nr_queued++;
spin_unlock_irq(&conf->device_lock);
+ /*
+ * In case freeze_array() is waiting for condition
+ * nr_pending == nr_queued + extra to be true.
+ */
+ wake_up(&conf->wait_barrier);
md_wakeup_thread(conf->mddev->thread);
} else {
if (test_bit(R10BIO_WriteError,
--
2.10.0
^ permalink raw reply related
* [PATCH RESEND] dm ioctl: Remove double parentheses
From: Matthias Kaehlcke @ 2017-04-17 18:05 UTC (permalink / raw)
To: Alasdair Kergon, Mike Snitzer, dm-devel
Cc: Shaohua Li, linux-raid, linux-kernel, Grant Grundler,
Greg Hackmann, Michael Davidson, Matthias Kaehlcke
The extra pair of parantheses is not needed and causes clang to generate
the following warning:
drivers/md/dm-ioctl.c:1776:11: error: equality comparison with extraneous parentheses [-Werror,-Wparentheses-equality]
if ((cmd == DM_DEV_CREATE_CMD)) {
~~~~^~~~~~~~~~~~~~~~~~~~
drivers/md/dm-ioctl.c:1776:11: note: remove extraneous parentheses around the comparison to silence this warning
if ((cmd == DM_DEV_CREATE_CMD)) {
~ ^ ~
drivers/md/dm-ioctl.c:1776:11: note: use '=' to turn this equality comparison into an assignment
if ((cmd == DM_DEV_CREATE_CMD)) {
^~
=
Also remove another double parentheses that don't cause a warning.
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
drivers/md/dm-ioctl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 4da6fc6b1ffd..acd8f12439e4 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1778,12 +1778,12 @@ static int validate_params(uint cmd, struct dm_ioctl *param)
cmd == DM_LIST_VERSIONS_CMD)
return 0;
- if ((cmd == DM_DEV_CREATE_CMD)) {
+ if (cmd == DM_DEV_CREATE_CMD) {
if (!*param->name) {
DMWARN("name not supplied when creating device");
return -EINVAL;
}
- } else if ((*param->uuid && *param->name)) {
+ } else if (*param->uuid && *param->name) {
DMWARN("only supply one of name or uuid, cmd(%u)", cmd);
return -EINVAL;
}
--
2.12.2.762.g0e3151a226-goog
^ permalink raw reply related
* Zombie dm device?
From: Phil Susi @ 2017-04-17 19:50 UTC (permalink / raw)
To: linux-raid
After running the parted test suite I seem to have a zombie md device:
root@devserv:~# mdadm -D /dev/md99
/dev/md99:
Version :
Raid Level : raid0
Total Devices : 0
State : inactive
Number Major Minor RaidDevice
Why is this thing still hanging around, and how do you get rid of it? I
tried mdadm --stop /dev/md99 but it didn't get rid of it. I also can't
find any mdadm switch or a delete file or similar in the sysfs node.
root@devserv:~# uname -a
Linux devserv 4.4.0-72-generic #93-Ubuntu SMP Fri Mar 31 14:07:41 UTC
2017 x86_64 x86_64 x86_64 GNU/Linux
^ permalink raw reply
* Re: [PATCH v2] md/r5cache: gracefully handle journal device errors for writeback mode
From: Song Liu @ 2017-04-17 20:18 UTC (permalink / raw)
To: Shaohua Li
Cc: linux-raid, Shaohua Li, NeilBrown, Kernel Team, Dan Williams,
Christoph Hellwig, jes.sorensen@gmail.com
In-Reply-To: <20170410162149.due7ihii7ibrz5pl@kernel.org>
> On Apr 10, 2017, at 9:21 AM, Shaohua Li <shli@kernel.org> wrote:
>
> On Wed, Mar 29, 2017 at 01:00:13AM -0700, Song Liu wrote:
>> For the raid456 with writeback cache, when journal device failed during
>> normal operation, it is still possible to persist all data, as all
>> pending data is still in stripe cache. However, it is necessary to handle
>> journal failure gracefully.
>>
>> During journal failures, this patch makes the follow changes to land data
>> in cache to raid disks gracefully:
>>
>> 1. In raid5_remove_disk(), flush all cached stripes;
>> 2. In handle_stripe(), allow stripes with data in journal (s.injournal > 0)
>> to make progress;
>> 3. In delay_towrite(), only process data in the cache (skip dev->towrite);
>> 4. In __get_priority_stripe(), set try_loprio to true, so no stripe stuck
>> in loprio_list
>> 5. In r5l_do_submit_io(), submit io->split_bio first (see inline comments
>> for details).
>> Signed-off-by: Song Liu <songliubraving@fb.com>
>> ---
>> drivers/md/raid5-cache.c | 27 ++++++++++++++++++---------
>> drivers/md/raid5.c | 28 ++++++++++++++++++++++++----
>> 2 files changed, 42 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
>> index b6194e0..0838617 100644
>> --- a/drivers/md/raid5-cache.c
>> +++ b/drivers/md/raid5-cache.c
>> @@ -632,20 +632,29 @@ static void r5l_do_submit_io(struct r5l_log *log, struct r5l_io_unit *io)
>> __r5l_set_io_unit_state(io, IO_UNIT_IO_START);
>> spin_unlock_irqrestore(&log->io_list_lock, flags);
>>
>> + /*
>> + * In case of journal device failures, submit_bio will get error
>> + * and calls endio, then active stripes will continue write
>> + * process. Therefore, it is not necessary to check Faulty bit
>> + * of journal device here.
>> + *
>> + * However, calling r5l_log_endio(current_bio) may change
>> + * split_bio. Therefore, it is necessary to check split_bio before
>> + * submit current_bio.
>> + */
>
> sorry, for the delay. what did you mean 'calling r5l_log_endio may change
> split_bio'? The split_bio is chained into current_bio. The endio of
> current_bio(r5l_log_endio) is only called after all chained bio completion. I
> didn't get the point why this change.
This happens when io->split_bio is NULL. In such cases, r5l_log_endio() may
free the io_unit, and thus change io->split_bio. I will revise the comments.
>
>> + if (io->split_bio) {
>> + if (io->has_flush)
>> + io->split_bio->bi_opf |= REQ_PREFLUSH;
>> + if (io->has_fua)
>> + io->split_bio->bi_opf |= REQ_FUA;
>> + submit_bio(io->split_bio);
>> + }
>> +
>> if (io->has_flush)
>> io->current_bio->bi_opf |= REQ_PREFLUSH;
>> if (io->has_fua)
>> io->current_bio->bi_opf |= REQ_FUA;
>> submit_bio(io->current_bio);
>> -
>> - if (!io->split_bio)
>> - return;
>> -
>> - if (io->has_flush)
>> - io->split_bio->bi_opf |= REQ_PREFLUSH;
>> - if (io->has_fua)
>> - io->split_bio->bi_opf |= REQ_FUA;
>> - submit_bio(io->split_bio);
>> }
>>
>> /* deferred io_unit will be dispatched here */
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index 6036d5e..4d3d1ab 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -3054,6 +3054,11 @@ sector_t raid5_compute_blocknr(struct stripe_head *sh, int i, int previous)
>> * When LOG_CRITICAL, stripes with injournal == 0 will be sent to
>> * no_space_stripes list.
>> *
>> + * 3. during journal failure
>> + * In journal failure, we try to flush all cached data to raid disks
>> + * based on data in stripe cache. The array is read-only to upper
>> + * layers, so we would skip all pending writes.
>> + *
>> */
>> static inline bool delay_towrite(struct r5conf *conf,
>> struct r5dev *dev,
>> @@ -3067,6 +3072,9 @@ static inline bool delay_towrite(struct r5conf *conf,
>> if (test_bit(R5C_LOG_CRITICAL, &conf->cache_state) &&
>> s->injournal > 0)
>> return true;
>> + /* case 3 above */
>> + if (s->log_failed && s->injournal)
>> + return true;
>> return false;
>> }
>>
>> @@ -4689,10 +4697,15 @@ static void handle_stripe(struct stripe_head *sh)
>> " to_write=%d failed=%d failed_num=%d,%d\n",
>> s.locked, s.uptodate, s.to_read, s.to_write, s.failed,
>> s.failed_num[0], s.failed_num[1]);
>> - /* check if the array has lost more than max_degraded devices and,
>> + /*
>> + * check if the array has lost more than max_degraded devices and,
>> * if so, some requests might need to be failed.
>> + *
>> + * When journal device failed (log_failed), we will only process
>> + * the stripe if there is data need write to raid disks
>> */
>> - if (s.failed > conf->max_degraded || s.log_failed) {
>> + if (s.failed > conf->max_degraded ||
>> + (s.log_failed && s.injournal == 0)) {
>> sh->check_state = 0;
>> sh->reconstruct_state = 0;
>> break_stripe_batch_list(sh, 0);
>> @@ -5272,7 +5285,8 @@ static struct stripe_head *__get_priority_stripe(struct r5conf *conf, int group)
>> struct list_head *handle_list = NULL;
>> struct r5worker_group *wg;
>> bool second_try = !r5c_is_writeback(conf->log);
>> - bool try_loprio = test_bit(R5C_LOG_TIGHT, &conf->cache_state);
>> + bool try_loprio = test_bit(R5C_LOG_TIGHT, &conf->cache_state) ||
>> + r5l_log_disk_error(conf);
>>
>> again:
>> wg = NULL;
>> @@ -7526,6 +7540,7 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
>> int number = rdev->raid_disk;
>> struct md_rdev **rdevp;
>> struct disk_info *p = conf->disks + number;
>> + unsigned long flags;
>>
>> print_raid5_conf(conf);
>> if (test_bit(Journal, &rdev->flags) && conf->log) {
>> @@ -7535,7 +7550,12 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
>> * neilb: there is no locking about new writes here,
>> * so this cannot be safe.
>> */
>> - if (atomic_read(&conf->active_stripes)) {
>> + if (atomic_read(&conf->active_stripes) ||
>> + atomic_read(&conf->r5c_cached_full_stripes) ||
>> + atomic_read(&conf->r5c_cached_partial_stripes)) {
>> + spin_lock_irqsave(&conf->device_lock, flags);
>> + r5c_flush_cache(conf, INT_MAX);
>> + spin_unlock_irqrestore(&conf->device_lock, flags);
>> return -EBUSY;
>
> It's weird this is called in raid5_remove_disk, shouldn't this be called in log
> disk error in case user doesn't remove the log disk? And this is a policy
> change. User might not want to do the flush, as this exposes write hole. I
> think at least we should print info out here to warn user the flush.
Yeah, it is better to flush cache in in raid5_error(). Let me try that.
I think flushing cache here is the best solution so far, as valid data is only
stored in DRAM before the flush. I will add a warning.
Thanks,
Song
^ permalink raw reply
* Re: [PATCH] md/raid10: wait up frozen array in handle_write_completed
From: NeilBrown @ 2017-04-17 23:04 UTC (permalink / raw)
To: Guoqing Jiang, linux-raid; +Cc: shli
In-Reply-To: <20170417091105.27180-1-gqjiang@suse.com>
[-- Attachment #1: Type: text/plain, Size: 1454 bytes --]
On Mon, Apr 17 2017, Guoqing Jiang wrote:
> Since nr_queued is changed, we need to call wake_up here
> if the array is already frozen and waiting for condition
> "nr_pending == nr_queued + extra" to be true.
>
> And commit 824e47daddbf ("RAID1: avoid unnecessary spin
> locks in I/O barrier code") which has already added the
> wake_up for raid1.
>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
> drivers/md/raid10.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 4167091..acb3f46 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -2704,6 +2704,11 @@ static void handle_write_completed(struct r10conf *conf, struct r10bio *r10_bio)
> list_add(&r10_bio->retry_list, &conf->bio_end_io_list);
> conf->nr_queued++;
> spin_unlock_irq(&conf->device_lock);
> + /*
> + * In case freeze_array() is waiting for condition
> + * nr_pending == nr_queued + extra to be true.
> + */
> + wake_up(&conf->wait_barrier);
> md_wakeup_thread(conf->mddev->thread);
> } else {
> if (test_bit(R10BIO_WriteError,
> --
> 2.10.0
Reviewed-by: NeilBrown <neilb@suse.com>
I think I looked at this once before and convinced myself that
md_wakeup_thread() would wake up the only thread that might be waiting
in freeze_array(). This is completely wrong, so I don't know why I
thought it:-(
Thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [md PATCH] md: handle read-only member devices better.
From: NeilBrown @ 2017-04-17 23:31 UTC (permalink / raw)
To: Anthony Youngman, Shaohua Li; +Cc: Linux-RAID, Nanda Kishore Chinnaram
In-Reply-To: <006d05e1-2517-ca5c-00b4-c699a5bd793d@youngman.org.uk>
[-- Attachment #1: Type: text/plain, Size: 2996 bytes --]
On Sat, Apr 15 2017, Anthony Youngman wrote:
> On 15/04/17 05:45, NeilBrown wrote:
>> So at present, callers of blkdev_get_by_dev() need to do their own
>> bdev_read_only() tests before writing.
>> We could discuss where in md.c is the best place to put them, but unless
>> you want to take on a largish project to 'fix' (or audit) all callers of
>> blkdev_get_by_dev(), they need to go in md somewhere.
>
> anthony@crappit:~/gitstuff/linux-stable> git grep -a blkdev_get_by_dev >
> blkdev_get_by_dev.txt
> anthony@crappit:~/gitstuff/linux-stable> cat blkdev_get_by_dev.txt
> drivers/block/xen-blkback/xenbus.c: bdev =
> blkdev_get_by_dev(vbd->pdevice, vbd->readonly ?
> drivers/md/dm.c: bdev = blkdev_get_by_dev(dev, td->dm_dev.mode |
> FMODE_EXCL, _claim_ptr);
> drivers/md/md.c: bdev = blkdev_get_by_dev(dev,
> FMODE_READ|FMODE_WRITE|FMODE_EXCL,
> drivers/mtd/devices/block2mtd.c: bdev =
> blkdev_get_by_dev(devt, mode, dev);
> fs/block_dev.c: * blkdev_get_by_dev - open a block device by device number
> fs/block_dev.c:struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t
> mode, void *holder)
> fs/block_dev.c:EXPORT_SYMBOL(blkdev_get_by_dev);
> fs/ext4/super.c: bdev = blkdev_get_by_dev(dev,
> FMODE_READ|FMODE_WRITE|FMODE_EXCL, sb);
> fs/f2fs/super.c:
> blkdev_get_by_dev(sbi->sb->s_bdev->bd_dev,
> fs/jfs/jfs_logmgr.c: bdev = blkdev_get_by_dev(sbi->logdev,
> FMODE_READ|FMODE_WRITE|FMODE_EXCL,
> fs/nfs/blocklayout/dev.c: bdev = blkdev_get_by_dev(dev, FMODE_READ
> | FMODE_WRITE, NULL);
> fs/reiserfs/journal.c: journal->j_dev_bd =
> blkdev_get_by_dev(jdev, blkdev_mode,
> include/linux/fs.h:extern struct block_device *blkdev_get_by_dev(dev_t
> dev, fmode_t mode,
> kernel/power/swap.c: hib_resume_bdev =
> blkdev_get_by_dev(swsusp_resume_device,
> anthony@crappit:~/gitstuff/linux-stable>
>
> So, if I'm mad enough to take this on, does that mean going through all
> of these checking/fixing them to test for a return code of -EACCESS?
Yes, where "fixing" means understanding how the bdev is used in
each case, and what mode is really required and making it behave
correctly in all circumstances.
Maybe that means just propagating the error upwards.
Maybe it means retrying with only FMODE_READ, and recording that no
write access was available. At the very least, this fact would be used
to pass the right fmode_t to blkdev_put.
Maybe it means always using FMODE_READ because FMODE_WRITE isn't
actually needed (though that seems unlikely).
Maybe it also means bd_write_holder could be changed to a counter
instead of a flag, if we can convince ourselves that the use of @mode
in blkdev_get/put() is no longer "too fragile".
NeilBrown
>
> No promises that I will, I keep on wanting to do this sort of stuff and
> then life gets in the way, but if I can't do it it'll go on the list of
> projects on the raid wiki.
>
> Cheers,
> Wol
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: GET_ARRAY_INFO assumptions?
From: NeilBrown @ 2017-04-17 23:48 UTC (permalink / raw)
To: Jes Sorensen, Shaohua Li; +Cc: linux-raid
In-Reply-To: <c0a29266-fdf4-2f07-aabd-6aa1f55d7d46@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3522 bytes --]
On Fri, Apr 14 2017, Jes Sorensen wrote:
> On 04/13/2017 05:06 PM, Jes Sorensen wrote:
>> On 04/13/2017 04:37 PM, Shaohua Li wrote:
>>> On Thu, Apr 13, 2017 at 01:50:06PM -0400, Jes Sorensen wrote:
>>>> Hi Neil,
>>>>
>>>> Looking at trying to phase out the ioctl usage, I am trying to
>>>> introduce a
>>>> helper for the 'is the array valid' situation.
>>>>
>>>> Now looking at places like Incremental.c (around like 557 in my current
>>>> tree):
>>>> /* 7b/ if yes, */
>>>> /* - if number of OK devices match expected, or -R and there */
>>>> /* are enough, */
>>>> /* + add any bitmap file */
>>>> /* + start the array (auto-readonly). */
>>>>
>>>> if (md_get_array_info(mdfd, &ainf) == 0) {
>>>> if (c->export) {
>>>> printf("MD_STARTED=already\n");
>>>> } else if (c->verbose >= 0)
>>>> pr_err("%s attached to %s which is already active.\n",
>>>> devname, chosen_name);
>>>> rv = 0;
>>>> goto out_unlock;
>>>> }
>>>>
>>>> I am wondering if there are any side effects/assumptions about
>>>> GET_ARRAY_INFO that I am not considering? Basically I am making the
>>>> assumption that if /sys/block/md<X>/md exists, the array is valid.
>>>
>>> what does 'valid' really mean? md<x>/md exists after a md device is
>>> allocated,
>>> the md device might not have any under layer disks bound yet.
>>>
>>>> The code in Incremental.c already deals with sysfs higher up in the
>>>> code, so
>>>> I guess the question is if the above test is even relevant anymore?
>>>>
>>>> Alternative, do we need export a new state in sysfs 'running'?
>>>
>>> I'd assume 'running' means the md device has a personality attached. See
>>> array_state_show(), !running == 'clear' or 'inactive'.
>>
>> Good point, I guess what I am trying to figure out is what is assumed
>> when ioctl(GET_ARRAY_INFO) returns 0 and how do we map it to sysfs?
>
> Looking some more at this, it may be simpler than I thought. How about
> this approach (only compile tested):
>
> int md_array_active(int fd)
> {
> struct mdinfo *sra;
> struct mdu_array_info_s array;
> int ret;
>
> sra = sysfs_read(fd, NULL, GET_VERSION | GET_DISKS);
> if (sra) {
> if (!sra->array.raid_disks &&
> !(sra->array.major_version == -1 &&
> sra->array.minor_version == -2))
> ret = -ENODEV;
> else
> ret = 0;
>
> free(sra);
> } else {
> ret = ioctl(fd, GET_ARRAY_INFO, &array);
> }
>
> return !ret;
> }
>
> Note 'major = -1 && minor = -2' is sysfs_read's way of saying 'external'.
>
> This pretty much mimics what the kernel does in the ioctl handler for
> GET_ARRAY_INFO:
>
> case GET_ARRAY_INFO:
> if (!mddev->raid_disks && !mddev->external)
> err = -ENODEV;
> else
> err = get_array_info(mddev, argp);
> goto out;
>
> What do you think?
I think that it accurately mimics what the current code does.
I'm not sure that is what we really want.
For testing in Incremental.c if an array is "active" we really
should be testing more than "raid_disks != 0".
We should be testing, as Shaohua suggested, if
array_state != 'clear' or 'inactive'.
You cannot get that info through the ioctl interface, so I suppose
I decided the current test was 'close enough'.
If we are going to stop supported kernels that don't have (e.g.)
array_state, then we should really fo the right thing and test
array_state.
Thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: Zombie dm device?
From: Zhilong Liu @ 2017-04-18 2:36 UTC (permalink / raw)
To: Phil Susi, linux-raid
In-Reply-To: <3cf28b7e-9716-3595-018a-fecb32b61751@ubuntu.com>
On 04/18/2017 03:50 AM, Phil Susi wrote:
> After running the parted test suite I seem to have a zombie md device:
>
> root@devserv:~# mdadm -D /dev/md99
> /dev/md99:
> Version :
> Raid Level : raid0
> Total Devices : 0
>
> State : inactive
>
> Number Major Minor RaidDevice
>
>
> Why is this thing still hanging around, and how do you get rid of it? I
> tried mdadm --stop /dev/md99 but it didn't get rid of it. I also can't
> find any mdadm switch or a delete file or similar in the sysfs node.
can you find the /sys/block/md99/md/array_state? issue the
# echo clear > /sys/block/md99/md/array_state to stop array.
The test suit is in improving progress, a few test cases cannot
work well currently. you can talk about your requirement, and
I would record it. more details, more appreciate. :-)
Thanks,
-Zhilong
> root@devserv:~# uname -a
> Linux devserv 4.4.0-72-generic #93-Ubuntu SMP Fri Mar 31 14:07:41 UTC
> 2017 x86_64 x86_64 x86_64 GNU/Linux
> --
> 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
>
^ permalink raw reply
* Re: [PATCH 05/22] drm/i915: Make use of the new sg_map helper function
From: Daniel Vetter @ 2017-04-18 6:44 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: Steve Wise, linux-nvme, Stephen Bates, target-devel, Sumit Semwal,
devel, rds-devel, Sagi Grimberg, linux-scsi, Matthew Wilcox,
linux-rdma, Christoph Hellwig, fcoe-devel, Ross Zwisler,
open-iscsi, linux-media, Ming Lin, intel-gfx, sparmaintainer,
linux-raid, dri-devel, Dan Williams, megaraidlinux.pdl,
Jens Axboe, linaro-mm-sig, Martin K. Petersen, Greg
In-Reply-To: <1492121135-4437-6-git-send-email-logang@deltatee.com>
On Thu, Apr 13, 2017 at 04:05:18PM -0600, Logan Gunthorpe wrote:
> This is a single straightforward conversion from kmap to sg_map.
>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Probably makes sense to merge through some other tree, but please be aware
of the considerable churn rate in i915 (i.e. make sure your tree is in
linux-next before you send a pull request for this). Plane B would be to
get the prep patch in first and then merge the i915 conversion one kernel
release later.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 27 ++++++++++++++++-----------
> 1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 67b1fc5..1b1b91a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2188,6 +2188,15 @@ static void __i915_gem_object_reset_page_iter(struct drm_i915_gem_object *obj)
> radix_tree_delete(&obj->mm.get_page.radix, iter.index);
> }
>
> +static void i915_gem_object_unmap(const struct drm_i915_gem_object *obj,
> + void *ptr)
> +{
> + if (is_vmalloc_addr(ptr))
> + vunmap(ptr);
> + else
> + sg_unmap(obj->mm.pages->sgl, ptr, SG_KMAP);
> +}
> +
> void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
> enum i915_mm_subclass subclass)
> {
> @@ -2215,10 +2224,7 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
> void *ptr;
>
> ptr = ptr_mask_bits(obj->mm.mapping);
> - if (is_vmalloc_addr(ptr))
> - vunmap(ptr);
> - else
> - kunmap(kmap_to_page(ptr));
> + i915_gem_object_unmap(obj, ptr);
>
> obj->mm.mapping = NULL;
> }
> @@ -2475,8 +2481,11 @@ static void *i915_gem_object_map(const struct drm_i915_gem_object *obj,
> void *addr;
>
> /* A single page can always be kmapped */
> - if (n_pages == 1 && type == I915_MAP_WB)
> - return kmap(sg_page(sgt->sgl));
> + if (n_pages == 1 && type == I915_MAP_WB) {
> + addr = sg_map(sgt->sgl, SG_KMAP);
> + if (IS_ERR(addr))
> + return NULL;
> + }
>
> if (n_pages > ARRAY_SIZE(stack_pages)) {
> /* Too big for stack -- allocate temporary array instead */
> @@ -2543,11 +2552,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
> goto err_unpin;
> }
>
> - if (is_vmalloc_addr(ptr))
> - vunmap(ptr);
> - else
> - kunmap(kmap_to_page(ptr));
> -
> + i915_gem_object_unmap(obj, ptr);
> ptr = obj->mm.mapping = NULL;
> }
>
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: Zombie dm device?
From: Phil Susi @ 2017-04-18 13:00 UTC (permalink / raw)
To: Zhilong Liu, linux-raid
In-Reply-To: <ff1512f1-f499-43f9-daef-85cdaf1cb2ef@suse.com>
On 4/17/2017 10:36 PM, Zhilong Liu wrote:
> can you find the /sys/block/md99/md/array_state? issue the
> # echo clear > /sys/block/md99/md/array_state to stop array.
> The test suit is in improving progress, a few test cases cannot
> work well currently. you can talk about your requirement, and
> I would record it. more details, more appreciate. :-)
It already said it was clear. Attempting to clear it again apparently
rebooted the virtual machine and for some reason, syslog and kern.log
are empty and the previous one is last dated 8 days ago.
Odd that it just rebooted too since I'm pretty sure a panic should have
just halted the system with the panic message written to the vnc console.
^ permalink raw reply
* RE: [PATCH 16/22] xen-blkfront: Make use of the new sg_map helper function
From: David Laight @ 2017-04-18 14:13 UTC (permalink / raw)
To: 'Logan Gunthorpe', Christoph Hellwig, Martin K. Petersen,
Sagi Grimberg, Jens Axboe, Tejun Heo, Greg Kroah-Hartman,
Dan Williams, Ross Zwisler, Matthew Wilcox, Sumit Semwal,
Ming Lin, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw@public.gmane.org,
intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
linux-raid-u79uwXL29TbrhsbdSgBK9A
Cc: Steve Wise
In-Reply-To: <1492121135-4437-17-git-send-email-logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
From: Logan Gunthorpe
> Sent: 13 April 2017 23:05
> Straightforward conversion to the new helper, except due to
> the lack of error path, we have to warn if unmapable memory
> is ever present in the sgl.
>
> Signed-off-by: Logan Gunthorpe <logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
> ---
> drivers/block/xen-blkfront.c | 33 +++++++++++++++++++++++++++------
> 1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 5067a0a..7dcf41d 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -807,8 +807,19 @@ static int blkif_queue_rw_req(struct request *req, struct blkfront_ring_info *ri
> BUG_ON(sg->offset + sg->length > PAGE_SIZE);
>
> if (setup.need_copy) {
> - setup.bvec_off = sg->offset;
> - setup.bvec_data = kmap_atomic(sg_page(sg));
> + setup.bvec_off = 0;
> + setup.bvec_data = sg_map(sg, SG_KMAP_ATOMIC);
> + if (IS_ERR(setup.bvec_data)) {
> + /*
> + * This should really never happen unless
> + * the code is changed to use memory that is
> + * not mappable in the sg. Seeing there is a
> + * questionable error path out of here,
> + * we WARN.
> + */
> + WARN(1, "Non-mappable memory used in sg!");
> + return 1;
> + }
...
Perhaps add a flag to mark failure as 'unexpected' and trace (and panic?)
inside sg_map().
David
^ permalink raw reply
* Re: [PATCH 16/22] xen-blkfront: Make use of the new sg_map helper function
From: Konrad Rzeszutek Wilk @ 2017-04-18 14:27 UTC (permalink / raw)
To: David Laight, xen-devel
Cc: linux-nvdimm@lists.01.org, Steve Wise,
linux-nvme@lists.infradead.org, target-devel@vger.kernel.org,
Sumit Semwal, devel@driverdev.osuosl.org,
rds-devel@oss.oracle.com, Sagi Grimberg,
linux-scsi@vger.kernel.org, Matthew Wilcox,
linux-rdma@vger.kernel.org, Christoph Hellwig,
fcoe-devel@open-fcoe.org, Ross Zwisler,
open-iscsi@googlegroups.com, linux-media@vger.kernel.org,
Ming Lin, intel-gfx
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DCFFD3CD7@AcuExch.aculab.com>
On Tue, Apr 18, 2017 at 02:13:59PM +0000, David Laight wrote:
> From: Logan Gunthorpe
> > Sent: 13 April 2017 23:05
> > Straightforward conversion to the new helper, except due to
> > the lack of error path, we have to warn if unmapable memory
> > is ever present in the sgl.
Interesting that you didn't CC any of the maintainers. Could you
do that in the future please?
> >
> > Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> > ---
> > drivers/block/xen-blkfront.c | 33 +++++++++++++++++++++++++++------
> > 1 file changed, 27 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> > index 5067a0a..7dcf41d 100644
> > --- a/drivers/block/xen-blkfront.c
> > +++ b/drivers/block/xen-blkfront.c
> > @@ -807,8 +807,19 @@ static int blkif_queue_rw_req(struct request *req, struct blkfront_ring_info *ri
> > BUG_ON(sg->offset + sg->length > PAGE_SIZE);
> >
> > if (setup.need_copy) {
> > - setup.bvec_off = sg->offset;
> > - setup.bvec_data = kmap_atomic(sg_page(sg));
> > + setup.bvec_off = 0;
> > + setup.bvec_data = sg_map(sg, SG_KMAP_ATOMIC);
> > + if (IS_ERR(setup.bvec_data)) {
> > + /*
> > + * This should really never happen unless
> > + * the code is changed to use memory that is
> > + * not mappable in the sg. Seeing there is a
> > + * questionable error path out of here,
> > + * we WARN.
> > + */
> > + WARN(1, "Non-mappable memory used in sg!");
> > + return 1;
> > + }
> ...
>
> Perhaps add a flag to mark failure as 'unexpected' and trace (and panic?)
> inside sg_map().
>
> David
>
>
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply
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