Linux RAID subsystem development
 help / color / mirror / Atom feed
* Re: [RFC 5/5] r5cache: naive reclaim approach
From: NeilBrown @ 2016-06-01  3:16 UTC (permalink / raw)
  To: linux-raid; +Cc: shli, dan.j.williams, hch, kernel-team, Song Liu
In-Reply-To: <1464326983-3798454-6-git-send-email-songliubraving@fb.com>

[-- Attachment #1: Type: text/plain, Size: 209 bytes --]

On Fri, May 27 2016, Song Liu wrote:

> +	sector_t end = -1L;

Please use "MaxSector", not "-1L".
sector_t might be "long long" so -1L might be wrong.

(I haven't looked at much else in this patch)

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

^ permalink raw reply

* Re: raid 5 crashed
From: Edward Kuns @ 2016-06-01  3:46 UTC (permalink / raw)
  To: Brad Campbell
  Cc: Wols Lists, Phil Turmel, bobzer, linux-raid, Mikael Abrahamsson
In-Reply-To: <95079572-f319-ca57-a3e9-e8d00ef40248@fnarfbargle.com>

On Tue, May 31, 2016 at 8:48 PM, Brad Campbell
<lists2009@fnarfbargle.com> wrote:
> Much better to try and get the array running in a read-only state with all
> disks in place and clone the data from the array rather than the disks after
> they've been ddrescued. In the case of a running array, a read error on one
> of the array members will see the RAID attempt to get the data from
> elsewhere (a reconstruction), whereas a read from a disc cloned with
> ddrescue will happily just report what was a faulty sector as a big pile of
> zeros, and *poof* your data is gone.

My understanding is that mdraid will kick out a drive with an unrecoverable
hardware error on a single sector.  (Is this incorrect?)  How do you add
the drive back in and get the raid in a read-only mode that won't kick out
drives for failures, thus allowing you to fully recover data on a (say)
raid5 with bad sectors on every drive at different sectors offsets?

I'm asking hypothetically.  I have my arrays scrubbed weekly to prevent
this kind of surprise, and I keep multiple backups of my most important and
irreplaceable data.

In the past, I had a mirror that kept kicking out one drive.  I never lost
any data.  Between a decent backup policy and luck, I never experienced two
drive failures at the same time.  It's likely that the drive was timing out
on an unrecoverable read error, but the OS gave up first.  This was before
I knew to fix that default (mis)tuning, as was discussed here recently.
Last fall, that drive totally failed, and since, I've been paying much more
attention to care-and-maintenance!

          Thanks,

           Eddie

P.S. Sorry for the double-response to those on the CC list... I forgot
to tell gmail to use plain text.

^ permalink raw reply

* Re: raid 5 crashed
From: Brad Campbell @ 2016-06-01  4:07 UTC (permalink / raw)
  To: Edward Kuns
  Cc: Wols Lists, Phil Turmel, bobzer, linux-raid, Mikael Abrahamsson
In-Reply-To: <CACsGCyQuPEXtgeW6QH-s_-eW9-SZ3J5s2s7CUzZRNemexTt3aw@mail.gmail.com>

On 01/06/16 11:46, Edward Kuns wrote:
> On Tue, May 31, 2016 at 8:48 PM, Brad Campbell
> <lists2009@fnarfbargle.com> wrote:
>> Much better to try and get the array running in a read-only state with all
>> disks in place and clone the data from the array rather than the disks after
>> they've been ddrescued. In the case of a running array, a read error on one
>> of the array members will see the RAID attempt to get the data from
>> elsewhere (a reconstruction), whereas a read from a disc cloned with
>> ddrescue will happily just report what was a faulty sector as a big pile of
>> zeros, and *poof* your data is gone.
>
> My understanding is that mdraid will kick out a drive with an unrecoverable
> hardware error on a single sector.  (Is this incorrect?)  How do you add
> the drive back in and get the raid in a read-only mode that won't kick out
> drives for failures, thus allowing you to fully recover data on a (say)
> raid5 with bad sectors on every drive at different sectors offsets?

Yes, that is incorrect. If your timeouts are configured correctly the 
drive will report an uncorrectable error up the stack. MD will try to 
get the data from elsewhere and it will try to re-write the bad sector 
with the reconstructed data.

If your timeouts are *wrong* however, the drive will go away and try 
desperately to read the sector. This most often will take well in excess 
of the default 30 second ata stack timeout. So the ata stack will poke 
the drive after 30 seconds, but the drive is still tied up trying to 
recover the data. It sees this as the drive going away, and reports to 
MD that the drive is gone. *Bang* it's out of the array.

There are other issues associated with md trying to re-write the sector 
and failing (and some complexities around whether or not you have a bad 
block list) that may kick the drive if the write fails, but invariably 
it's a read timeout issue that causes this problem.

> I'm asking hypothetically.  I have my arrays scrubbed weekly to prevent
> this kind of surprise, and I keep multiple backups of my most important and
> irreplaceable data.

Certainly best practice. I was more lax until I encountered a 
misbehaving SIL controller that silently corrupted a significant 
proportion of a 16TB array.

> In the past, I had a mirror that kept kicking out one drive.  I never lost
> any data.  Between a decent backup policy and luck, I never experienced two
> drive failures at the same time.  It's likely that the drive was timing out
> on an unrecoverable read error, but the OS gave up first.  This was before
> I knew to fix that default (mis)tuning, as was discussed here recently.
> Last fall, that drive totally failed, and since, I've been paying much more
> attention to care-and-maintenance!

I do daily short SMART tests, weekly long SMART tests and monthly 
"check" scrubs of the RAID(s). I'm not saying it's best practice, but 
I've had several drives turn up SMART failures and managed to address 
those before it became an issue on any of the arrays.

I've used the '--force' parameter to mdadm to assemble an array after a 
sata controller failure and had good results, but I wasn't dealing with 
bad drives and the need to make things read-only, so I defer to those 
with more experience in that regard.

I have however done a *lot* of data recovery on single drives over the 
years and can absolutely vouch that dd will leave you in tears.


Regards,
Brad


^ permalink raw reply

* RE: [PATCH] dm-crypt: Fix error with too large bios (was: bcache gets stuck flushing writeback cache when used in combination with LUKS/dm-crypt and non-default bucket size)
From: James Johnston @ 2016-06-01  4:19 UTC (permalink / raw)
  To: 'Mikulas Patocka'
  Cc: 'Eric Wheeler', 'Tim Small',
	'Kent Overstreet', 'Alasdair Kergon',
	'Mike Snitzer', linux-bcache, dm-devel, dm-crypt,
	'Neil Brown', linux-raid
In-Reply-To: <alpine.LRH.2.02.1605271037470.2109@file01.intranet.prod.int.rdu2.redhat.com>

On Fri, 27 May 2016, Mikulas Patocka wrote:
> dm-crypt: Fix error with too large bios
> 
> When dm-crypt processes writes, it allocates a new bio in the function
> crypt_alloc_buffer. The bio is allocated from a bio set and it can have at
> most BIO_MAX_PAGES vector entries, however the incoming bio can be larger
> if it was allocated by other means. For example, bcache creates bios
> larger than BIO_MAX_PAGES. If the incoming bio is larger, bio_alloc_bioset
> fails and error is returned.
> 
> To avoid the error, we test for too large bio in the function crypt_map
> and dm_accept_partial_bio to split the bio. dm_accept_partial_bio trims
> the current bio to the desired size and requests that the device mapper
> core sends another bio with the rest of the data.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Cc: stable@vger.kernel.org	# v3.16+

Tested-by: James Johnston <johnstonj.public@codenest.com>

I tested this patch by:

1.  Building v4.7-rc1 from Torvalds git repo.  Confirmed that original bug
    still occurs on Ubuntu 15.10.

2.  Applying your patch to v4.7-rc1.  My kill sequence no longer works,
    and the writeback cache is now successfully flushed to disk, and the
    cache can be detached from the backing device.

3.  To check data integrity, copied 250 MB of /dev/urandom to some file
    on main volume.  Then, dd copy this file to /dev/bcache0.  Then,
    detached the cache device from the backing device.  Then, rebooted.
    Then, dd copy /dev/bcache0 to another file on main volume.  Then,
    diff the files and confirm no changes.

So it looks like it works, based on this admittedly brief testing.  Thanks!

Best regards,

James Johnston



^ permalink raw reply

* RE: [PATCH] dm-log-writes: fix bug with too large bios
From: James Johnston @ 2016-06-01  4:23 UTC (permalink / raw)
  To: 'Mikulas Patocka', 'Alasdair G. Kergon',
	'Mike Snitzer', 'Josef Bacik'
  Cc: dm-devel, 'Eric Wheeler', 'Tim Small',
	'Kent Overstreet', linux-bcache, dm-crypt,
	'Neil Brown', linux-raid
In-Reply-To: <alpine.LRH.2.02.1605271047280.2109@file01.intranet.prod.int.rdu2.redhat.com>

Hi Mikulas,

> bio_alloc can allocate a bio with at most BIO_MAX_PAGES (256) vector
> entries. However, the incoming bio may have more vector entries if it was
> allocated by other means. For example, bcache submits bios with more than
> BIO_MAX_PAGES entries. This results in bio_alloc failure.
> 
> To avoid the failure, change the code so that it allocates bio with at
> most BIO_MAX_PAGES entries. If the incoming bio has more entries,
> bio_add_page will fail and a new bio will be allocated - the code that
> handles bio_add_page failure already exists in the dm-log-writes target.
> 
> Also, move atomic_inc(&lc->io_blocks) before bio_alloc to fix a bug that
> the target hangs if bio_alloc fails. The error path does put_io_block(lc),
> so we must do atomic_inc(&lc->io_blocks) before invoking the error path to
> avoid underflow of lc->io_blocks.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Cc: stable@vger.kernel.org	# v4.1+

How does this relate to the previous patch you made to dm-crypt?  How best
should I test this?  It looks like the dm-crypt patch fixed the problem.

Should I test by applying this patch ONLY and reverting the dm-crypt patch?
(i.e. does this patch also fix the problem.)  Or should I just test with
both patches applied simultaneously?

James

^ permalink raw reply

* [RFC v3 0/4] Introduce the bulk mode method when sending request to crypto layer
From: Baolin Wang @ 2016-06-01  4:58 UTC (permalink / raw)
  To: axboe, agk, snitzer, dm-devel, herbert, davem
  Cc: ebiggers3, js1304, tadeusz.struk, smueller, standby24x7, shli,
	dan.j.williams, martin.petersen, sagig, kent.overstreet,
	keith.busch, tj, ming.lei, broonie, arnd, linux-crypto,
	linux-block, linux-raid, linux-kernel, baolin.wang

This patchset will check if the cipher can support bulk mode, then dm-crypt
will handle different ways to send requests to crypto layer according to
cipher mode. For bulk mode, we can use sg table to map the whole bio and
send all scatterlists of one bio to crypto engine to encrypt or decrypt,
which can improve the hardware engine's efficiency.

Changes since v2:
 - Add one cipher user with CRYPTO_ALG_BULK flag to support bulk mode.
 - Add one atomic variable to avoid the sg table race.

Changes since v1:
 - Refactor the blk_bio_map_sg() function to avoid duplicated code.
 - Move the sg table allocation to crypt_ctr_cipher() function to avoid memory
   allocation in the IO path.
 - Remove the crypt_sg_entry() function.
 - Other optimization.

Baolin Wang (4):
  block: Introduce blk_bio_map_sg() to map one bio
  crypto: Introduce CRYPTO_ALG_BULK flag
  md: dm-crypt: Introduce the bulk mode method when sending request
  crypto: Add the CRYPTO_ALG_BULK flag for ecb(aes) cipher

 block/blk-merge.c         |   36 ++++++++--
 drivers/crypto/omap-aes.c |    2 +-
 drivers/md/dm-crypt.c     |  159 ++++++++++++++++++++++++++++++++++++++++++++-
 include/crypto/skcipher.h |    7 ++
 include/linux/blkdev.h    |    2 +
 include/linux/crypto.h    |    6 ++
 6 files changed, 205 insertions(+), 7 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* [RFC v3 1/4] block: Introduce blk_bio_map_sg() to map one bio
From: Baolin Wang @ 2016-06-01  4:58 UTC (permalink / raw)
  To: axboe, agk, snitzer, dm-devel, herbert, davem
  Cc: ebiggers3, js1304, tadeusz.struk, smueller, standby24x7, shli,
	dan.j.williams, martin.petersen, sagig, kent.overstreet,
	keith.busch, tj, ming.lei, broonie, arnd, linux-crypto,
	linux-block, linux-raid, linux-kernel, baolin.wang
In-Reply-To: <cover.1464756501.git.baolin.wang@linaro.org>

In dm-crypt, it need to map one bio to scatterlist for improving the
hardware engine encryption efficiency. Thus this patch introduces the
blk_bio_map_sg() function to map one bio with scatterlists.

For avoiding the duplicated code in __blk_bios_map_sg() function, add
one parameter to distinguish bio map or request map.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 block/blk-merge.c      |   36 +++++++++++++++++++++++++++++++-----
 include/linux/blkdev.h |    2 ++
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 2613531..badae44 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -376,7 +376,7 @@ new_segment:
 
 static int __blk_bios_map_sg(struct request_queue *q, struct bio *bio,
 			     struct scatterlist *sglist,
-			     struct scatterlist **sg)
+			     struct scatterlist **sg, bool single_bio)
 {
 	struct bio_vec bvec, bvprv = { NULL };
 	struct bvec_iter iter;
@@ -408,13 +408,39 @@ single_segment:
 		return 1;
 	}
 
-	for_each_bio(bio)
+	if (!single_bio) {
+		for_each_bio(bio)
+			bio_for_each_segment(bvec, bio, iter)
+				__blk_segment_map_sg(q, &bvec, sglist, &bvprv,
+						     sg, &nsegs, &cluster);
+	} else {
 		bio_for_each_segment(bvec, bio, iter)
-			__blk_segment_map_sg(q, &bvec, sglist, &bvprv, sg,
-					     &nsegs, &cluster);
+			__blk_segment_map_sg(q, &bvec, sglist, &bvprv,
+					     sg, &nsegs, &cluster);
+	}
+
+	return nsegs;
+}
+
+/*
+ * Map a bio to scatterlist, return number of sg entries setup. Caller must
+ * make sure sg can hold bio segments entries.
+ */
+int blk_bio_map_sg(struct request_queue *q, struct bio *bio,
+		   struct scatterlist *sglist)
+{
+	struct scatterlist *sg = NULL;
+	int nsegs = 0;
+
+	if (bio)
+		nsegs = __blk_bios_map_sg(q, bio, sglist, &sg, true);
+
+	if (sg)
+		sg_mark_end(sg);
 
 	return nsegs;
 }
+EXPORT_SYMBOL(blk_bio_map_sg);
 
 /*
  * map a request to scatterlist, return number of sg entries setup. Caller
@@ -427,7 +453,7 @@ int blk_rq_map_sg(struct request_queue *q, struct request *rq,
 	int nsegs = 0;
 
 	if (rq->bio)
-		nsegs = __blk_bios_map_sg(q, rq->bio, sglist, &sg);
+		nsegs = __blk_bios_map_sg(q, rq->bio, sglist, &sg, false);
 
 	if (unlikely(rq->cmd_flags & REQ_COPY_USER) &&
 	    (blk_rq_bytes(rq) & q->dma_pad_mask)) {
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 1fd8fdf..5868062 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1013,6 +1013,8 @@ extern void blk_queue_write_cache(struct request_queue *q, bool enabled, bool fu
 extern struct backing_dev_info *blk_get_backing_dev_info(struct block_device *bdev);
 
 extern int blk_rq_map_sg(struct request_queue *, struct request *, struct scatterlist *);
+extern int blk_bio_map_sg(struct request_queue *q, struct bio *bio,
+			  struct scatterlist *sglist);
 extern void blk_dump_rq_flags(struct request *, char *);
 extern long nr_blockdev_pages(void);
 
-- 
1.7.9.5

^ permalink raw reply related

* [RFC v3 2/4] crypto: Introduce CRYPTO_ALG_BULK flag
From: Baolin Wang @ 2016-06-01  4:58 UTC (permalink / raw)
  To: axboe, agk, snitzer, dm-devel, herbert, davem
  Cc: ebiggers3, js1304, tadeusz.struk, smueller, standby24x7, shli,
	dan.j.williams, martin.petersen, sagig, kent.overstreet,
	keith.busch, tj, ming.lei, broonie, arnd, linux-crypto,
	linux-block, linux-raid, linux-kernel, baolin.wang
In-Reply-To: <cover.1464756501.git.baolin.wang@linaro.org>

Now some cipher hardware engines prefer to handle bulk block rather than one
sector (512 bytes) created by dm-crypt, cause these cipher engines can handle
the intermediate values (IV) by themselves in one bulk block. This means we
can increase the size of the request by merging request rather than always 512
bytes and thus increase the hardware engine processing speed.

So introduce 'CRYPTO_ALG_BULK' flag to indicate this cipher can support bulk
mode.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 include/crypto/skcipher.h |    7 +++++++
 include/linux/crypto.h    |    6 ++++++
 2 files changed, 13 insertions(+)

diff --git a/include/crypto/skcipher.h b/include/crypto/skcipher.h
index 0f987f5..d89d29a 100644
--- a/include/crypto/skcipher.h
+++ b/include/crypto/skcipher.h
@@ -519,5 +519,12 @@ static inline void skcipher_request_set_crypt(
 	req->iv = iv;
 }
 
+static inline unsigned int skcipher_is_bulk_mode(struct crypto_skcipher *sk_tfm)
+{
+	struct crypto_tfm *tfm = crypto_skcipher_tfm(sk_tfm);
+
+	return crypto_tfm_alg_bulk(tfm);
+}
+
 #endif	/* _CRYPTO_SKCIPHER_H */
 
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 6e28c89..a315487 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -63,6 +63,7 @@
 #define CRYPTO_ALG_DEAD			0x00000020
 #define CRYPTO_ALG_DYING		0x00000040
 #define CRYPTO_ALG_ASYNC		0x00000080
+#define CRYPTO_ALG_BULK			0x00000100
 
 /*
  * Set this bit if and only if the algorithm requires another algorithm of
@@ -623,6 +624,11 @@ static inline u32 crypto_tfm_alg_type(struct crypto_tfm *tfm)
 	return tfm->__crt_alg->cra_flags & CRYPTO_ALG_TYPE_MASK;
 }
 
+static inline unsigned int crypto_tfm_alg_bulk(struct crypto_tfm *tfm)
+{
+	return tfm->__crt_alg->cra_flags & CRYPTO_ALG_BULK;
+}
+
 static inline unsigned int crypto_tfm_alg_blocksize(struct crypto_tfm *tfm)
 {
 	return tfm->__crt_alg->cra_blocksize;
-- 
1.7.9.5

^ permalink raw reply related

* [RFC v3 3/4] md: dm-crypt: Introduce the bulk mode method when sending request
From: Baolin Wang @ 2016-06-01  4:58 UTC (permalink / raw)
  To: axboe, agk, snitzer, dm-devel, herbert, davem
  Cc: ebiggers3, js1304, tadeusz.struk, smueller, standby24x7, shli,
	dan.j.williams, martin.petersen, sagig, kent.overstreet,
	keith.busch, tj, ming.lei, broonie, arnd, linux-crypto,
	linux-block, linux-raid, linux-kernel, baolin.wang
In-Reply-To: <cover.1464756501.git.baolin.wang@linaro.org>

In now dm-crypt code, it is ineffective to map one segment (always one
sector) of one bio with just only one scatterlist at one time for hardware
crypto engine. Especially for some encryption mode (like ecb or xts mode)
cooperating with the crypto engine, they just need one initial IV or null
IV instead of different IV for each sector. In this situation We can consider
to use multiple scatterlists to map the whole bio and send all scatterlists
of one bio to crypto engine to encrypt or decrypt, which can improve the
hardware engine's efficiency.

With this optimization, On my test setup (beaglebone black board and dd testing)
using 64KB I/Os on an eMMC storage device I saw about 127% improvement in
throughput for encrypted writes, and about 206% improvement for encrypted reads.
But this is not fit for other modes which need different IV for each sector.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/md/dm-crypt.c |  159 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 158 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 4f3cb35..0b1d452 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -33,6 +33,7 @@
 #include <linux/device-mapper.h>
 
 #define DM_MSG_PREFIX "crypt"
+#define DM_MAX_SG_LIST	512
 
 /*
  * context holding the current state of a multi-part conversion
@@ -142,6 +143,11 @@ struct crypt_config {
 	char *cipher;
 	char *cipher_string;
 
+	struct sg_table sgt_in;
+	struct sg_table sgt_out;
+	atomic_t sgt_init_done;
+	struct completion sgt_restart;
+
 	struct crypt_iv_operations *iv_gen_ops;
 	union {
 		struct iv_essiv_private essiv;
@@ -837,6 +843,141 @@ static u8 *iv_of_dmreq(struct crypt_config *cc,
 		crypto_skcipher_alignmask(any_tfm(cc)) + 1);
 }
 
+static void crypt_init_sg_table(struct scatterlist *sgl)
+{
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sg(sgl, sg, DM_MAX_SG_LIST, i) {
+		if (i < DM_MAX_SG_LIST - 1 && sg_is_last(sg))
+			sg_unmark_end(sg);
+		else if (i == DM_MAX_SG_LIST - 1)
+			sg_mark_end(sg);
+	}
+
+	for_each_sg(sgl, sg, DM_MAX_SG_LIST, i) {
+		memset(sg, 0, sizeof(struct scatterlist));
+
+		if (i == DM_MAX_SG_LIST - 1)
+			sg_mark_end(sg);
+	}
+}
+
+static void crypt_reinit_sg_table(struct crypt_config *cc)
+{
+	if (!cc->sgt_in.orig_nents || !cc->sgt_out.orig_nents)
+		return;
+
+	crypt_init_sg_table(cc->sgt_in.sgl);
+	crypt_init_sg_table(cc->sgt_out.sgl);
+
+	if (atomic_inc_and_test(&cc->sgt_init_done))
+		complete(&cc->sgt_restart);
+	atomic_set(&cc->sgt_init_done, 1);
+}
+
+static int crypt_alloc_sg_table(struct crypt_config *cc)
+{
+	unsigned int bulk_mode = skcipher_is_bulk_mode(any_tfm(cc));
+	int ret = 0;
+
+	if (!bulk_mode)
+		goto out_skip_alloc;
+
+	ret = sg_alloc_table(&cc->sgt_in, DM_MAX_SG_LIST, GFP_KERNEL);
+	if (ret)
+		goto out_skip_alloc;
+
+	ret = sg_alloc_table(&cc->sgt_out, DM_MAX_SG_LIST, GFP_KERNEL);
+	if (ret)
+		goto out_free_table;
+
+	init_completion(&cc->sgt_restart);
+	atomic_set(&cc->sgt_init_done, 1);
+	return 0;
+
+out_free_table:
+	sg_free_table(&cc->sgt_in);
+out_skip_alloc:
+	cc->sgt_in.orig_nents = 0;
+	cc->sgt_out.orig_nents = 0;
+
+	return ret;
+}
+
+static int crypt_convert_bulk_block(struct crypt_config *cc,
+				    struct convert_context *ctx,
+				    struct skcipher_request *req)
+{
+	struct bio *bio_in = ctx->bio_in;
+	struct bio *bio_out = ctx->bio_out;
+	unsigned int total_bytes = bio_in->bi_iter.bi_size;
+	unsigned int total_sg_in, total_sg_out;
+	struct scatterlist *sg_in, *sg_out;
+	struct dm_crypt_request *dmreq;
+	u8 *iv;
+	int r;
+
+	if (!cc->sgt_in.orig_nents || !cc->sgt_out.orig_nents)
+		return -EINVAL;
+
+	if (!atomic_dec_and_test(&cc->sgt_init_done)) {
+		wait_for_completion(&cc->sgt_restart);
+		reinit_completion(&cc->sgt_restart);
+	}
+
+	dmreq = dmreq_of_req(cc, req);
+	iv = iv_of_dmreq(cc, dmreq);
+	dmreq->iv_sector = ctx->cc_sector;
+	dmreq->ctx = ctx;
+
+	total_sg_in = blk_bio_map_sg(bdev_get_queue(bio_in->bi_bdev),
+				     bio_in, cc->sgt_in.sgl);
+	if ((total_sg_in <= 0) || (total_sg_in > DM_MAX_SG_LIST)) {
+		DMERR("%s in sg map error %d, sg table nents[%d]\n",
+		      __func__, total_sg_in, cc->sgt_in.orig_nents);
+		return -EINVAL;
+	}
+
+	ctx->iter_in.bi_size -= total_bytes;
+	sg_in = cc->sgt_in.sgl;
+	sg_out = cc->sgt_in.sgl;
+
+	if (bio_data_dir(bio_in) == READ)
+		goto set_crypt;
+
+	total_sg_out = blk_bio_map_sg(bdev_get_queue(bio_out->bi_bdev),
+				      bio_out, cc->sgt_out.sgl);
+	if ((total_sg_out <= 0) || (total_sg_out > DM_MAX_SG_LIST)) {
+		DMERR("%s out sg map error %d, sg table nents[%d]\n",
+		      __func__, total_sg_out, cc->sgt_out.orig_nents);
+		return -EINVAL;
+	}
+
+	ctx->iter_out.bi_size -= total_bytes;
+	sg_out = cc->sgt_out.sgl;
+
+set_crypt:
+	if (cc->iv_gen_ops) {
+		r = cc->iv_gen_ops->generator(cc, iv, dmreq);
+		if (r < 0)
+			return r;
+	}
+
+	atomic_set(&cc->sgt_init_done, 0);
+	skcipher_request_set_crypt(req, sg_in, sg_out, total_bytes, iv);
+
+	if (bio_data_dir(ctx->bio_in) == WRITE)
+		r = crypto_skcipher_encrypt(req);
+	else
+		r = crypto_skcipher_decrypt(req);
+
+	if (!r && cc->iv_gen_ops && cc->iv_gen_ops->post)
+		r = cc->iv_gen_ops->post(cc, iv, dmreq);
+
+	return r;
+}
+
 static int crypt_convert_block(struct crypt_config *cc,
 			       struct convert_context *ctx,
 			       struct skcipher_request *req)
@@ -920,6 +1061,7 @@ static void crypt_free_req(struct crypt_config *cc,
 static int crypt_convert(struct crypt_config *cc,
 			 struct convert_context *ctx)
 {
+	unsigned int bulk_mode;
 	int r;
 
 	atomic_set(&ctx->cc_pending, 1);
@@ -930,7 +1072,14 @@ static int crypt_convert(struct crypt_config *cc,
 
 		atomic_inc(&ctx->cc_pending);
 
-		r = crypt_convert_block(cc, ctx, ctx->req);
+		bulk_mode = skcipher_is_bulk_mode(any_tfm(cc));
+		if (!bulk_mode) {
+			r = crypt_convert_block(cc, ctx, ctx->req);
+		} else {
+			r = crypt_convert_bulk_block(cc, ctx, ctx->req);
+			if (r == -EINVAL)
+				r = crypt_convert_block(cc, ctx, ctx->req);
+		}
 
 		switch (r) {
 		/*
@@ -1081,6 +1230,7 @@ static void crypt_dec_pending(struct dm_crypt_io *io)
 	if (io->ctx.req)
 		crypt_free_req(cc, io->ctx.req, base_bio);
 
+	crypt_reinit_sg_table(cc);
 	base_bio->bi_error = error;
 	bio_endio(base_bio);
 }
@@ -1563,6 +1713,9 @@ static void crypt_dtr(struct dm_target *ti)
 	kzfree(cc->cipher);
 	kzfree(cc->cipher_string);
 
+	sg_free_table(&cc->sgt_in);
+	sg_free_table(&cc->sgt_out);
+
 	/* Must zero key material before freeing */
 	kzfree(cc);
 }
@@ -1718,6 +1871,10 @@ static int crypt_ctr_cipher(struct dm_target *ti,
 		}
 	}
 
+	ret = crypt_alloc_sg_table(cc);
+	if (ret)
+		DMWARN("Allocate sg table for bulk mode failed");
+
 	ret = 0;
 bad:
 	kfree(cipher_api);
-- 
1.7.9.5

^ permalink raw reply related

* [RFC v3 4/4] crypto: Add the CRYPTO_ALG_BULK flag for ecb(aes) cipher
From: Baolin Wang @ 2016-06-01  4:58 UTC (permalink / raw)
  To: axboe, agk, snitzer, dm-devel, herbert, davem
  Cc: ebiggers3, js1304, tadeusz.struk, smueller, standby24x7, shli,
	dan.j.williams, martin.petersen, sagig, kent.overstreet,
	keith.busch, tj, ming.lei, broonie, arnd, linux-crypto,
	linux-block, linux-raid, linux-kernel, baolin.wang
In-Reply-To: <cover.1464756501.git.baolin.wang@linaro.org>

Since the ecb(aes) cipher does not need to handle the IV things for encryption
or decryption, that means it can support for bulk block when handling data.
Thus this patch adds the CRYPTO_ALG_BULK flag for ecb(aes) cipher to improve
the hardware aes engine's efficiency.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/crypto/omap-aes.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/omap-aes.c b/drivers/crypto/omap-aes.c
index ce174d3..ab09429 100644
--- a/drivers/crypto/omap-aes.c
+++ b/drivers/crypto/omap-aes.c
@@ -804,7 +804,7 @@ static struct crypto_alg algs_ecb_cbc[] = {
 	.cra_priority		= 300,
 	.cra_flags		= CRYPTO_ALG_TYPE_ABLKCIPHER |
 				  CRYPTO_ALG_KERN_DRIVER_ONLY |
-				  CRYPTO_ALG_ASYNC,
+				  CRYPTO_ALG_ASYNC | CRYPTO_ALG_BULK,
 	.cra_blocksize		= AES_BLOCK_SIZE,
 	.cra_ctxsize		= sizeof(struct omap_aes_ctx),
 	.cra_alignmask		= 0,
-- 
1.7.9.5

^ permalink raw reply related

* Re: BLKZEROOUT not zeroing md dev on VMDK
From: Sitsofe Wheeler @ 2016-06-01  5:04 UTC (permalink / raw)
  To: Tom Yan
  Cc: Darrick J. Wong, Shaohua Li, Jens Axboe, Arvind Kumar,
	VMware PV-Drivers, linux-raid, linux-scsi@vger.kernel.org,
	linux-block, linux-kernel@vger.kernel.org
In-Reply-To: <CAGnHSEn0ntEw_dwFNqG6CdgwcpHzQie88jgA21uioKhBbcByTg@mail.gmail.com>

On 27 May 2016 at 10:30, Tom Yan <tom.ty89@gmail.com> wrote:
> There seems to be some sort of race condition between
> blkdev_issue_zeroout() and the scsi disk driver (disabling write same
> after an illegal request). On my UAS drive, sometimes `blkdiscard -z
> /dev/sdX` will return right away, even though if I then check
> `write_same_max_bytes` it has turned 0. Sometimes it will just write
> zero with SCSI WRITE even if `write_same_max_bytes` is 33553920 before
> I issue `blkdiscard -z` (`write_same_max_bytes` also turned 0, as
> expected).
>
> Not sure if it is directly related to the case here though.

I'm not aware of hitting that particular problem myself directly on
the underlying "SCSI" device but the patch on
https://patchwork.kernel.org/patch/9137311/ should be able to resolve
that issue. Could you test it and follow up on
http://permalink.gmane.org/gmane.linux.kernel/2229377 ? I'm hoping
more testing reports will lead to the patch being reviewed and
accepted sooner rather than later as it's currently stalled...

-- 
Sitsofe | http://sucs.org/~sits/

^ permalink raw reply

* Re: raid 5 crashed
From: Edward Kuns @ 2016-06-01  5:23 UTC (permalink / raw)
  To: Brad Campbell
  Cc: Wols Lists, Phil Turmel, bobzer, linux-raid, Mikael Abrahamsson
In-Reply-To: <20b1858b-0cd7-a7de-82af-9167dbcfb09e@fnarfbargle.com>

On 01/06/16 11:46, Edward Kuns wrote:
> My understanding is that mdraid will kick out a drive with an
> unrecoverable hardware error on a single sector.  (Is this incorrect?)

On Tue, May 31, 2016 at 11:07 PM, Brad Campbell
<lists2009@fnarfbargle.com> wrote:
> Yes, that is incorrect. If your timeouts are configured correctly the drive
> will report an uncorrectable error up the stack. MD will try to get the data
> from elsewhere and it will try to re-write the bad sector with the
> reconstructed data.

Ah ha.  OK.  That makes a lot more sense.  Thanks.  And I *know* I had
bad timeouts on consumer drives before I replaced this drive.

> I do daily short SMART tests, weekly long SMART tests and monthly "check"
> scrubs of the RAID(s). I'm not saying it's best practice, but I've had
> several drives turn up SMART failures and managed to address those before it
> became an issue on any of the arrays.

As far as SMART, I'm running with the default Fedora behavior in that
area.  I don't believe it does any automatic tests, unless the default
logwatch behavior does SMART tests.  I'll add this to my "todo" list.
Thanks.

              Eddie

^ permalink raw reply

* Re: raid 5 crashed
From: Brad Campbell @ 2016-06-01  5:28 UTC (permalink / raw)
  To: Edward Kuns
  Cc: Wols Lists, Phil Turmel, bobzer, linux-raid, Mikael Abrahamsson
In-Reply-To: <CACsGCySR8NwbwRY=xe32YzyeZPFMmeDbnqp-LbykPoGBc537qA@mail.gmail.com>

On 01/06/16 13:23, Edward Kuns wrote:

>> I do daily short SMART tests, weekly long SMART tests and monthly "check"
>> scrubs of the RAID(s). I'm not saying it's best practice, but I've had
>> several drives turn up SMART failures and managed to address those before it
>> became an issue on any of the arrays.
>
> As far as SMART, I'm running with the default Fedora behavior in that
> area.  I don't believe it does any automatic tests, unless the default
> logwatch behavior does SMART tests.  I'll add this to my "todo" list.
> Thanks.

I just add this into smartd.conf on every machine I have (all Debian 
based systems). Make sure it's the first DEVICESCAN line as all lines 
after this are ignored.

Mail to brad, short tests every day at 2am and long tests at 4am on a 
Sunday.

DEVICESCAN -m brad -s (L/../../7/04|S/../.././02) -M exec 
/usr/share/smartmontools/smartd-runner

By default I've never had any system schedule smart tests.


^ permalink raw reply

* Re: RAID6 recovery with 6/9 drives out-of-sync
From: Peckins, Steven E @ 2016-06-01 11:32 UTC (permalink / raw)
  To: Phil Turmel; +Cc: linux-raid@vger.kernel.org
In-Reply-To: <574DE3B0.8000409@turmel.org>


On May 31, 2016, at 2:19 PM, Phil Turmel <philip@turmel.org> wrote:

> On 05/30/2016 10:43 PM, Peckins, Steven E wrote:
>> 
>> The component devices in the array are supposed to be multipath devices (dm-multipath), but for some reason, when the server was restarted, md grabbed both dm-* components and raw devices.  I *think* that this is what caused the problem.
> 
> Quite possible.  You probably need a DEVICES clause in your mdadm.conf
> to exclude the raw devices from the arrays.

I had a typo in the DEVICE glob for the system disks (/dev/sd[ab]* instead of /dev/sd[ab][12]).


>> I'm seeking advice on how to proceed at this point.  If more information is required, please ask.
> 
> Hmmm.  The partial success on mdadm --force suggests trying that again.
> Possible with --force twice on the command line.
> 
> Forced assembly is precisely what you need -- don't despair and attempt
> anything else.

Repeating the command was not successful; it is still reporting "/dev/md10 assembled from 5 drives and 1 spare - not enough to start the array."  Four drives are listed as “possibly out of date."  I assume those are the four that are not being incorporated.

Output from --assemble --force 1x and 2x:  http://pastebin.com/k1dT2zYC

--steve--
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: RAID6 recovery with 6/9 drives out-of-sync
From: Phil Turmel @ 2016-06-01 12:06 UTC (permalink / raw)
  To: Peckins, Steven E; +Cc: linux-raid@vger.kernel.org
In-Reply-To: <5D9C8773-D76E-409F-8961-41EB3ACD177A@illinois.edu>

On 06/01/2016 07:32 AM, Peckins, Steven E wrote:
> 
> On May 31, 2016, at 2:19 PM, Phil Turmel <philip@turmel.org> wrote:
> 
>> On 05/30/2016 10:43 PM, Peckins, Steven E wrote:
>>>
>>> The component devices in the array are supposed to be multipath devices (dm-multipath), but for some reason, when the server was restarted, md grabbed both dm-* components and raw devices.  I *think* that this is what caused the problem.
>>
>> Quite possible.  You probably need a DEVICES clause in your mdadm.conf
>> to exclude the raw devices from the arrays.
> 
> I had a typo in the DEVICE glob for the system disks (/dev/sd[ab]* instead of /dev/sd[ab][12]).

Understood, but be aware that if you have to hotswap one of these system
devices, they may not get the sda or sdb name, preventing a re-add or a
replacement from joining the array.

Since you are having to use /dev/mapper entries for some arrays,
consider using /dev/disk/by*/ symlinks for your system arrays.

>>> I'm seeking advice on how to proceed at this point.  If more information is required, please ask.
>>
>> Hmmm.  The partial success on mdadm --force suggests trying that again.
>> Possible with --force twice on the command line.
>>
>> Forced assembly is precisely what you need -- don't despair and attempt
>> anything else.
> 
> Repeating the command was not successful; it is still reporting "/dev/md10 assembled from 5 drives and 1 spare - not enough to start the array."  Four drives are listed as “possibly out of date."  I assume those are the four that are not being incorporated.
> 
> Output from --assemble --force 1x and 2x:  http://pastebin.com/k1dT2zYC

{ In the future, please paste these in-line so the archives will have
them.  The size limit for this list is ~ 100k. }

I vaguely recall a bug in forced reassembly for many out-of-date drives.
 Please clone and build the latest mdadm userspace[1] and run that mdadm
binary for the forced assembly.  Also show the portion of dmesg that
corresponds to the attempt.

Phil

[1] https://github.com/neilbrown/mdadm

--
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: [RFC 4/5] r5cache: write part of r5cache
From: Song Liu @ 2016-06-01 13:13 UTC (permalink / raw)
  To: Guoqing Jiang, linux-raid@vger.kernel.org
  Cc: Shaohua Li, nfbrown@novell.com, dan.j.williams@intel.com,
	hch@infradead.org, Kernel Team
In-Reply-To: <574D52BE.7040703@suse.com>


On 5/31/16, 5:00 PM, "Guoqing Jiang" <gqjiang@suse.com> wrote:

>Hi,
>
>I don't know a lot about raid5-cache, just a quick glance about syntax.
>
>On 05/27/2016 01:29 AM, Song Liu wrote:
>> This is the write part of r5cache. The cache is integrated with
>> stripe cache of raid456. It leverages code of r5l_log to write
>> data to journal device.
>>
>> r5cache split current write path into 2 parts: the write path
>> and the reclaim path. The write path is as following:
>> 1. write data to journal
>> 2. call bio_endio
>>
>> Then the reclaim path is as:
>> 1. Freeze the stripe (no more writes coming in)
>> 2. Calcualte parity (reconstruct or RMW)
>> 3. Write parity to journal device (data is already written to it)
>> 4. Write data and parity to RAID disks
>>
>> With r5cache, write operation does not wait for parity calculation
>> and write out, so the write latency is lower (1 write to journal
>> device vs. read and then write to raid disks). Also, r5cache will
>> reduce RAID overhead (multipile IO due to read-modify-write of
>> parity) and provide more opportunities of full stripe writes.
>>
>> r5cache adds a new state of each stripe: enum r5c_states. The write
>> path runs in state CLEAN and RUNNING (data in cache). Cache writes
>> start from r5c_handle_stripe_dirtying(), where bit R5_Wantcache is
>> set for devices with bio in towrite. Then, the data is written to
>> the journal through r5l_log implementation. Once the data is in the
>> journal, we set bit R5_InCache, and presue bio_endio for these
>> writes.
>>
>> The reclaim path starts by freezing the stripe (no more writes).
>> This stripes back to raid5 state machine, where
>> handle_stripe_dirtying will evaluate the stripe for reconstruct
>> writes or RMW writes (read data and calculate parity).
>>
>> For RMW, the code allocates extra page for prexor. Specifically,
>> a new page is allocated for r5dev->page to do prexor; while
>> r5dev->orig_page keeps the cached data. The extra page is freed
>> after prexor.
>>
>> r5cache naturally excludes SkipCopy. With R5_Wantcache bit set,
>> async_copy_data will not skip copy.
>>
>> Before writing data to RAID disks, the r5l_log logic stores
>> parity (and non-overwrite data) to the journal.
>>
>> Instead of inactive_list, stripes with cached data are tracked in
>> r5conf->r5c_cached_list. r5conf->r5c_cached_stripes tracks how
>> many stripes has dirty data in the cache.
>>
>> Two sysfs entries are provided for the write cache:
>> 1. r5c_cached_stripes shows how many stripes have cached data.
>>     Writing anything to r5c_cached_stripes will flush all data
>>     to RAID disks.
>> 2. r5c_cache_mode provides knob to switch the system between
>>     write-back or write-through (only write-log).
>>
>> There are some known limitations of the cache implementation:
>>
>> 1. Write cache only covers full page writes (R5_OVERWRITE). Writes
>>     of smaller granularity are write through.
>> 2. Only one log io (sh->log_io) for each stripe at anytime. Later
>>     writes for the same stripe have to wait. This can be improved by
>>     moving log_io to r5dev.
>>
>> Signed-off-by: Song Liu <songliubraving@fb.com>
>> Signed-off-by: Shaohua Li <shli@fb.com>
>> ---
>>   drivers/md/raid5-cache.c | 399 +++++++++++++++++++++++++++++++++++++++++++++--
>>   drivers/md/raid5.c       | 172 +++++++++++++++++---
>>   drivers/md/raid5.h       |  38 ++++-
>>   3 files changed, 577 insertions(+), 32 deletions(-)
>>
>
>[snip]
>
>> +
>> +/*
>> + * this journal write must contain full parity,
>> + * it may also contain data of none-overwrites
>> + */
>> +static void r5c_handle_parity_cached(struct stripe_head *sh)
>> +{
>> +	int i;
>> +
>> +	for (i = sh->disks; i--; )
>> +		if (test_bit(R5_InCache, &sh->dev[i].flags))
>> +			set_bit(R5_Wantwrite, &sh->dev[i].flags);
>> +	r5c_set_state(sh, R5C_STATE_PARITY_DONE);
>> +}
>> +
>> +static void r5c_finish_cache_stripe(struct stripe_head *sh)
>> +{
>> +	switch (sh->r5c_state) {
>> +	case R5C_STATE_PARITY_RUN:
>> +		r5c_handle_parity_cached(sh);
>> +		break;
>> +	case R5C_STATE_CLEAN:
>> +		r5c_set_state(sh, R5C_STATE_RUNNING);
>
>Maybe you missed break here?

I meant not to have the break here. I will revise this function (probably by a lot). 

>
>> +	case R5C_STATE_RUNNING:
>> +		r5c_handle_data_cached(sh);
>> +		break;
>> +	default:
>> +		BUG();
>> +	}
>> +}
>> +
>
>BTW: there are lots of issues reported by checkpatch.

I will run checkpatch and fix them. 

Thanks,
Song



^ permalink raw reply

* Re: RAID6 recovery with 6/9 drives out-of-sync
From: Peckins, Steven E @ 2016-06-01 13:16 UTC (permalink / raw)
  To: Phil Turmel; +Cc: linux-raid@vger.kernel.org
In-Reply-To: <574ECFE2.9050602@turmel.org>


On Jun 1, 2016, at 7:06 AM, Phil Turmel <philip@turmel.org> wrote:
> 
> Understood, but be aware that if you have to hotswap one of these system
> devices, they may not get the sda or sdb name, preventing a re-add or a
> replacement from joining the array.
> 
> Since you are having to use /dev/mapper entries for some arrays,
> consider using /dev/disk/by*/ symlinks for your system arrays.

Noted and updated.  (Those two drives are connected to the motherboard SATA, and the kernel names have been stable.  All other drives are connected through on-board SAS controllers and HBAs, etc.)


> I vaguely recall a bug in forced reassembly for many out-of-date drives.
> Please clone and build the latest mdadm userspace[1] and run that mdadm
> binary for the forced assembly.  Also show the portion of dmesg that
> corresponds to the attempt.

Good call!  The latest mdadm was able to assemble this array.

ocarina mdadm-latest # ./mdadm --version
mdadm - v3.4 - 28th January 2016
ocarina mdadm-latest # ./mdadm --assemble /dev/md10 --force --verbose /dev/dm-{0,1,11,12,13,14,15,16,17,28}
mdadm: looking for devices for /dev/md10
mdadm: /dev/dm-0 is identified as a member of /dev/md10, slot 0.
mdadm: /dev/dm-1 is identified as a member of /dev/md10, slot 1.
mdadm: /dev/dm-11 is identified as a member of /dev/md10, slot 2.
mdadm: /dev/dm-12 is identified as a member of /dev/md10, slot 3.
mdadm: /dev/dm-13 is identified as a member of /dev/md10, slot 4.
mdadm: /dev/dm-14 is identified as a member of /dev/md10, slot 5.
mdadm: /dev/dm-15 is identified as a member of /dev/md10, slot 6.
mdadm: /dev/dm-16 is identified as a member of /dev/md10, slot 7.
mdadm: /dev/dm-17 is identified as a member of /dev/md10, slot 8.
mdadm: /dev/dm-28 is identified as a member of /dev/md10, slot -1.
mdadm: forcing event count in /dev/dm-14(5) from 35 upto 44
mdadm: forcing event count in /dev/dm-15(6) from 35 upto 44
mdadm: forcing event count in /dev/dm-16(7) from 35 upto 44
mdadm: forcing event count in /dev/dm-17(8) from 35 upto 44
mdadm: clearing FAULTY flag for device 5 in /dev/md10 for /dev/dm-14
mdadm: clearing FAULTY flag for device 6 in /dev/md10 for /dev/dm-15
mdadm: clearing FAULTY flag for device 7 in /dev/md10 for /dev/dm-16
mdadm: clearing FAULTY flag for device 8 in /dev/md10 for /dev/dm-17
mdadm: Marking array /dev/md10 as 'clean'
mdadm: added /dev/dm-1 to /dev/md10 as 1
mdadm: added /dev/dm-11 to /dev/md10 as 2
mdadm: added /dev/dm-12 to /dev/md10 as 3
mdadm: added /dev/dm-13 to /dev/md10 as 4
mdadm: added /dev/dm-14 to /dev/md10 as 5
mdadm: added /dev/dm-15 to /dev/md10 as 6
mdadm: added /dev/dm-16 to /dev/md10 as 7
mdadm: added /dev/dm-17 to /dev/md10 as 8
mdadm: added /dev/dm-28 to /dev/md10 as -1
mdadm: added /dev/dm-0 to /dev/md10 as 0
mdadm: /dev/md10 has been started with 9 drives and 1 spare.

Output from dmesg for successful --assemble --force with latest mdadm binary:

[Wed Jun  1 08:23:15 2016] md: md10 stopped.
[Wed Jun  1 08:23:15 2016] md: bind<dm-1>
[Wed Jun  1 08:23:15 2016] md: bind<dm-11>
[Wed Jun  1 08:23:15 2016] md: bind<dm-12>
[Wed Jun  1 08:23:15 2016] md: bind<dm-13>
[Wed Jun  1 08:23:15 2016] md: bind<dm-14>
[Wed Jun  1 08:23:15 2016] md: bind<dm-15>
[Wed Jun  1 08:23:15 2016] md: bind<dm-16>
[Wed Jun  1 08:23:15 2016] md: bind<dm-17>
[Wed Jun  1 08:23:15 2016] md: bind<dm-28>
[Wed Jun  1 08:23:15 2016] md: bind<dm-0>
[Wed Jun  1 08:23:15 2016] md/raid:md10: device dm-0 operational as raid disk 0
[Wed Jun  1 08:23:15 2016] md/raid:md10: device dm-17 operational as raid disk 8
[Wed Jun  1 08:23:15 2016] md/raid:md10: device dm-16 operational as raid disk 7
[Wed Jun  1 08:23:15 2016] md/raid:md10: device dm-15 operational as raid disk 6
[Wed Jun  1 08:23:15 2016] md/raid:md10: device dm-14 operational as raid disk 5
[Wed Jun  1 08:23:15 2016] md/raid:md10: device dm-13 operational as raid disk 4
[Wed Jun  1 08:23:15 2016] md/raid:md10: device dm-12 operational as raid disk 3
[Wed Jun  1 08:23:15 2016] md/raid:md10: device dm-11 operational as raid disk 2
[Wed Jun  1 08:23:15 2016] md/raid:md10: device dm-1 operational as raid disk 1
[Wed Jun  1 08:23:15 2016] md/raid:md10: allocated 9558kB
[Wed Jun  1 08:23:15 2016] md/raid:md10: raid level 6 active with 9 out of 9 devices, algorithm 2
[Wed Jun  1 08:23:15 2016] RAID conf printout:
[Wed Jun  1 08:23:15 2016]  --- level:6 rd:9 wd:9
[Wed Jun  1 08:23:15 2016]  disk 0, o:1, dev:dm-0
[Wed Jun  1 08:23:15 2016]  disk 1, o:1, dev:dm-1
[Wed Jun  1 08:23:15 2016]  disk 2, o:1, dev:dm-11
[Wed Jun  1 08:23:15 2016]  disk 3, o:1, dev:dm-12
[Wed Jun  1 08:23:15 2016]  disk 4, o:1, dev:dm-13
[Wed Jun  1 08:23:15 2016]  disk 5, o:1, dev:dm-14
[Wed Jun  1 08:23:15 2016]  disk 6, o:1, dev:dm-15
[Wed Jun  1 08:23:15 2016]  disk 7, o:1, dev:dm-16
[Wed Jun  1 08:23:15 2016]  disk 8, o:1, dev:dm-17
[Wed Jun  1 08:23:15 2016] md10: detected capacity change from 0 to 14002780897280
[Wed Jun  1 08:23:15 2016] RAID conf printout:
[Wed Jun  1 08:23:15 2016]  --- level:6 rd:9 wd:9
[Wed Jun  1 08:23:15 2016]  disk 0, o:1, dev:dm-0
[Wed Jun  1 08:23:15 2016]  disk 1, o:1, dev:dm-1
[Wed Jun  1 08:23:15 2016]  disk 2, o:1, dev:dm-11
[Wed Jun  1 08:23:15 2016]  disk 3, o:1, dev:dm-12
[Wed Jun  1 08:23:15 2016]  disk 4, o:1, dev:dm-13
[Wed Jun  1 08:23:15 2016]  disk 5, o:1, dev:dm-14
[Wed Jun  1 08:23:15 2016]  disk 6, o:1, dev:dm-15
[Wed Jun  1 08:23:15 2016]  disk 7, o:1, dev:dm-16
[Wed Jun  1 08:23:15 2016]  disk 8, o:1, dev:dm-17
[Wed Jun  1 08:23:15 2016]  md10: unknown partition table

Uneventful dmesg output from EARLIER unsuccessful attempt with mdadm 3.3:

[Wed Jun  1 07:35:22 2016] md: md10 stopped.
[Wed Jun  1 07:35:22 2016] md: bind<dm-1>
[Wed Jun  1 07:35:22 2016] md: bind<dm-11>
[Wed Jun  1 07:35:22 2016] md: bind<dm-12>
[Wed Jun  1 07:35:22 2016] md: bind<dm-13>
[Wed Jun  1 07:35:22 2016] md: bind<dm-14>
[Wed Jun  1 07:35:22 2016] md: bind<dm-15>
[Wed Jun  1 07:35:22 2016] md: bind<dm-16>
[Wed Jun  1 07:35:22 2016] md: bind<dm-17>
[Wed Jun  1 07:35:22 2016] md: bind<dm-28>
[Wed Jun  1 07:35:22 2016] md: bind<dm-0>
[Wed Jun  1 07:35:22 2016] md: md10 stopped.
[Wed Jun  1 07:35:22 2016] md: unbind<dm-0>
[Wed Jun  1 07:35:22 2016] md: export_rdev(dm-0)
[Wed Jun  1 07:35:22 2016] md: unbind<dm-28>
[Wed Jun  1 07:35:22 2016] md: export_rdev(dm-28)
[Wed Jun  1 07:35:22 2016] md: unbind<dm-17>
[Wed Jun  1 07:35:22 2016] md: export_rdev(dm-17)
[Wed Jun  1 07:35:22 2016] md: unbind<dm-16>
[Wed Jun  1 07:35:22 2016] md: export_rdev(dm-16)
[Wed Jun  1 07:35:22 2016] md: unbind<dm-15>
[Wed Jun  1 07:35:22 2016] md: export_rdev(dm-15)
[Wed Jun  1 07:35:22 2016] md: unbind<dm-14>
[Wed Jun  1 07:35:22 2016] md: export_rdev(dm-14)
[Wed Jun  1 07:35:22 2016] md: unbind<dm-13>
[Wed Jun  1 07:35:22 2016] md: export_rdev(dm-13)
[Wed Jun  1 07:35:22 2016] md: unbind<dm-12>
[Wed Jun  1 07:35:22 2016] md: export_rdev(dm-12)
[Wed Jun  1 07:35:22 2016] md: unbind<dm-11>
[Wed Jun  1 07:35:22 2016] md: export_rdev(dm-11)
[Wed Jun  1 07:35:22 2016] md: unbind<dm-1>
[Wed Jun  1 07:35:22 2016] md: export_rdev(dm-1)

I activated the lvm volume and mounted the filesystem.  Everything looks intact.

Thanks for your help recovering this array!  I had been avoiding updating mdadm while I was recovering this as I had read of potential issues while recovering arrays created with earlier versions.

—steve


--
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: [RFC 1/5] add bio_split_mddev
From: Song Liu @ 2016-06-01 13:18 UTC (permalink / raw)
  To: Guoqing Jiang, linux-raid@vger.kernel.org
  Cc: Shaohua Li, nfbrown@novell.com, dan.j.williams@intel.com,
	hch@infradead.org, Kernel Team
In-Reply-To: <574D55AF.1030005@suse.com>

>> +	return bio_split(bio, sectors, gfp, mddev->bio_set);
>> +}
>> +EXPORT_SYMBOL_GPL(bio_split_mddev);
>> +
>
>Compared with bio_alloc_mddev and bio_clone_mddev, there is no 
>bio_split_bioset
>func, I think use bio_split directly is enough. Also why the last 
>parameter of the
>first bio_split is NULL instead of fs_bio_set?
>
>>   /*
>>    * We have a system wide 'event count' that is incremented
>>    * on any 'interesting' event, and readers of /proc/mdstat

I see. Let’s just use bio_split for now. 

Thanks,
Song



^ permalink raw reply

* Re: RAID6 recovery with 6/9 drives out-of-sync
From: Phil Turmel @ 2016-06-01 13:22 UTC (permalink / raw)
  To: Peckins, Steven E; +Cc: linux-raid@vger.kernel.org
In-Reply-To: <5BE0D8AF-C1BF-4DFB-BEAE-8C93898EAC87@illinois.edu>

On 06/01/2016 09:16 AM, Peckins, Steven E wrote:

> I activated the lvm volume and mounted the filesystem.  Everything
> looks intact.

Yay!

> Thanks for your help recovering this array!

You're welcome.

> I had been avoiding updating mdadm while I was recovering this as I
> had read of potential issues while recovering arrays created with
> earlier versions.

Viewing bug reports and bug fix patches is the primary reason I'm
subscribed to this list.  It lets me make my own stable upgrade decisions.

Phil

^ permalink raw reply

* Re: [RFC 3/5] r5cache: look up stripe cache for chunk_aligned_read
From: Song Liu @ 2016-06-01 13:23 UTC (permalink / raw)
  To: NeilBrown, linux-raid@vger.kernel.org
  Cc: Shaohua Li, dan.j.williams@intel.com, hch@infradead.org,
	Kernel Team
In-Reply-To: <8737oxsjrs.fsf@notabene.neil.brown.name>

>> stripe cache; for read_miss, raid5_read_one_chunk reads all data from
>> the disk; for read_partial_hit, raid5_read_one_chunk reads data from
>> disk, and amends the data with data in stripe cache in endio
>> (r5c_chunk_aligned_read_endio).
>
>I wonder if all this complexity is really justified.
>If we cannot bypass the cache, then we can just perform the read using
>the cache with the existing code.  That will be slower than the direct
>bypass, but is it so much slower that the complexity is needed.
>
>Alternately.... is a full hit at all likely?  If data is in the stripe
>cache, then there is a good chance it is in the page cache too, so it
>won't be read.
>If we assume that all reads will either be partial-hits or misses, then
>your approach will always read from the disk, then add updates (possibly
>none) from the cache.  In that case it might be simpler to do exactly
>that.
>i.e. schedule the read and then when that finishes, find all the stripes
>which overlap and if they contain unwritten data, attach the bio and
>queue for normal handling.
>
>I suspect the code would end up being much simpler as it would reuse
>more existing code.

Let me try both directions and see which ends up simplest/fast. 
>
>>
>> Sysfs entry is added to show statistics of read_full_hits,
>> read_partial_hits, and read_misses.
>
>I'm not sure putting statistics like this in /sys is a good idea.  If
>they are just for debugging, then put them in debugfs.

Will move them to debugfs. 

>>
>> Signed-off-by: Song Liu <songliubraving@fb.com>
>> Signed-off-by: Shaohua Li <shli@fb.com>
>> ---
>>  drivers/md/raid5-cache.c | 238 +++++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/md/raid5.c       |  23 ++++-
>>  drivers/md/raid5.h       |   6 ++
>>  3 files changed, 265 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
>> index e889e2d..5f0d96f 100644
>> --- a/drivers/md/raid5-cache.c
>> +++ b/drivers/md/raid5-cache.c
>> @@ -40,8 +40,15 @@
>>   */
>>  #define R5L_POOL_SIZE	4
>>  
>> +struct r5c_cache {
>> +	atomic64_t read_full_hits;	/* the whole chunk in cache */
>> +	atomic64_t read_partial_hits;	/* some pages of the chunk in cache */
>> +	atomic64_t read_misses;		/* the whold chunk is not in cache */
>> +};
>
>If this structure just holds statistics, then the name of the structure
>should indicate that.  "r5c_cache" makes it seem like it is a cache...
>
>> +
>>  struct r5l_log {
>>  	struct md_rdev *rdev;
>> +	struct r5c_cache cache;
>>  
>>  	u32 uuid_checksum;
>>  
>> @@ -134,6 +141,21 @@ enum r5l_io_unit_state {
>>  	IO_UNIT_STRIPE_END = 3,	/* stripes data finished writing to raid */
>>  };
>>  
>> +struct r5c_chunk_map {
>> +	int sh_count;
>> +	struct r5conf *conf;
>> +	struct bio *parent_bi;
>> +	int dd_idx;
>> +	struct stripe_head *sh_array[0];
>> +};
>> +
>> +static void init_r5c_cache(struct r5conf *conf, struct r5c_cache *cache)
>> +{
>> +	atomic64_set(&cache->read_full_hits, 0);
>> +	atomic64_set(&cache->read_partial_hits, 0);
>> +	atomic64_set(&cache->read_misses, 0);
>> +}
>> +
>>  static sector_t r5l_ring_add(struct r5l_log *log, sector_t start, sector_t inc)
>>  {
>>  	start += inc;
>> @@ -1120,6 +1142,220 @@ static void r5l_write_super(struct r5l_log *log, sector_t cp)
>>  	set_bit(MD_CHANGE_DEVS, &mddev->flags);
>>  }
>>  
>> +/* TODO: use async copy */
>> +static void r5c_copy_data_to_bvec(struct r5dev *rdev, int sh_offset,
>> +				  struct bio_vec *bvec, int bvec_offset, int len)
>> +{
>> +	/* We always copy data from orig_page. This is because in R-M-W, we use
>> +	 * page to do prexor of parity */
>> +	void *src_p = kmap_atomic(rdev->orig_page);
>> +	void *dst_p = kmap_atomic(bvec->bv_page);
>> +	memcpy(dst_p + bvec_offset, src_p + sh_offset, len);
>> +	kunmap_atomic(dst_p);
>> +	kunmap_atomic(src_p);
>> +}
>> +
>> +/*
>> + * copy data from a chunk_map to a bio
>> + */
>> +static void r5c_copy_chunk_map_to_bio(struct r5c_chunk_map *chunk_map,
>> +			 struct bio *bio)
>> +{
>> +	struct bvec_iter iter;
>> +	struct bio_vec bvec;
>> +	int sh_idx;
>> +	unsigned sh_offset;
>> +
>> +	sh_idx = 0;
>> +	sh_offset = (bio->bi_iter.bi_sector & ((sector_t)STRIPE_SECTORS-1)) << 9;
>> +
>> +	/*
>> +	 * If bio is not page aligned, the chunk_map will have 1 more sh than bvecs
>> +	 * in the bio. Chunk_map may also have NULL-sh. To copy the right data, we
>> +	 * need to walk through the chunk_map carefully. In this implementation,
>> +	 * bvec/bvec_offset always matches with sh_array[sh_idx]/sh_offset.
>> +	 *
>> +	 * In the following example, the nested loop will run 4 times; and
>> +	 * r5c_copy_data_to_bvec will be called for the first and last iteration.
>> +	 *
>> +	 *             --------------------------------
>> +	 * chunk_map   | valid sh |  NULL  | valid sh |
>> +	 *             --------------------------------
>> +	 *                   ---------------------
>> +	 * bio               |         |         |
>> +	 *                   ---------------------
>> +	 *
>> +	 *                   |    |    |   |     |
>> +	 * copy_data         | Y  | N  | N |  Y  |
>> +	 */
>> +	bio_for_each_segment(bvec, bio, iter) {
>> +		int len;
>> +		unsigned bvec_offset = bvec.bv_offset;
>> +		while (bvec_offset < PAGE_SIZE) {
>> +			len = min_t(unsigned, PAGE_SIZE - bvec_offset, PAGE_SIZE - sh_offset);
>> +			if (chunk_map->sh_array[sh_idx])
>> +				r5c_copy_data_to_bvec(&chunk_map->sh_array[sh_idx]->dev[chunk_map->dd_idx], sh_offset,
>> +						      &bvec, bvec_offset, len);
>> +			bvec_offset += len;
>> +			sh_offset += len;
>> +			if (sh_offset == PAGE_SIZE) {
>> +				sh_idx += 1;
>> +				sh_offset = 0;
>> +			}
>> +		}
>> +	}
>> +	return;
>> +}
>> +
>> +/*
>> + * release stripes in chunk_map and free the chunk_map
>> + */
>> +static void free_r5c_chunk_map(struct r5c_chunk_map *chunk_map)
>> +{
>> +	unsigned sh_idx;
>> +	struct stripe_head *sh;
>> +
>> +	for (sh_idx = 0; sh_idx < chunk_map->sh_count; ++sh_idx) {
>> +		sh = chunk_map->sh_array[sh_idx];
>> +		if (sh) {
>> +			set_bit(STRIPE_HANDLE, &sh->state);
>> +			raid5_release_stripe(sh);
>> +		}
>> +	}
>> +	kfree(chunk_map);
>> +}
>> +
>> +static void r5c_chunk_aligned_read_endio(struct bio *bio)
>> +{
>> +	struct r5c_chunk_map *chunk_map = (struct r5c_chunk_map *) bio->bi_private;
>> +	struct bio *parent_bi = chunk_map->parent_bi;
>> +
>> +	r5c_copy_chunk_map_to_bio(chunk_map, bio);
>> +	free_r5c_chunk_map(chunk_map);
>> +	bio_put(bio);
>> +	bio_endio(parent_bi);
>> +}
>> +
>> +/*
>> + * look up bio in stripe cache
>> + * return raid_bio	-> no data in cache, read the chunk from disk
>> + * return new r5c_bio	-> partial data in cache, read from disk, and amend in r5c_align_endio
>> + * return NULL		-> all data in cache, no need to read disk
>> + */
>> +struct bio *r5c_lookup_chunk(struct r5l_log *log, struct bio *raid_bio)
>> +{
>> +	struct r5conf *conf;
>> +	sector_t logical_sector;
>> +	sector_t first_stripe, last_stripe;  /* first (inclusive) stripe and last (exclusive) */
>> +	int dd_idx;
>> +	struct stripe_head *sh;
>> +	unsigned sh_count, sh_idx, sh_cached;
>> +	struct r5c_chunk_map *chunk_map;
>> +	struct bio *r5c_bio;
>> +	int hash;
>> +	unsigned long flags;
>> +
>> +	if (!log)
>> +		return raid_bio;
>> +
>> +	conf = log->rdev->mddev->private;
>> +
>> +	logical_sector = raid_bio->bi_iter.bi_sector &
>> +		~((sector_t)STRIPE_SECTORS-1);
>> +	sh_count = DIV_ROUND_UP_SECTOR_T(bio_end_sector(raid_bio) - logical_sector, STRIPE_SECTORS);
>> +
>> +	first_stripe = raid5_compute_sector(conf, logical_sector, 0, &dd_idx, NULL);
>> +	last_stripe = first_stripe + STRIPE_SECTORS * sh_count;
>> +
>> +	chunk_map = kzalloc(sizeof(struct r5c_chunk_map) + sh_count * sizeof(struct stripe_head*), GFP_NOIO);
>> +	sh_cached = 0;
>> +
>> +	for (sh_idx = 0; sh_idx < sh_count; ++sh_idx) {
>> +		hash = stripe_hash_locks_hash(first_stripe + sh_idx * STRIPE_SECTORS);
>> +		spin_lock_irqsave(conf->hash_locks + hash, flags);
>> +		sh = __find_stripe(conf, first_stripe + sh_idx * STRIPE_SECTORS, conf->generation);
>> +		if (sh &&
>> +		    test_bit(R5_UPTODATE, &sh->dev[dd_idx].flags)) {
>> +			if (!atomic_inc_not_zero(&sh->count)) {
>> +				spin_lock(&conf->device_lock);
>> +				if (!atomic_read(&sh->count)) {
>> +					if (!test_bit(STRIPE_HANDLE, &sh->state))
>> +						atomic_inc(&conf->active_stripes);
>> +					BUG_ON(list_empty(&sh->lru) &&
>> +					       !test_bit(STRIPE_EXPANDING, &sh->state));
>> +					list_del_init(&sh->lru);
>> +					if (sh->group) {
>> +						sh->group->stripes_cnt--;
>> +						sh->group = NULL;
>> +					}
>> +				}
>> +				atomic_inc(&sh->count);
>> +				spin_unlock(&conf->device_lock);
>> +			}
>> +			chunk_map->sh_array[sh_idx] = sh;
>> +			++sh_cached;
>> +		}
>> +		spin_unlock_irqrestore(conf->hash_locks + hash, flags);
>> +	}
>> +
>> +	if (sh_cached == 0) {
>> +		atomic64_inc(&log->cache.read_misses);
>> +		kfree(chunk_map);
>> +		return raid_bio;
>> +	}
>> +
>> +	chunk_map->sh_count = sh_count;
>> +	chunk_map->dd_idx = dd_idx;
>> +
>> +	if (sh_cached == sh_count) {
>> +		atomic64_inc(&log->cache.read_full_hits);
>> +		r5c_copy_chunk_map_to_bio(chunk_map, raid_bio);
>> +		free_r5c_chunk_map(chunk_map);
>> +		bio_endio(raid_bio);
>> +		return NULL;
>> +	}
>> +
>> +	chunk_map->parent_bi = raid_bio;
>> +	chunk_map->conf = conf;
>> +
>> +	atomic64_inc(&log->cache.read_partial_hits);
>> +
>> +	/* TODO: handle bio_clone failure? */
>> +	r5c_bio = bio_clone_mddev(raid_bio, GFP_NOIO, log->rdev->mddev);
>> +
>> +	r5c_bio->bi_private = chunk_map;
>> +	r5c_bio->bi_end_io = r5c_chunk_aligned_read_endio;
>> +
>> +	return r5c_bio;
>> +}
>> +
>> +ssize_t
>> +r5c_stat_show(struct mddev *mddev, char* page)
>> +{
>> +	struct r5conf *conf = mddev->private;
>> +	struct r5l_log *log;
>> +	int ret = 0;
>> +
>> +	if (!conf)
>> +		return 0;
>> +
>> +	log = conf->log;
>> +
>> +	if (!log)
>> +		return 0;
>> +
>> +	ret += snprintf(page + ret, PAGE_SIZE - ret, "r5c_read_full_hits: %llu\n",
>> +			(unsigned long long) atomic64_read(&log->cache.read_full_hits));
>> +
>> +	ret += snprintf(page + ret, PAGE_SIZE - ret, "r5c_read_partial_hits: %llu\n",
>> +			(unsigned long long) atomic64_read(&log->cache.read_partial_hits));
>> +
>> +	ret += snprintf(page + ret, PAGE_SIZE - ret, "r5c_read_misses: %llu\n",
>> +			(unsigned long long) atomic64_read(&log->cache.read_misses));
>> +
>> +	return ret;
>> +}
>> +
>>  static int r5l_load_log(struct r5l_log *log)
>>  {
>>  	struct md_rdev *rdev = log->rdev;
>> @@ -1239,6 +1475,8 @@ int r5l_init_log(struct r5conf *conf, struct md_rdev *rdev)
>>  	INIT_LIST_HEAD(&log->no_space_stripes);
>>  	spin_lock_init(&log->no_space_stripes_lock);
>>  
>> +	init_r5c_cache(conf, &log->cache);
>> +
>>  	if (r5l_load_log(log))
>>  		goto error;
>>  
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index dc24b664..cdd9c4b 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -503,7 +503,7 @@ retry:
>>  	set_bit(STRIPE_BATCH_READY, &sh->state);
>>  }
>>  
>> -static struct stripe_head *__find_stripe(struct r5conf *conf, sector_t sector,
>> +struct stripe_head *__find_stripe(struct r5conf *conf, sector_t sector,
>>  					 short generation)
>>  {
>>  	struct stripe_head *sh;
>> @@ -515,6 +515,7 @@ static struct stripe_head *__find_stripe(struct r5conf *conf, sector_t sector,
>>  	pr_debug("__stripe %llu not in cache\n", (unsigned long long)sector);
>>  	return NULL;
>>  }
>> +EXPORT_SYMBOL(__find_stripe);
>
>Why are you exporting this?  raid5-cache is not a separate module.
>If you were going the export it the name would need to be changed to
>include "md_".

I should have removed it before sending. 

Thanks,

Song


>
>
>NeilBrown
>
>
>>  
>>  /*
>>   * Need to check if array has failed when deciding whether to:
>> @@ -4726,7 +4727,8 @@ static int raid5_read_one_chunk(struct mddev *mddev, struct bio *raid_bio)
>>  {
>>  	struct r5conf *conf = mddev->private;
>>  	int dd_idx;
>> -	struct bio* align_bi;
>> +	struct bio *align_bi;
>> +	struct bio *r5c_bio;
>>  	struct md_rdev *rdev;
>>  	sector_t end_sector;
>>  
>> @@ -4734,6 +4736,18 @@ static int raid5_read_one_chunk(struct mddev *mddev, struct bio *raid_bio)
>>  		pr_debug("%s: non aligned\n", __func__);
>>  		return 0;
>>  	}
>> +
>> +	r5c_bio = r5c_lookup_chunk(conf->log, raid_bio);
>> +
>> +	if (r5c_bio == NULL) {
>> +		pr_debug("Read all data from stripe cache\n");
>> +		return 1;
>> +	} else if (r5c_bio == raid_bio)
>> +		pr_debug("No data in stripe cache, read all from disk\n");
>> +	else {
>> +		pr_debug("Partial data in stripe cache, read and amend\n");
>> +		raid_bio = r5c_bio;
>> +	}
>>  	/*
>>  	 * use bio_clone_mddev to make a copy of the bio
>>  	 */
>> @@ -6157,6 +6171,10 @@ raid5_group_thread_cnt = __ATTR(group_thread_cnt, S_IRUGO | S_IWUSR,
>>  				raid5_show_group_thread_cnt,
>>  				raid5_store_group_thread_cnt);
>>  
>> +static struct md_sysfs_entry
>> +r5c_cache_stats = __ATTR(r5c_cache_stats, S_IRUGO,
>> +			 r5c_stat_show, NULL);
>> +
>>  static struct attribute *raid5_attrs[] =  {
>>  	&raid5_stripecache_size.attr,
>>  	&raid5_stripecache_active.attr,
>> @@ -6164,6 +6182,7 @@ static struct attribute *raid5_attrs[] =  {
>>  	&raid5_group_thread_cnt.attr,
>>  	&raid5_skip_copy.attr,
>>  	&raid5_rmw_level.attr,
>> +	&r5c_cache_stats.attr,
>>  	NULL,
>>  };
>>  static struct attribute_group raid5_attrs_group = {
>> diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
>> index 3b68d4f..de11514 100644
>> --- a/drivers/md/raid5.h
>> +++ b/drivers/md/raid5.h
>> @@ -690,4 +690,10 @@ extern void r5l_stripe_write_finished(struct stripe_head *sh);
>>  extern int r5l_handle_flush_request(struct r5l_log *log, struct bio *bio);
>>  extern void r5l_quiesce(struct r5l_log *log, int state);
>>  extern bool r5l_log_disk_error(struct r5conf *conf);
>> +
>> +extern struct bio *r5c_lookup_chunk(struct r5l_log *log, struct bio *raid_bio);
>> +extern struct stripe_head *__find_stripe(struct r5conf *conf, sector_t sector,
>> +					 short generation);
>> +
>> +extern ssize_t r5c_stat_show(struct mddev *mddev, char* page);
>>  #endif
>> -- 
>> 2.8.0.rc2


^ permalink raw reply

* Re: [RFC 5/5] r5cache: naive reclaim approach
From: Song Liu @ 2016-06-01 13:24 UTC (permalink / raw)
  To: NeilBrown, linux-raid@vger.kernel.org
  Cc: Shaohua Li, dan.j.williams@intel.com, hch@infradead.org,
	Kernel Team
In-Reply-To: <87wpm9r42y.fsf@notabene.neil.brown.name>


On 6/1/16, 11:16 AM, "NeilBrown" <nfbrown@novell.com> wrote:

>On Fri, May 27 2016, Song Liu wrote:
>
>> +	sector_t end = -1L;
>
>Please use "MaxSector", not "-1L".
>sector_t might be "long long" so -1L might be wrong.
>
>(I haven't looked at much else in this patch)
>
>NeilBrown

I will fix this one in next revision. 

Thanks,
Song



^ permalink raw reply

* Re: [RFC 4/5] r5cache: write part of r5cache
From: Song Liu @ 2016-06-01 13:36 UTC (permalink / raw)
  To: NeilBrown, linux-raid@vger.kernel.org
  Cc: Shaohua Li, dan.j.williams@intel.com, hch@infradead.org,
	Kernel Team
In-Reply-To: <87zir5r496.fsf@notabene.neil.brown.name>

>> path runs in state CLEAN and RUNNING (data in cache). Cache writes
>> start from r5c_handle_stripe_dirtying(), where bit R5_Wantcache is
>> set for devices with bio in towrite. Then, the data is written to
>> the journal through r5l_log implementation. Once the data is in the
>> journal, we set bit R5_InCache, and presue bio_endio for these
>> writes.
>
>I don't think this new state field is a good idea.  We already have
>plenty of state information and it would be best to fit in with that.
>When handle_stripe() runs it assesses the state of the stripe and
>decides what to do next.  The sh->count is incremented and it doesn't
>return to zero until all the actions that were found to be necessary
>have completely.  When it does return to zero, handle_stripe() runs
>again to see what else is needed.

Let me try to avoid the new state field. 

>
>You may well need to add sh->state or dev->flags flags to record new
>aspects of the state, such as whether data is safe in the log or safe in
>the RAID, but I don't think a new state variable will really help.
>
>>
>> The reclaim path starts by freezing the stripe (no more writes).
>> This stripes back to raid5 state machine, where
>> handle_stripe_dirtying will evaluate the stripe for reconstruct
>> writes or RMW writes (read data and calculate parity).
>>
>> For RMW, the code allocates extra page for prexor. Specifically,
>> a new page is allocated for r5dev->page to do prexor; while
>> r5dev->orig_page keeps the cached data. The extra page is freed
>> after prexor.
>
>It isn't at all clear to me why you need the extra page.  Could you
>explain when and why the ->page and the ->orig_page would contain
>different content and both of the content would be needed?
>

In prexor, we need read old data from the disk to perform the 
substract-xor. In the meanwhile, we need a buffer for the new data. 
When there is no write cache, the new data is in the page of bio. 
With write cache, however, the bio might be already released. 
Therefore, we will need an extra page here: orig_page for new 
Cached data, while newly allocated page just hold old data for prexor. 



>>
>> r5cache naturally excludes SkipCopy. With R5_Wantcache bit set,
>> async_copy_data will not skip copy.
>>
>> Before writing data to RAID disks, the r5l_log logic stores
>> parity (and non-overwrite data) to the journal.
>>
>> Instead of inactive_list, stripes with cached data are tracked in
>> r5conf->r5c_cached_list. r5conf->r5c_cached_stripes tracks how
>> many stripes has dirty data in the cache.
>>
>> Two sysfs entries are provided for the write cache:
>> 1. r5c_cached_stripes shows how many stripes have cached data.
>>    Writing anything to r5c_cached_stripes will flush all data
>>    to RAID disks.
>> 2. r5c_cache_mode provides knob to switch the system between
>>    write-back or write-through (only write-log).
>>
>> There are some known limitations of the cache implementation:
>>
>> 1. Write cache only covers full page writes (R5_OVERWRITE). Writes
>>    of smaller granularity are write through.
>> 2. Only one log io (sh->log_io) for each stripe at anytime. Later
>>    writes for the same stripe have to wait. This can be improved by
>>    moving log_io to r5dev.
>>
>> Signed-off-by: Song Liu <songliubraving@fb.com>
>> Signed-off-by: Shaohua Li <shli@fb.com>
>> ---
>>  drivers/md/raid5-cache.c | 399 +++++++++++++++++++++++++++++++++++++++++++++--
>>  drivers/md/raid5.c       | 172 +++++++++++++++++---
>>  drivers/md/raid5.h       |  38 ++++-
>>  3 files changed, 577 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
>> index 5f0d96f..66a3cd5 100644
>> --- a/drivers/md/raid5-cache.c
>> +++ b/drivers/md/raid5-cache.c
>> @@ -40,7 +40,19 @@
>>   */
>>  #define R5L_POOL_SIZE	4
>>  
>> +enum r5c_cache_mode {
>> +	R5C_MODE_NO_CACHE = 0,
>> +	R5C_MODE_WRITE_THROUGH = 1,
>> +	R5C_MODE_WRITE_BACK = 2,
>> +};
>> +
>> +static char *r5c_cache_mode_str[] = {"no-cache", "write-through", "write-back"};
>> +
>>  struct r5c_cache {
>> +	int flush_threshold;		/* flush the stripe when flush_threshold buffers are dirty  */
>> +	int mode;			/* enum r5c_cache_mode */
>
> why isn't this just "enum r5c_cache_mode mode;" ??
>
>Clearly this structure is more than just statistics.  But now I wonder
>why these fields aren't simply added to struct r5conf.  Or maybe in
>r5l_log.

I will try whether it is better to add these to r5conf. 

Thanks,
Songd
>
>
>NeilBrown


^ permalink raw reply

* Re: dm-log-writes: fix bug with too large bios
From: Mike Snitzer @ 2016-06-01 14:02 UTC (permalink / raw)
  To: James Johnston
  Cc: 'Mikulas Patocka', 'Alasdair G. Kergon',
	'Josef Bacik', 'Eric Wheeler', dm-crypt, dm-devel,
	'Neil Brown', linux-raid, linux-bcache,
	'Kent Overstreet', 'Tim Small'
In-Reply-To: <07bf01d1bbbd$53991750$facb45f0$@codenest.com>

On Wed, Jun 01 2016 at 12:23am -0400,
James Johnston <johnstonj.public@codenest.com> wrote:

> Hi Mikulas,
> 
> > bio_alloc can allocate a bio with at most BIO_MAX_PAGES (256) vector
> > entries. However, the incoming bio may have more vector entries if it was
> > allocated by other means. For example, bcache submits bios with more than
> > BIO_MAX_PAGES entries. This results in bio_alloc failure.
> > 
> > To avoid the failure, change the code so that it allocates bio with at
> > most BIO_MAX_PAGES entries. If the incoming bio has more entries,
> > bio_add_page will fail and a new bio will be allocated - the code that
> > handles bio_add_page failure already exists in the dm-log-writes target.
> > 
> > Also, move atomic_inc(&lc->io_blocks) before bio_alloc to fix a bug that
> > the target hangs if bio_alloc fails. The error path does put_io_block(lc),
> > so we must do atomic_inc(&lc->io_blocks) before invoking the error path to
> > avoid underflow of lc->io_blocks.
> > 
> > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> > Cc: stable@vger.kernel.org	# v4.1+
> 
> How does this relate to the previous patch you made to dm-crypt?  How best
> should I test this?  It looks like the dm-crypt patch fixed the problem.
> 
> Should I test by applying this patch ONLY and reverting the dm-crypt patch?
> (i.e. does this patch also fix the problem.)  Or should I just test with
> both patches applied simultaneously?

The dm-log-writes patch has nothing to do with the dm-crypt patch.  It
is just that both targets have comparable issues with bcache issuing
really large bios.

^ permalink raw reply

* Re: raid 5 crashed
From: Wols Lists @ 2016-06-01 15:36 UTC (permalink / raw)
  To: Brad Campbell, Edward Kuns
  Cc: Phil Turmel, bobzer, linux-raid, Mikael Abrahamsson
In-Reply-To: <20b1858b-0cd7-a7de-82af-9167dbcfb09e@fnarfbargle.com>

On 01/06/16 05:07, Brad Campbell wrote:
> I have however done a *lot* of data recovery on single drives over the
> years and can absolutely vouch that dd will leave you in tears.

Good to know! I've regularly used dd on drives, but not on ones that
were in trouble (maybe once ...)

But now drives are at the point that you cannot guarantee an error-free
scan even on just one pass, I guess I'll have to make sure I use
ddrescue from now on - I usually just read an (old) drive into a file on
a new larger hard disk, loopmount it, and proceed from there ... never
had any trouble so far ...

Cheers,
Wol

^ permalink raw reply

* Re: raid 5 crashed
From: Wols Lists @ 2016-06-01 15:42 UTC (permalink / raw)
  To: Brad Campbell, Phil Turmel, bobzer; +Cc: linux-raid, Mikael Abrahamsson
In-Reply-To: <95079572-f319-ca57-a3e9-e8d00ef40248@fnarfbargle.com>

On 01/06/16 02:48, Brad Campbell wrote:
> Now, having said that :
> 
> Much better to try and get the array running in a read-only state with
> all disks in place and clone the data from the array rather than the
> disks after they've been ddrescued. In the case of a running array, a
> read error on one of the array members will see the RAID attempt to get
> the data from elsewhere (a reconstruction), whereas a read from a disc
> cloned with ddrescue will happily just report what was a faulty sector
> as a big pile of zeros, and *poof* your data is gone.
> 
> Set the timeouts appropriately (and conservatively) to give the disks
> time to actually report they can't read the sector. This will allow md
> to try and get it elsewhere rather than kicking the disc out because the
> storage stack timed it out as faulty.

Okay - so would this be better (a lot slower, possibly, but safe ...)

Use dd - so it DOES bomb on error! - and only replace the drive once
you've got a clean read off it. With 2TB drives, that should work so
long as they're not faulty. And if it's - JUST - a timeout issue,
this'll work fine?

Cheers,
Wol

^ permalink raw reply


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