* Re: [PATCH] md: avoid warning for 32-bit sector_t
From: NeilBrown @ 2015-12-16 4:02 UTC (permalink / raw)
To: Arnd Bergmann, linux-raid; +Cc: linux-kernel, linux-arm-kernel
In-Reply-To: <65226011.NWtA6TWNA3@wuerfel>
[-- Attachment #1: Type: text/plain, Size: 2301 bytes --]
On Tue, Nov 24 2015, Arnd Bergmann wrote:
> When CONFIG_LBDAF is not set, sector_t is only 32-bits wide, which
> means we cannot have devices with more than 2TB, and the code that
> is trying to handle compatibility support for large devices in
> md version 0.90 is meaningless but also causes a compile-time warning:
>
> drivers/md/md.c: In function 'super_90_load':
> drivers/md/md.c:1029:19: warning: large integer implicitly truncated to unsigned type [-Woverflow]
> drivers/md/md.c: In function 'super_90_rdev_size_change':
> drivers/md/md.c:1323:17: warning: large integer implicitly truncated to unsigned type [-Woverflow]
>
> This adds a check for CONFIG_LBDAF to avoid even getting into this
> code path, and also adds an explicit cast to let the compiler know
> it doesn't have to warn about the truncation.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Noticed on ARM randconfig builds with recent gcc versions.
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 0b48c5d7c489..ee9a7ab44f32 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -1025,8 +1025,9 @@ static int super_90_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor
> * (not needed for Linear and RAID0 as metadata doesn't
> * record this size)
> */
> - if (rdev->sectors >= (2ULL << 32) && sb->level >= 1)
> - rdev->sectors = (2ULL << 32) - 2;
> + if (IS_ENABLED(CONFIG_LBDAF) && (u64)rdev->sectors >= (2ULL << 32) &&
> + sb->level >= 1)
> + rdev->sectors = (sector_t)(2ULL << 32) - 2;
>
> if (rdev->sectors < ((sector_t)sb->size) * 2 && sb->level >= 1)
> /* "this cannot possibly happen" ... */
> @@ -1319,8 +1320,9 @@ super_90_rdev_size_change(struct md_rdev *rdev, sector_t num_sectors)
> /* Limit to 4TB as metadata cannot record more than that.
> * 4TB == 2^32 KB, or 2*2^32 sectors.
> */
> - if (num_sectors >= (2ULL << 32) && rdev->mddev->level >= 1)
> - num_sectors = (2ULL << 32) - 2;
> + if (IS_ENABLED(CONFIG_LBDAF) && (u64)num_sectors >= (2ULL << 32) &&
> + rdev->mddev->level >= 1)
> + num_sectors = (sector_t)(2ULL << 32) - 2;
> md_super_write(rdev->mddev, rdev, rdev->sb_start, rdev->sb_size,
> rdev->sb_page);
> md_super_wait(rdev->mddev);
applied, thanks.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: [PATCH] raid5-cache: add journal hot add/remove support
From: NeilBrown @ 2015-12-16 3:42 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, Kernel-team, songliubraving, hch
In-Reply-To: <20151216013310.GA741432@devbig084.prn1.facebook.com>
[-- Attachment #1: Type: text/plain, Size: 729 bytes --]
On Wed, Dec 16 2015, Shaohua Li wrote:
>> And I'm a bit uncertain about setting rdev->raid_disk to
>> mddev->raid_disks.
>> I thought we decided that "Journal" devices had a different namespace
>> for raid_disk than data disks, so ->raid_disk of 0 was appropriate?
>> Setting the journal raid_disk to mddev->raid_disks might cause problems
>> when we ultimately support reshaping an array with a journal.
>
> 0 will introduce confusion in sysfs entry, eg, which disk should
> xxx/md/rd0 point to? Create a special sys file? This appears the only
> reason we don't use 0.
sysfs_link_rdev already avoids the "rd%d" link of Replacement devices.
Change it and sysfs_unlink_rdev to ignore the Journal device too.
Thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* [PATCH v2 2/2] md: dm-crypt: Introduce the bulk IV mode for bulk crypto
From: Baolin Wang @ 2015-12-16 3:18 UTC (permalink / raw)
To: axboe, agk, snitzer, dm-devel
Cc: neilb, dan.j.williams, martin.petersen, sagig, kent.overstreet,
keith.busch, tj, broonie, arnd, linux-block, linux-raid,
linux-kernel, baolin.wang
In-Reply-To: <cover.1450233399.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) using 64KB
I/Os on an eMMC storage device I saw about 60% improvement in throughput for
encrypted writes, and about 100% 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 | 333 ++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 327 insertions(+), 6 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 917d47e..dc2e5e6 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -32,6 +32,7 @@
#include <linux/device-mapper.h>
#define DM_MSG_PREFIX "crypt"
+#define DM_MAX_SG_LIST 1024
/*
* context holding the current state of a multi-part conversion
@@ -68,6 +69,8 @@ struct dm_crypt_request {
struct convert_context *ctx;
struct scatterlist sg_in;
struct scatterlist sg_out;
+ struct sg_table sgt_in;
+ struct sg_table sgt_out;
sector_t iv_sector;
};
@@ -140,6 +143,7 @@ struct crypt_config {
char *cipher;
char *cipher_string;
+ int bulk_crypto;
struct crypt_iv_operations *iv_gen_ops;
union {
struct iv_essiv_private essiv;
@@ -238,6 +242,9 @@ static struct crypto_ablkcipher *any_tfm(struct crypt_config *cc)
*
* plumb: unimplemented, see:
* http://article.gmane.org/gmane.linux.kernel.device-mapper.dm-crypt/454
+ *
+ * bulk: the initial vector is the 64-bit little-endian version of the sector
+ * number, which is used as just one initial IV for one bulk data.
*/
static int crypt_iv_plain_gen(struct crypt_config *cc, u8 *iv,
@@ -755,6 +762,15 @@ static int crypt_iv_tcw_post(struct crypt_config *cc, u8 *iv,
return r;
}
+static int crypt_iv_bulk_gen(struct crypt_config *cc, u8 *iv,
+ struct dm_crypt_request *dmreq)
+{
+ memset(iv, 0, cc->iv_size);
+ *(__le64 *)iv = cpu_to_le64(dmreq->iv_sector);
+
+ return 0;
+}
+
static struct crypt_iv_operations crypt_iv_plain_ops = {
.generator = crypt_iv_plain_gen
};
@@ -799,6 +815,10 @@ static struct crypt_iv_operations crypt_iv_tcw_ops = {
.post = crypt_iv_tcw_post
};
+static struct crypt_iv_operations crypt_iv_bulk_ops = {
+ .generator = crypt_iv_bulk_gen
+};
+
static void crypt_convert_init(struct crypt_config *cc,
struct convert_context *ctx,
struct bio *bio_out, struct bio *bio_in,
@@ -833,6 +853,11 @@ static u8 *iv_of_dmreq(struct crypt_config *cc,
crypto_ablkcipher_alignmask(any_tfm(cc)) + 1);
}
+static int crypt_is_bulk_mode(struct crypt_config *cc)
+{
+ return cc->bulk_crypto;
+}
+
static int crypt_convert_block(struct crypt_config *cc,
struct convert_context *ctx,
struct ablkcipher_request *req)
@@ -881,24 +906,40 @@ static int crypt_convert_block(struct crypt_config *cc,
static void kcryptd_async_done(struct crypto_async_request *async_req,
int error);
+static void kcryptd_async_all_done(struct crypto_async_request *async_req,
+ int error);
static void crypt_alloc_req(struct crypt_config *cc,
struct convert_context *ctx)
{
unsigned key_index = ctx->cc_sector & (cc->tfms_count - 1);
+ struct dm_crypt_request *dmreq;
if (!ctx->req)
ctx->req = mempool_alloc(cc->req_pool, GFP_NOIO);
+ dmreq = dmreq_of_req(cc, ctx->req);
+ dmreq->sgt_in.orig_nents = 0;
+ dmreq->sgt_out.orig_nents = 0;
+
ablkcipher_request_set_tfm(ctx->req, cc->tfms[key_index]);
/*
* Use REQ_MAY_BACKLOG so a cipher driver internally backlogs
* requests if driver request queue is full.
*/
- ablkcipher_request_set_callback(ctx->req,
- CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
- kcryptd_async_done, dmreq_of_req(cc, ctx->req));
+ if (crypt_is_bulk_mode(cc))
+ ablkcipher_request_set_callback(ctx->req,
+ CRYPTO_TFM_REQ_MAY_BACKLOG
+ | CRYPTO_TFM_REQ_MAY_SLEEP,
+ kcryptd_async_all_done,
+ dmreq_of_req(cc, ctx->req));
+ else
+ ablkcipher_request_set_callback(ctx->req,
+ CRYPTO_TFM_REQ_MAY_BACKLOG
+ | CRYPTO_TFM_REQ_MAY_SLEEP,
+ kcryptd_async_done,
+ dmreq_of_req(cc, ctx->req));
}
static void crypt_free_req(struct crypt_config *cc,
@@ -911,6 +952,221 @@ static void crypt_free_req(struct crypt_config *cc,
}
/*
+ * Check how many sg entry numbers are needed when map one bio
+ * with scatterlist in advance.
+ */
+static unsigned int crypt_sg_entry(struct bio *bio_t)
+{
+ struct request_queue *q = bdev_get_queue(bio_t->bi_bdev);
+ int cluster = blk_queue_cluster(q);
+ struct bio_vec bvec, bvprv = { NULL };
+ struct bvec_iter biter;
+ unsigned long nbytes = 0, sg_length = 0;
+ unsigned int sg_cnt = 0;
+
+ if (bio_t->bi_rw & REQ_DISCARD) {
+ if (bio_t->bi_vcnt)
+ return 1;
+ return 0;
+ }
+
+ if (bio_t->bi_rw & REQ_WRITE_SAME)
+ return 1;
+
+ bio_for_each_segment(bvec, bio_t, biter) {
+ nbytes = bvec.bv_len;
+
+ if (!cluster) {
+ sg_cnt++;
+ continue;
+ }
+
+ if (sg_length + nbytes > queue_max_segment_size(q)) {
+ sg_length = nbytes;
+ sg_cnt++;
+ goto next;
+ }
+
+ if (!BIOVEC_PHYS_MERGEABLE(&bvprv, &bvec)) {
+ sg_length = nbytes;
+ sg_cnt++;
+ goto next;
+ }
+
+ if (!BIOVEC_SEG_BOUNDARY(q, &bvprv, &bvec)) {
+ sg_length = nbytes;
+ sg_cnt++;
+ goto next;
+ }
+
+ sg_length += nbytes;
+next:
+ memcpy(&bvprv, &bvec, sizeof(struct bio_vec));
+ }
+
+ return sg_cnt;
+}
+
+static int crypt_convert_all_blocks(struct crypt_config *cc,
+ struct convert_context *ctx,
+ struct ablkcipher_request *req)
+{
+ struct dm_crypt_io *io =
+ container_of(ctx, struct dm_crypt_io, ctx);
+ struct dm_crypt_request *dmreq = dmreq_of_req(cc, req);
+ u8 *iv = iv_of_dmreq(cc, dmreq);
+ struct bio *orig_bio = io->base_bio;
+ struct bio *bio_in = ctx->bio_in;
+ struct bio *bio_out = ctx->bio_out;
+ unsigned int total_bytes = orig_bio->bi_iter.bi_size;
+ struct scatterlist *sg_in = NULL;
+ struct scatterlist *sg_out = NULL;
+ struct scatterlist *sg = NULL;
+ unsigned int total_sg_len_in = 0;
+ unsigned int total_sg_len_out = 0;
+ unsigned int sg_in_max = 0, sg_out_max = 0;
+ int ret;
+
+ dmreq->iv_sector = ctx->cc_sector;
+ dmreq->ctx = ctx;
+
+ /*
+ * Need to calculate how many sg entry need to be used
+ * for this bio.
+ */
+ sg_in_max = crypt_sg_entry(bio_in) + 1;
+ if (sg_in_max > DM_MAX_SG_LIST || sg_in_max <= 0) {
+ DMERR("%s sg entry too large or none %d\n",
+ __func__, sg_in_max);
+ return -EINVAL;
+ } else if (sg_in_max == 2) {
+ sg_in = &dmreq->sg_in;
+ }
+
+ if (!sg_in) {
+ ret = sg_alloc_table(&dmreq->sgt_in, sg_in_max, GFP_KERNEL);
+ if (ret) {
+ DMERR("%s sg in allocation failed\n", __func__);
+ return -ENOMEM;
+ }
+
+ sg_in = dmreq->sgt_in.sgl;
+ }
+
+ total_sg_len_in = __blk_bios_map_sg(bdev_get_queue(bio_in->bi_bdev),
+ bio_in, sg_in, &sg);
+ if ((total_sg_len_in <= 0)
+ || (total_sg_len_in > sg_in_max)) {
+ DMERR("%s in sg map error %d, sg_in_max[%d]\n",
+ __func__, total_sg_len_in, sg_in_max);
+ return -EINVAL;
+ }
+
+ if (sg)
+ sg_mark_end(sg);
+
+ ctx->iter_in.bi_size -= total_bytes;
+
+ if (bio_data_dir(orig_bio) == READ)
+ goto set_crypt;
+
+ sg_out_max = crypt_sg_entry(bio_out) + 1;
+ if (sg_out_max > DM_MAX_SG_LIST || sg_out_max <= 0) {
+ DMERR("%s sg entry too large or none %d\n",
+ __func__, sg_out_max);
+ return -EINVAL;
+ } else if (sg_out_max == 2) {
+ sg_out = &dmreq->sg_out;
+ }
+
+ if (!sg_out) {
+ ret = sg_alloc_table(&dmreq->sgt_out, sg_out_max, GFP_KERNEL);
+ if (ret) {
+ DMERR("%s sg out allocation failed\n", __func__);
+ return -ENOMEM;
+ }
+
+ sg_out = dmreq->sgt_out.sgl;
+ }
+
+ sg = NULL;
+ total_sg_len_out = __blk_bios_map_sg(bdev_get_queue(bio_out->bi_bdev),
+ bio_out, sg_out, &sg);
+ if ((total_sg_len_out <= 0) ||
+ (total_sg_len_out > sg_out_max)) {
+ DMERR("%s out sg map error %d, sg_out_max[%d]\n",
+ __func__, total_sg_len_out, sg_out_max);
+ return -EINVAL;
+ }
+
+ if (sg)
+ sg_mark_end(sg);
+
+ ctx->iter_out.bi_size -= total_bytes;
+set_crypt:
+ if (cc->iv_gen_ops) {
+ ret = cc->iv_gen_ops->generator(cc, iv, dmreq);
+ if (ret < 0) {
+ DMERR("%s generator iv error %d\n", __func__, ret);
+ return ret;
+ }
+ }
+
+ if (bio_data_dir(orig_bio) == WRITE) {
+ ablkcipher_request_set_crypt(req, sg_in,
+ sg_out, total_bytes, iv);
+
+ ret = crypto_ablkcipher_encrypt(req);
+ } else {
+ ablkcipher_request_set_crypt(req, sg_in,
+ sg_in, total_bytes, iv);
+
+ ret = crypto_ablkcipher_decrypt(req);
+ }
+
+ if (!ret && cc->iv_gen_ops && cc->iv_gen_ops->post)
+ ret = cc->iv_gen_ops->post(cc, iv, dmreq);
+
+ return ret;
+}
+
+/*
+ * Encrypt / decrypt data from one whole bio at one time.
+ */
+static int crypt_convert_io(struct crypt_config *cc,
+ struct convert_context *ctx)
+{
+ int r;
+
+ atomic_set(&ctx->cc_pending, 1);
+ crypt_alloc_req(cc, ctx);
+ atomic_inc(&ctx->cc_pending);
+
+ r = crypt_convert_all_blocks(cc, ctx, ctx->req);
+ switch (r) {
+ case -EBUSY:
+ /*
+ * Lets make this synchronous bio by waiting on
+ * in progress as well.
+ */
+ case -EINPROGRESS:
+ wait_for_completion(&ctx->restart);
+ ctx->req = NULL;
+ break;
+ case 0:
+ atomic_dec(&ctx->cc_pending);
+ cond_resched();
+ break;
+ /* There was an error while processing the request. */
+ default:
+ atomic_dec(&ctx->cc_pending);
+ return r;
+ }
+
+ return 0;
+}
+
+/*
* Encrypt / decrypt data from one bio to another one (can be the same one)
*/
static int crypt_convert(struct crypt_config *cc,
@@ -1070,12 +1326,18 @@ static void crypt_dec_pending(struct dm_crypt_io *io)
struct crypt_config *cc = io->cc;
struct bio *base_bio = io->base_bio;
int error = io->error;
+ struct dm_crypt_request *dmreq;
if (!atomic_dec_and_test(&io->io_pending))
return;
- if (io->ctx.req)
+ if (io->ctx.req) {
+ dmreq = dmreq_of_req(cc, io->ctx.req);
+ sg_free_table(&dmreq->sgt_out);
+ sg_free_table(&dmreq->sgt_in);
+
crypt_free_req(cc, io->ctx.req, base_bio);
+ }
base_bio->bi_error = error;
bio_endio(base_bio);
@@ -1312,7 +1574,11 @@ static void kcryptd_crypt_write_convert(struct dm_crypt_io *io)
sector += bio_sectors(clone);
crypt_inc_pending(io);
- r = crypt_convert(cc, &io->ctx);
+ if (crypt_is_bulk_mode(cc))
+ r = crypt_convert_io(cc, &io->ctx);
+ else
+ r = crypt_convert(cc, &io->ctx);
+
if (r)
io->error = -EIO;
crypt_finished = atomic_dec_and_test(&io->ctx.cc_pending);
@@ -1342,7 +1608,11 @@ static void kcryptd_crypt_read_convert(struct dm_crypt_io *io)
crypt_convert_init(cc, &io->ctx, io->base_bio, io->base_bio,
io->sector);
- r = crypt_convert(cc, &io->ctx);
+ if (crypt_is_bulk_mode(cc))
+ r = crypt_convert_io(cc, &io->ctx);
+ else
+ r = crypt_convert(cc, &io->ctx);
+
if (r < 0)
io->error = -EIO;
@@ -1387,6 +1657,40 @@ static void kcryptd_async_done(struct crypto_async_request *async_req,
kcryptd_crypt_write_io_submit(io, 1);
}
+static void kcryptd_async_all_done(struct crypto_async_request *async_req,
+ int error)
+{
+ struct dm_crypt_request *dmreq = async_req->data;
+ struct convert_context *ctx = dmreq->ctx;
+ struct dm_crypt_io *io = container_of(ctx, struct dm_crypt_io, ctx);
+ struct crypt_config *cc = io->cc;
+
+ if (error == -EINPROGRESS)
+ return;
+
+ if (!error && cc->iv_gen_ops && cc->iv_gen_ops->post)
+ error = cc->iv_gen_ops->post(cc, iv_of_dmreq(cc, dmreq), dmreq);
+
+ if (error < 0)
+ io->error = error;
+
+ sg_free_table(&dmreq->sgt_out);
+ sg_free_table(&dmreq->sgt_in);
+
+ crypt_free_req(cc, req_of_dmreq(cc, dmreq), io->base_bio);
+
+ if (!atomic_dec_and_test(&ctx->cc_pending)) {
+ complete(&io->ctx.restart);
+ return;
+ }
+
+ complete(&io->ctx.restart);
+ if (bio_data_dir(io->base_bio) == READ)
+ kcryptd_crypt_read_done(io);
+ else
+ kcryptd_crypt_write_io_submit(io, 1);
+}
+
static void kcryptd_crypt(struct work_struct *work)
{
struct dm_crypt_io *io = container_of(work, struct dm_crypt_io, work);
@@ -1633,6 +1937,21 @@ static int crypt_ctr_cipher(struct dm_target *ti,
goto bad_mem;
}
+ /*
+ * Here we need to check if it can be encrypted or decrypted with
+ * bulk block, which means these encryption modes don't need IV or
+ * just need one initial IV. For bulk mode, we can expand the
+ * scatterlist entries to map the bio, then send all the scatterlists
+ * to the hardware engine at one time to improve the crypto engine
+ * efficiency. But it does not fit for other IV modes, it has to do
+ * encryption and decryption sector by sector because every sector
+ * has different IV.
+ */
+ if (!ivmode || !strcmp(ivmode, "bulk") || !strcmp(ivmode, "null"))
+ cc->bulk_crypto = 1;
+ else
+ cc->bulk_crypto = 0;
+
/* Allocate cipher */
ret = crypt_alloc_tfms(cc, cipher_api);
if (ret < 0) {
@@ -1680,6 +1999,8 @@ static int crypt_ctr_cipher(struct dm_target *ti,
cc->iv_gen_ops = &crypt_iv_tcw_ops;
cc->key_parts += 2; /* IV + whitening */
cc->key_extra_size = cc->iv_size + TCW_WHITENING_SIZE;
+ } else if (strcmp(ivmode, "bulk") == 0) {
+ cc->iv_gen_ops = &crypt_iv_bulk_ops;
} else {
ret = -EINVAL;
ti->error = "Invalid IV mode";
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 1/2] block: Export the __blk_bios_map_sg() to map one bio
From: Baolin Wang @ 2015-12-16 3:18 UTC (permalink / raw)
To: axboe, agk, snitzer, dm-devel
Cc: neilb, dan.j.williams, martin.petersen, sagig, kent.overstreet,
keith.busch, tj, broonie, arnd, linux-block, linux-raid,
linux-kernel, baolin.wang
In-Reply-To: <cover.1450233399.git.baolin.wang@linaro.org>
In dm-crypt, it need to map one bio to scatterlist for improving the
encryption efficiency. Thus this patch exports the __blk_bios_map_sg()
function to map one bio with scatterlists.
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
block/blk-merge.c | 7 ++++---
include/linux/blkdev.h | 3 +++
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/block/blk-merge.c b/block/blk-merge.c
index de5716d8..09cc7c4 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -331,9 +331,9 @@ new_segment:
*bvprv = *bvec;
}
-static int __blk_bios_map_sg(struct request_queue *q, struct bio *bio,
- struct scatterlist *sglist,
- struct scatterlist **sg)
+int __blk_bios_map_sg(struct request_queue *q, struct bio *bio,
+ struct scatterlist *sglist,
+ struct scatterlist **sg)
{
struct bio_vec bvec, bvprv = { NULL };
struct bvec_iter iter;
@@ -372,6 +372,7 @@ single_segment:
return nsegs;
}
+EXPORT_SYMBOL(__blk_bios_map_sg);
/*
* map a request to scatterlist, return number of sg entries setup. Caller
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 3fe27f8..dd8d10f 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1004,6 +1004,9 @@ extern void blk_queue_flush_queueable(struct request_queue *q, bool queueable);
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_bios_map_sg(struct request_queue *q, struct bio *bio,
+ struct scatterlist *sglist,
+ struct scatterlist **sg);
extern void blk_dump_rq_flags(struct request *, char *);
extern long nr_blockdev_pages(void);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 0/2] Introduce the bulk IV mode for improving the crypto engine efficiency
From: Baolin Wang @ 2015-12-16 3:18 UTC (permalink / raw)
To: axboe, agk, snitzer, dm-devel
Cc: neilb, dan.j.williams, martin.petersen, sagig, kent.overstreet,
keith.busch, tj, broonie, arnd, linux-block, linux-raid,
linux-kernel, baolin.wang
From the dm-crypt performance report, we found it shows low efficiency
with crypto engine for some mode (like ecb or xts mode). Because in dm
crypt, it will map the IO data buffer with scatterlist, and send the
scatterlist of one bio to the encryption engine, if send more scatterlists
with bigger size at one time, that helps the engine palys best performance,
which means a high encryption speed.
But now the dm-crypt only map one segment (always one sector) of one bio
with one scatterlist to the crypto engine at one time. which is more
time-consuming and ineffective for the crypto engine. Especially for some
modes which don't need different IV for each sector, we can map the whole
bio with multiple scatterlists to improve the engine performance.
But this optimization is not support some ciphers and IV modes which should
do sector by sector and need different IV for each sector.
Change since v1:
- Introduce one different IV mode.
- Change the conditions for bulk mode.
Baolin Wang (2):
block: Export the __blk_bios_map_sg() to map one bio
md: dm-crypt: Introduce the bulk IV mode for bulk crypto
block/blk-merge.c | 7 +-
drivers/md/dm-crypt.c | 333 +++++++++++++++++++++++++++++++++++++++++++++++-
include/linux/blkdev.h | 3 +
3 files changed, 334 insertions(+), 9 deletions(-)
--
1.7.9.5
^ permalink raw reply
* Re: [PATCH 4/4] mdadm: do not display bitmap info if it is cleared
From: NeilBrown @ 2015-12-16 2:24 UTC (permalink / raw)
To: linux-raid; +Cc: rgoldwyn, Guoqing Jiang
In-Reply-To: <1448987412-3932-4-git-send-email-gqjiang@suse.com>
[-- Attachment #1: Type: text/plain, Size: 3833 bytes --]
On Wed, Dec 02 2015, Guoqing Jiang wrote:
> "mdadm -X DISK" is used to report information about a bitmap
> file, it is better to not display all the related infos if
> bitmap is cleared with "--bitmap=none" under grow mode.
>
> To do that, the locate_bitmap is changed a little to have a
> return value based on MD_FEATURE_BITMAP_OFFSET.
>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
> bitmap.c | 8 ++++++--
> mdadm.h | 2 +-
> super0.c | 7 ++++---
> super1.c | 12 +++++++++---
> 4 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/bitmap.c b/bitmap.c
> index 803eda3..dab674b 100644
> --- a/bitmap.c
> +++ b/bitmap.c
> @@ -221,8 +221,12 @@ int bitmap_file_open(char *filename, struct supertype **stp)
> pr_err("No bitmap possible with %s metadata\n",
> st->ss->name);
> return -1;
> - } else
> - st->ss->locate_bitmap(st, fd);
> + } else {
> + if (st->ss->locate_bitmap(st, fd)) {
> + pr_err("%s doesn't have bitmap\n", filename);
> + fd = -1;
> + }
> + }
>
> *stp = st;
> } else {
> diff --git a/mdadm.h b/mdadm.h
> index 5d5e97f..aad0fa8 100644
> --- a/mdadm.h
> +++ b/mdadm.h
> @@ -873,7 +873,7 @@ extern struct superswitch {
> /* Seek 'fd' to start of write-intent-bitmap. Must be an
> * md-native format bitmap
> */
> - void (*locate_bitmap)(struct supertype *st, int fd);
> + int (*locate_bitmap)(struct supertype *st, int fd);
> /* if add_internal_bitmap succeeded for existing array, this
> * writes it out.
> */
> diff --git a/super0.c b/super0.c
> index 6ad9d39..7f80014 100644
> --- a/super0.c
> +++ b/super0.c
> @@ -1155,16 +1155,16 @@ static int add_internal_bitmap0(struct supertype *st, int *chunkp,
> return 1;
> }
>
> -static void locate_bitmap0(struct supertype *st, int fd)
> +static int locate_bitmap0(struct supertype *st, int fd)
> {
> unsigned long long dsize;
> unsigned long long offset;
>
> if (!get_dev_size(fd, NULL, &dsize))
> - return;
> + return -1;
>
> if (dsize < MD_RESERVED_SECTORS*512)
> - return;
> + return -1;
>
> offset = MD_NEW_SIZE_SECTORS(dsize>>9);
>
> @@ -1173,6 +1173,7 @@ static void locate_bitmap0(struct supertype *st, int fd)
> offset += MD_SB_BYTES;
>
> lseek64(fd, offset, 0);
> + return 0;
> }
>
> static int write_bitmap0(struct supertype *st, int fd, enum bitmap_update update)
> diff --git a/super1.c b/super1.c
> index 062d9e7..7a1156d 100644
> --- a/super1.c
> +++ b/super1.c
> @@ -1520,7 +1520,7 @@ static int add_to_super1(struct supertype *st, mdu_disk_info_t *dk,
> }
> #endif
>
> -static void locate_bitmap1(struct supertype *st, int fd);
> +static int locate_bitmap1(struct supertype *st, int fd);
>
> static int store_super1(struct supertype *st, int fd)
> {
> @@ -2304,24 +2304,30 @@ add_internal_bitmap1(struct supertype *st,
> return 1;
> }
>
> -static void locate_bitmap1(struct supertype *st, int fd)
> +static int locate_bitmap1(struct supertype *st, int fd)
> {
> unsigned long long offset;
> struct mdp_superblock_1 *sb;
> int mustfree = 0;
> + int ret;
>
> if (!st->sb) {
> if (st->ss->load_super(st, fd, NULL))
> - return; /* no error I hope... */
> + return -1; /* no error I hope... */
> mustfree = 1;
> }
> sb = st->sb;
>
> + if ((__le32_to_cpu(sb->feature_map) & MD_FEATURE_BITMAP_OFFSET))
> + ret = 0;
> + else
> + ret = -1;
> offset = __le64_to_cpu(sb->super_offset);
> offset += (int32_t) __le32_to_cpu(sb->bitmap_offset);
> if (mustfree)
> free(sb);
> lseek64(fd, offset<<9, 0);
> + return ret;
> }
>
> static int write_bitmap1(struct supertype *st, int fd, enum bitmap_update update)
> --
> 2.1.4
All 4 applied,
thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: [PATCH 00/17] mdadm: SCSI enclosure based array
From: NeilBrown @ 2015-12-16 2:01 UTC (permalink / raw)
To: linux-raid; +Cc: dan.j.williams, shli, Song Liu
In-Reply-To: <1449015933-255689-1-git-send-email-songliubraving@fb.com>
[-- Attachment #1: Type: text/plain, Size: 758 bytes --]
On Wed, Dec 02 2015, Song Liu wrote:
> These patches by Dan Williams enable creating RAID array based on
> SCSI enclosures.
Is there any background reading available so I can have some idea what
these patches are supposed to do.
I have a vague idea what an "enclosure" is but I didn't know that Linux
knew about them. Is there a link to the LWN article about them or some
other useful documentation? "git grep -i enclosur Documentation/"
didn't show anything likely to be useful.
And what is "/dev/disk/by-slot"?? I've never come across it before
and google doesn't help much. Just these patches and
http://comments.gmane.org/gmane.linux.scsi/93809
which doesn't seem very conclusive.
So currently I am not able to review these patches.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: [PATCH v2 0/2] mdadm: re-add journal to array with bad journal
From: NeilBrown @ 2015-12-16 1:45 UTC (permalink / raw)
To: linux-raid; +Cc: dan.j.williams, shli, hch, kernel-team, Song Liu
In-Reply-To: <1450143823-1145846-1-git-send-email-songliubraving@fb.com>
[-- Attachment #1: Type: text/plain, Size: 546 bytes --]
On Tue, Dec 15 2015, Song Liu wrote:
> These patches enable recreation of missing/faulty journal. This requires
> kernel changes as well, which Shaohua will send out later.
>
> Song Liu (2):
> add sysfs_array_state to struct mdinfo
> [mdadm] recreate journal in mdadm
>
> Manage.c | 42 +++++++++++++++++++++++++++++++++++++++---
> ReadMe.c | 1 +
> mdadm.c | 8 ++++++++
> mdadm.h | 5 +++++
> super1.c | 3 ++-
> sysfs.c | 7 +++++++
> 6 files changed, 62 insertions(+), 4 deletions(-)
>
> --
> 2.4.6
applied, thanks.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: [PATCH] raid5-cache: add journal hot add/remove support
From: Shaohua Li @ 2015-12-16 1:33 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, Kernel-team, songliubraving, hch
In-Reply-To: <87r3inb3eh.fsf@notabene.neil.brown.name>
On Wed, Dec 16, 2015 at 12:14:46PM +1100, NeilBrown wrote:
> On Tue, Dec 15 2015, Shaohua Li wrote:
>
> > Add support for journal disk hot add/remove. Mostly trival checks in md
> > part. The raid5 part is a little tricky. For hot-remove, we can't wait
> > pending write as it's called from raid5d. The wait will cause deadlock.
> > We simplily fail the hot-remove. A hot-remove retry can success
> > eventually since if journal disk is faulty all pending write will be
> > failed and finish. For hot-add, since an array supporting journal but
> > without journal disk will be marked read-only, we are safe to hot add
> > journal without stopping IO (should be read IO, while journal only
> > handles write IO).
> >
> > Signed-off-by: Shaohua Li <shli@fb.com>
> > ---
> > drivers/md/md.c | 44 +++++++++++++++++++++++++++++++++-----------
> > drivers/md/raid5.c | 31 +++++++++++++++++++++++--------
> > 2 files changed, 56 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index 807095f..de9e3a1 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -2053,7 +2053,9 @@ static int bind_rdev_to_array(struct md_rdev *rdev, struct mddev *mddev)
> >
> > /* make sure rdev->sectors exceeds mddev->dev_sectors */
> > if (rdev->sectors && (mddev->dev_sectors == 0 ||
> > - rdev->sectors < mddev->dev_sectors)) {
> > + rdev->sectors < mddev->dev_sectors) &&
> > + !(test_bit(MD_HAS_JOURNAL, &mddev->flags) &&
> > + test_bit(Journal, &rdev->flags))) {
> > if (mddev->pers) {
> > /* Cannot change size, so fail
> > * If mddev->level <= 0, then we don't care
>
> This is (nearly) right, but it took me far too long to see that it was
> right - and that makes it wrong.
> If the end result was:
>
> /* make sure rdev->sectors exceeds mddev->dev_sectors for
> * non-journal devices
> */
> if (!test_bit(Journal, &rdev->flags) &&
> rdev->sectors &&
> (mddev->dev_sectors == 0 || rdev->sectors < mddev->dev_sectors)) {
> .....
>
> it would be a lot more obviously correct. I removed the test on
> MD_HAS_JOURNAL because it seems irrelevant. We don't want to perform
> that test on an rdev with Journal set whether MD_HAS_JOURNAL is set or
> not.
Ok
>
> > @@ -2084,7 +2086,9 @@ static int bind_rdev_to_array(struct md_rdev *rdev, struct mddev *mddev)
> > }
> > }
> > rcu_read_unlock();
> > - if (mddev->max_disks && rdev->desc_nr >= mddev->max_disks) {
> > + if (mddev->max_disks && rdev->desc_nr >= mddev->max_disks &&
> > + !(test_bit(MD_HAS_JOURNAL, &mddev->flags) &&
> > + test_bit(Journal, &rdev->flags))) {
> > printk(KERN_WARNING "md: %s: array is limited to %d devices\n",
> > mdname(mddev), mddev->max_disks);
> > return -EBUSY;
>
> Same here. Removed the test on MD_HAS_JOURNAL is it is just confusing.
> I wold prefer the test on Journal came first, but would accept having it
> last if you prefer that.
I'm ok with it
> > @@ -6031,8 +6035,25 @@ static int add_new_disk(struct mddev *mddev, mdu_disk_info_t *info)
> > else
> > clear_bit(WriteMostly, &rdev->flags);
> >
> > - if (info->state & (1<<MD_DISK_JOURNAL))
> > + if (info->state & (1<<MD_DISK_JOURNAL)) {
> > + struct md_rdev *rdev2;
> > + bool has_journal = false;
> > +
> > + /* make sure no existing journal disk */
> > + rcu_read_lock();
> > + rdev_for_each(rdev2, mddev) {
> > + if (test_bit(Journal, &rdev2->flags)) {
> > + has_journal = true;
> > + break;
> > + }
> > + }
> > + rcu_read_unlock();
> > + if (has_journal) {
> > + export_rdev(rdev);
> > + return -EBUSY;
> > + }
> > set_bit(Journal, &rdev->flags);
> > + }
> > /*
> > * check whether the device shows up in other nodes
> > */
>
> rcu_read_lock() isn't really needed here as we hold ->reconfig_mutex so
> rdevs cannot disappear.
> Maybe it is OK to leave it there as it costs almost nothing. But maybe
> it could lead other people to think RCU protection is needed in this
> case, which is wrong....
>
I'll delete it
> > @@ -8162,19 +8183,20 @@ static int remove_and_add_spares(struct mddev *mddev,
> > continue;
> > if (test_bit(Faulty, &rdev->flags))
> > continue;
> > - if (test_bit(Journal, &rdev->flags))
> > - continue;
> > - if (mddev->ro &&
> > - ! (rdev->saved_raid_disk >= 0 &&
> > - !test_bit(Bitmap_sync, &rdev->flags)))
> > - continue;
> > + if (!test_bit(Journal, &rdev->flags)) {
> > + if (mddev->ro &&
> > + ! (rdev->saved_raid_disk >= 0 &&
> > + !test_bit(Bitmap_sync, &rdev->flags)))
> > + continue;
> >
> > - rdev->recovery_offset = 0;
> > + rdev->recovery_offset = 0;
> > + }
> > if (mddev->pers->
> > hot_add_disk(mddev, rdev) == 0) {
> > if (sysfs_link_rdev(mddev, rdev))
> > /* failure here is OK */;
> > - spares++;
> > + if (!test_bit(Journal, &rdev->flags))
> > + spares++;
> > md_new_event(mddev);
> > set_bit(MD_CHANGE_DEVS, &mddev->flags);
> > }
> > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> > index 704ef7f..f8f3d52 100644
> > --- a/drivers/md/raid5.c
> > +++ b/drivers/md/raid5.c
> > @@ -7141,14 +7141,16 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
> > struct disk_info *p = conf->disks + number;
> >
> > print_raid5_conf(conf);
> > - if (test_bit(Journal, &rdev->flags)) {
> > + if (test_bit(Journal, &rdev->flags) && conf->log) {
> > /*
> > - * journal disk is not removable, but we need give a chance to
> > - * update superblock of other disks. Otherwise journal disk
> > - * will be considered as 'fresh'
> > + * we can't wait pending write here, as this is called in
> > + * raid5d, wait will deadlock.
> > */
> > - set_bit(MD_CHANGE_DEVS, &mddev->flags);
> > - return -EINVAL;
> > + if (atomic_read(&mddev->writes_pending))
> > + return -EBUSY;
> > + r5l_exit_log(conf->log);
> > + conf->log = NULL;
> > + return 0;
>
> Failing with EBUSY if ->writes_pending is correct.
> I wonder if extra care is needed when ->writes_pending is zero.
> Note that for removing data devices, we set *rdevp to NULL,
> call synchronize_rcu(), then check ->nr_pending and possibly restoring
> the pointer.
> This is because there is no other interlock between new writes arriving
> and a device being removed.
> The possible race would be that between the moment when
> raid5_remove_disk tests ->writes_pending and when it sets conf->log to
> NULL, a new write request comes in and gets as far as calling
> r5_log_disk_error().
> If r5l_log_disk_error() finds that ->log is not NULL, then
> rl5_exit_log() in the other thread frees the log, and
> r5l_log_disk_error() tests a bit in conf->log->rdev->flags, then it
> could be accessing freed memory and anything could happen.
>
> I think you need something like:
>
> log = conf->log;
> conf->log = NULL;
> synchronize_rcu();
> r5l_exit_log(log);
>
> and then r5l_log_disk_error() should do:
> rcu_read_lock();
> log = rcu_dereference(conf->log);
> if (log)
> err = test_bit(Faulty, &log->rdev->flags);
> else
> err = test_bit(MD_HAS_JOURNAL, &conf->mddev->flags);
> rcu_read_unlock();
> return err;
>
> The race is, admittedly, extremely unlikely. But it is best to be safe,
> especially with virtualised environments which can put unexpected delays
> in strange places.
>
> You could possibly use rcu_free() to free the log and avoid the
> synchonize_rcu().
Good catch!
> > }
> > if (rdev == p->rdev)
> > rdevp = &p->rdev;
> > @@ -7212,8 +7214,21 @@ static int raid5_add_disk(struct mddev *mddev, struct md_rdev *rdev)
> > int first = 0;
> > int last = conf->raid_disks - 1;
> >
> > - if (test_bit(Journal, &rdev->flags))
> > - return -EINVAL;
> > + if (test_bit(Journal, &rdev->flags)) {
> > + char b[BDEVNAME_SIZE];
> > + if (conf->log)
> > + return -EBUSY;
> > +
> > + rdev->raid_disk = mddev->raid_disks;
> > + /*
> > + * The array is in readonly mode if journal is missing, so no
> > + * write requests running. We should be safe
> > + */
> > + r5l_init_log(conf, rdev);
> > + printk(KERN_INFO"md/raid:%s: using device %s as journal\n",
> > + mdname(mddev), bdevname(rdev->bdev, b));
> > + return 0;
> > + }
>
> you'll need an "rcu_assign_pointer" in r5l_init_log() for the
> conf->log = log
> to ensure proper ordering of memory writes.
>
> And I'm a bit uncertain about setting rdev->raid_disk to
> mddev->raid_disks.
> I thought we decided that "Journal" devices had a different namespace
> for raid_disk than data disks, so ->raid_disk of 0 was appropriate?
> Setting the journal raid_disk to mddev->raid_disks might cause problems
> when we ultimately support reshaping an array with a journal.
0 will introduce confusion in sysfs entry, eg, which disk should
xxx/md/rd0 point to? Create a special sys file? This appears the only
reason we don't use 0.
Thanks,
Shaohua
^ permalink raw reply
* Re: [PATCH] md/raid10: fix data corruption and crash during resync
From: NeilBrown @ 2015-12-16 1:29 UTC (permalink / raw)
To: Shaohua Li, Artur Paszkiewicz; +Cc: linux-raid, pawel.baldysiak
In-Reply-To: <20151104223353.GA99478@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 5714 bytes --]
On Thu, Nov 05 2015, Shaohua Li wrote:
> On Wed, Nov 04, 2015 at 05:30:30PM +0100, Artur Paszkiewicz wrote:
>> The commit c31df25f20e3 ("md/raid10: make sync_request_write() call
>> bio_copy_data()") replaced manual data copying with bio_copy_data() but
>> it doesn't work as intended. The source bio (fbio) is already processed,
>> so its bvec_iter has bi_size == 0 and bi_idx == bi_vcnt. Because of
>> this, bio_copy_data() either does not copy anything, or worse, copies
>> data from the ->bi_next bio if it is set. This causes wrong data to be
>> written to drives during resync and sometimes lockups/crashes in
>> bio_copy_data():
>>
>> [ 517.338478] NMI watchdog: BUG: soft lockup - CPU#0 stuck for 22s! [md126_raid10:3319]
>> [ 517.347324] Modules linked in: raid10 xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 xt_conntrack ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw iptable_filter ip_tables x86_pkg_temp_thermal coretemp kvm_intel kvm crct10dif_pclmul crc32_pclmul cryptd shpchp pcspkr ipmi_si ipmi_msghandler tpm_crb acpi_power_meter acpi_cpufreq ext4 mbcache jbd2 sr_mod cdrom sd_mod e1000e ax88179_178a usbnet mii ahci ata_generic crc32c_intel libahci ptp pata_acpi libata pps_core wmi sunrpc dm_mirror dm_region_hash dm_log dm_mod
>> [ 517.440555] CPU: 0 PID: 3319 Comm: md126_raid10 Not tainted 4.3.0-rc6+ #1
>> [ 517.448384] Hardware name: Intel Corporation PURLEY/PURLEY, BIOS PLYDCRB1.86B.0055.D14.1509221924 09/22/2015
>> [ 517.459768] task: ffff880153773980 ti: ffff880150df8000 task.ti: ffff880150df8000
>> [ 517.468529] RIP: 0010:[<ffffffff812e1888>] [<ffffffff812e1888>] bio_copy_data+0xc8/0x3c0
>> [ 517.478164] RSP: 0018:ffff880150dfbc98 EFLAGS: 00000246
>> [ 517.484341] RAX: ffff880169356688 RBX: 0000000000001000 RCX: 0000000000000000
>> [ 517.492558] RDX: 0000000000000000 RSI: ffffea0001ac2980 RDI: ffffea0000d835c0
>> [ 517.500773] RBP: ffff880150dfbd08 R08: 0000000000000001 R09: ffff880153773980
>> [ 517.508987] R10: ffff880169356600 R11: 0000000000001000 R12: 0000000000010000
>> [ 517.517199] R13: 000000000000e000 R14: 0000000000000000 R15: 0000000000001000
>> [ 517.525412] FS: 0000000000000000(0000) GS:ffff880174a00000(0000) knlGS:0000000000000000
>> [ 517.534844] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [ 517.541507] CR2: 00007f8a044d5fed CR3: 0000000169504000 CR4: 00000000001406f0
>> [ 517.549722] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> [ 517.557929] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> [ 517.566144] Stack:
>> [ 517.568626] ffff880174a16bc0 ffff880153773980 ffff880169356600 0000000000000000
>> [ 517.577659] 0000000000000001 0000000000000001 ffff880153773980 ffff88016a61a800
>> [ 517.586715] ffff880150dfbcf8 0000000000000001 ffff88016dd209e0 0000000000001000
>> [ 517.595773] Call Trace:
>> [ 517.598747] [<ffffffffa043ef95>] raid10d+0xfc5/0x1690 [raid10]
>> [ 517.605610] [<ffffffff816697ae>] ? __schedule+0x29e/0x8e2
>> [ 517.611987] [<ffffffff814ff206>] md_thread+0x106/0x140
>> [ 517.618072] [<ffffffff810c1d80>] ? wait_woken+0x80/0x80
>> [ 517.624252] [<ffffffff814ff100>] ? super_1_load+0x520/0x520
>> [ 517.630817] [<ffffffff8109ef89>] kthread+0xc9/0xe0
>> [ 517.636506] [<ffffffff8109eec0>] ? flush_kthread_worker+0x70/0x70
>> [ 517.643653] [<ffffffff8166d99f>] ret_from_fork+0x3f/0x70
>> [ 517.649929] [<ffffffff8109eec0>] ? flush_kthread_worker+0x70/0x70
>>
>> Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
>> ---
>> drivers/md/raid10.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index 96f3659..23bbe61 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
>> @@ -1944,6 +1944,8 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
>>
>> first = i;
>> fbio = r10_bio->devs[i].bio;
>> + fbio->bi_iter.bi_size = r10_bio->sectors << 9;
>> + fbio->bi_iter.bi_idx = 0;
>>
>> vcnt = (r10_bio->sectors + (PAGE_SIZE >> 9) - 1) >> (PAGE_SHIFT - 9);
>> /* now find blocks with errors */
>> @@ -1987,7 +1989,7 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
>> bio_reset(tbio);
>>
>> tbio->bi_vcnt = vcnt;
>> - tbio->bi_iter.bi_size = r10_bio->sectors << 9;
>> + tbio->bi_iter.bi_size = fbio->bi_iter.bi_size;
>> tbio->bi_rw = WRITE;
>> tbio->bi_private = r10_bio;
>> tbio->bi_iter.bi_sector = r10_bio->devs[i].addr;
>
> Looks good. Reviewed-by: Shaohua Li <shli@kernel.org>
Thanks for the patch and the review.
I've added:
Cc: stable@vger.kernel.org (v4.2+)
Fixes: c31df25f20e3 ("md/raid10: make sync_request_write() call bio_copy_data()")
Signed-off-by: NeilBrown <neilb@suse.com>
and will hopefully submit to Linus in a day or so.
>
> A nitpick, I'm wondering if we should do a full reset like raid1 does to make this more clear.
Might make sense. I'm happy with the code as it is, but if doing a full
reset makes the code cleared I'd accept that too.
Thanks,
NeilBrown
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: [PATCH] raid5-cache: add journal hot add/remove support
From: NeilBrown @ 2015-12-16 1:14 UTC (permalink / raw)
To: Shaohua Li, linux-raid; +Cc: Kernel-team, songliubraving, hch
In-Reply-To: <48dfb85817d51f68d8da85368711ed14f945caa4.1450159900.git.shli@fb.com>
[-- Attachment #1: Type: text/plain, Size: 8869 bytes --]
On Tue, Dec 15 2015, Shaohua Li wrote:
> Add support for journal disk hot add/remove. Mostly trival checks in md
> part. The raid5 part is a little tricky. For hot-remove, we can't wait
> pending write as it's called from raid5d. The wait will cause deadlock.
> We simplily fail the hot-remove. A hot-remove retry can success
> eventually since if journal disk is faulty all pending write will be
> failed and finish. For hot-add, since an array supporting journal but
> without journal disk will be marked read-only, we are safe to hot add
> journal without stopping IO (should be read IO, while journal only
> handles write IO).
>
> Signed-off-by: Shaohua Li <shli@fb.com>
> ---
> drivers/md/md.c | 44 +++++++++++++++++++++++++++++++++-----------
> drivers/md/raid5.c | 31 +++++++++++++++++++++++--------
> 2 files changed, 56 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 807095f..de9e3a1 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -2053,7 +2053,9 @@ static int bind_rdev_to_array(struct md_rdev *rdev, struct mddev *mddev)
>
> /* make sure rdev->sectors exceeds mddev->dev_sectors */
> if (rdev->sectors && (mddev->dev_sectors == 0 ||
> - rdev->sectors < mddev->dev_sectors)) {
> + rdev->sectors < mddev->dev_sectors) &&
> + !(test_bit(MD_HAS_JOURNAL, &mddev->flags) &&
> + test_bit(Journal, &rdev->flags))) {
> if (mddev->pers) {
> /* Cannot change size, so fail
> * If mddev->level <= 0, then we don't care
This is (nearly) right, but it took me far too long to see that it was
right - and that makes it wrong.
If the end result was:
/* make sure rdev->sectors exceeds mddev->dev_sectors for
* non-journal devices
*/
if (!test_bit(Journal, &rdev->flags) &&
rdev->sectors &&
(mddev->dev_sectors == 0 || rdev->sectors < mddev->dev_sectors)) {
.....
it would be a lot more obviously correct. I removed the test on
MD_HAS_JOURNAL because it seems irrelevant. We don't want to perform
that test on an rdev with Journal set whether MD_HAS_JOURNAL is set or
not.
> @@ -2084,7 +2086,9 @@ static int bind_rdev_to_array(struct md_rdev *rdev, struct mddev *mddev)
> }
> }
> rcu_read_unlock();
> - if (mddev->max_disks && rdev->desc_nr >= mddev->max_disks) {
> + if (mddev->max_disks && rdev->desc_nr >= mddev->max_disks &&
> + !(test_bit(MD_HAS_JOURNAL, &mddev->flags) &&
> + test_bit(Journal, &rdev->flags))) {
> printk(KERN_WARNING "md: %s: array is limited to %d devices\n",
> mdname(mddev), mddev->max_disks);
> return -EBUSY;
Same here. Removed the test on MD_HAS_JOURNAL is it is just confusing.
I wold prefer the test on Journal came first, but would accept having it
last if you prefer that.
> @@ -6031,8 +6035,25 @@ static int add_new_disk(struct mddev *mddev, mdu_disk_info_t *info)
> else
> clear_bit(WriteMostly, &rdev->flags);
>
> - if (info->state & (1<<MD_DISK_JOURNAL))
> + if (info->state & (1<<MD_DISK_JOURNAL)) {
> + struct md_rdev *rdev2;
> + bool has_journal = false;
> +
> + /* make sure no existing journal disk */
> + rcu_read_lock();
> + rdev_for_each(rdev2, mddev) {
> + if (test_bit(Journal, &rdev2->flags)) {
> + has_journal = true;
> + break;
> + }
> + }
> + rcu_read_unlock();
> + if (has_journal) {
> + export_rdev(rdev);
> + return -EBUSY;
> + }
> set_bit(Journal, &rdev->flags);
> + }
> /*
> * check whether the device shows up in other nodes
> */
rcu_read_lock() isn't really needed here as we hold ->reconfig_mutex so
rdevs cannot disappear.
Maybe it is OK to leave it there as it costs almost nothing. But maybe
it could lead other people to think RCU protection is needed in this
case, which is wrong....
> @@ -8162,19 +8183,20 @@ static int remove_and_add_spares(struct mddev *mddev,
> continue;
> if (test_bit(Faulty, &rdev->flags))
> continue;
> - if (test_bit(Journal, &rdev->flags))
> - continue;
> - if (mddev->ro &&
> - ! (rdev->saved_raid_disk >= 0 &&
> - !test_bit(Bitmap_sync, &rdev->flags)))
> - continue;
> + if (!test_bit(Journal, &rdev->flags)) {
> + if (mddev->ro &&
> + ! (rdev->saved_raid_disk >= 0 &&
> + !test_bit(Bitmap_sync, &rdev->flags)))
> + continue;
>
> - rdev->recovery_offset = 0;
> + rdev->recovery_offset = 0;
> + }
> if (mddev->pers->
> hot_add_disk(mddev, rdev) == 0) {
> if (sysfs_link_rdev(mddev, rdev))
> /* failure here is OK */;
> - spares++;
> + if (!test_bit(Journal, &rdev->flags))
> + spares++;
> md_new_event(mddev);
> set_bit(MD_CHANGE_DEVS, &mddev->flags);
> }
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 704ef7f..f8f3d52 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -7141,14 +7141,16 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
> struct disk_info *p = conf->disks + number;
>
> print_raid5_conf(conf);
> - if (test_bit(Journal, &rdev->flags)) {
> + if (test_bit(Journal, &rdev->flags) && conf->log) {
> /*
> - * journal disk is not removable, but we need give a chance to
> - * update superblock of other disks. Otherwise journal disk
> - * will be considered as 'fresh'
> + * we can't wait pending write here, as this is called in
> + * raid5d, wait will deadlock.
> */
> - set_bit(MD_CHANGE_DEVS, &mddev->flags);
> - return -EINVAL;
> + if (atomic_read(&mddev->writes_pending))
> + return -EBUSY;
> + r5l_exit_log(conf->log);
> + conf->log = NULL;
> + return 0;
Failing with EBUSY if ->writes_pending is correct.
I wonder if extra care is needed when ->writes_pending is zero.
Note that for removing data devices, we set *rdevp to NULL,
call synchronize_rcu(), then check ->nr_pending and possibly restoring
the pointer.
This is because there is no other interlock between new writes arriving
and a device being removed.
The possible race would be that between the moment when
raid5_remove_disk tests ->writes_pending and when it sets conf->log to
NULL, a new write request comes in and gets as far as calling
r5_log_disk_error().
If r5l_log_disk_error() finds that ->log is not NULL, then
rl5_exit_log() in the other thread frees the log, and
r5l_log_disk_error() tests a bit in conf->log->rdev->flags, then it
could be accessing freed memory and anything could happen.
I think you need something like:
log = conf->log;
conf->log = NULL;
synchronize_rcu();
r5l_exit_log(log);
and then r5l_log_disk_error() should do:
rcu_read_lock();
log = rcu_dereference(conf->log);
if (log)
err = test_bit(Faulty, &log->rdev->flags);
else
err = test_bit(MD_HAS_JOURNAL, &conf->mddev->flags);
rcu_read_unlock();
return err;
The race is, admittedly, extremely unlikely. But it is best to be safe,
especially with virtualised environments which can put unexpected delays
in strange places.
You could possibly use rcu_free() to free the log and avoid the
synchonize_rcu().
> }
> if (rdev == p->rdev)
> rdevp = &p->rdev;
> @@ -7212,8 +7214,21 @@ static int raid5_add_disk(struct mddev *mddev, struct md_rdev *rdev)
> int first = 0;
> int last = conf->raid_disks - 1;
>
> - if (test_bit(Journal, &rdev->flags))
> - return -EINVAL;
> + if (test_bit(Journal, &rdev->flags)) {
> + char b[BDEVNAME_SIZE];
> + if (conf->log)
> + return -EBUSY;
> +
> + rdev->raid_disk = mddev->raid_disks;
> + /*
> + * The array is in readonly mode if journal is missing, so no
> + * write requests running. We should be safe
> + */
> + r5l_init_log(conf, rdev);
> + printk(KERN_INFO"md/raid:%s: using device %s as journal\n",
> + mdname(mddev), bdevname(rdev->bdev, b));
> + return 0;
> + }
you'll need an "rcu_assign_pointer" in r5l_init_log() for the
conf->log = log
to ensure proper ordering of memory writes.
And I'm a bit uncertain about setting rdev->raid_disk to
mddev->raid_disks.
I thought we decided that "Journal" devices had a different namespace
for raid_disk than data disks, so ->raid_disk of 0 was appropriate?
Setting the journal raid_disk to mddev->raid_disks might cause problems
when we ultimately support reshaping an array with a journal.
So: mostly good, but a few little issues to resolve.
Thanks,
NeilBrown
> if (mddev->recovery_disabled == conf->recovery_disabled)
> return -EBUSY;
>
> --
> 2.4.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: best base / worst case RAID 5,6 write speeds
From: Dallas Clement @ 2015-12-15 23:07 UTC (permalink / raw)
To: John Stoffel; +Cc: Mark Knecht, Phil Turmel, Linux-RAID
In-Reply-To: <22128.35881.182823.556362@quad.stoffel.home>
On Tue, Dec 15, 2015 at 3:54 PM, John Stoffel <john@stoffel.org> wrote:
>>>>>> "Dallas" == Dallas Clement <dallas.a.clement@gmail.com> writes:
>
> Dallas> Thanks guys for all the ideas and help.
> Dallas> Phil,
>
>>> Very interesting indeed. I wonder if the extra I/O in flight at high
>>> depths is consuming all available stripe cache space, possibly not
>>> consistently. I'd raise and lower that in various combinations with
>>> various combinations of iodepth. Running out of stripe cache will cause
>>> premature RMWs.
>
> Dallas> Okay, I'll play with that today. I have to confess I'm not
> Dallas> sure that I completely understand how the stripe cache works.
> Dallas> I think the idea is to batch I/Os into a complete stripe if
> Dallas> possible and write out to the disks all in one go to avoid
> Dallas> RMWs. Other than alignment issues, I'm unclear on what
> Dallas> triggers RMWs. It seems like as Robert mentioned that if the
> Dallas> I/Os block size is stripe aligned, there should never be RMWs.
>
> Remember, there's a bounding limit on both how large the stripe cache
> is, and how long (timewise) it will let the cache sit around waiting
> for new blocks to come in. That's probably what you're hitting at
> times with the high queue depth numbers.
>
> I assume the blocktrace info would tell you more, but I haven't really
> a clue how to interpret it.
>
>
> Dallas> My stripe cache is 8192 btw.
>
> Dallas> John,
>
>>> I suspect you've hit a known problem-ish area with Linux disk io, which is that big queue depths aren't optimal.
>
> Dallas> Yes, certainly looks that way. But maybe as Phil indicated I might be
> Dallas> exceeding my stripe cache. I am still surprised that there are so
> Dallas> many RMWs even if the stripe cache has been exhausted.
>
>>> As you can see, it peaks at a queue depth of 4, and then tends
>>> downward before falling off a cliff. So now what I'd do is keep the
>>> queue depth at 4, but vary the block size and other parameters and see
>>> how things change there.
>
> Dallas> Why do you think there is a gradual drop off after queue depth
> Dallas> of 4 and before it falls off the cliff?
>
> I think because the in-kernel sizes start getting bigger, and so the
> kernel spends more time queuing and caching the data and moving it
> around, instead of just shoveling it down to the disks as quick as it
> can.
>
> Dallas> I with this were for fun! ;) Although this has been a fun
> Dallas> discussion. I've learned a ton. This effort is for work
> Dallas> though. I'd be all over the SSDs and caching otherwise. I'm
> Dallas> trying to characterize and then squeeze all of the performance
> Dallas> I can out of a legacy NAS product. I am constrained by the
> Dallas> existing hardware. Unfortunately I do not have the option of
> Dallas> using SSDs or hardware RAID controllers. I have to rely
> Dallas> completely on Linux RAID.
>
> Ah... in that case, you need to do your testing from the NAS side,
> don't bother going to this level. I'd honestly now just set your
> queue depth to 4 and move on to testing the NAS side of things, where
> you have one, two, four, eight, or more test boxes hitting the NAS
> box.
>
> Dallas> I also need to optimize for large sequential writes (streaming
> Dallas> video, audio, large file transfers), iSCSI (mostly used for
> Dallas> hosting VMs), and random I/O (small and big files) as you
> Dallas> would expect with a NAS.
>
> So you want to do everything at all once. Fun. So really I'd move
> back to the Network side, because unless your NAS box has more than
> 1GigE interface, and supports Bonding/trunking, you've hit the
> performance wall.
>
> Also, even if you get a ton of performance with large streaming
> writes, when you sprinkle in a small set of random IO/s, you're going
> to hit the cliff much sooner. And in that case... it's another set of
> optimizations.
>
> Are you going to use NFSv3? TCP? UDP? 1500 MTU, 9000 MTU? How many
> clients? How active?
>
> Can you give up disk space for IOP/s? So get away from the RAID6 and
> move to RAID1 mirrors with a strip atop it, so that you maximize how
> many IOPs you can get.
>
Hi John.
> Remember, there's a bounding limit on both how large the stripe cache
> is, and how long (timewise) it will let the cache sit around waiting
> for new blocks to come in. That's probably what you're hitting at
> times with the high queue depth numbers.
Okay, good to know. I did try doubling the size of the stripe cache
just to see if it would reduce the # of RMWs at iodepth>=64. It did
not. So it looks like cache timing out as you mentioned.
> So you want to do everything at all once. Fun. So really I'd move
> back to the Network side, because unless your NAS box has more than
> 1GigE interface, and supports Bonding/trunking, you've hit the
> performance wall.
I'm not sure I necessarily want to tune everything at once.
Surprisingly this box does have 10GigE interfaces. I just want to get
RAID tuned the best I can before I start testing over the network.
With 10 GigE this box should be able to write 1200 MB/s max. But as
reported earlier, I'm not even able to get that with fio running
locally on the box writing to the RAID device.
After Phil's explanation I now better understand what triggers the
RMWs. Clearly I would like to minimize these to get the best
performance for both sequential and random patterns. Messing with the
stripe cache size doesn't seem to change anything with performance, so
will probably play with a smaller chunk size next to see if that
helps.
> Also, even if you get a ton of performance with large streaming
> writes, when you sprinkle in a small set of random IO/s, you're going
> to hit the cliff much sooner. And in that case... it's another set of
> optimizations.
Yes, I get that. There are definitely some customers that do a little
of everything with these NAS boxes. But from what I've seen, a lot of
them also use a NAS for just one thing - host VMs or stream media or
file server or database / web app.
> Are you going to use NFSv3? TCP? UDP? 1500 MTU, 9000 MTU?
Yes, all of these are supported.
> How many clients? How active?
These boxes tend to get used pretty hard. Probably the biggest
application is iSCSI or Samba backups, and then hosting VMs. For
backups it's usually small number of clients but heavy volume. For
VMs there can be quite a few.
One other consideration is that these kind of products undergo lots of
bench-mark testing with the usual host of tools. Probably my main
goal at this point is for the tests which are primarily focused on
sequential throughput (small and large blocks) are the best that they
can be given the hardware limitations. 10 Gbps iSCSI throughput is
probably the most important benchmark. If I can somehow get the RAID
5,6 write speeds up over 1 GB/s I would be very happy. Right now 10
Gbps iSCSI write performance is limited by the RAID device performance
sadly.
> Can you give up disk space for IOP/s? So get away from the RAID6 and
> move to RAID1 mirrors with a strip atop it, so that you maximize how
> many IOPs you can get.
Yes, this box already supports RAID 0, 1, 5, 6, 10, 50, 60
^ permalink raw reply
* Re: core dump on initial sync of raid10 please help
From: John Stoffel @ 2015-12-15 22:10 UTC (permalink / raw)
To: Micheal Blue; +Cc: linux-raid
In-Reply-To: <trinity-3e0abfe9-de99-450e-a215-4af82f2177be-1450121480258@3capp-mailcom-bs12>
>>>>> "Micheal" == Micheal Blue <mblue@gmx.us> writes:
>> Sent: Monday, December 14, 2015 at 8:48 AM
>> From: "Micheal Blue" <>
>> To: linux-raid@vger.kernel.org
>> Subject: core dump on initial sync of raid10 please help
>>
>> I noticed my initial sync when making a 4 disk raid10 is really slow. I looked in dmesg and found this.
>> I do not know what it means. Suggestions please :)
>>
>> [ 600.959945] INFO: task md0_resync:676 blocked for more than 120 seconds.
>> [ 600.959976] Not tainted 4.3.2-1-ARCH #1
>> [ 600.959994] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>> [ 600.960024] md0_resync D ffff88021fa956c0 0 676 2 0x00000000
>> [ 600.960028] ffff8800ca143b40 0000000000000046 ffff880216659b80 ffff8800377244c0
>> [ 600.960030] ffff8800ca144000 ffff88003765a000 ffff8800ca143b68 ffff88003765a100
>> [ 600.960032] ffff88003765a000 ffff8800ca143b58 ffffffff8157f92a ffff88003765a0dc
>> [ 600.960034] Call Trace:
>> [ 600.960040] [<ffffffff8157f92a>] schedule+0x3a/0x90
>> [ 600.960044] [<ffffffffa0548f52>] raise_barrier+0x122/0x190 [raid10]
>> [ 600.960049] [<ffffffff810b5dc0>] ? wake_atomic_t_function+0x60/0x60
>> [ 600.960051] [<ffffffffa054eea1>] sync_request+0x731/0x19b0 [raid10]
Micheal> ...
Micheal> It seems that the kernel itself is to blame for this. If I
Micheal> boot into kernel version 4.1.14 everything seems to be
Micheal> running fine as I create the array.
Try to run with kernel v4.4-rc4 or newer I think. I ran into
something similiar on my system.
John
^ permalink raw reply
* Re: best base / worst case RAID 5,6 write speeds
From: John Stoffel @ 2015-12-15 21:54 UTC (permalink / raw)
To: Dallas Clement; +Cc: John Stoffel, Mark Knecht, Phil Turmel, Linux-RAID
In-Reply-To: <CAE9DZURepRB3k-pBnRg2Tx8GCzAr6zCzv+LJy4mK4CDdAkBYVQ@mail.gmail.com>
>>>>> "Dallas" == Dallas Clement <dallas.a.clement@gmail.com> writes:
Dallas> Thanks guys for all the ideas and help.
Dallas> Phil,
>> Very interesting indeed. I wonder if the extra I/O in flight at high
>> depths is consuming all available stripe cache space, possibly not
>> consistently. I'd raise and lower that in various combinations with
>> various combinations of iodepth. Running out of stripe cache will cause
>> premature RMWs.
Dallas> Okay, I'll play with that today. I have to confess I'm not
Dallas> sure that I completely understand how the stripe cache works.
Dallas> I think the idea is to batch I/Os into a complete stripe if
Dallas> possible and write out to the disks all in one go to avoid
Dallas> RMWs. Other than alignment issues, I'm unclear on what
Dallas> triggers RMWs. It seems like as Robert mentioned that if the
Dallas> I/Os block size is stripe aligned, there should never be RMWs.
Remember, there's a bounding limit on both how large the stripe cache
is, and how long (timewise) it will let the cache sit around waiting
for new blocks to come in. That's probably what you're hitting at
times with the high queue depth numbers.
I assume the blocktrace info would tell you more, but I haven't really
a clue how to interpret it.
Dallas> My stripe cache is 8192 btw.
Dallas> John,
>> I suspect you've hit a known problem-ish area with Linux disk io, which is that big queue depths aren't optimal.
Dallas> Yes, certainly looks that way. But maybe as Phil indicated I might be
Dallas> exceeding my stripe cache. I am still surprised that there are so
Dallas> many RMWs even if the stripe cache has been exhausted.
>> As you can see, it peaks at a queue depth of 4, and then tends
>> downward before falling off a cliff. So now what I'd do is keep the
>> queue depth at 4, but vary the block size and other parameters and see
>> how things change there.
Dallas> Why do you think there is a gradual drop off after queue depth
Dallas> of 4 and before it falls off the cliff?
I think because the in-kernel sizes start getting bigger, and so the
kernel spends more time queuing and caching the data and moving it
around, instead of just shoveling it down to the disks as quick as it
can.
Dallas> I with this were for fun! ;) Although this has been a fun
Dallas> discussion. I've learned a ton. This effort is for work
Dallas> though. I'd be all over the SSDs and caching otherwise. I'm
Dallas> trying to characterize and then squeeze all of the performance
Dallas> I can out of a legacy NAS product. I am constrained by the
Dallas> existing hardware. Unfortunately I do not have the option of
Dallas> using SSDs or hardware RAID controllers. I have to rely
Dallas> completely on Linux RAID.
Ah... in that case, you need to do your testing from the NAS side,
don't bother going to this level. I'd honestly now just set your
queue depth to 4 and move on to testing the NAS side of things, where
you have one, two, four, eight, or more test boxes hitting the NAS
box.
Dallas> I also need to optimize for large sequential writes (streaming
Dallas> video, audio, large file transfers), iSCSI (mostly used for
Dallas> hosting VMs), and random I/O (small and big files) as you
Dallas> would expect with a NAS.
So you want to do everything at all once. Fun. So really I'd move
back to the Network side, because unless your NAS box has more than
1GigE interface, and supports Bonding/trunking, you've hit the
performance wall.
Also, even if you get a ton of performance with large streaming
writes, when you sprinkle in a small set of random IO/s, you're going
to hit the cliff much sooner. And in that case... it's another set of
optimizations.
Are you going to use NFSv3? TCP? UDP? 1500 MTU, 9000 MTU? How many
clients? How active?
Can you give up disk space for IOP/s? So get away from the RAID6 and
move to RAID1 mirrors with a strip atop it, so that you maximize how
many IOPs you can get.
^ permalink raw reply
* Re: best base / worst case RAID 5,6 write speeds
From: Phil Turmel @ 2015-12-15 19:52 UTC (permalink / raw)
To: Dallas Clement; +Cc: John Stoffel, Mark Knecht, Linux-RAID
In-Reply-To: <CAE9DZUS=AVydRoDZav-Mvq9VyPuM4Z+0iQgxy9KDJG9yB2NZ8g@mail.gmail.com>
On 12/15/2015 02:44 PM, Dallas Clement wrote:
> Wow! Thanks a ton Phil. This is incredibly helpful! It looks like I
> need to do some experimenting with smaller chunk sizes. Just one more
> question: what stripe cache size do you recommend for this system?
> It has 8 GB of RAM, but can't use all of it for RAID as this NAS needs
> to run multiple applications. I understand that in the >= 4.1 kernels
> the stripe cache grows dynamically.
I don't really know. I use the default 256, but all of my parity raid
arrays have 16k chunks and are relatively lightly loaded. Consider
sampling stripe_cache_active once a second for a normal workday on real
workloads to figure out what you need.
Phil
^ permalink raw reply
* Re: best base / worst case RAID 5,6 write speeds
From: Dallas Clement @ 2015-12-15 19:44 UTC (permalink / raw)
To: Phil Turmel; +Cc: John Stoffel, Mark Knecht, Linux-RAID
In-Reply-To: <56706858.2040908@turmel.org>
On Tue, Dec 15, 2015 at 1:22 PM, Phil Turmel <philip@turmel.org> wrote:
> Hi Dallas,
>
> On 12/15/2015 12:30 PM, Dallas Clement wrote:
>> Thanks guys for all the ideas and help.
>>
>> Phil,
>>
>>> Very interesting indeed. I wonder if the extra I/O in flight at high
>>> depths is consuming all available stripe cache space, possibly not
>>> consistently. I'd raise and lower that in various combinations with
>>> various combinations of iodepth. Running out of stripe cache will cause
>>> premature RMWs.
>>
>> Okay, I'll play with that today. I have to confess I'm not sure that
>> I completely understand how the stripe cache works. I think the idea
>> is to batch I/Os into a complete stripe if possible and write out to
>> the disks all in one go to avoid RMWs. Other than alignment issues,
>> I'm unclear on what triggers RMWs. It seems like as Robert mentioned
>> that if the I/Os block size is stripe aligned, there should never be
>> RMWs.
>>
>> My stripe cache is 8192 btw.
>>
>
> Stripe cache is the kernel's workspace to compute parity or to recover
> data from parity. It works on 4k blocks. Per "man md", the units are
> number of such blocks per device. *The blocks in each cache stripe are
> separated from each other on disk by the chunk size*.
>
> Let's examine some scenarios for your 128k chunk size, 12 devices. You
> have 8192 cache stripes of 12 blocks each:
>
> 1) Random write of 16k. 4 stripes will be allocated from the cache for
> *all* of the devices, and filled for the devices written. The raid5
> state machine lets them sit briefly for a chance for more writes to the
> other blocks in each stripe.
>
> 1a) If none come in, MD will request a read of the old data blocks and
> the old parities. When those arrive, it'll compute the new parities and
> write both parities and new data blocks. Total I/O: 32k read, 32k write.
>
> 1b) If other random writes come in for those stripes, chunk size spaced,
> MD will wait a bit more. Then it will read in any blocks that weren't
> written, compute parity, and write all the new data and parity. Total
> I/O: 16k * n, possibly some reads, the rest writes.
>
> 2) Sequential write of stripe-aligned 1408k. The first 128k allocates
> 64 cache stripes and fills their first block. The next 128k fills the
> second block of each cache stripe. And so on, filling all the data
> blocks in the cache stripes. MD shortly notices a full cache stripe
> write on each, so it just computes the parities and submits all of those
> writes.
>
> 3) Sequential write of 256k, aligned or not. As above, but you only
> fill two blocks in each cache stripe. MD then reads 1152k, computes
> parity, and writes 384k.
>
> 4) Multiple back-to-back writes of 1408k aligned. First grabs 64 cache
> stripes and shortly queues all of those writes. Next grabs another 64
> cache stripes and queues more writes. And then another 64 caches stripes
> and writes. Underlying layer, as its queue grows, notices the adjacency
> of chunk writes from multiple top-level writes and starts merging.
> Stripe caches are still held, though, until each write is completed. If
> 128 top-level writes are in flight (8192/64), you've exhausted your
> stripe cache. Note that this is writes in flight in your application
> *and* writes in flight from anything else. Keeping in mind that merging
> might actually raise the completion latency for the earlier writes.
>
> I'm sure you can come up with more. The key is that stripe parity
> calculations must be performed on blocks separated on disk by the chunk
> size. Really big chunk sizes don't actually help parity raid, since
> everything is broken down to 4k for the stripe cache, then re-merged
> underneath it.
>
>> I with this were for fun! ;) Although this has been a fun discussion.
>> I've learned a ton. This effort is for work though. I'd be all over
>> the SSDs and caching otherwise. I'm trying to characterize and then
>> squeeze all of the performance I can out of a legacy NAS product. I
>> am constrained by the existing hardware. Unfortunately I do not have
>> the option of using SSDs or hardware RAID controllers. I have to rely
>> completely on Linux RAID.
>>
>> I also need to optimize for large sequential writes (streaming video,
>> audio, large file transfers), iSCSI (mostly used for hosting VMs), and
>> random I/O (small and big files) as you would expect with a NAS.
>
> On spinning rust, once you introduce any random writes, you've
> effectively made the entire stack a random workload. This is true for
> all raid levels, but particularly true for parity raid due to the RMW
> cycles. If you really need great sequential performance, you can't
> allow the VMs and the databases and small files on the same disks.
>
> That said, I recommend a parity raid chunk size of 16k or 32k for all
> workloads. Greatly improves spatial locality for random writes, reduces
> stripe cache hogging for sequential writes, and doesn't hurt sequential
> reads too much.
>
> Phil
Wow! Thanks a ton Phil. This is incredibly helpful! It looks like I
need to do some experimenting with smaller chunk sizes. Just one more
question: what stripe cache size do you recommend for this system?
It has 8 GB of RAM, but can't use all of it for RAID as this NAS needs
to run multiple applications. I understand that in the >= 4.1 kernels
the stripe cache grows dynamically.
^ permalink raw reply
* Re: best base / worst case RAID 5,6 write speeds
From: Phil Turmel @ 2015-12-15 19:22 UTC (permalink / raw)
To: Dallas Clement, John Stoffel; +Cc: Mark Knecht, Linux-RAID
In-Reply-To: <CAE9DZURepRB3k-pBnRg2Tx8GCzAr6zCzv+LJy4mK4CDdAkBYVQ@mail.gmail.com>
Hi Dallas,
On 12/15/2015 12:30 PM, Dallas Clement wrote:
> Thanks guys for all the ideas and help.
>
> Phil,
>
>> Very interesting indeed. I wonder if the extra I/O in flight at high
>> depths is consuming all available stripe cache space, possibly not
>> consistently. I'd raise and lower that in various combinations with
>> various combinations of iodepth. Running out of stripe cache will cause
>> premature RMWs.
>
> Okay, I'll play with that today. I have to confess I'm not sure that
> I completely understand how the stripe cache works. I think the idea
> is to batch I/Os into a complete stripe if possible and write out to
> the disks all in one go to avoid RMWs. Other than alignment issues,
> I'm unclear on what triggers RMWs. It seems like as Robert mentioned
> that if the I/Os block size is stripe aligned, there should never be
> RMWs.
>
> My stripe cache is 8192 btw.
>
Stripe cache is the kernel's workspace to compute parity or to recover
data from parity. It works on 4k blocks. Per "man md", the units are
number of such blocks per device. *The blocks in each cache stripe are
separated from each other on disk by the chunk size*.
Let's examine some scenarios for your 128k chunk size, 12 devices. You
have 8192 cache stripes of 12 blocks each:
1) Random write of 16k. 4 stripes will be allocated from the cache for
*all* of the devices, and filled for the devices written. The raid5
state machine lets them sit briefly for a chance for more writes to the
other blocks in each stripe.
1a) If none come in, MD will request a read of the old data blocks and
the old parities. When those arrive, it'll compute the new parities and
write both parities and new data blocks. Total I/O: 32k read, 32k write.
1b) If other random writes come in for those stripes, chunk size spaced,
MD will wait a bit more. Then it will read in any blocks that weren't
written, compute parity, and write all the new data and parity. Total
I/O: 16k * n, possibly some reads, the rest writes.
2) Sequential write of stripe-aligned 1408k. The first 128k allocates
64 cache stripes and fills their first block. The next 128k fills the
second block of each cache stripe. And so on, filling all the data
blocks in the cache stripes. MD shortly notices a full cache stripe
write on each, so it just computes the parities and submits all of those
writes.
3) Sequential write of 256k, aligned or not. As above, but you only
fill two blocks in each cache stripe. MD then reads 1152k, computes
parity, and writes 384k.
4) Multiple back-to-back writes of 1408k aligned. First grabs 64 cache
stripes and shortly queues all of those writes. Next grabs another 64
cache stripes and queues more writes. And then another 64 caches stripes
and writes. Underlying layer, as its queue grows, notices the adjacency
of chunk writes from multiple top-level writes and starts merging.
Stripe caches are still held, though, until each write is completed. If
128 top-level writes are in flight (8192/64), you've exhausted your
stripe cache. Note that this is writes in flight in your application
*and* writes in flight from anything else. Keeping in mind that merging
might actually raise the completion latency for the earlier writes.
I'm sure you can come up with more. The key is that stripe parity
calculations must be performed on blocks separated on disk by the chunk
size. Really big chunk sizes don't actually help parity raid, since
everything is broken down to 4k for the stripe cache, then re-merged
underneath it.
> I with this were for fun! ;) Although this has been a fun discussion.
> I've learned a ton. This effort is for work though. I'd be all over
> the SSDs and caching otherwise. I'm trying to characterize and then
> squeeze all of the performance I can out of a legacy NAS product. I
> am constrained by the existing hardware. Unfortunately I do not have
> the option of using SSDs or hardware RAID controllers. I have to rely
> completely on Linux RAID.
>
> I also need to optimize for large sequential writes (streaming video,
> audio, large file transfers), iSCSI (mostly used for hosting VMs), and
> random I/O (small and big files) as you would expect with a NAS.
On spinning rust, once you introduce any random writes, you've
effectively made the entire stack a random workload. This is true for
all raid levels, but particularly true for parity raid due to the RMW
cycles. If you really need great sequential performance, you can't
allow the VMs and the databases and small files on the same disks.
That said, I recommend a parity raid chunk size of 16k or 32k for all
workloads. Greatly improves spatial locality for random writes, reduces
stripe cache hogging for sequential writes, and doesn't hurt sequential
reads too much.
Phil
^ permalink raw reply
* Re: best base / worst case RAID 5,6 write speeds
From: Dallas Clement @ 2015-12-15 17:30 UTC (permalink / raw)
To: John Stoffel; +Cc: Mark Knecht, Phil Turmel, Linux-RAID
In-Reply-To: <22128.11867.847781.946791@quad.stoffel.home>
Thanks guys for all the ideas and help.
Phil,
> Very interesting indeed. I wonder if the extra I/O in flight at high
> depths is consuming all available stripe cache space, possibly not
> consistently. I'd raise and lower that in various combinations with
> various combinations of iodepth. Running out of stripe cache will cause
> premature RMWs.
Okay, I'll play with that today. I have to confess I'm not sure that
I completely understand how the stripe cache works. I think the idea
is to batch I/Os into a complete stripe if possible and write out to
the disks all in one go to avoid RMWs. Other than alignment issues,
I'm unclear on what triggers RMWs. It seems like as Robert mentioned
that if the I/Os block size is stripe aligned, there should never be
RMWs.
My stripe cache is 8192 btw.
John,
> I suspect you've hit a known problem-ish area with Linux disk io, which is that big queue depths aren't optimal.
Yes, certainly looks that way. But maybe as Phil indicated I might be
exceeding my stripe cache. I am still surprised that there are so
many RMWs even if the stripe cache has been exhausted.
> As you can see, it peaks at a queue depth of 4, and then tends
> downward before falling off a cliff. So now what I'd do is keep the
> queue depth at 4, but vary the block size and other parameters and see
> how things change there.
Why do you think there is a gradual drop off after queue depth of 4
and before it falls off the cliff?
> Now this is all fun, but I also think you need to backup and re-think
> about the big picture. What workloads are you looking to optimize
> for? Lots of small file writes? Lots of big file writes? Random
> reads of big/small files?
> Are you looking for backing stores for VMs?
I with this were for fun! ;) Although this has been a fun discussion.
I've learned a ton. This effort is for work though. I'd be all over
the SSDs and caching otherwise. I'm trying to characterize and then
squeeze all of the performance I can out of a legacy NAS product. I
am constrained by the existing hardware. Unfortunately I do not have
the option of using SSDs or hardware RAID controllers. I have to rely
completely on Linux RAID.
I also need to optimize for large sequential writes (streaming video,
audio, large file transfers), iSCSI (mostly used for hosting VMs), and
random I/O (small and big files) as you would expect with a NAS.
^ permalink raw reply
* Re: start dm-multipath before mdadm raid
From: John Stoffel @ 2015-12-15 15:42 UTC (permalink / raw)
To: P. Remek; +Cc: John Stoffel, Phil Turmel, linux-raid
In-Reply-To: <CABdHLQ5mHPymE6teRq9mCOGas5ydGZhFwUXoS5-j0NjsmNHp+g@mail.gmail.com>
Sorry, haven't had a chance yet. Maybe later today/tonight.
It might be a good thing to compile your own kernel, using the latest
Linux version (v4.4-rc5) and see how that works for you.
John
P> have you managed to look at the information in the link below? I think
P> dm-multihost is as module.
P> Regards,
P> Remek
>>
>> Here is all the information I collected: http://pastebin.com/VF4Sfrq5
>>
>>
>> Regarding the testing setup, we can safely consider whole system as a
>> test, we will rebuild the system once we figure final config anyway.
>> So we can play around with the current raid with no risk of data loss.
^ permalink raw reply
* Re: best base / worst case RAID 5,6 write speeds
From: John Stoffel @ 2015-12-15 15:14 UTC (permalink / raw)
To: Dallas Clement; +Cc: Mark Knecht, Phil Turmel, John Stoffel, Linux-RAID
In-Reply-To: <CAE9DZUQxT_5L0bW5m9SZ_d2GU6sZS8k0qD=g+o112qM4V=cJkw@mail.gmail.com>
Dallas,
I suspect you've hit a known problem-ish area with Linux disk io,
which is that big queue depths aren't optimal. They're better for
systems where you're talking to a big backend disk array with lots of
cache/memory which is all battery backed, and which can acknowledge
those writes immmediately, but then retire them to disk in a more
optimal manner.
So using a queue depth of 4, which is per-device, means that you can
have upto 48 writes outstanding at a time. Just doubling that to 8,
means you can have 96 writes outstanding, which takes up memory
buffers on the system, etc.
As you can see, it peaks at a queue depth of 4, and then tends
downward before falling off a cliff. So now what I'd do is keep the
queue depth at 4, but vary the block size and other parameters and see
how things change there.
Now this is all fun, but I also think you need to backup and re-think
about the big picture. What workloads are you looking to optimize
for? Lots of small file writes? Lots of big file writes? Random
reads of big/small files?
Are you looking for backing stores for VMs?
Have you looked into battery backed RAID cards? They used to be alot
common, but these days CPUs are more than fast enough, and JBOD works
really well with more flexibility and less chance of your data getting
lost due to vendor-lockin.
Another option, if you're looking for performance might be using
lvmcache with a pair of mirrored SSDs, and if you KNOW you have UPS
support on the system, you could change the cache policy from
writeback (both SSDs and backing store writes need to complete) to
write through (SSDs writes done, backing store later...) so that you
get the most speed.
I've just recently done this setup on my home machine (not nearly as
beefy as this) and my off the cuff feeling is that it's a nice
speedup.
But back to the task at hand, what is the goal here? To find the
sweet spot for your hardware? For fun? I'm all up for fun, this is a
great discussion.
It's too bad there's no auto-tuning script for testing a setup and
running fio to get test results, and then having the next knob tweaked
and tested in an automated way.
^ permalink raw reply
* Re: start dm-multipath before mdadm raid
From: Phil Turmel @ 2015-12-15 15:07 UTC (permalink / raw)
To: P. Remek, John Stoffel; +Cc: linux-raid
In-Reply-To: <CABdHLQ5mHPymE6teRq9mCOGas5ydGZhFwUXoS5-j0NjsmNHp+g@mail.gmail.com>
On 12/15/2015 10:00 AM, P. Remek wrote:
> Hello John, Phil,
>
> have you managed to look at the information in the link below? I think
> dm-multihost is as module.
I skimmed it but didn't have any insight to add. As I mentioned, I
don't run Ubuntu on bare metal or with any advanced disk setup. (And I
have no systems with multipath at all.) I expect that if the list has
the requisite knowledge, they'll pipe up.
Phil
^ permalink raw reply
* Re: start dm-multipath before mdadm raid
From: P. Remek @ 2015-12-15 15:00 UTC (permalink / raw)
To: John Stoffel; +Cc: Phil Turmel, linux-raid
In-Reply-To: <CABdHLQ5jy1jNQ5uKF5AqJftHXw6CG3sEsrwguhgqP3mNr10aWA@mail.gmail.com>
Hello John, Phil,
have you managed to look at the information in the link below? I think
dm-multihost is as module.
Regards,
Remek
>
> Here is all the information I collected: http://pastebin.com/VF4Sfrq5
>
>
> Regarding the testing setup, we can safely consider whole system as a
> test, we will rebuild the system once we figure final config anyway.
> So we can play around with the current raid with no risk of data loss.
^ permalink raw reply
* Re: Mdadm with data offsets
From: TheGerwazy . @ 2015-12-15 14:11 UTC (permalink / raw)
To: Phil Turmel; +Cc: linux-raid
In-Reply-To: <56701944.1040505@turmel.org>
2015-12-15 14:44 GMT+01:00 Phil Turmel <philip@turmel.org>:
> On 12/15/2015 06:30 AM, TheGerwazy . wrote:
>
>> The main problem is the data offsets are not stored elsewhere than in
>> superblock. I have some drives from debian 6.0.5 and some were changed
>> in debian 7.5. In 6.0.5 default data offset was at 2048 sector while
>> int 7.5 it is 262144.
>
> There have been other defaults, and various adjustments to the
> algorithms that handle special cases. The purpose of the superblock is
> to store the data needed by the features supported. Where else
> would/could you store this information?
>
maybe mdadm.conf, maybe syslog ?
It the same problem like partitions, ext3 superblocks
We need only a few parameters to recover whole array - some of them we
could guess.
Meybe its a good idea to spare some backups of mdadm superblock like in EXT3 ?
Even though mdadm is, in my opinion, reliable system and I suggest it
to my clients because of easy recovery.
> This highlights why --create is such a terrible way to recover an array,
> and highlights the danger posed by well-intentioned bloggers who
> cavalierly suggest using it.
>
> Pretty much everyone who comes here for advice *before* trying --create
> has been successfully helped. The results after --create are decidedly
> mixed.
Due to that there are production systems, sometimes there is no time
to wait even an hour for advice - your boss would ask you "I need my
data, why you are waitng ?"
Another problem is when you are asking on the Internet - you sometimes
receive completly wrong advice what lead you to losing the data...
>
>> Thanks for Phil
>
> You're welcome.
>
> Phil
>
Best regards, Gerard
^ permalink raw reply
* Re: RAID 6 "Failed to restore critical section for reshape, sorry." - recovery advice?
From: Mikael Abrahamsson @ 2015-12-15 14:11 UTC (permalink / raw)
To: George Rapp; +Cc: linux-raid
In-Reply-To: <CAF-KpgYe9Vxgcy2E5T1_9HdEM0YMfATMzN7WcjHra_EE+sOOTg@mail.gmail.com>
On Thu, 10 Dec 2015, George Rapp wrote:
> I appear to be too early in the reshape for auto-recovery, but too far
> along to just say "never mind on that whole reshape business". Any other
> thoughts?
what does "cat /proc/mdstat" say after these commands?
--
Mikael Abrahamsson email: swmike@swm.pp.se
^ permalink raw reply
* RE: best base / worst case RAID 5,6 write speeds
From: Robert Kierski @ 2015-12-15 14:09 UTC (permalink / raw)
To: Dallas Clement, Mark Knecht; +Cc: Phil Turmel, John Stoffel, Linux-RAID
In-Reply-To: <CAE9DZUQxT_5L0bW5m9SZ_d2GU6sZS8k0qD=g+o112qM4V=cJkw@mail.gmail.com>
Dallas,
The threshold between iodepth=32 and iodepth=64 might be caused by exceeding the stripe cache size.
In my opinion, if you're writing chunk aligned data, you shouldn't be doing any RMW's. That you're doing small numbers of RMW's with small iodepth, indicates that you're using the stripe cache.
I've tried similar tests where I've set the stripe cache size to 17 (the smallest you can set it to), and then did perfectly aligned IO's. The results showed that I was doing massive amounts of RMW, and my performance was horrible.
Your FIO job file looks ordinary (and by that I mean.... "good"). While I wouldn't have picked bs=1408k, because it's aligned, I wouldn't expect that to be a cause for problem.
You should be able to set iodepth to whatever you want. You could set it to a billion. The OS should block additional requests until the underlying device's queue has available space. Iodepth shouldn't affect RMW when you're doing aligned writes. In my opnion, increasing iodepth should only help... not hurt. If it causes low memory, that could be an issue, but it shouldn't increase the amount of RMW.
Bob Kierski
Senior Storage Performance Engineer
Cray Inc.
380 Jackson Street
Suite 210
St. Paul, MN 55101
Tele: 651-967-9590
Fax: 651-605-9001
Cell: 651-890-7461
^ 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