* Re: [PATCH 0/5] block: a virtual block device driver for testing
From: Shaohua Li @ 2017-08-08 21:05 UTC (permalink / raw)
To: Jens Axboe
Cc: Hannes Reinecke, linux-block, linux-raid, kernel-team,
Kyungchan Koh, Shaohua Li
In-Reply-To: <f6896ce5-1752-feba-fadb-dbc89dbf4e20@kernel.dk>
On Tue, Aug 08, 2017 at 02:31:54PM -0600, Jens Axboe wrote:
> On 08/07/2017 10:36 AM, Shaohua Li wrote:
> > On Mon, Aug 07, 2017 at 10:29:05AM +0200, Hannes Reinecke wrote:
> >> On 08/05/2017 05:51 PM, Shaohua Li wrote:
> >>> From: Shaohua Li <shli@fb.com>
> >>>
> >>> In testing software RAID, I usually found it's hard to cover specific cases.
> >>> RAID is supposed to work even disk is in semi good state, for example, some
> >>> sectors are broken. Since we can't control the behavior of hardware, it's
> >>> difficult to create test suites to do destructive tests. But we can control the
> >>> behavior of software, software based disk is an obvious choice for such tests.
> >>> While we already have several software based disks for testing (eg, null_blk,
> >>> scsi_debug), none is for destructive testing, this is the reason we create a
> >>> new test block device.
> >>>
> >>> Currently the driver can create disk with following features:
> >>> - Bandwidth control. A raid array consists of several disks. The disks could
> >>> run in different speed, for example, one disk is SSD and the other is HD.
> >>> Actually raid1 has a feature called write behind just for this. To test such
> >>> raid1 feature, we'd like the disks speed could be controlled.
> >>> - Emulate disk cache. Software must flush disk cache to guarantee data is
> >>> safely stored in media after a power failure. To verify if software works
> >>> well, we can't simply use physical disk, because even software doesn't flush
> >>> cache, the hardware probably will flush the cache. With a software
> >>> implementation of disk cache, we can fully control how we flush disk cache in a
> >>> power failure.
> >>> - Badblock. If only part of a disk is broken, software raid continues working.
> >>> To test if software raid works well, disks must include some broken parts or
> >>> bad blocks. Bad blocks can be easily implemented in software.
> >>>
> >>> While this is inspired by software raid testing, the driver is very flexible
> >>> for extension. We can easily add new features into the driver. The interface is
> >>> configfs, which can be configured with a shell script. There is a 'features'
> >>> attribute exposing all supported features. By checking this, we don't need to
> >>> worry about compability issues. For configuration details, please check the
> >>> first patch.
> >>>
> >> Any particular reason why you can't fold these changes into brd or null_blk?
> >> After all, without those testing features it is 'just' another ramdisk
> >> driver...
> >
> > null_blk isn't a good fit. ramdisk might be, but I try to not. We are adding
> > new interfaces, locks and so on. Adding the features into ramdisk driver will
> > mess it up. Binding it to ramdisk driver will make adding new features harder
> > too, because the test driver doesn't really care about performance while
> > ramdisk does.
>
> I'm curious why null_blk isn't a good fit? You'd just need to add RAM
> storage to it. That would just be a separate option that should be set,
> ram_backing=1 or something like that. That would make it less critical
> than using the RAM disk driver as well, since only people that want a "real"
> data backing would enable it.
>
> It's not that I'm extremely opposed to adding a(nother) test block driver,
> but we at least need some sort of reasoning behind why, which isn't just
> "not a good fit".
Ah, I thought the 'null' of null_blk means we do nothing for the disks. Of
course we can rename it, which means this point less meaningful. I think the
main reason is the interface. We will configure the disks with different
parameters and do power on/off for each disks (which is the key we can emulate
disk cache and power loss). The module paramter interface of null_blk doesn't
work for the usage. Of course, these issues can be fixed, for example, we can
make null_blk use the configfs interface. If you really prefer a single driver
for all test purpose, I can move the test_blk functionalities to null_blk.
Thanks,
Shaohua
^ permalink raw reply
* Re: [PATCH 4/9] md: raid5 nowait support
From: Shaohua Li @ 2017-08-08 20:43 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: linux-block, hch, jack, linux-raid, dm-devel, Goldwyn Rodrigues
In-Reply-To: <20170726235806.12148-5-rgoldwyn@suse.de>
On Wed, Jul 26, 2017 at 06:58:01PM -0500, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
>
> Return EAGAIN in case RAID5 would block because of waiting due to:
> + Reshaping
> + Suspension
> + Stripe Expansion
>
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> ---
> drivers/md/raid5.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index aeeb8d6854e2..d1b3bcf26d29 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -5635,6 +5635,11 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
> ? logical_sector < conf->reshape_safe
> : logical_sector >= conf->reshape_safe) {
> spin_unlock_irq(&conf->device_lock);
> + if (bi->bi_opf & REQ_NOWAIT) {
> + bio_wouldblock_error(bi);
> + finish_wait(&conf->wait_for_overlap, &w);
> + return true;
> + }
A bio could use several stripes. If one stripe block, simpliy return bio here
doesn't really make the whole bio finish.
> schedule();
> do_prepare = true;
> goto retry;
> @@ -5672,6 +5677,11 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
> spin_unlock_irq(&conf->device_lock);
> if (must_retry) {
> raid5_release_stripe(sh);
> + if (bi->bi_opf & REQ_NOWAIT) {
> + bio_wouldblock_error(bi);
> + finish_wait(&conf->wait_for_overlap, &w);
> + return true;
> + }
> schedule();
> do_prepare = true;
> goto retry;
> @@ -5700,6 +5710,11 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
> sigset_t full, old;
> sigfillset(&full);
> sigprocmask(SIG_BLOCK, &full, &old);
> + if (bi->bi_opf & REQ_NOWAIT) {
> + bio_wouldblock_error(bi);
> + finish_wait(&conf->wait_for_overlap, &w);
> + return true;
> + }
> schedule();
> sigprocmask(SIG_SETMASK, &old, NULL);
> do_prepare = true;
> @@ -5715,6 +5730,11 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
> */
> md_wakeup_thread(mddev->thread);
> raid5_release_stripe(sh);
> + if (bi->bi_opf & REQ_NOWAIT) {
> + bio_wouldblock_error(bi);
> + finish_wait(&conf->wait_for_overlap, &w);
> + return true;
> + }
> schedule();
> do_prepare = true;
> goto retry;
> --
> 2.12.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 5/9] md: raid10 nowait support
From: Shaohua Li @ 2017-08-08 20:40 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: linux-block, hch, jack, linux-raid, dm-devel, Goldwyn Rodrigues
In-Reply-To: <20170726235806.12148-6-rgoldwyn@suse.de>
On Wed, Jul 26, 2017 at 06:58:02PM -0500, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
>
> Bail and status to EAGAIN if raid10 is going to wait for:
> + barriers
> + reshape operation
> + Too many queued requests
>
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> ---
> drivers/md/raid10.c | 62 ++++++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 47 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 5026e7ad51d3..6d80438c5040 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -978,8 +978,9 @@ static void lower_barrier(struct r10conf *conf)
> wake_up(&conf->wait_barrier);
> }
>
> -static void wait_barrier(struct r10conf *conf)
> +static bool wait_barrier(struct r10conf *conf, bool nowait)
> {
> + bool ret = true;
> spin_lock_irq(&conf->resync_lock);
> if (conf->barrier) {
> conf->nr_waiting++;
> @@ -993,19 +994,23 @@ static void wait_barrier(struct r10conf *conf)
> * count down.
> */
> raid10_log(conf->mddev, "wait barrier");
> - wait_event_lock_irq(conf->wait_barrier,
> - !conf->barrier ||
> - (atomic_read(&conf->nr_pending) &&
> - current->bio_list &&
> - (!bio_list_empty(¤t->bio_list[0]) ||
> - !bio_list_empty(¤t->bio_list[1]))),
> - conf->resync_lock);
> + if (!nowait)
> + wait_event_lock_irq(conf->wait_barrier,
> + !conf->barrier ||
> + (atomic_read(&conf->nr_pending) &&
> + current->bio_list &&
> + (!bio_list_empty(¤t->bio_list[0]) ||
> + !bio_list_empty(¤t->bio_list[1]))),
> + conf->resync_lock);
> + else
> + ret = false;
same here, nr_pending shouldn't be increased
> conf->nr_waiting--;
> if (!conf->nr_waiting)
> wake_up(&conf->wait_barrier);
> }
> atomic_inc(&conf->nr_pending);
> spin_unlock_irq(&conf->resync_lock);
> + return ret;
> }
>
> static void allow_barrier(struct r10conf *conf)
> @@ -1158,7 +1163,10 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
> * thread has put up a bar for new requests.
> * Continue immediately if no resync is active currently.
> */
> - wait_barrier(conf);
> + if (!wait_barrier(conf, bio->bi_opf & REQ_NOWAIT)) {
> + bio_wouldblock_error(bio);
> + return;
> + }
>
> sectors = r10_bio->sectors;
> while (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
> @@ -1169,12 +1177,16 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
> * pass
> */
> raid10_log(conf->mddev, "wait reshape");
> + if (bio->bi_opf & REQ_NOWAIT) {
> + bio_wouldblock_error(bio);
> + return;
> + }
> allow_barrier(conf);
> wait_event(conf->wait_barrier,
> conf->reshape_progress <= bio->bi_iter.bi_sector ||
> conf->reshape_progress >= bio->bi_iter.bi_sector +
> sectors);
> - wait_barrier(conf);
> + wait_barrier(conf, false);
> }
>
> rdev = read_balance(conf, r10_bio, &max_sectors);
> @@ -1308,7 +1320,10 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
> * thread has put up a bar for new requests.
> * Continue immediately if no resync is active currently.
> */
> - wait_barrier(conf);
> + if (!wait_barrier(conf, bio->bi_opf & REQ_NOWAIT)) {
> + bio_wouldblock_error(bio);
> + return;
> + }
>
> sectors = r10_bio->sectors;
> while (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
> @@ -1319,12 +1334,16 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
> * pass
> */
> raid10_log(conf->mddev, "wait reshape");
> + if (bio->bi_opf & REQ_NOWAIT) {
> + bio_wouldblock_error(bio);
> + return;
> + }
> allow_barrier(conf);
> wait_event(conf->wait_barrier,
> conf->reshape_progress <= bio->bi_iter.bi_sector ||
> conf->reshape_progress >= bio->bi_iter.bi_sector +
> sectors);
> - wait_barrier(conf);
> + wait_barrier(conf, false);
> }
>
> if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
> @@ -1339,6 +1358,10 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
> BIT(MD_SB_CHANGE_DEVS) | BIT(MD_SB_CHANGE_PENDING));
> md_wakeup_thread(mddev->thread);
> raid10_log(conf->mddev, "wait reshape metadata");
> + if (bio->bi_opf & REQ_NOWAIT) {
> + bio_wouldblock_error(bio);
> + return;
> + }
> wait_event(mddev->sb_wait,
> !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags));
>
> @@ -1348,6 +1371,10 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
> if (conf->pending_count >= max_queued_requests) {
> md_wakeup_thread(mddev->thread);
> raid10_log(mddev, "wait queued");
> + if (bio->bi_opf & REQ_NOWAIT) {
> + bio_wouldblock_error(bio);
> + return;
> + }
> wait_event(conf->wait_barrier,
> conf->pending_count < max_queued_requests);
> }
> @@ -1454,6 +1481,11 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
> int j;
> int d;
>
> + if (bio->bi_opf & REQ_NOWAIT) {
> + bio_wouldblock_error(bio);
> + return;
> + }
> +
> for (j = 0; j < i; j++) {
> if (r10_bio->devs[j].bio) {
> d = r10_bio->devs[j].devnum;
> @@ -1474,7 +1506,7 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
> allow_barrier(conf);
> raid10_log(conf->mddev, "wait rdev %d blocked", blocked_rdev->raid_disk);
> md_wait_for_blocked_rdev(blocked_rdev, mddev);
> - wait_barrier(conf);
> + wait_barrier(conf, false);
> goto retry_write;
> }
>
> @@ -1703,7 +1735,7 @@ static void print_conf(struct r10conf *conf)
>
> static void close_sync(struct r10conf *conf)
> {
> - wait_barrier(conf);
> + wait_barrier(conf, false);
> allow_barrier(conf);
>
> mempool_destroy(conf->r10buf_pool);
> @@ -4347,7 +4379,7 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
> if (need_flush ||
> time_after(jiffies, conf->reshape_checkpoint + 10*HZ)) {
> /* Need to update reshape_position in metadata */
> - wait_barrier(conf);
> + wait_barrier(conf, false);
> mddev->reshape_position = conf->reshape_progress;
> if (mddev->reshape_backwards)
> mddev->curr_resync_completed = raid10_size(mddev, 0, 0)
> --
> 2.12.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/9] md: raid1 nowait support
From: Shaohua Li @ 2017-08-08 20:39 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: linux-block, hch, jack, linux-raid, dm-devel, Goldwyn Rodrigues
In-Reply-To: <20170726235806.12148-4-rgoldwyn@suse.de>
On Wed, Jul 26, 2017 at 06:58:00PM -0500, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
>
> The RAID1 driver would bail with EAGAIN in case of:
> + I/O has to wait for a barrier
> + array is frozen
> + Area is suspended
> + There are too many pending I/O that it will be queued.
>
> To facilitate error for wait barriers, wait_barrier() is
> returning bool. True in case if there was a wait (or is not
> required). False in case a wait was required, but was not performed.
>
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> ---
> drivers/md/raid1.c | 74 +++++++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 57 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 3febfc8391fb..66ca4288e3e8 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -903,8 +903,9 @@ static void lower_barrier(struct r1conf *conf, sector_t sector_nr)
> wake_up(&conf->wait_barrier);
> }
>
> -static void _wait_barrier(struct r1conf *conf, int idx)
> +static bool _wait_barrier(struct r1conf *conf, int idx, bool nowait)
> {
> + bool ret = true;
> /*
> * We need to increase conf->nr_pending[idx] very early here,
> * then raise_barrier() can be blocked when it waits for
> @@ -935,7 +936,7 @@ static void _wait_barrier(struct r1conf *conf, int idx)
> */
> if (!READ_ONCE(conf->array_frozen) &&
> !atomic_read(&conf->barrier[idx]))
> - return;
> + return ret;
>
> /*
> * After holding conf->resync_lock, conf->nr_pending[idx]
> @@ -953,18 +954,26 @@ static void _wait_barrier(struct r1conf *conf, int idx)
> */
> wake_up(&conf->wait_barrier);
> /* Wait for the barrier in same barrier unit bucket to drop. */
> - wait_event_lock_irq(conf->wait_barrier,
> - !conf->array_frozen &&
> - !atomic_read(&conf->barrier[idx]),
> - conf->resync_lock);
> + if (conf->array_frozen || atomic_read(&conf->barrier[idx])) {
> + if (nowait)
> + ret = false;
In this case, we nr_pending shouldn't be increased
> + else
> + wait_event_lock_irq(conf->wait_barrier,
> + !conf->array_frozen &&
> + !atomic_read(&conf->barrier[idx]),
> + conf->resync_lock);
> + }
> atomic_inc(&conf->nr_pending[idx]);
> atomic_dec(&conf->nr_waiting[idx]);
> spin_unlock_irq(&conf->resync_lock);
> + return ret;
> }
>
> -static void wait_read_barrier(struct r1conf *conf, sector_t sector_nr)
> +static bool wait_read_barrier(struct r1conf *conf, sector_t sector_nr,
> + bool nowait)
> {
> int idx = sector_to_idx(sector_nr);
> + bool ret = true;
>
> /*
> * Very similar to _wait_barrier(). The difference is, for read
> @@ -976,7 +985,7 @@ static void wait_read_barrier(struct r1conf *conf, sector_t sector_nr)
> atomic_inc(&conf->nr_pending[idx]);
>
> if (!READ_ONCE(conf->array_frozen))
> - return;
> + return ret;
>
> spin_lock_irq(&conf->resync_lock);
> atomic_inc(&conf->nr_waiting[idx]);
> @@ -987,19 +996,28 @@ static void wait_read_barrier(struct r1conf *conf, sector_t sector_nr)
> */
> wake_up(&conf->wait_barrier);
> /* Wait for array to be unfrozen */
> - wait_event_lock_irq(conf->wait_barrier,
> - !conf->array_frozen,
> - conf->resync_lock);
> + if (conf->array_frozen) {
> + /* If nowait flag is set, return false to
> + * show we did not wait
> + */
> + if (nowait)
> + ret = false;
ditto
> + else
> + wait_event_lock_irq(conf->wait_barrier,
> + !conf->array_frozen,
> + conf->resync_lock);
> + }
> atomic_inc(&conf->nr_pending[idx]);
> atomic_dec(&conf->nr_waiting[idx]);
> spin_unlock_irq(&conf->resync_lock);
> + return ret;
> }
>
> -static void wait_barrier(struct r1conf *conf, sector_t sector_nr)
> +static bool wait_barrier(struct r1conf *conf, sector_t sector_nr, bool nowait)
> {
> int idx = sector_to_idx(sector_nr);
>
> - _wait_barrier(conf, idx);
> + return _wait_barrier(conf, idx, nowait);
> }
>
> static void wait_all_barriers(struct r1conf *conf)
> @@ -1007,7 +1025,7 @@ static void wait_all_barriers(struct r1conf *conf)
> int idx;
>
> for (idx = 0; idx < BARRIER_BUCKETS_NR; idx++)
> - _wait_barrier(conf, idx);
> + _wait_barrier(conf, idx, false);
> }
>
> static void _allow_barrier(struct r1conf *conf, int idx)
> @@ -1223,7 +1241,11 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
> * Still need barrier for READ in case that whole
> * array is frozen.
> */
> - wait_read_barrier(conf, bio->bi_iter.bi_sector);
> + if (!wait_read_barrier(conf, bio->bi_iter.bi_sector,
> + bio->bi_opf & REQ_NOWAIT)) {
> + bio_wouldblock_error(bio);
> + return;
> + }
>
> if (!r1_bio)
> r1_bio = alloc_r1bio(mddev, bio);
> @@ -1333,6 +1355,11 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
> * an interruptible wait.
> */
> DEFINE_WAIT(w);
> + if (bio->bi_opf & REQ_NOWAIT) {
> + bio_wouldblock_error(bio);
> + return;
> + }
> +
> for (;;) {
> sigset_t full, old;
> prepare_to_wait(&conf->wait_barrier,
> @@ -1351,7 +1378,11 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
> }
> finish_wait(&conf->wait_barrier, &w);
> }
> - wait_barrier(conf, bio->bi_iter.bi_sector);
> + if (!wait_barrier(conf, bio->bi_iter.bi_sector,
> + bio->bi_opf & REQ_NOWAIT)) {
> + bio_wouldblock_error(bio);
> + return;
> + }
>
> r1_bio = alloc_r1bio(mddev, bio);
> r1_bio->sectors = max_write_sectors;
> @@ -1359,6 +1390,10 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
> if (conf->pending_count >= max_queued_requests) {
> md_wakeup_thread(mddev->thread);
> raid1_log(mddev, "wait queued");
> + if (bio->bi_opf & REQ_NOWAIT) {
> + bio_wouldblock_error(bio);
> + return;
> + }
> wait_event(conf->wait_barrier,
> conf->pending_count < max_queued_requests);
> }
> @@ -1442,6 +1477,11 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
> /* Wait for this device to become unblocked */
> int j;
>
> + if (bio->bi_opf & REQ_NOWAIT) {
> + bio_wouldblock_error(bio);
> + return;
> + }
> +
> for (j = 0; j < i; j++)
> if (r1_bio->bios[j])
> rdev_dec_pending(conf->mirrors[j].rdev, mddev);
> @@ -1449,7 +1489,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
> allow_barrier(conf, bio->bi_iter.bi_sector);
> raid1_log(mddev, "wait rdev %d blocked", blocked_rdev->raid_disk);
> md_wait_for_blocked_rdev(blocked_rdev, mddev);
> - wait_barrier(conf, bio->bi_iter.bi_sector);
> + wait_barrier(conf, bio->bi_iter.bi_sector, false);
There are other cases we could block, for example, md_wait_for_blocked_rdev
here. Is the goal just avoid block for normal situations?
> goto retry_write;
> }
>
> --
> 2.12.3
>
> --
> 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
* [PATCH v2] md/bitmap: preserve whole sb_page when initializing bitmap
From: Song Liu @ 2017-08-08 20:36 UTC (permalink / raw)
To: linux-raid, shli
Cc: Song Liu, shli, neilb, kernel-team, dan.j.williams, hch,
jes.sorensen
When bitmap_resize() is used to initialize the bitmap, it is
necessary to preserve the whole page of sb_page instead of just
the header (bitmap_super_t). This is because the sb_page may
contain bitmap read from the disks (initialized by mdadm).
Note that, this code path is only triggered with certain
combinations of parameters. One example is when raid456 array
is created with write journal.
To make the sb_page consistent across mutliple device, we also
update IO block size in write_sb_page() and read_sb_page() to
PAGE_SIZE.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
drivers/md/bitmap.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
index 40f3cd7..384e45d 100644
--- a/drivers/md/bitmap.c
+++ b/drivers/md/bitmap.c
@@ -162,8 +162,7 @@ static int read_sb_page(struct mddev *mddev, loff_t offset,
target = offset + index * (PAGE_SIZE/512);
- if (sync_page_io(rdev, target,
- roundup(size, bdev_logical_block_size(rdev->bdev)),
+ if (sync_page_io(rdev, target, PAGE_SIZE,
page, REQ_OP_READ, 0, true)) {
page->index = index;
return 0;
@@ -227,8 +226,7 @@ static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
int last_page_size = store->bytes & (PAGE_SIZE-1);
if (last_page_size == 0)
last_page_size = PAGE_SIZE;
- size = roundup(last_page_size,
- bdev_logical_block_size(bdev));
+ size = roundup(last_page_size, PAGE_SIZE);
}
/* Just make sure we aren't corrupting data or
* metadata
@@ -2117,8 +2115,7 @@ int bitmap_resize(struct bitmap *bitmap, sector_t blocks,
if (store.sb_page && bitmap->storage.sb_page)
memcpy(page_address(store.sb_page),
- page_address(bitmap->storage.sb_page),
- sizeof(bitmap_super_t));
+ page_address(bitmap->storage.sb_page), PAGE_SIZE);
bitmap_file_unmap(&bitmap->storage);
bitmap->storage = store;
--
2.9.3
^ permalink raw reply related
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Jens Axboe @ 2017-08-08 20:36 UTC (permalink / raw)
To: Shaohua Li, Goldwyn Rodrigues
Cc: linux-block, hch, jack, linux-raid, dm-devel, Goldwyn Rodrigues
In-Reply-To: <20170808203246.sdjnpzixxz5brjaf@kernel.org>
On 08/08/2017 02:32 PM, Shaohua Li wrote:
>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>> index 25f6a0cb27d3..fae021ebec1b 100644
>> --- a/include/linux/blkdev.h
>> +++ b/include/linux/blkdev.h
>> @@ -633,6 +633,7 @@ struct request_queue {
>> #define QUEUE_FLAG_REGISTERED 29 /* queue has been registered to a disk */
>> #define QUEUE_FLAG_SCSI_PASSTHROUGH 30 /* queue supports SCSI commands */
>> #define QUEUE_FLAG_QUIESCED 31 /* queue has been quiesced */
>> +#define QUEUE_FLAG_NOWAIT 32 /* stack device driver supports REQ_NOWAIT */
Does this work on 32-bit, where sizeof(unsigned long) == 32?
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 2/5] testb: implement block device operations
From: Jens Axboe @ 2017-08-08 20:34 UTC (permalink / raw)
To: Shaohua Li, linux-block, linux-raid
Cc: kernel-team, Kyungchan Koh, Kyungchan Koh
In-Reply-To: <971572972425927f702b33da37be14a9ab646a94.1501945859.git.shli@fb.com>
On 08/05/2017 09:51 AM, Shaohua Li wrote:
> +static struct testb_page *testb_insert_page(struct testb *testb,
> + sector_t sector, unsigned long *lock_flag)
> +{
> + u64 idx;
> + struct testb_page *t_page;
> +
> + assert_spin_locked(&testb->t_dev->lock);
> +
> + t_page = testb_lookup_page(testb, sector, true);
> + if (t_page)
> + return t_page;
> +
> + spin_unlock_irqrestore(&testb->t_dev->lock, *lock_flag);
Passing in lock flags through several functions is kind of iffy. It also
used to break on some architectures, though I don't think that's the
case anymore. The problem is that it usually ends up being a code
locking, instead of a data structure locking. The latter is much
cleaner.
> +static int copy_from_testb(struct testb *testb, struct page *dest,
> + unsigned int off, sector_t sector, size_t n, unsigned long *lock_flag)
This also gets the lock flags passed in, but doesn't use it.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 2/9] md: Add nowait support to md
From: Shaohua Li @ 2017-08-08 20:34 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: linux-block, hch, jack, linux-raid, dm-devel, Goldwyn Rodrigues
In-Reply-To: <20170726235806.12148-3-rgoldwyn@suse.de>
On Wed, Jul 26, 2017 at 06:57:59PM -0500, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
>
> Set queue flags to QUEUE_FLAG_NOWAIT to indicate REQ_NOWAIT
> will be handled.
>
> If an I/O on the md will be delayed, it would bail by calling
> bio_wouldblock_error(). The conditions when this could happen are:
>
> + MD is suspended
> + There is a change pending on the SB, and current I/O would
> block until that is complete.
>
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> ---
> drivers/md/md.c | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 8cdca0296749..d96c27d16841 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -285,6 +285,13 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
> bio_endio(bio);
> return BLK_QC_T_NONE;
> }
> +
> + if (mddev->suspended && (bio->bi_opf & REQ_NOWAIT)) {
> + bio_wouldblock_error(bio);
> + rcu_read_unlock();
this unlock is not required.
> + return BLK_QC_T_NONE;
> + }
> +
> check_suspended:
> rcu_read_lock();
> if (mddev->suspended) {
> @@ -5274,6 +5281,10 @@ static int md_alloc(dev_t dev, char *name)
> mddev->queue = NULL;
> goto abort;
> }
> +
> + /* Set the NOWAIT flags to show support */
> + queue_flag_set_unlocked(QUEUE_FLAG_NOWAIT, mddev->queue);
> +
> disk->major = MAJOR(mddev->unit);
> disk->first_minor = unit << shift;
> if (name)
> @@ -8010,8 +8021,20 @@ bool md_write_start(struct mddev *mddev, struct bio *bi)
> rcu_read_unlock();
> if (did_change)
> sysfs_notify_dirent_safe(mddev->sysfs_state);
> - wait_event(mddev->sb_wait,
> - !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags) && !mddev->suspended);
> +
> + /* Don't wait for sb writes if marked with REQ_NOWAIT */
> + if (test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags) ||
> + mddev->suspended) {
> + if (bi->bi_opf & REQ_NOWAIT) {
> + bio_wouldblock_error(bi);
> + percpu_ref_put(&mddev->writes_pending);
> + return false;
> + }
> +
> + wait_event(mddev->sb_wait,
> + !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags) && !mddev->suspended);
> + }
> +
> if (test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags)) {
> percpu_ref_put(&mddev->writes_pending);
> return false;
> --
> 2.12.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Shaohua Li @ 2017-08-08 20:32 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: linux-block, hch, jack, linux-raid, dm-devel, Goldwyn Rodrigues
In-Reply-To: <20170726235806.12148-2-rgoldwyn@suse.de>
On Wed, Jul 26, 2017 at 06:57:58PM -0500, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
>
> Nowait is a feature of direct AIO, where users can request
> to return immediately if the I/O is going to block. This translates
> to REQ_NOWAIT in bio.bi_opf flags. While request based devices
> don't wait, stacked devices such as md/dm will.
>
> In order to explicitly mark stacked devices as supported, we
> set the QUEUE_FLAG_NOWAIT in the queue_flags and return -EAGAIN
> whenever the device would block.
probably you should route this patch to Jens first, DM/MD are different trees.
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> ---
> block/blk-core.c | 3 ++-
> include/linux/blkdev.h | 2 ++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 970b9c9638c5..1c9a981d88e5 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -2025,7 +2025,8 @@ generic_make_request_checks(struct bio *bio)
> * if queue is not a request based queue.
> */
>
> - if ((bio->bi_opf & REQ_NOWAIT) && !queue_is_rq_based(q))
> + if ((bio->bi_opf & REQ_NOWAIT) && !queue_is_rq_based(q) &&
> + !blk_queue_supports_nowait(q))
> goto not_supported;
>
> part = bio->bi_bdev->bd_part;
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 25f6a0cb27d3..fae021ebec1b 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -633,6 +633,7 @@ struct request_queue {
> #define QUEUE_FLAG_REGISTERED 29 /* queue has been registered to a disk */
> #define QUEUE_FLAG_SCSI_PASSTHROUGH 30 /* queue supports SCSI commands */
> #define QUEUE_FLAG_QUIESCED 31 /* queue has been quiesced */
> +#define QUEUE_FLAG_NOWAIT 32 /* stack device driver supports REQ_NOWAIT */
>
> #define QUEUE_FLAG_DEFAULT ((1 << QUEUE_FLAG_IO_STAT) | \
> (1 << QUEUE_FLAG_STACKABLE) | \
> @@ -732,6 +733,7 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
> #define blk_queue_dax(q) test_bit(QUEUE_FLAG_DAX, &(q)->queue_flags)
> #define blk_queue_scsi_passthrough(q) \
> test_bit(QUEUE_FLAG_SCSI_PASSTHROUGH, &(q)->queue_flags)
> +#define blk_queue_supports_nowait(q) test_bit(QUEUE_FLAG_NOWAIT, &(q)->queue_flags)
Should this bit consider under layer disks? For example, one raid array disk
doesn't support NOWAIT, shouldn't we disable NOWAIT for the array?
I have another generic question. If a bio is splitted into 2 bios, one bio
doesn't need to wait but the other need to wait. We will return -EAGAIN for the
second bio, so the whole bio will return -EAGAIN, but the first bio is already
dispatched to disk. Is this correct behavior?
^ permalink raw reply
* Re: [PATCH 0/5] block: a virtual block device driver for testing
From: Jens Axboe @ 2017-08-08 20:31 UTC (permalink / raw)
To: Shaohua Li, Hannes Reinecke
Cc: linux-block, linux-raid, kernel-team, Kyungchan Koh, Shaohua Li
In-Reply-To: <20170807163607.msflun2r4q5tlpsk@kernel.org>
On 08/07/2017 10:36 AM, Shaohua Li wrote:
> On Mon, Aug 07, 2017 at 10:29:05AM +0200, Hannes Reinecke wrote:
>> On 08/05/2017 05:51 PM, Shaohua Li wrote:
>>> From: Shaohua Li <shli@fb.com>
>>>
>>> In testing software RAID, I usually found it's hard to cover specific cases.
>>> RAID is supposed to work even disk is in semi good state, for example, some
>>> sectors are broken. Since we can't control the behavior of hardware, it's
>>> difficult to create test suites to do destructive tests. But we can control the
>>> behavior of software, software based disk is an obvious choice for such tests.
>>> While we already have several software based disks for testing (eg, null_blk,
>>> scsi_debug), none is for destructive testing, this is the reason we create a
>>> new test block device.
>>>
>>> Currently the driver can create disk with following features:
>>> - Bandwidth control. A raid array consists of several disks. The disks could
>>> run in different speed, for example, one disk is SSD and the other is HD.
>>> Actually raid1 has a feature called write behind just for this. To test such
>>> raid1 feature, we'd like the disks speed could be controlled.
>>> - Emulate disk cache. Software must flush disk cache to guarantee data is
>>> safely stored in media after a power failure. To verify if software works
>>> well, we can't simply use physical disk, because even software doesn't flush
>>> cache, the hardware probably will flush the cache. With a software
>>> implementation of disk cache, we can fully control how we flush disk cache in a
>>> power failure.
>>> - Badblock. If only part of a disk is broken, software raid continues working.
>>> To test if software raid works well, disks must include some broken parts or
>>> bad blocks. Bad blocks can be easily implemented in software.
>>>
>>> While this is inspired by software raid testing, the driver is very flexible
>>> for extension. We can easily add new features into the driver. The interface is
>>> configfs, which can be configured with a shell script. There is a 'features'
>>> attribute exposing all supported features. By checking this, we don't need to
>>> worry about compability issues. For configuration details, please check the
>>> first patch.
>>>
>> Any particular reason why you can't fold these changes into brd or null_blk?
>> After all, without those testing features it is 'just' another ramdisk
>> driver...
>
> null_blk isn't a good fit. ramdisk might be, but I try to not. We are adding
> new interfaces, locks and so on. Adding the features into ramdisk driver will
> mess it up. Binding it to ramdisk driver will make adding new features harder
> too, because the test driver doesn't really care about performance while
> ramdisk does.
I'm curious why null_blk isn't a good fit? You'd just need to add RAM
storage to it. That would just be a separate option that should be set,
ram_backing=1 or something like that. That would make it less critical
than using the RAM disk driver as well, since only people that want a "real"
data backing would enable it.
It's not that I'm extremely opposed to adding a(nother) test block driver,
but we at least need some sort of reasoning behind why, which isn't just
"not a good fit".
--
Jens Axboe
^ permalink raw reply
* Re: RecovData Handshk error
From: David C. Rankin @ 2017-08-08 20:08 UTC (permalink / raw)
To: Alex; +Cc: mdraid
In-Reply-To: <CAB1R3shMZX7Dg0G3iVa-oMrM77DFSJzDTt64VqGr4OF1Ys8RwA@mail.gmail.com>
On 08/05/2017 02:43 PM, Alex wrote:
> Hi all, I sent the message below last week and haven't received any
> response. Is there a place that might be more appropriate for help
> with my MegaRAID and possible disk failure?
Alex,
Sorry, the reason you got no response is because the LSI (or whatever
vendor) MegaRAID software is a for a proprietary HARDWARE raid implementation.
This list is for opensource Linux SOFTWARE raid. The MegaRAID software used to
manage the proprietary hardware RAID card and drives (while opensourced a
while back) has nothing at all to do with Linux software RAID discussed here.
Once you get your problem sorted out, (or if you can't), you might consider
just using your raid card as a simple disk controller and creating a Linux
software RAID array on that -- then your questions would be relevant here.
(and unless you are saturating your hardware raid setup relying on a
battery-powered write-back cache, then good old free software raid here will
likely match or exceed the I/O performance of your current hardware card :)
--
David C. Rankin, J.D.,P.E.
^ permalink raw reply
* (unknown),
From: citydesk @ 2017-08-08 19:40 UTC (permalink / raw)
To: linux-raid
[-- Attachment #1: 4143572985.zip --]
[-- Type: application/zip, Size: 2790 bytes --]
^ permalink raw reply
* Re: [PATCH] md/bitmap: preserve whole sb_page when initializing bitmap
From: Song Liu @ 2017-08-08 18:50 UTC (permalink / raw)
To: Shaohua Li
Cc: linux-raid@vger.kernel.org, Shaohua Li, neilb@suse.com,
Kernel Team, dan.j.williams@intel.com, hch@infradead.org,
jes.sorensen@gmail.com
In-Reply-To: <20170808152856.bhoikpeuw7pg6yy5@kernel.org>
>> On 8/8/17, 8:30 AM, "Shaohua Li" <shli@kernel.org> wrote:
On Tue, Aug 01, 2017 at 11:11:37PM -0700, Song Liu wrote:
> When bitmap_resize() is used to initialize the bitmap, it is
> necessary to preserve the whole page of sb_page instead of just
> the header (bitmap_super_t). This is because the sb_page may
> contain bitmap read from the disks (initialized by mdadm).
>
> Note that, this code path is only triggered with certain
> combinations of parameters. One example is when raid456 array
> is created with write journal
Good catch, this issue probably only exists with journal, because we write
superblock there.
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
> drivers/md/bitmap.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
> index 40f3cd7..93dd809 100644
> --- a/drivers/md/bitmap.c
> +++ b/drivers/md/bitmap.c
> @@ -2118,7 +2118,7 @@ int bitmap_resize(struct bitmap *bitmap, sector_t blocks,
> if (store.sb_page && bitmap->storage.sb_page)
> memcpy(page_address(store.sb_page),
> page_address(bitmap->storage.sb_page),
> - sizeof(bitmap_super_t));
> + init ? PAGE_SIZE : sizeof(bitmap_super_t));
I think this should be 'roundup(sizeof(bitmap_super_t),
bdev_logical_block_size(rdev->bdev))', we not always read one page.
The sb_page is read from from one device, but updated in all devices. So when
different devices have different block sizes, we may still write wrong bitmap
to the devices. How about we make read_sb_page() and write_sb_page() both access
full page instead?
Thanks,
Song
^ permalink raw reply
* [PATCH] Assemble: prevent segfault with faulty "best" devices
From: Andrea Righi @ 2017-08-08 17:48 UTC (permalink / raw)
To: Jes Sorensen; +Cc: Robert LeBlanc, NeilBrown, linux-raid
I was able to trigger this curious problem that seems to happen only on
one of our server:
# mdadm --assemble /dev/md/10.4.237.12-volume --name 10.4.237.12-volume
Segmentation fault
This md volume is a raid1 volume made of 2 device mapper (dm-multipath)
devices and the underlying LUNs are imported via iSCSI.
Applying the following patch (see below) seems to fix the problem:
# ./mdadm --assemble /dev/md/10.4.237.12-volume --name 10.4.237.12-volume
mdadm: /dev/md/10.4.237.12-volume has been started with 2 drives.
But I'm not sure if it's the right fix or if there're some other
problems that I'm missing.
More details about the md superblocks that might help to better
understand the nature of the problem:
# for i in 36001405a04ed0c104881{1,2}00000000000p2; do echo dev: ${i}; mdadm --examine /dev/mapper/${i}; done
dev: 36001405a04ed0c104881100000000000p2
/dev/mapper/36001405a04ed0c104881100000000000p2:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x1
Array UUID : 5f3e8283:7f831b85:bc1958b9:6f2787a4
Name : 10.4.237.12-volume
Creation Time : Thu Jul 27 14:43:16 2017
Raid Level : raid1
Raid Devices : 2
Avail Dev Size : 1073729503 (511.99 GiB 549.75 GB)
Array Size : 536864704 (511.99 GiB 549.75 GB)
Used Dev Size : 1073729408 (511.99 GiB 549.75 GB)
Data Offset : 8192 sectors
Super Offset : 8 sectors
Unused Space : before=8104 sectors, after=95 sectors
State : clean
Device UUID : 16dae7e3:42f3487f:fbeac43a:71cf1f63
Internal Bitmap : 8 sectors from superblock
Update Time : Tue Aug 8 11:12:22 2017
Bad Block Log : 512 entries available at offset 72 sectors
Checksum : 518c443e - correct
Events : 167
Device Role : Active device 0
Array State : AA ('A' == active, '.' == missing, 'R' == replacing)
dev: 36001405a04ed0c104881200000000000p2
/dev/mapper/36001405a04ed0c104881200000000000p2:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x1
Array UUID : 5f3e8283:7f831b85:bc1958b9:6f2787a4
Name : 10.4.237.12-volume
Creation Time : Thu Jul 27 14:43:16 2017
Raid Level : raid1
Raid Devices : 2
Avail Dev Size : 1073729503 (511.99 GiB 549.75 GB)
Array Size : 536864704 (511.99 GiB 549.75 GB)
Used Dev Size : 1073729408 (511.99 GiB 549.75 GB)
Data Offset : 8192 sectors
Super Offset : 8 sectors
Unused Space : before=8104 sectors, after=95 sectors
State : clean
Device UUID : ef612bdd:e475fe02:5d3fc55e:53612f34
Internal Bitmap : 8 sectors from superblock
Update Time : Tue Aug 8 11:12:22 2017
Bad Block Log : 512 entries available at offset 72 sectors
Checksum : c39534fd - correct
Events : 167
Device Role : Active device 1
Array State : AA ('A' == active, '.' == missing, 'R' == replacing)
# for i in 36001405a04ed0c104881{1,2}00000000000p2; do echo dev: ${i}; hexdump -s 4096 -n 4189696 -C /dev/mapper/${i}; done
dev: 36001405a04ed0c104881100000000000p2
00001000 fc 4e 2b a9 01 00 00 00 01 00 00 00 00 00 00 00 |.N+.............|
00001010 5f 3e 82 83 7f 83 1b 85 bc 19 58 b9 6f 27 87 a4 |_>........X.o'..|
00001020 31 30 2e 34 2e 32 33 37 2e 31 32 2d 76 6f 6c 75 |10.4.237.12-volu|
00001030 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |me..............|
00001040 64 50 7a 59 00 00 00 00 01 00 00 00 00 00 00 00 |dPzY............|
00001050 80 cf ff 3f 00 00 00 00 00 00 00 00 02 00 00 00 |...?............|
00001060 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00001070 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00001080 00 20 00 00 00 00 00 00 df cf ff 3f 00 00 00 00 |. .........?....|
00001090 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
000010a0 00 00 00 00 00 00 00 00 16 da e7 e3 42 f3 48 7f |............B.H.|
000010b0 fb ea c4 3a 71 cf 1f 63 00 00 08 00 48 00 00 00 |...:q..c....H...|
000010c0 54 f0 89 59 00 00 00 00 a7 00 00 00 00 00 00 00 |T..Y............|
000010d0 ff ff ff ff ff ff ff ff 9c 43 8c 51 80 00 00 00 |.........C.Q....|
000010e0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
00001100 00 00 01 00 fe ff fe ff fe ff fe ff fe ff fe ff |................|
00001110 fe ff fe ff fe ff fe ff fe ff fe ff fe ff fe ff |................|
*
00001200 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
00002000 62 69 74 6d 04 00 00 00 5f 3e 82 83 7f 83 1b 85 |bitm...._>......|
00002010 bc 19 58 b9 6f 27 87 a4 a7 00 00 00 00 00 00 00 |..X.o'..........|
00002020 a7 00 00 00 00 00 00 00 80 cf ff 3f 00 00 00 00 |...........?....|
00002030 00 00 00 00 00 00 00 01 05 00 00 00 00 00 00 00 |................|
00002040 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
00003100 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
*
00004000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
003ffe00
dev: 36001405a04ed0c104881200000000000p2
00001000 fc 4e 2b a9 01 00 00 00 01 00 00 00 00 00 00 00 |.N+.............|
00001010 5f 3e 82 83 7f 83 1b 85 bc 19 58 b9 6f 27 87 a4 |_>........X.o'..|
00001020 31 30 2e 34 2e 32 33 37 2e 31 32 2d 76 6f 6c 75 |10.4.237.12-volu|
00001030 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |me..............|
00001040 64 50 7a 59 00 00 00 00 01 00 00 00 00 00 00 00 |dPzY............|
00001050 80 cf ff 3f 00 00 00 00 00 00 00 00 02 00 00 00 |...?............|
00001060 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00001070 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00001080 00 20 00 00 00 00 00 00 df cf ff 3f 00 00 00 00 |. .........?....|
00001090 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
000010a0 01 00 00 00 00 00 00 00 ef 61 2b dd e4 75 fe 02 |.........a+..u..|
000010b0 5d 3f c5 5e 53 61 2f 34 00 00 08 00 48 00 00 00 |]?.^Sa/4....H...|
000010c0 54 f0 89 59 00 00 00 00 a7 00 00 00 00 00 00 00 |T..Y............|
000010d0 ff ff ff ff ff ff ff ff 5b 34 95 c3 80 00 00 00 |........[4......|
000010e0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
00001100 00 00 01 00 fe ff fe ff fe ff fe ff fe ff fe ff |................|
00001110 fe ff fe ff fe ff fe ff fe ff fe ff fe ff fe ff |................|
*
00001200 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
00002000 62 69 74 6d 04 00 00 00 5f 3e 82 83 7f 83 1b 85 |bitm...._>......|
00002010 bc 19 58 b9 6f 27 87 a4 a7 00 00 00 00 00 00 00 |..X.o'..........|
00002020 a7 00 00 00 00 00 00 00 80 cf ff 3f 00 00 00 00 |...........?....|
00002030 00 00 00 00 00 00 00 01 05 00 00 00 00 00 00 00 |................|
00002040 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
00003100 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
*
00004000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
003ffe00
---
Assemble: prevent segfault with faulty "best" devices
In Assemble(), after context reload, best[i] can be -1 in some cases,
and before checking if this value is negative we use it to access
devices[j].i.disk.raid_disk, potentially causing a segfault.
Check if best[i] is negative before using it to prevent this potential
segfault.
Signed-off-by: Andrea Righi <andrea@betterlinux.com>
Signed-off-by: Robert LeBlanc <robert@leblancnet.us>
---
Assemble.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Assemble.c b/Assemble.c
index 3da0903..fc681eb 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -1669,6 +1669,8 @@ try_again:
int j = best[i];
unsigned int desired_state;
+ if (j < 0)
+ continue;
if (devices[j].i.disk.raid_disk == MD_DISK_ROLE_JOURNAL)
desired_state = (1<<MD_DISK_JOURNAL);
else if (i >= content->array.raid_disks * 2)
@@ -1678,8 +1680,6 @@ try_again:
else
desired_state = (1<<MD_DISK_ACTIVE) | (1<<MD_DISK_SYNC);
- if (j<0)
- continue;
if (!devices[j].uptodate)
continue;
^ permalink raw reply related
* Re: [PATCH] md/bitmap: preserve whole sb_page when initializing bitmap
From: Shaohua Li @ 2017-08-08 15:29 UTC (permalink / raw)
To: Song Liu
Cc: linux-raid, shli, neilb, kernel-team, dan.j.williams, hch,
jes.sorensen
In-Reply-To: <20170802061137.2434370-1-songliubraving@fb.com>
On Tue, Aug 01, 2017 at 11:11:37PM -0700, Song Liu wrote:
> When bitmap_resize() is used to initialize the bitmap, it is
> necessary to preserve the whole page of sb_page instead of just
> the header (bitmap_super_t). This is because the sb_page may
> contain bitmap read from the disks (initialized by mdadm).
>
> Note that, this code path is only triggered with certain
> combinations of parameters. One example is when raid456 array
> is created with write journal
Good catch, this issue probably only exists with journal, because we write
superblock there.
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
> drivers/md/bitmap.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
> index 40f3cd7..93dd809 100644
> --- a/drivers/md/bitmap.c
> +++ b/drivers/md/bitmap.c
> @@ -2118,7 +2118,7 @@ int bitmap_resize(struct bitmap *bitmap, sector_t blocks,
> if (store.sb_page && bitmap->storage.sb_page)
> memcpy(page_address(store.sb_page),
> page_address(bitmap->storage.sb_page),
> - sizeof(bitmap_super_t));
> + init ? PAGE_SIZE : sizeof(bitmap_super_t));
I think this should be 'roundup(sizeof(bitmap_super_t),
bdev_logical_block_size(rdev->bdev))', we not always read one page.
The init check is unnecessary too, because if it isn't init, we will initialize
the bitmaps later in bitmap_resize, so the copy doesn't matter.
Thanks,
Shaohua
> bitmap_file_unmap(&bitmap->storage);
> bitmap->storage = store;
>
> --
> 2.9.3
>
^ permalink raw reply
* Re: [PATCH v4 02/19] crypto: ccm: use -EAGAIN for transient busy indication
From: Gary R Hook @ 2017-08-08 15:24 UTC (permalink / raw)
To: Gilad Ben-Yossef, Herbert Xu, David S. Miller, Jonathan Corbet,
David Howells, Lendacky, Thomas, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer,
dm-devel@redhat.com, Shaohua Li, Steve French,
Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serg
Cc: Ofir Drang
In-Reply-To: <1502193834-11289-3-git-send-email-gilad@benyossef.com>
On 08/08/2017 07:03 AM, Gilad Ben-Yossef wrote:
> Replace -EBUSY with -EAGAIN when reporting transient busy
> indication in the absence of backlog.
>
> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Could we use "ccp" in the subject line, please?
> ---
> drivers/crypto/ccp/ccp-crypto-main.c | 8 +++-----
> drivers/crypto/ccp/ccp-dev.c | 7 +++++--
> 2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/crypto/ccp/ccp-crypto-main.c b/drivers/crypto/ccp/ccp-crypto-main.c
> index 35a9de7..403ff0a 100644
> --- a/drivers/crypto/ccp/ccp-crypto-main.c
> +++ b/drivers/crypto/ccp/ccp-crypto-main.c
> @@ -222,9 +222,10 @@ static int ccp_crypto_enqueue_cmd(struct ccp_crypto_cmd *crypto_cmd)
>
> /* Check if the cmd can/should be queued */
> if (req_queue.cmd_count >= CCP_CRYPTO_MAX_QLEN) {
> - ret = -EBUSY;
> - if (!(crypto_cmd->cmd->flags & CCP_CMD_MAY_BACKLOG))
> + if (!(crypto_cmd->cmd->flags & CCP_CMD_MAY_BACKLOG)) {
> + ret = -EAGAIN;
> goto e_lock;
> + }
> }
>
> /* Look for an entry with the same tfm. If there is a cmd
> @@ -243,9 +244,6 @@ static int ccp_crypto_enqueue_cmd(struct ccp_crypto_cmd *crypto_cmd)
> ret = ccp_enqueue_cmd(crypto_cmd->cmd);
> if (!ccp_crypto_success(ret))
> goto e_lock; /* Error, don't queue it */
> - if ((ret == -EBUSY) &&
> - !(crypto_cmd->cmd->flags & CCP_CMD_MAY_BACKLOG))
> - goto e_lock; /* Not backlogging, don't queue it */
> }
>
> if (req_queue.cmd_count >= CCP_CRYPTO_MAX_QLEN) {
> diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c
> index 4e029b1..3d637e3 100644
> --- a/drivers/crypto/ccp/ccp-dev.c
> +++ b/drivers/crypto/ccp/ccp-dev.c
> @@ -292,9 +292,12 @@ int ccp_enqueue_cmd(struct ccp_cmd *cmd)
> i = ccp->cmd_q_count;
>
> if (ccp->cmd_count >= MAX_CMD_QLEN) {
> - ret = -EBUSY;
> - if (cmd->flags & CCP_CMD_MAY_BACKLOG)
> + if (cmd->flags & CCP_CMD_MAY_BACKLOG) {
> + ret = -EBUSY;
> list_add_tail(&cmd->entry, &ccp->backlog);
> + } else {
> + ret = -EAGAIN;
> + }
> } else {
> ret = -EINPROGRESS;
> ccp->cmd_count++;
>
^ permalink raw reply
* Re: [md PATCH 1/2] md: always clear ->safemode when md_check_recovery gets the mddev lock.
From: Shaohua Li @ 2017-08-08 14:40 UTC (permalink / raw)
To: NeilBrown; +Cc: Dominik Brodowski, David R, linux-raid
In-Reply-To: <150217539687.23065.4449077069418495000.stgit@noble>
On Tue, Aug 08, 2017 at 04:56:36PM +1000, Neil Brown wrote:
> If ->safemode == 1, md_check_recovery() will try to get the mddev lock
> and perform various other checks.
> If mddev->in_sync is zero, it will call set_in_sync, and clear
> ->safemode. However if mddev->in_sync is not zero, ->safemode will not
> be cleared.
>
> When md_check_recovery() drops the mddev lock, the thread is woken
> up again. Normally it would just check if there was anything else to
> do, find nothing, and go to sleep. However as ->safemode was not
> cleared, it will take the mddev lock again, then wake itself up
> when unlocking.
>
> This results in an infinite loop, repeatedly calling
> md_check_recovery(), which RCU or the soft-lockup detector
> will eventually complain about.
>
> Prior to commit 4ad23a976413 ("MD: use per-cpu counter for
> writes_pending"), safemode would only be set to one when the
> writes_pending counter reached zero, and would be cleared again
> when writes_pending is incremented. Since that patch, safemode
> is set more freely, but is not reliably cleared.
>
> So in md_check_recovery() clear ->safemode before checking ->in_sync.
Nice catch! Applied both patches.
I spent hours to check why md_check_recovery loops, apparently I missed
set_in_sync is only called when in_sync is not set, silly me.
Thanks,
Shaohua
> Fixes: 4ad23a976413 ("MD: use per-cpu counter for writes_pending")
> Cc: stable@vger.kernel.org (4.12+)
> Reported-by: Dominik Brodowski <linux@dominikbrodowski.net>
> Reported-by: David R <david@unsolicited.net>
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
> drivers/md/md.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index c99634612fc4..d84aceede1cb 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -8656,6 +8656,9 @@ void md_check_recovery(struct mddev *mddev)
> if (mddev_trylock(mddev)) {
> int spares = 0;
>
> + if (mddev->safemode == 1)
> + mddev->safemode = 0;
> +
> if (mddev->ro) {
> struct md_rdev *rdev;
> if (!mddev->external && mddev->in_sync)
>
>
^ permalink raw reply
* Re: [PATCH v4 06/19] crypto: move algif to generic async completion
From: Gilad Ben-Yossef @ 2017-08-08 13:45 UTC (permalink / raw)
To: Stephan Mueller
Cc: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer,
device-mapper development, Shaohua Li, Steve French,
Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge
In-Reply-To: <8585274.Umn6qnzcUd-b2PLbiJbNv8ftSvlWXw0+g@public.gmane.org>
On Tue, Aug 8, 2017 at 4:10 PM, Stephan Mueller <smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org> wrote:
> Am Dienstag, 8. August 2017, 14:03:37 CEST schrieb Gilad Ben-Yossef:
>
> Hi Gilad,
>
>> algif starts several async crypto ops and waits for their completion.
>> Move it over to generic code doing the same.
>
> Just FYI: your patch set and my patch [1] will clash.
>
> [1] https://patchwork.kernel.org/patch/9875959/
I'll try to rebase on linux-next + your patch than.
Thanks for letting me know.
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply
* Re: [PATCH v4 06/19] crypto: move algif to generic async completion
From: Stephan Mueller @ 2017-08-08 13:10 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Mike Snitzer, linux-doc, Gary Hook, David Howells,
samba.-technical, dm-devel, keyrings, linux-ima-devel,
Alasdair Kergon, Boris Brezillon, Herbert Xu, Jonathan Corbet,
Mimi Zohar, Serge E. Hallyn, Tom Lendacky, linux-cifs,
linux-ima-user, Arnaud Ebalard, linux-raid, linux-fscrypt,
linux-mediatek, James Morris, Matthias Brugger, Jaegeuk Kim,
linux-arm-kernel
In-Reply-To: <1502193834-11289-7-git-send-email-gilad@benyossef.com>
Am Dienstag, 8. August 2017, 14:03:37 CEST schrieb Gilad Ben-Yossef:
Hi Gilad,
> algif starts several async crypto ops and waits for their completion.
> Move it over to generic code doing the same.
Just FYI: your patch set and my patch [1] will clash.
[1] https://patchwork.kernel.org/patch/9875959/
Ciao
Stephan
^ permalink raw reply
* [PATCH v4 19/19] crypto: adapt api sample to use async. op wait
From: Gilad Ben-Yossef @ 2017-08-08 12:03 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, Shaohua Li, Steve French,
Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
keyrings-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel
Cc: Ofir Drang
In-Reply-To: <1502193834-11289-1-git-send-email-gilad-6S/DczAoZh3WXxRugSxzZg@public.gmane.org>
The code sample is waiting for an async. crypto op completion.
Adapt sample to use the new generic infrastructure to do the same.
This also fixes a possible data coruption bug created by the
use of wait_for_completion_interruptible() without dealing
correctly with an interrupt aborting the wait prior to the
async op finishing.
Signed-off-by: Gilad Ben-Yossef <gilad-6S/DczAoZh3WXxRugSxzZg@public.gmane.org>
---
Documentation/crypto/api-samples.rst | 52 +++++++-----------------------------
1 file changed, 10 insertions(+), 42 deletions(-)
diff --git a/Documentation/crypto/api-samples.rst b/Documentation/crypto/api-samples.rst
index 2531948..006827e 100644
--- a/Documentation/crypto/api-samples.rst
+++ b/Documentation/crypto/api-samples.rst
@@ -7,59 +7,27 @@ Code Example For Symmetric Key Cipher Operation
::
- struct tcrypt_result {
- struct completion completion;
- int err;
- };
-
/* tie all data structures together */
struct skcipher_def {
struct scatterlist sg;
struct crypto_skcipher *tfm;
struct skcipher_request *req;
- struct tcrypt_result result;
+ struct crypto_wait wait;
};
- /* Callback function */
- static void test_skcipher_cb(struct crypto_async_request *req, int error)
- {
- struct tcrypt_result *result = req->data;
-
- if (error == -EINPROGRESS)
- return;
- result->err = error;
- complete(&result->completion);
- pr_info("Encryption finished successfully\n");
- }
-
/* Perform cipher operation */
static unsigned int test_skcipher_encdec(struct skcipher_def *sk,
int enc)
{
- int rc = 0;
+ int rc;
if (enc)
- rc = crypto_skcipher_encrypt(sk->req);
+ rc = crypto_wait_req(crypto_skcipher_encrypt(sk->req), &sk->wait);
else
- rc = crypto_skcipher_decrypt(sk->req);
-
- switch (rc) {
- case 0:
- break;
- case -EINPROGRESS:
- case -EBUSY:
- rc = wait_for_completion_interruptible(
- &sk->result.completion);
- if (!rc && !sk->result.err) {
- reinit_completion(&sk->result.completion);
- break;
- }
- default:
- pr_info("skcipher encrypt returned with %d result %d\n",
- rc, sk->result.err);
- break;
- }
- init_completion(&sk->result.completion);
+ rc = crypto_wait_req(crypto_skcipher_decrypt(sk->req), &sk->wait);
+
+ if (rc)
+ pr_info("skcipher encrypt returned with result %d\n", rc);
return rc;
}
@@ -89,8 +57,8 @@ Code Example For Symmetric Key Cipher Operation
}
skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- test_skcipher_cb,
- &sk.result);
+ crypto_req_done,
+ &sk.wait);
/* AES 256 with random key */
get_random_bytes(&key, 32);
@@ -122,7 +90,7 @@ Code Example For Symmetric Key Cipher Operation
/* We encrypt one block */
sg_init_one(&sk.sg, scratchpad, 16);
skcipher_request_set_crypt(req, &sk.sg, &sk.sg, 16, ivdata);
- init_completion(&sk.result.completion);
+ crypto_init_wait(&sk.wait);
/* encrypt data */
ret = test_skcipher_encdec(&sk, 1);
--
2.1.4
^ permalink raw reply related
* [PATCH v4 19/19] crypto: adapt api sample to use async. op wait
From: Gilad Ben-Yossef @ 2017-08-08 12:03 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Shaohua Li, Steve French, Theodore Y. Ts'o, Jaegeuk Kim,
Mimi Zohar, Dmitry Kasatkin, James Morris, Serge E. Hallyn,
linux-crypto, linux-doc, linux-kernel, keyrings, linux-arm-kernel
Cc: Ofir Drang
In-Reply-To: <1502193834-11289-1-git-send-email-gilad@benyossef.com>
The code sample is waiting for an async. crypto op completion.
Adapt sample to use the new generic infrastructure to do the same.
This also fixes a possible data coruption bug created by the
use of wait_for_completion_interruptible() without dealing
correctly with an interrupt aborting the wait prior to the
async op finishing.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
Documentation/crypto/api-samples.rst | 52 +++++++-----------------------------
1 file changed, 10 insertions(+), 42 deletions(-)
diff --git a/Documentation/crypto/api-samples.rst b/Documentation/crypto/api-samples.rst
index 2531948..006827e 100644
--- a/Documentation/crypto/api-samples.rst
+++ b/Documentation/crypto/api-samples.rst
@@ -7,59 +7,27 @@ Code Example For Symmetric Key Cipher Operation
::
- struct tcrypt_result {
- struct completion completion;
- int err;
- };
-
/* tie all data structures together */
struct skcipher_def {
struct scatterlist sg;
struct crypto_skcipher *tfm;
struct skcipher_request *req;
- struct tcrypt_result result;
+ struct crypto_wait wait;
};
- /* Callback function */
- static void test_skcipher_cb(struct crypto_async_request *req, int error)
- {
- struct tcrypt_result *result = req->data;
-
- if (error == -EINPROGRESS)
- return;
- result->err = error;
- complete(&result->completion);
- pr_info("Encryption finished successfully\n");
- }
-
/* Perform cipher operation */
static unsigned int test_skcipher_encdec(struct skcipher_def *sk,
int enc)
{
- int rc = 0;
+ int rc;
if (enc)
- rc = crypto_skcipher_encrypt(sk->req);
+ rc = crypto_wait_req(crypto_skcipher_encrypt(sk->req), &sk->wait);
else
- rc = crypto_skcipher_decrypt(sk->req);
-
- switch (rc) {
- case 0:
- break;
- case -EINPROGRESS:
- case -EBUSY:
- rc = wait_for_completion_interruptible(
- &sk->result.completion);
- if (!rc && !sk->result.err) {
- reinit_completion(&sk->result.completion);
- break;
- }
- default:
- pr_info("skcipher encrypt returned with %d result %d\n",
- rc, sk->result.err);
- break;
- }
- init_completion(&sk->result.completion);
+ rc = crypto_wait_req(crypto_skcipher_decrypt(sk->req), &sk->wait);
+
+ if (rc)
+ pr_info("skcipher encrypt returned with result %d\n", rc);
return rc;
}
@@ -89,8 +57,8 @@ Code Example For Symmetric Key Cipher Operation
}
skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- test_skcipher_cb,
- &sk.result);
+ crypto_req_done,
+ &sk.wait);
/* AES 256 with random key */
get_random_bytes(&key, 32);
@@ -122,7 +90,7 @@ Code Example For Symmetric Key Cipher Operation
/* We encrypt one block */
sg_init_one(&sk.sg, scratchpad, 16);
skcipher_request_set_crypt(req, &sk.sg, &sk.sg, 16, ivdata);
- init_completion(&sk.result.completion);
+ crypto_init_wait(&sk.wait);
/* encrypt data */
ret = test_skcipher_encdec(&sk, 1);
--
2.1.4
^ permalink raw reply related
* [PATCH v4 19/19] crypto: adapt api sample to use async. op wait
From: Gilad Ben-Yossef @ 2017-08-08 12:03 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Shaohua Li, Steve French, Theodore Y. Ts'o, Jaegeuk Kim,
Mimi Zohar, Dmitry Kasatkin, James Morris, Serge E. Hallyn,
linux-crypto
Cc: Ofir Drang
In-Reply-To: <1502193834-11289-1-git-send-email-gilad@benyossef.com>
The code sample is waiting for an async. crypto op completion.
Adapt sample to use the new generic infrastructure to do the same.
This also fixes a possible data coruption bug created by the
use of wait_for_completion_interruptible() without dealing
correctly with an interrupt aborting the wait prior to the
async op finishing.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
Documentation/crypto/api-samples.rst | 52 +++++++-----------------------------
1 file changed, 10 insertions(+), 42 deletions(-)
diff --git a/Documentation/crypto/api-samples.rst b/Documentation/crypto/api-samples.rst
index 2531948..006827e 100644
--- a/Documentation/crypto/api-samples.rst
+++ b/Documentation/crypto/api-samples.rst
@@ -7,59 +7,27 @@ Code Example For Symmetric Key Cipher Operation
::
- struct tcrypt_result {
- struct completion completion;
- int err;
- };
-
/* tie all data structures together */
struct skcipher_def {
struct scatterlist sg;
struct crypto_skcipher *tfm;
struct skcipher_request *req;
- struct tcrypt_result result;
+ struct crypto_wait wait;
};
- /* Callback function */
- static void test_skcipher_cb(struct crypto_async_request *req, int error)
- {
- struct tcrypt_result *result = req->data;
-
- if (error == -EINPROGRESS)
- return;
- result->err = error;
- complete(&result->completion);
- pr_info("Encryption finished successfully\n");
- }
-
/* Perform cipher operation */
static unsigned int test_skcipher_encdec(struct skcipher_def *sk,
int enc)
{
- int rc = 0;
+ int rc;
if (enc)
- rc = crypto_skcipher_encrypt(sk->req);
+ rc = crypto_wait_req(crypto_skcipher_encrypt(sk->req), &sk->wait);
else
- rc = crypto_skcipher_decrypt(sk->req);
-
- switch (rc) {
- case 0:
- break;
- case -EINPROGRESS:
- case -EBUSY:
- rc = wait_for_completion_interruptible(
- &sk->result.completion);
- if (!rc && !sk->result.err) {
- reinit_completion(&sk->result.completion);
- break;
- }
- default:
- pr_info("skcipher encrypt returned with %d result %d\n",
- rc, sk->result.err);
- break;
- }
- init_completion(&sk->result.completion);
+ rc = crypto_wait_req(crypto_skcipher_decrypt(sk->req), &sk->wait);
+
+ if (rc)
+ pr_info("skcipher encrypt returned with result %d\n", rc);
return rc;
}
@@ -89,8 +57,8 @@ Code Example For Symmetric Key Cipher Operation
}
skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- test_skcipher_cb,
- &sk.result);
+ crypto_req_done,
+ &sk.wait);
/* AES 256 with random key */
get_random_bytes(&key, 32);
@@ -122,7 +90,7 @@ Code Example For Symmetric Key Cipher Operation
/* We encrypt one block */
sg_init_one(&sk.sg, scratchpad, 16);
skcipher_request_set_crypt(req, &sk.sg, &sk.sg, 16, ivdata);
- init_completion(&sk.result.completion);
+ crypto_init_wait(&sk.wait);
/* encrypt data */
ret = test_skcipher_encdec(&sk, 1);
--
2.1.4
^ permalink raw reply related
* [PATCH v4 18/19] crypto: mediatek: move to generic async completion
From: Gilad Ben-Yossef @ 2017-08-08 12:03 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Shaohua Li, Steve French, Theodore Y. Ts'o, Jaegeuk Kim,
Mimi Zohar, Dmitry Kasatkin, James Morris, Serge E. Hallyn,
linux-crypto, linux-doc, linux-kernel, keyrings, linux-arm-kernel
Cc: Ofir Drang
In-Reply-To: <1502193834-11289-1-git-send-email-gilad@benyossef.com>
The mediatek driver starts several async crypto ops and waits for their
completions. Move it over to generic code doing the same.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
drivers/crypto/mediatek/mtk-aes.c | 31 +++++--------------------------
1 file changed, 5 insertions(+), 26 deletions(-)
diff --git a/drivers/crypto/mediatek/mtk-aes.c b/drivers/crypto/mediatek/mtk-aes.c
index 9e845e8..e2c7c95 100644
--- a/drivers/crypto/mediatek/mtk-aes.c
+++ b/drivers/crypto/mediatek/mtk-aes.c
@@ -137,11 +137,6 @@ struct mtk_aes_gcm_ctx {
struct crypto_skcipher *ctr;
};
-struct mtk_aes_gcm_setkey_result {
- int err;
- struct completion completion;
-};
-
struct mtk_aes_drv {
struct list_head dev_list;
/* Device list lock */
@@ -936,17 +931,6 @@ static int mtk_aes_gcm_crypt(struct aead_request *req, u64 mode)
&req->base);
}
-static void mtk_gcm_setkey_done(struct crypto_async_request *req, int err)
-{
- struct mtk_aes_gcm_setkey_result *result = req->data;
-
- if (err == -EINPROGRESS)
- return;
-
- result->err = err;
- complete(&result->completion);
-}
-
/*
* Because of the hardware limitation, we need to pre-calculate key(H)
* for the GHASH operation. The result of the encryption operation
@@ -962,7 +946,7 @@ static int mtk_aes_gcm_setkey(struct crypto_aead *aead, const u8 *key,
u32 hash[4];
u8 iv[8];
- struct mtk_aes_gcm_setkey_result result;
+ struct crypto_wait wait;
struct scatterlist sg[1];
struct skcipher_request req;
@@ -1002,22 +986,17 @@ static int mtk_aes_gcm_setkey(struct crypto_aead *aead, const u8 *key,
if (!data)
return -ENOMEM;
- init_completion(&data->result.completion);
+ crypto_init_wait(&data->wait);
sg_init_one(data->sg, &data->hash, AES_BLOCK_SIZE);
skcipher_request_set_tfm(&data->req, ctr);
skcipher_request_set_callback(&data->req, CRYPTO_TFM_REQ_MAY_SLEEP |
CRYPTO_TFM_REQ_MAY_BACKLOG,
- mtk_gcm_setkey_done, &data->result);
+ crypto_req_done, &data->wait);
skcipher_request_set_crypt(&data->req, data->sg, data->sg,
AES_BLOCK_SIZE, data->iv);
- err = crypto_skcipher_encrypt(&data->req);
- if (err == -EINPROGRESS || err == -EBUSY) {
- err = wait_for_completion_interruptible(
- &data->result.completion);
- if (!err)
- err = data->result.err;
- }
+ err = crypto_wait_req(crypto_skcipher_encrypt(&data->req),
+ &data->wait);
if (err)
goto out;
--
2.1.4
^ permalink raw reply related
* [PATCH v4 18/19] crypto: mediatek: move to generic async completion
From: Gilad Ben-Yossef @ 2017-08-08 12:03 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, Shaohua Li, Steve French,
Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
keyrings-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel
Cc: Ofir Drang
In-Reply-To: <1502193834-11289-1-git-send-email-gilad-6S/DczAoZh3WXxRugSxzZg@public.gmane.org>
The mediatek driver starts several async crypto ops and waits for their
completions. Move it over to generic code doing the same.
Signed-off-by: Gilad Ben-Yossef <gilad-6S/DczAoZh3WXxRugSxzZg@public.gmane.org>
---
drivers/crypto/mediatek/mtk-aes.c | 31 +++++--------------------------
1 file changed, 5 insertions(+), 26 deletions(-)
diff --git a/drivers/crypto/mediatek/mtk-aes.c b/drivers/crypto/mediatek/mtk-aes.c
index 9e845e8..e2c7c95 100644
--- a/drivers/crypto/mediatek/mtk-aes.c
+++ b/drivers/crypto/mediatek/mtk-aes.c
@@ -137,11 +137,6 @@ struct mtk_aes_gcm_ctx {
struct crypto_skcipher *ctr;
};
-struct mtk_aes_gcm_setkey_result {
- int err;
- struct completion completion;
-};
-
struct mtk_aes_drv {
struct list_head dev_list;
/* Device list lock */
@@ -936,17 +931,6 @@ static int mtk_aes_gcm_crypt(struct aead_request *req, u64 mode)
&req->base);
}
-static void mtk_gcm_setkey_done(struct crypto_async_request *req, int err)
-{
- struct mtk_aes_gcm_setkey_result *result = req->data;
-
- if (err == -EINPROGRESS)
- return;
-
- result->err = err;
- complete(&result->completion);
-}
-
/*
* Because of the hardware limitation, we need to pre-calculate key(H)
* for the GHASH operation. The result of the encryption operation
@@ -962,7 +946,7 @@ static int mtk_aes_gcm_setkey(struct crypto_aead *aead, const u8 *key,
u32 hash[4];
u8 iv[8];
- struct mtk_aes_gcm_setkey_result result;
+ struct crypto_wait wait;
struct scatterlist sg[1];
struct skcipher_request req;
@@ -1002,22 +986,17 @@ static int mtk_aes_gcm_setkey(struct crypto_aead *aead, const u8 *key,
if (!data)
return -ENOMEM;
- init_completion(&data->result.completion);
+ crypto_init_wait(&data->wait);
sg_init_one(data->sg, &data->hash, AES_BLOCK_SIZE);
skcipher_request_set_tfm(&data->req, ctr);
skcipher_request_set_callback(&data->req, CRYPTO_TFM_REQ_MAY_SLEEP |
CRYPTO_TFM_REQ_MAY_BACKLOG,
- mtk_gcm_setkey_done, &data->result);
+ crypto_req_done, &data->wait);
skcipher_request_set_crypt(&data->req, data->sg, data->sg,
AES_BLOCK_SIZE, data->iv);
- err = crypto_skcipher_encrypt(&data->req);
- if (err == -EINPROGRESS || err == -EBUSY) {
- err = wait_for_completion_interruptible(
- &data->result.completion);
- if (!err)
- err = data->result.err;
- }
+ err = crypto_wait_req(crypto_skcipher_encrypt(&data->req),
+ &data->wait);
if (err)
goto out;
--
2.1.4
^ permalink raw reply related
* [PATCH v4 18/19] crypto: mediatek: move to generic async completion
From: Gilad Ben-Yossef @ 2017-08-08 12:03 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Shaohua Li, Steve French, Theodore Y. Ts'o, Jaegeuk Kim,
Mimi Zohar, Dmitry Kasatkin, James Morris, Serge E. Hallyn,
linux-crypto
Cc: Ofir Drang
In-Reply-To: <1502193834-11289-1-git-send-email-gilad@benyossef.com>
The mediatek driver starts several async crypto ops and waits for their
completions. Move it over to generic code doing the same.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
drivers/crypto/mediatek/mtk-aes.c | 31 +++++--------------------------
1 file changed, 5 insertions(+), 26 deletions(-)
diff --git a/drivers/crypto/mediatek/mtk-aes.c b/drivers/crypto/mediatek/mtk-aes.c
index 9e845e8..e2c7c95 100644
--- a/drivers/crypto/mediatek/mtk-aes.c
+++ b/drivers/crypto/mediatek/mtk-aes.c
@@ -137,11 +137,6 @@ struct mtk_aes_gcm_ctx {
struct crypto_skcipher *ctr;
};
-struct mtk_aes_gcm_setkey_result {
- int err;
- struct completion completion;
-};
-
struct mtk_aes_drv {
struct list_head dev_list;
/* Device list lock */
@@ -936,17 +931,6 @@ static int mtk_aes_gcm_crypt(struct aead_request *req, u64 mode)
&req->base);
}
-static void mtk_gcm_setkey_done(struct crypto_async_request *req, int err)
-{
- struct mtk_aes_gcm_setkey_result *result = req->data;
-
- if (err == -EINPROGRESS)
- return;
-
- result->err = err;
- complete(&result->completion);
-}
-
/*
* Because of the hardware limitation, we need to pre-calculate key(H)
* for the GHASH operation. The result of the encryption operation
@@ -962,7 +946,7 @@ static int mtk_aes_gcm_setkey(struct crypto_aead *aead, const u8 *key,
u32 hash[4];
u8 iv[8];
- struct mtk_aes_gcm_setkey_result result;
+ struct crypto_wait wait;
struct scatterlist sg[1];
struct skcipher_request req;
@@ -1002,22 +986,17 @@ static int mtk_aes_gcm_setkey(struct crypto_aead *aead, const u8 *key,
if (!data)
return -ENOMEM;
- init_completion(&data->result.completion);
+ crypto_init_wait(&data->wait);
sg_init_one(data->sg, &data->hash, AES_BLOCK_SIZE);
skcipher_request_set_tfm(&data->req, ctr);
skcipher_request_set_callback(&data->req, CRYPTO_TFM_REQ_MAY_SLEEP |
CRYPTO_TFM_REQ_MAY_BACKLOG,
- mtk_gcm_setkey_done, &data->result);
+ crypto_req_done, &data->wait);
skcipher_request_set_crypt(&data->req, data->sg, data->sg,
AES_BLOCK_SIZE, data->iv);
- err = crypto_skcipher_encrypt(&data->req);
- if (err == -EINPROGRESS || err == -EBUSY) {
- err = wait_for_completion_interruptible(
- &data->result.completion);
- if (!err)
- err = data->result.err;
- }
+ err = crypto_wait_req(crypto_skcipher_encrypt(&data->req),
+ &data->wait);
if (err)
goto out;
--
2.1.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox