Linux RAID subsystem development
 help / color / mirror / Atom feed
* [PATCH 02/12] raid5-cache: free I/O units earlier
From: Christoph Hellwig @ 2015-09-12  6:17 UTC (permalink / raw)
  To: Shaohua Li, neilb; +Cc: linux-raid, Kernel-team, dan.j.williams
In-Reply-To: <1442038638-6947-1-git-send-email-hch@lst.de>

There is no good reason to keep the I/O unit structures around after the
stripe has been written back to the RAID array.  The only information
we need is the log sequence number, and the checkpoint offset of the
highest successfull writeback.  Store those in the log structure, and
free the IO units from __r5l_stripe_write_finished.

Besides simplifying the code this also avoid having to keep the allocation
for the I/O unit around for a potentially long time as superblock updates
that checkpoint the log do not happen very often.

This also fixes the previously incorrect calculation of 'free' in
r5l_do_reclaim as a side effect: previous if took the last unit which
isn't checkpointed into account.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/raid5-cache.c | 145 ++++++++++++++++++-----------------------------
 1 file changed, 55 insertions(+), 90 deletions(-)

diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index c345479..803bcc6 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -51,6 +51,9 @@ struct r5l_log {
 	sector_t log_start; /* log head. where new data appends */
 	u64 seq; /* log head sequence */
 
+	sector_t next_checkpoint;
+	u64 next_cp_seq;
+
 	struct mutex io_mutex;
 	struct r5l_io_unit *current_io; /* current io_unit accepting new data */
 
@@ -65,10 +68,6 @@ struct r5l_log {
 					* cache flush */
 	struct list_head flushed_ios; /* io_units which settle down in log disk */
 	struct bio flush_bio;
-	struct list_head stripe_end_ios; /* io_units which have been
-					  * completely written to the RAID *
-					  * but have not yet been considered *
-					  * for updating super */
 
 	struct kmem_cache *io_kc;
 
@@ -185,35 +184,6 @@ static void r5l_move_io_unit_list(struct list_head *from, struct list_head *to,
        }
 }
 
-/*
- * We don't want too many io_units reside in stripe_end_ios list, which will
- * waste a lot of memory. So we try to remove some. But we must keep at least 2
- * io_units. The superblock must point to a valid meta, if it's the last meta,
- * recovery can scan less
- * */
-static void r5l_compress_stripe_end_list(struct r5l_log *log)
-{
-	struct r5l_io_unit *first, *last, *io;
-
-	first = list_first_entry(&log->stripe_end_ios,
-		struct r5l_io_unit, log_sibling);
-	last = list_last_entry(&log->stripe_end_ios,
-		struct r5l_io_unit, log_sibling);
-	if (first == last)
-		return;
-	list_del(&first->log_sibling);
-	list_del(&last->log_sibling);
-	while (!list_empty(&log->stripe_end_ios)) {
-		io = list_first_entry(&log->stripe_end_ios,
-			struct r5l_io_unit, log_sibling);
-		list_del(&io->log_sibling);
-		first->log_end = io->log_end;
-		r5l_free_io_unit(log, io);
-	}
-	list_add_tail(&first->log_sibling, &log->stripe_end_ios);
-	list_add_tail(&last->log_sibling, &log->stripe_end_ios);
-}
-
 static void __r5l_set_io_unit_state(struct r5l_io_unit *io,
 	enum r5l_io_unit_state state)
 {
@@ -540,31 +510,53 @@ static void r5l_run_no_space_stripes(struct r5l_log *log)
 	spin_unlock(&log->no_space_stripes_lock);
 }
 
+static sector_t r5l_reclaimable_space(struct r5l_log *log)
+{
+	return r5l_ring_distance(log, log->last_checkpoint,
+				 log->next_checkpoint);
+}
+
+static bool r5l_complete_flushed_ios(struct r5l_log *log)
+{
+	struct r5l_io_unit *io, *next;
+	bool found = false;
+
+	assert_spin_locked(&log->io_list_lock);
+
+	list_for_each_entry_safe(io, next, &log->flushed_ios, log_sibling) {
+		/* don't change list order */
+		if (io->state < IO_UNIT_STRIPE_END)
+			break;
+
+		log->next_checkpoint = io->log_start;
+		log->next_cp_seq = io->seq;
+
+		list_del(&io->log_sibling);
+		r5l_free_io_unit(log, io);
+
+		found = true;
+	}
+	
+
+	return found;
+}
+
 static void __r5l_stripe_write_finished(struct r5l_io_unit *io)
 {
 	struct r5l_log *log = io->log;
-	struct r5l_io_unit *last;
-	sector_t reclaimable_space;
 	unsigned long flags;
 
 	spin_lock_irqsave(&log->io_list_lock, flags);
 	__r5l_set_io_unit_state(io, IO_UNIT_STRIPE_END);
-	/* might move 0 entry */
-	r5l_move_io_unit_list(&log->flushed_ios, &log->stripe_end_ios,
-		IO_UNIT_STRIPE_END);
-	if (list_empty(&log->stripe_end_ios)) {
+
+	if (!r5l_complete_flushed_ios(log)) {
 		spin_unlock_irqrestore(&log->io_list_lock, flags);
 		return;
 	}
 
-	last = list_last_entry(&log->stripe_end_ios,
-			struct r5l_io_unit, log_sibling);
-	reclaimable_space = r5l_ring_distance(log, log->last_checkpoint,
-				last->log_end);
-	if (reclaimable_space >= log->max_free_space)
+	if (r5l_reclaimable_space(log) > log->max_free_space)
 		r5l_wake_reclaim(log, 0);
 
-	r5l_compress_stripe_end_list(log);
 	spin_unlock_irqrestore(&log->io_list_lock, flags);
 	wake_up(&log->iounit_wait);
 }
@@ -640,13 +632,6 @@ void r5l_flush_stripe_to_raid(struct r5l_log *log)
 	submit_bio(WRITE_FLUSH, &log->flush_bio);
 }
 
-static void r5l_kick_io_unit(struct r5l_log *log)
-{
-	md_wakeup_thread(log->rdev->mddev->thread);
-	wait_event_lock_irq(log->iounit_wait, !list_empty(&log->stripe_end_ios),
-		log->io_list_lock);
-}
-
 static void r5l_write_super(struct r5l_log *log, sector_t cp);
 static void r5l_write_super_and_discard_space(struct r5l_log *log,
 	sector_t end)
@@ -678,10 +663,10 @@ static void r5l_write_super_and_discard_space(struct r5l_log *log,
 
 static void r5l_do_reclaim(struct r5l_log *log)
 {
-	struct r5l_io_unit *io, *last;
-	LIST_HEAD(list);
-	sector_t free = 0;
 	sector_t reclaim_target = xchg(&log->reclaim_target, 0);
+	sector_t reclaimable;
+	sector_t next_checkpoint;
+	u64 next_cp_seq;
 
 	spin_lock_irq(&log->io_list_lock);
 	/*
@@ -690,60 +675,41 @@ static void r5l_do_reclaim(struct r5l_log *log)
 	 * shouldn't reuse space of an unreclaimable io_unit
 	 * */
 	while (1) {
-		struct list_head *target_list = NULL;
-
-		while (!list_empty(&log->stripe_end_ios)) {
-			io = list_first_entry(&log->stripe_end_ios,
-				struct r5l_io_unit, log_sibling);
-			list_move_tail(&io->log_sibling, &list);
-			free += r5l_ring_distance(log, io->log_start,
-						io->log_end);
-		}
-
-		if (free >= reclaim_target ||
+		reclaimable = r5l_reclaimable_space(log);
+		if (reclaimable >= reclaim_target ||
 		    (list_empty(&log->running_ios) &&
 		     list_empty(&log->io_end_ios) &&
 		     list_empty(&log->flushing_ios) &&
 		     list_empty(&log->flushed_ios)))
 			break;
 
-		/* Below waiting mostly happens when we shutdown the raid */
-		if (!list_empty(&log->flushed_ios))
-			target_list = &log->flushed_ios;
-		else if (!list_empty(&log->flushing_ios))
-			target_list = &log->flushing_ios;
-		else if (!list_empty(&log->io_end_ios))
-			target_list = &log->io_end_ios;
-		else if (!list_empty(&log->running_ios))
-			target_list = &log->running_ios;
-
-		r5l_kick_io_unit(log);
+		md_wakeup_thread(log->rdev->mddev->thread);
+		wait_event_lock_irq(log->iounit_wait,
+				    r5l_reclaimable_space(log) > reclaimable,
+				    log->io_list_lock);
 	}
+
+	next_checkpoint = log->next_checkpoint;
+	next_cp_seq = log->next_cp_seq;
 	spin_unlock_irq(&log->io_list_lock);
 
-	if (list_empty(&list))
+	BUG_ON(reclaimable < 0);
+	if (reclaimable == 0)
 		return;
 
-	/* super always point to last valid meta */
-	last = list_last_entry(&list, struct r5l_io_unit, log_sibling);
 	/*
 	 * write_super will flush cache of each raid disk. We must write super
 	 * here, because the log area might be reused soon and we don't want to
 	 * confuse recovery
 	 * */
-	r5l_write_super_and_discard_space(log, last->log_start);
+	r5l_write_super_and_discard_space(log, next_checkpoint);
 
 	mutex_lock(&log->io_mutex);
-	log->last_checkpoint = last->log_start;
-	log->last_cp_seq = last->seq;
+	log->last_checkpoint = next_checkpoint;
+	log->last_cp_seq = next_cp_seq;
 	mutex_unlock(&log->io_mutex);
-	r5l_run_no_space_stripes(log);
 
-	while (!list_empty(&list)) {
-		io = list_first_entry(&list, struct r5l_io_unit, log_sibling);
-		list_del(&io->log_sibling);
-		r5l_free_io_unit(log, io);
-	}
+	r5l_run_no_space_stripes(log);
 }
 
 static void r5l_reclaim_thread(struct md_thread *thread)
@@ -1105,7 +1071,6 @@ int r5l_init_log(struct r5conf *conf, struct md_rdev *rdev)
 	spin_lock_init(&log->io_list_lock);
 	INIT_LIST_HEAD(&log->running_ios);
 	INIT_LIST_HEAD(&log->io_end_ios);
-	INIT_LIST_HEAD(&log->stripe_end_ios);
 	INIT_LIST_HEAD(&log->flushing_ios);
 	INIT_LIST_HEAD(&log->flushed_ios);
 	bio_init(&log->flush_bio);
-- 
1.9.1


^ permalink raw reply related

* [PATCH 01/12] raid5-cache: port to 4.3-rc
From: Christoph Hellwig @ 2015-09-12  6:17 UTC (permalink / raw)
  To: Shaohua Li, neilb; +Cc: linux-raid, Kernel-team, dan.j.williams
In-Reply-To: <1442038638-6947-1-git-send-email-hch@lst.de>

Port for changes in the block layer:  bio endio callers don't get passed
a separate error, and bio_get_nr_vecs is gone.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/raid5-cache.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 785749b1..c345479 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -222,7 +222,8 @@ static void __r5l_set_io_unit_state(struct r5l_io_unit *io,
 	io->state = state;
 }
 
-static inline void r5l_log_endio(struct bio *bio, int error)
+/* XXX: totally ignores I/O errors */
+static void r5l_log_endio(struct bio *bio)
 {
 	struct r5l_io_unit *io = bio->bi_private;
 	struct r5l_log *log = io->log;
@@ -288,8 +289,7 @@ static struct r5l_io_unit *r5l_new_meta(struct r5l_log *log)
 	io->meta_offset = sizeof(struct r5l_meta_block);
 	io->seq = log->seq;
 
-	bio = bio_kmalloc(GFP_NOIO | __GFP_NOFAIL,
-		bio_get_nr_vecs(log->rdev->bdev));
+	bio = bio_kmalloc(GFP_NOIO | __GFP_NOFAIL, BIO_MAX_PAGES);
 	io->current_bio = bio;
 	bio->bi_rw = WRITE;
 	bio->bi_bdev = log->rdev->bdev;
@@ -358,8 +358,7 @@ static void r5l_append_payload_page(struct r5l_log *log, struct page *page)
 alloc_bio:
 	if (!io->current_bio) {
 		struct bio *bio;
-		bio = bio_kmalloc(GFP_NOIO | __GFP_NOFAIL,
-			bio_get_nr_vecs(log->rdev->bdev));
+		bio = bio_kmalloc(GFP_NOIO | __GFP_NOFAIL, BIO_MAX_PAGES);
 		bio->bi_rw = WRITE;
 		bio->bi_bdev = log->rdev->bdev;
 		bio->bi_iter.bi_sector = log->log_start;
@@ -518,7 +517,7 @@ int r5l_handle_flush_request(struct r5l_log *log, struct bio *bio)
 	 * don't need to flush again
 	 * */
 	if (bio->bi_iter.bi_size == 0) {
-		bio_endio(bio, 0);
+		bio_endio(bio);
 		return 0;
 	}
 	bio->bi_rw &= ~REQ_FLUSH;
@@ -581,7 +580,7 @@ void r5l_stripe_write_finished(struct stripe_head *sh)
 		__r5l_stripe_write_finished(io);
 }
 
-static void r5l_log_flush_endio(struct bio *bio, int error)
+static void r5l_log_flush_endio(struct bio *bio)
 {
 	struct r5l_log *log = container_of(bio, struct r5l_log,
 		flush_bio);
-- 
1.9.1


^ permalink raw reply related

* raid5-cache I/O path improvements V2
From: Christoph Hellwig @ 2015-09-12  6:17 UTC (permalink / raw)
  To: Shaohua Li, neilb; +Cc: linux-raid, Kernel-team, dan.j.williams

Hi Shaohua, hi Neil,

this series contains a few updates to the raid5-cache feature.

The first patch just ports it to the post-4.2 block layer.  As part of that
I noticed that it currently doesn't handle I/O errors - fixes for that will
follow.

The second and third patch simplify the I/O unit state machine and reduce
latency and memory usage for the I/O units.  The remainder are just a couple
of cleanups in this area that I stumbled upon.

Changes since V1:
 - only use REQ_FUA if supported natively by the log device


^ permalink raw reply

* Re: raid1 resync stuck
From: Jes Sorensen @ 2015-09-11 17:44 UTC (permalink / raw)
  To: NeilBrown; +Cc: majianpeng, linux-raid, nate.dailey
In-Reply-To: <wrfjmvwvz9ke.fsf@redhat.com>

Jes Sorensen <Jes.Sorensen@redhat.com> writes:
> Neil,
>
> We're chasing a case where the raid1 code gets stuck during resync. Nate
> is able to reproduce it much more reliably than me - so attaching his
> reproducing script. Basically run it on an existing raid1 with internal
> bitmap on rotating disk.
>
> Nate was able to bisect it to 79ef3a8aa1cb1523cc231c9a90a278333c21f761,
> the original iobarrier rewrite patch, and it can be reproduced in
> current Linus' top of trunk a794b4f3292160bb3fd0f1f90ec8df454e3b17b3.
>
> In Nate's analysis it hangs in raise_barrier():
>
> static void raise_barrier(struct r1conf *conf, sector_t sector_nr)
> {
> 	spin_lock_irq(&conf->resync_lock);
>
> 	/* Wait until no block IO is waiting */
> 	wait_event_lock_irq(conf->wait_barrier, !conf->nr_waiting,
> 			    conf->resync_lock);
>
> 	/* block any new IO from starting */
> 	conf->barrier++;
> 	conf->next_resync = sector_nr;
>
> 	/* For these conditions we must wait:
> 	 * A: while the array is in frozen state
> 	 * B: while barrier >= RESYNC_DEPTH, meaning resync reach
> 	 *    the max count which allowed.
> 	 * C: next_resync + RESYNC_SECTORS > start_next_window, meaning
> 	 *    next resync will reach to the window which normal bios are
> 	 *    handling.
> 	 * D: while there are any active requests in the current window.
> 	 */
> 	wait_event_lock_irq(conf->wait_barrier,
> 			    !conf->array_frozen &&
> 			    conf->barrier < RESYNC_DEPTH &&
> 			    conf->current_window_requests == 0 &&
> 			    (conf->start_next_window >=
> 			     conf->next_resync + RESYNC_SECTORS),
> 			    conf->resync_lock);
>
> crash> r1conf 0xffff882028f3e600 | grep -e array_frozen -e barrier -e start_next_window -e next_resync
>   barrier = 0x1,                      (conf->barrier < RESYNC_DEPTH)
>   array_frozen = 0x0,                 (!conf->array_frozen)
>   next_resync = 0x3000,
>   start_next_window = 0x3000,
>
> ie. next_resync == start_next_window, which will never wake up since
> start_next_window is smaller than next_resync + RESYNC_SECTORS.
>
> Have you seen anything like this?

Looking further at this together with Nate. It looks like you had a
patch resolving something similar:

commit 669cc7ba77864e7b1ac39c9f2b2afb8730f341f4
Author: NeilBrown <neilb@suse.de>
Date:   Thu Sep 4 16:30:38 2014 +1000

    md/raid1: clean up request counts properly in close_sync()
    
    If there are outstanding writes when close_sync is called,
    the change to ->start_next_window might cause them to
    decrement the wrong counter when they complete.  Fix this
    by merging the two counters into the one that will be decremented.
    
    Having an incorrect value in a counter can cause raise_barrier()
    to hangs, so this is suitable for -stable.
    
    Fixes: 79ef3a8aa1cb1523cc231c9a90a278333c21f761
    cc: stable@vger.kernel.org (v3.13+)
    Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index ad0468c..a31c92b 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1545,8 +1545,13 @@ static void close_sync(struct r1conf *conf)
        mempool_destroy(conf->r1buf_pool);
        conf->r1buf_pool = NULL;
 
+       spin_lock_irq(&conf->resync_lock);
        conf->next_resync = 0;
        conf->start_next_window = MaxSector;
+       conf->current_window_requests +=
+               conf->next_window_requests;
+       conf->next_window_requests = 0;
+       spin_unlock_irq(&conf->resync_lock);
 }
 
It looks to us like close_sync()'s conf->start_next_window = MaxSector
results in wait_barrier() triggering this when the outstanding IO
completes:

	if (bio && bio_data_dir(bio) == WRITE) {
		if (bio->bi_sector >=
		    conf->mddev->curr_resync_completed) {
			if (conf->start_next_window == MaxSector)
				conf->start_next_window =
					conf->next_resync +
					NEXT_NORMALIO_DISTANCE;

putting us into the situation where raise_barrier()'s condition never
completes:

	wait_event_lock_irq(conf->wait_barrier,
			    !conf->array_frozen &&
			    conf->barrier < RESYNC_DEPTH &&
			    conf->current_window_requests == 0 &&
			    (conf->start_next_window >=
			     conf->next_resync + RESYNC_SECTORS),
			    conf->resync_lock);

So the question is, is it wrong for close_sync() to be setting
conf->start_next_window = MaxSector in the first place, or should it
only be doing this once all outstanding I/O has completed?

Nate tested a case where removing the MaxSector assignment from
close_sync() but are there any side effects to doing that?

Cheers,
Jes

^ permalink raw reply related

* Re: raid role number off on one array?
From: David C. Rankin @ 2015-09-11 15:21 UTC (permalink / raw)
  To: mdraid
In-Reply-To: <55F2DD22.3030406@turmel.org>

On 09/11/2015 08:54 AM, Phil Turmel wrote:
> /proc/mdstat doesn't show role numbers.  It shows slot numbers.  Slot
> numbers are the indices into the role number tracking tables in the
> superblock.  They happen to match role numbers on a freshly-created
> array simply because the slots are allocated starting from zero, just
> like the roles.  As you add and remove devices, the slot numbers and
> role numbers may no longer match.

Ah hah!

   That was the piece of the puzzle I was missing. Thanks you. I guess I'm just 
a bit overly concerned about making sure I understand everything I'm seeing with 
my arrays. After having used mdadm for roughly a decade without a single loss, 
then having that little "no attempt to activate sda7 Oops", I just want to do 
what I can to avoid a repeat :-)

-- 
David C. Rankin, J.D.,P.E.

^ permalink raw reply

* Re: raid role number off on one array?
From: Phil Turmel @ 2015-09-11 13:54 UTC (permalink / raw)
  To: David C. Rankin, mdraid
In-Reply-To: <55F249DB.5010908@suddenlinkmail.com>

Hi David,

On 09/10/2015 11:26 PM, David C. Rankin wrote:

[trim /]

> md1 : active raid1 sda7[2] sdb7[1]
> 
>   My concern is why is sda7 shown as being in role [2] and sdb7 shown in
> [1]? All other arrays are [0][1]. What concerns me is the information at:

/proc/mdstat doesn't show role numbers.  It shows slot numbers.  Slot
numbers are the indices into the role number tracking tables in the
superblock.  They happen to match role numbers on a freshly-created
array simply because the slots are allocated starting from zero, just
like the roles.  As you add and remove devices, the slot numbers and
role numbers may no longer match.

Phil


^ permalink raw reply

* raid role number off on one array?
From: David C. Rankin @ 2015-09-11  3:26 UTC (permalink / raw)
  To: mdraid

All,

   This is a follow-up to "Re-add of raid1 drive resulted in strange loss of 
data on Archlinux?" posted 9/1. As a brief background there, running the 4.1.6 
kernel, for some reason on boot, the system never made an attempt to activate 
sda7, part of my raid1 array of the root filesystem. Updates were done before 
the degraded array was discovered. Upon re-add of sda7 to the array, re-sync 
appeared to have completed fine, but on next reboot, the system crashed due to 
some 440 0-byte files in /lib and (and in the kernel module tree). It was as if 
the re-sync corrupted all files in the filesystem that had been updated while 
the array was in degraded mode, rather than properly copying them from sdb7 to 
sda7 to restore the array. I still have no idea how that could occur.

   Now checking my arrays with mdstat I find:

# cat /proc/mdstat
Personalities : [raid1]
md1 : active raid1 sda7[2] sdb7[1]
       52396032 blocks super 1.2 [2/2] [UU]

md3 : active raid1 sda6[0] sdb6[1]
       1047552 blocks super 1.2 [2/2] [UU]

md0 : active raid1 sdb5[1] sda5[0]
       204608 blocks super 1.2 [2/2] [UU]

md2 : active raid1 sdb8[1] sda8[0]
       922944192 blocks super 1.2 [2/2] [UU]
       bitmap: 0/7 pages [0KB], 65536KB chunk

unused devices: <none>

   What has me worried is the "raid role numbers" following md1, e.g.:

md1 : active raid1 sda7[2] sdb7[1]

   My concern is why is sda7 shown as being in role [2] and sdb7 shown in [1]? 
All other arrays are [0][1]. What concerns me is the information at:

http://tldp.org/HOWTO/Software-RAID-HOWTO-6.html

   Specifically discussing an array with n-devices (2 in my case), the howto states:

"Any device with "n" or higher are spare disks. 0,1,..,n-1 are for the working 
array."

   Huh? A spare?

   Checking the detail shows everything is OK, but not knowing more about what 
the significance of the role number, or what it is saying in my case (in light 
of the tldp quote, I thought I would check here to make sure there isn't 
something going on with this array I should be concerned with. Here is the detail:

# mdadm -D /dev/md1
/dev/md1:
         Version : 1.2
   Creation Time : Wed Nov 27 04:35:49 2013
      Raid Level : raid1
      Array Size : 52396032 (49.97 GiB 53.65 GB)
   Used Dev Size : 52396032 (49.97 GiB 53.65 GB)
    Raid Devices : 2
   Total Devices : 2
     Persistence : Superblock is persistent

     Update Time : Thu Sep 10 21:33:11 2015
           State : clean
  Active Devices : 2
Working Devices : 2
  Failed Devices : 0
   Spare Devices : 0

            Name : archiso:1
            UUID : 320d86f7:22999af5:5eeefee1:35cd8970
          Events : 103373

     Number   Major   Minor   RaidDevice State
        2       8        7        0      active sync   /dev/sda7
        1       8       23        1      active sync   /dev/sdb7

   How can sda77 be RaidDevice '0' but Number '2' in the array? This is running on:

Linux phoinix 4.1.6-1-ARCH #1 SMP PREEMPT Mon Aug 17 08:52:28 CEST 2015 x86_64 
GNU/Linux

   What say the experts?


-- 
David C. Rankin, J.D.,P.E.

^ permalink raw reply

* [PATCH] Remove unused argument in md_new_event()
From: Goldwyn Rodrigues @ 2015-09-10 12:41 UTC (permalink / raw)
  To: linux-raid

Code cleanup:

  - Removed md_new_event_inintr() because md_new_event() and
    md_new_event_inintr() are the same.

  - Removed mddev argument passed to md_new_event() because it is
    not used.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 drivers/md/md.c     | 37 ++++++++++++++-----------------------
 drivers/md/md.h     |  2 +-
 drivers/md/raid10.c |  2 +-
 drivers/md/raid5.c  |  2 +-
 4 files changed, 17 insertions(+), 26 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 4f5ecbe..c4b2c1e 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -198,22 +198,13 @@ EXPORT_SYMBOL_GPL(bio_clone_mddev);
  */
 static DECLARE_WAIT_QUEUE_HEAD(md_event_waiters);
 static atomic_t md_event_count;
-void md_new_event(struct mddev *mddev)
+void md_new_event()
 {
 	atomic_inc(&md_event_count);
 	wake_up(&md_event_waiters);
 }
 EXPORT_SYMBOL_GPL(md_new_event);
 
-/* Alternate version that can be called from interrupts
- * when calling sysfs_notify isn't needed.
- */
-static void md_new_event_inintr(struct mddev *mddev)
-{
-	atomic_inc(&md_event_count);
-	wake_up(&md_event_waiters);
-}
-
 /*
  * Enables to iterate over all existing md arrays
  * all_mddevs_lock protects this list.
@@ -2384,7 +2375,7 @@ static int add_bound_rdev(struct md_rdev *rdev)
 	if (mddev->degraded)
 		set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
 	set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
-	md_new_event(mddev);
+	md_new_event();
 	md_wakeup_thread(mddev->thread);
 	return 0;
 }
@@ -2497,7 +2488,7 @@ state_store(struct md_rdev *rdev, const char *buf, size_t len)
 				md_cluster_ops->metadata_update_start(mddev);
 			if (mddev->pers)
 				md_update_sb(mddev, 1);
-			md_new_event(mddev);
+			md_new_event();
 			if (mddev_is_clustered(mddev))
 				md_cluster_ops->metadata_update_finish(mddev);
 			err = 0;
@@ -3502,7 +3493,7 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
 	if (!mddev->thread)
 		md_update_sb(mddev, 1);
 	sysfs_notify(&mddev->kobj, NULL, "level");
-	md_new_event(mddev);
+	md_new_event();
 	rv = len;
 out_unlock:
 	mddev_unlock(mddev);
@@ -5205,7 +5196,7 @@ int md_run(struct mddev *mddev)
 	if (mddev->flags & MD_UPDATE_SB_FLAGS)
 		md_update_sb(mddev, 0);
 
-	md_new_event(mddev);
+	md_new_event();
 	sysfs_notify_dirent_safe(mddev->sysfs_state);
 	sysfs_notify_dirent_safe(mddev->sysfs_action);
 	sysfs_notify(&mddev->kobj, NULL, "degraded");
@@ -5539,7 +5530,7 @@ static int do_md_stop(struct mddev *mddev, int mode,
 			mddev->hold_active = 0;
 	}
 	blk_integrity_unregister(disk);
-	md_new_event(mddev);
+	md_new_event();
 	sysfs_notify_dirent_safe(mddev->sysfs_state);
 	return 0;
 }
@@ -6008,7 +5999,7 @@ static int hot_remove_disk(struct mddev *mddev, dev_t dev)
 
 	md_kick_rdev_from_array(rdev);
 	md_update_sb(mddev, 1);
-	md_new_event(mddev);
+	md_new_event();
 
 	if (mddev_is_clustered(mddev))
 		md_cluster_ops->metadata_update_finish(mddev);
@@ -6093,7 +6084,7 @@ static int hot_add_disk(struct mddev *mddev, dev_t dev)
 	 */
 	set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
 	md_wakeup_thread(mddev->thread);
-	md_new_event(mddev);
+	md_new_event();
 	return 0;
 
 abort_clustered:
@@ -7050,7 +7041,7 @@ void md_error(struct mddev *mddev, struct md_rdev *rdev)
 	md_wakeup_thread(mddev->thread);
 	if (mddev->event_work.func)
 		queue_work(md_misc_wq, &mddev->event_work);
-	md_new_event_inintr(mddev);
+	md_new_event();
 }
 EXPORT_SYMBOL(md_error);
 
@@ -7795,7 +7786,7 @@ void md_do_sync(struct md_thread *thread)
 		mddev->curr_resync = 3; /* no longer delayed */
 	mddev->curr_resync_completed = j;
 	sysfs_notify(&mddev->kobj, NULL, "sync_completed");
-	md_new_event(mddev);
+	md_new_event();
 	update_time = jiffies;
 
 	if (mddev_is_clustered(mddev))
@@ -7871,7 +7862,7 @@ void md_do_sync(struct md_thread *thread)
 			/* this is the earliest that rebuild will be
 			 * visible in /proc/mdstat
 			 */
-			md_new_event(mddev);
+			md_new_event();
 
 		if (last_check + window > io_sectors || j == max_sectors)
 			continue;
@@ -8043,7 +8034,7 @@ static int remove_and_add_spares(struct mddev *mddev,
 			if (sysfs_link_rdev(mddev, rdev))
 				/* failure here is OK */;
 			spares++;
-			md_new_event(mddev);
+			md_new_event();
 			set_bit(MD_CHANGE_DEVS, &mddev->flags);
 		}
 	}
@@ -8078,7 +8069,7 @@ static void md_start_sync(struct work_struct *ws)
 	} else
 		md_wakeup_thread(mddev->sync_thread);
 	sysfs_notify_dirent_safe(mddev->sysfs_action);
-	md_new_event(mddev);
+	md_new_event();
 }
 
 /*
@@ -8311,7 +8302,7 @@ void md_reap_sync_thread(struct mddev *mddev)
 	/* flag recovery needed just to double check */
 	set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
 	sysfs_notify_dirent_safe(mddev->sysfs_action);
-	md_new_event(mddev);
+	md_new_event();
 	if (mddev->event_work.func)
 		queue_work(md_misc_wq, &mddev->event_work);
 }
diff --git a/drivers/md/md.h b/drivers/md/md.h
index ab33957..c8f2519c 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -634,7 +634,7 @@ extern void md_super_wait(struct mddev *mddev);
 extern int sync_page_io(struct md_rdev *rdev, sector_t sector, int size,
 			struct page *page, int rw, bool metadata_op);
 extern void md_do_sync(struct md_thread *thread);
-extern void md_new_event(struct mddev *mddev);
+extern void md_new_event(void);
 extern int md_allow_write(struct mddev *mddev);
 extern void md_wait_for_blocked_rdev(struct md_rdev *rdev, struct mddev *mddev);
 extern void md_set_array_sectors(struct mddev *mddev, sector_t array_sectors);
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 0fc33eb..c24fc98 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4068,7 +4068,7 @@ static int raid10_start_reshape(struct mddev *mddev)
 	}
 	conf->reshape_checkpoint = jiffies;
 	md_wakeup_thread(mddev->sync_thread);
-	md_new_event(mddev);
+	md_new_event();
 	return 0;
 
 abort:
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 15ef2c6..ea4052a 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -7420,7 +7420,7 @@ static int raid5_start_reshape(struct mddev *mddev)
 	}
 	conf->reshape_checkpoint = jiffies;
 	md_wakeup_thread(mddev->sync_thread);
-	md_new_event(mddev);
+	md_new_event();
 	return 0;
 }
 
-- 
2.1.4


^ permalink raw reply related

* Re: raid1 resync stuck
From: Jes Sorensen @ 2015-09-09 19:48 UTC (permalink / raw)
  To: NeilBrown; +Cc: majianpeng, linux-raid, nate.dailey
In-Reply-To: <wrfjmvwvz9ke.fsf@redhat.com>

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

Jes Sorensen <Jes.Sorensen@redhat.com> writes:
> Neil,
>
> We're chasing a case where the raid1 code gets stuck during resync. Nate
> is able to reproduce it much more reliably than me - so attaching his
> reproducing script. Basically run it on an existing raid1 with internal
> bitmap on rotating disk.
>
> Nate was able to bisect it to 79ef3a8aa1cb1523cc231c9a90a278333c21f761,
> the original iobarrier rewrite patch, and it can be reproduced in
> current Linus' top of trunk a794b4f3292160bb3fd0f1f90ec8df454e3b17b3.
>
> In Nate's analysis it hangs in raise_barrier():
>
> static void raise_barrier(struct r1conf *conf, sector_t sector_nr)
> {
> 	spin_lock_irq(&conf->resync_lock);
>
> 	/* Wait until no block IO is waiting */
> 	wait_event_lock_irq(conf->wait_barrier, !conf->nr_waiting,
> 			    conf->resync_lock);
>
> 	/* block any new IO from starting */
> 	conf->barrier++;
> 	conf->next_resync = sector_nr;
>
> 	/* For these conditions we must wait:
> 	 * A: while the array is in frozen state
> 	 * B: while barrier >= RESYNC_DEPTH, meaning resync reach
> 	 *    the max count which allowed.
> 	 * C: next_resync + RESYNC_SECTORS > start_next_window, meaning
> 	 *    next resync will reach to the window which normal bios are
> 	 *    handling.
> 	 * D: while there are any active requests in the current window.
> 	 */
> 	wait_event_lock_irq(conf->wait_barrier,
> 			    !conf->array_frozen &&
> 			    conf->barrier < RESYNC_DEPTH &&
> 			    conf->current_window_requests == 0 &&
> 			    (conf->start_next_window >=
> 			     conf->next_resync + RESYNC_SECTORS),
> 			    conf->resync_lock);
>
> crash> r1conf 0xffff882028f3e600 | grep -e array_frozen -e barrier -e start_next_window -e next_resync
>   barrier = 0x1,                      (conf->barrier < RESYNC_DEPTH)
>   array_frozen = 0x0,                 (!conf->array_frozen)
>   next_resync = 0x3000,
>   start_next_window = 0x3000,
>
> ie. next_resync == start_next_window, which will never wake up since
> start_next_window is smaller than next_resync + RESYNC_SECTORS.
>
> Have you seen anything like this?
>
> Cheers,
> Jes

Grrr - doing too many things in parallel :( I knew I forgot something -
here is Nate's script.

Jes

[-- Attachment #2: md_stuck_resync.sh --]
[-- Type: text/plain, Size: 1201 bytes --]

#!/bin/bash

wait_idle() {
    while [ "$(cat /sys/block/$sysmd/md/sync_action)" != "idle" ] ; do
	sleep 10s
    done
}

[ $# -ne 1 ] && echo "usage: $0 <md>" && exit 1
sysmd=$1
devmd="/dev/$(udevadm info -q name --path=/sys/block/$sysmd)"
[ ! -b "$devmd" ] && echo "failed to find md devnode" && exit 1

devsd=
for m in /sys/block/$sysmd/md/dev-*
{
    realpath=$(cd $m/block && pwd -P)
    devsd="/dev/$(udevadm info -q name --path=$realpath)"
    break
}
[ ! -b "$devsd" ] && echo "failed to find member disk devnode" && exit 1

iter() {
    echo
    echo "Remove $devsd"
    mdadm -f $devmd $devsd
    sleep 1s
    mdadm -r $devmd $devsd

    echo "Start 1GB IO"
    dd if=/dev/urandom of=$devmd bs=1M count=1024 &

    for j in 1 2 3 4 5
    {
	sleep 15s

	echo
	echo "$j: Add $devsd"
	mdadm -a $devmd $devsd
	sleep 1s

	echo
	echo "$j: Remove $devsd"
	mdadm -f $devmd $devsd
	echo "idle" > /sys/block/$sysmd/md/sync_action
	sleep 1s
	mdadm -r $devmd $devsd
    }

    echo
    echo "Add $devsd"
    mdadm -a $devmd $devsd

    echo
    echo "Wait for $sysmd recovery..."
    wait_idle
}

i=1
while [ 1 ] ; do
    echo "$(date) ********** Iteration $i **********"
    iter
    i=$(($i + 1))
done

^ permalink raw reply

* raid1 resync stuck
From: Jes Sorensen @ 2015-09-09 19:17 UTC (permalink / raw)
  To: NeilBrown; +Cc: majianpeng, linux-raid, nate.dailey

Neil,

We're chasing a case where the raid1 code gets stuck during resync. Nate
is able to reproduce it much more reliably than me - so attaching his
reproducing script. Basically run it on an existing raid1 with internal
bitmap on rotating disk.

Nate was able to bisect it to 79ef3a8aa1cb1523cc231c9a90a278333c21f761,
the original iobarrier rewrite patch, and it can be reproduced in
current Linus' top of trunk a794b4f3292160bb3fd0f1f90ec8df454e3b17b3.

In Nate's analysis it hangs in raise_barrier():

static void raise_barrier(struct r1conf *conf, sector_t sector_nr)
{
	spin_lock_irq(&conf->resync_lock);

	/* Wait until no block IO is waiting */
	wait_event_lock_irq(conf->wait_barrier, !conf->nr_waiting,
			    conf->resync_lock);

	/* block any new IO from starting */
	conf->barrier++;
	conf->next_resync = sector_nr;

	/* For these conditions we must wait:
	 * A: while the array is in frozen state
	 * B: while barrier >= RESYNC_DEPTH, meaning resync reach
	 *    the max count which allowed.
	 * C: next_resync + RESYNC_SECTORS > start_next_window, meaning
	 *    next resync will reach to the window which normal bios are
	 *    handling.
	 * D: while there are any active requests in the current window.
	 */
	wait_event_lock_irq(conf->wait_barrier,
			    !conf->array_frozen &&
			    conf->barrier < RESYNC_DEPTH &&
			    conf->current_window_requests == 0 &&
			    (conf->start_next_window >=
			     conf->next_resync + RESYNC_SECTORS),
			    conf->resync_lock);

crash> r1conf 0xffff882028f3e600 | grep -e array_frozen -e barrier -e start_next_window -e next_resync
  barrier = 0x1,                      (conf->barrier < RESYNC_DEPTH)
  array_frozen = 0x0,                 (!conf->array_frozen)
  next_resync = 0x3000,
  start_next_window = 0x3000,

ie. next_resync == start_next_window, which will never wake up since
start_next_window is smaller than next_resync + RESYNC_SECTORS.

Have you seen anything like this?

Cheers,
Jes

^ permalink raw reply

* Re: raid5-cache I/O path improvements
From: Christoph Hellwig @ 2015-09-09 15:59 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Shaohua Li, Christoph Hellwig, neilb, linux-raid, Kernel-team,
	dan.j.williams, Martin K. Petersen, linux-ide
In-Reply-To: <20150908173420.GJ13749@mtj.duckdns.org>

On Tue, Sep 08, 2015 at 01:34:20PM -0400, Tejun Heo wrote:
> Hmmm... grep tells me that dm and md actually are branching on whether
> the underlying device supports FUA.  This is tricky.  I didn't even
> mean flush_flags to be used directly by upper layers.  For rotational
> devices, doing multiple FUAs compared multiple writes followed by
> REQ_FLUSH is probably a lot worse - the head gets moved multiple times
> likely skipping over data which can be written out while traversing
> and it's not like stalling write pipeline and draining write queue has
> much impact on hard drives.

Well, that's what we'd need to do for the raid cache as well, given
the resulst that Shaohua sees.  Unless you have a good idea for another
way to handle the issue we'll need to support both behaviors there.

^ permalink raw reply

* Re: raid5-cache I/O path improvements
From: Tejun Heo @ 2015-09-08 17:34 UTC (permalink / raw)
  To: Shaohua Li
  Cc: Christoph Hellwig, neilb, linux-raid, Kernel-team, dan.j.williams,
	Martin K. Petersen, linux-ide
In-Reply-To: <20150908170736.GA423191@devbig257.prn2.facebook.com>

Hello,

On Tue, Sep 08, 2015 at 10:07:36AM -0700, Shaohua Li wrote:
> I need double confirm. But for write + flush, we aggregrate several
> writes and do a flush; for FUA, we do every meta write with FUA. So this
> is not apple to apple comparison.

Does that mean that upper layers are taking different actions
depending on whether the underlying device supports FUA?  That at
least wasn't the original model that I had on mind when implementing
the current incarnation of REQ_FLUSH and FUA.  The only difference FUA
was expected to make was optimizing out the flush after REQ_FUA and
the upper layers were expected to issue REQ_FLUSH/FUA the same whether
the device supports FUA or not.

Hmmm... grep tells me that dm and md actually are branching on whether
the underlying device supports FUA.  This is tricky.  I didn't even
mean flush_flags to be used directly by upper layers.  For rotational
devices, doing multiple FUAs compared multiple writes followed by
REQ_FLUSH is probably a lot worse - the head gets moved multiple times
likely skipping over data which can be written out while traversing
and it's not like stalling write pipeline and draining write queue has
much impact on hard drives.

Maybe it's different on SSDs.  I'm not sure about how expensive flush
itself would be given that a lot of write cost is paid asynchronously
anyway during gc but flush stalls IO pipeline and that could be very
noticeable on high iops devices.  Also, unless the implementation is
braindead FUA IOs are unlikely to be expensive on SSDs, so maybe what
we should do is making block layer hint upper layers regarding what's
likely to perform better.

But, ultimately, I don't think it'd be too difficult for high-depth
SSD devices to report write-through w/o losing any performance one way
or the other and it'd be great if we eventually can get there.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: raid5-cache I/O path improvements
From: Shaohua Li @ 2015-09-08 17:07 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Christoph Hellwig, neilb, linux-raid, Kernel-team, dan.j.williams,
	Martin K. Petersen, linux-ide
In-Reply-To: <20150908170226.GI13749@mtj.duckdns.org>

On Tue, Sep 08, 2015 at 01:02:26PM -0400, Tejun Heo wrote:
> Hello,
> 
> On Tue, Sep 08, 2015 at 09:56:22AM -0700, Shaohua Li wrote:
> > I'll benchmark on a SSD from another vendor, which supports FUA, but FUA
> > write has poor performance in my last test.
> 
> lolwut?  Was it slower than write + flush?

I need double confirm. But for write + flush, we aggregrate several
writes and do a flush; for FUA, we do every meta write with FUA. So this
is not apple to apple comparison.

Thanks,
Shaohua

^ permalink raw reply

* Re: raid5-cache I/O path improvements
From: Tejun Heo @ 2015-09-08 17:02 UTC (permalink / raw)
  To: Shaohua Li
  Cc: Christoph Hellwig, neilb, linux-raid, Kernel-team, dan.j.williams,
	Martin K. Petersen, linux-ide
In-Reply-To: <20150908165611.GA371379@devbig257.prn2.facebook.com>

Hello,

On Tue, Sep 08, 2015 at 09:56:22AM -0700, Shaohua Li wrote:
> I'll benchmark on a SSD from another vendor, which supports FUA, but FUA
> write has poor performance in my last test.

lolwut?  Was it slower than write + flush?

thanks.

-- 
tejun

^ permalink raw reply

* Re: raid5-cache I/O path improvements
From: Shaohua Li @ 2015-09-08 16:56 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: neilb, linux-raid, Kernel-team, dan.j.williams, Tejun Heo,
	Martin K. Petersen, linux-ide
In-Reply-To: <20150908061215.GA23833@lst.de>

On Tue, Sep 08, 2015 at 08:12:15AM +0200, Christoph Hellwig wrote:
> On Mon, Sep 07, 2015 at 05:28:55PM -0700, Shaohua Li wrote:
> > Hi Christoph,
> > Thanks for these work. Yes, I/O error handling is in the plan. We could
> > simplify panic (people here like this option) or report error and bypass
> > log. Any way an option is good.
> 
> I think the sensible thing in general is to fail the I/O.  Once we have
> a cache devie the assumption is that a) write holes are properly handled,
> and we b) do all kinds of optimizations based on the presensce of the
> log device like not passing through flush requests or skippign resync.
> 
> Having the cache device suddenly disappear will alwasy break a) and
> require a lot of hairy code only used in failure cases to undo the
> rest.

Failing the I/O is ok too.
 
> > For the patches, FUA write does simplify things a lot. However, I tried
> > it before, the performance is quite bad in SSD. FUA is off in SATA by
> > default, the emulation is farily slow because FLUSH request isn't NCQ
> > command. I tried to enable FUA in SATA too, FUA write is still slow in
> > the SSD I tested. Other than this one, other patches look good:
> 
> Pretty much every SSD (and modern disk driver) supports FUA.  Please
> benchmark with libata.fua=Y, as I think the simplifcation is absolutely
> worth it.  On my SSDs using it gives far lower latency for writes,
> nevermind nvmdimm where it's also essential as the flush statemchine
> increases the write latency by an order of magnitude.
> 
> Tejun, do you have any updates on libata vs FUA?  We onable it
> by default for a while in 2012, but then Jeff reverted it with a rather
> non-descriptive commit message.
> 
> Also NVMe or SAS SSDs will benefit heavily from the FUA bit.

I agree the benefit of FUA. In the system I'm testing, an Intel ssd
supports FUA, a sandisk SSD doesn't support FUA (this is the SSD we will
deploy for the log). This is AHCI with libata.fua=1. FUA isn't supported
by every SSD. If the log uses FUA by default, we will have a lot of FUA
write and performance is impacted.

I'll benchmark on a SSD from another vendor, which supports FUA, but FUA
write has poor performance in my last test.

Thanks,
Shaohua

^ permalink raw reply

* Re: raid5-cache I/O path improvements
From: Christoph Hellwig @ 2015-09-08 15:40 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Shaohua Li, neilb, linux-raid, Kernel-team, dan.j.williams,
	Martin K. Petersen, linux-ide
In-Reply-To: <20150908152546.GB13749@mtj.duckdns.org>

On Tue, Sep 08, 2015 at 11:25:46AM -0400, Tejun Heo wrote:
> IIRC, some controllers and/or controllers were choking on it and it
> didn't make any noticeable difference on rotating disks.  Maybe we can
> try again with controller white list and enabling by default on SSds.

I guess we could start with AHCI only as a good approximation for a not
too crappy controller and driver.

^ permalink raw reply

* Re: raid5-cache I/O path improvements
From: Tejun Heo @ 2015-09-08 15:26 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Shaohua Li, neilb, linux-raid, Kernel-team, dan.j.williams,
	Martin K. Petersen, linux-ide
In-Reply-To: <20150908152546.GB13749@mtj.duckdns.org>

On Tue, Sep 08, 2015 at 11:25:46AM -0400, Tejun Heo wrote:
...
> IIRC, some controllers and/or controllers were choking on it and it
                                ^
				drives

> didn't make any noticeable difference on rotating disks.  Maybe we can
> try again with controller white list and enabling by default on SSds.

-- 
tejun

^ permalink raw reply

* Re: raid5-cache I/O path improvements
From: Tejun Heo @ 2015-09-08 15:25 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Shaohua Li, neilb, linux-raid, Kernel-team, dan.j.williams,
	Martin K. Petersen, linux-ide
In-Reply-To: <20150908061215.GA23833@lst.de>

Hello, Christoph.

On Tue, Sep 08, 2015 at 08:12:15AM +0200, Christoph Hellwig wrote:
> Tejun, do you have any updates on libata vs FUA?  We onable it
> by default for a while in 2012, but then Jeff reverted it with a rather
> non-descriptive commit message.

IIRC, some controllers and/or controllers were choking on it and it
didn't make any noticeable difference on rotating disks.  Maybe we can
try again with controller white list and enabling by default on SSds.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: raid5-cache I/O path improvements
From: Christoph Hellwig @ 2015-09-08  6:12 UTC (permalink / raw)
  To: Shaohua Li
  Cc: neilb, linux-raid, Kernel-team, dan.j.williams, Tejun Heo,
	Martin K. Petersen, linux-ide
In-Reply-To: <20150908002840.GA3196542@devbig257.prn2.facebook.com>

On Mon, Sep 07, 2015 at 05:28:55PM -0700, Shaohua Li wrote:
> Hi Christoph,
> Thanks for these work. Yes, I/O error handling is in the plan. We could
> simplify panic (people here like this option) or report error and bypass
> log. Any way an option is good.

I think the sensible thing in general is to fail the I/O.  Once we have
a cache devie the assumption is that a) write holes are properly handled,
and we b) do all kinds of optimizations based on the presensce of the
log device like not passing through flush requests or skippign resync.

Having the cache device suddenly disappear will alwasy break a) and
require a lot of hairy code only used in failure cases to undo the
rest.

> For the patches, FUA write does simplify things a lot. However, I tried
> it before, the performance is quite bad in SSD. FUA is off in SATA by
> default, the emulation is farily slow because FLUSH request isn't NCQ
> command. I tried to enable FUA in SATA too, FUA write is still slow in
> the SSD I tested. Other than this one, other patches look good:

Pretty much every SSD (and modern disk driver) supports FUA.  Please
benchmark with libata.fua=Y, as I think the simplifcation is absolutely
worth it.  On my SSDs using it gives far lower latency for writes,
nevermind nvmdimm where it's also essential as the flush statemchine
increases the write latency by an order of magnitude.

Tejun, do you have any updates on libata vs FUA?  We onable it
by default for a while in 2012, but then Jeff reverted it with a rather
non-descriptive commit message.

Also NVMe or SAS SSDs will benefit heavily from the FUA bit.

^ permalink raw reply

* Re: RAID4/5/6 reshape grow stuck
From: Yi Zhang @ 2015-09-08  4:33 UTC (permalink / raw)
  To: NeilBrown; +Cc: linux-raid
In-Reply-To: <1025553941.22272591.1441628393915.JavaMail.zimbra@redhat.com>

I have test with the latest mdadm package, the issue fixed.

Thanks
Yi

Best Regards,
  Yi Zhang


----- Original Message -----
From: "Yi Zhang" <yizhan@redhat.com>
To: "NeilBrown" <neilb@suse.com>
Cc: linux-raid@vger.kernel.org
Sent: Monday, September 7, 2015 8:19:53 PM
Subject: RAID4/5/6 reshape grow stuck

Hi Neil

When testing 07revert-grow, found the RAID4/5/6 grow stuck issue, pls check below info:

Steps:
mdadm --quiet -CR --assume-clean /dev/md0 -l6 -n4 -x1 /dev/loop0 /dev/loop1 /dev/loop2 /dev/loop3 /dev/loop4
sleep 3
mdadm --wait /dev/md0
mdadm -G /dev/md0 -n 5

[root@dhcp-12-171 bug]# cat /proc/mdstat 
Personalities : [raid6] [raid5] [raid4] 
md0 : active raid6 loop4[4] loop3[3] loop2[2] loop1[1] loop0[0]
      38912 blocks super 1.2 level 6, 512k chunk, algorithm 2 [5/5] [UUUUU]
      [>....................]  reshape =  0.0% (0/19456) finish=3344.0min speed=0K/sec
      
unused devices: <none>
kernel: 4.2.0
dmesg:
[  563.248753] md: bind<loop0>
[  563.248825] md: bind<loop1>
[  563.248892] md: bind<loop2>
[  563.248954] md: bind<loop3>
[  563.251515] md: bind<loop4>
[  563.257778] md/raid:md0: device loop3 operational as raid disk 3
[  563.257784] md/raid:md0: device loop2 operational as raid disk 2
[  563.257787] md/raid:md0: device loop1 operational as raid disk 1
[  563.257789] md/raid:md0: device loop0 operational as raid disk 0
[  563.258543] md/raid:md0: allocated 4366kB
[  563.258613] md/raid:md0: raid level 6 active with 4 out of 4 devices, algorithm 2
[  563.258617] RAID conf printout:
[  563.258621]  --- level:6 rd:4 wd:4
[  563.258624]  disk 0, o:1, dev:loop0
[  563.258628]  disk 1, o:1, dev:loop1
[  563.258632]  disk 2, o:1, dev:loop2
[  563.258634]  disk 3, o:1, dev:loop3
[  563.258651] md/raid456: discard support disabled due to uncertainty.
[  563.258653] Set raid456.devices_handle_discard_safely=Y to override.
[  563.258694] md0: detected capacity change from 0 to 39845888
[  563.258814] RAID conf printout:
[  563.258821]  --- level:6 rd:4 wd:4
[  563.258825]  disk 0, o:1, dev:loop0
[  563.258829]  disk 1, o:1, dev:loop1
[  563.258833]  disk 2, o:1, dev:loop2
[  563.258838]  disk 3, o:1, dev:loop3
[  566.566752] RAID conf printout:
[  566.566758]  --- level:6 rd:5 wd:5
[  566.566762]  disk 0, o:1, dev:loop0
[  566.566765]  disk 1, o:1, dev:loop1
[  566.566767]  disk 2, o:1, dev:loop2
[  566.566770]  disk 3, o:1, dev:loop3
[  566.566772]  disk 4, o:1, dev:loop4
[  566.567770] md: reshape of RAID array md0
[  566.567776] md: minimum _guaranteed_  speed: 1000 KB/sec/disk.
[  566.567779] md: using maximum available idle IO bandwidth (but not more than 200000 KB/sec) for reshape.
[  566.567785] md: using 128k window, over a total of 19456k


Best Regards,
  Yi Zhang


--
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: raid5-cache I/O path improvements
From: Shaohua Li @ 2015-09-08  0:28 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: neilb, linux-raid, Kernel-team, dan.j.williams
In-Reply-To: <1441603250-5119-1-git-send-email-hch@lst.de>

On Mon, Sep 07, 2015 at 07:20:40AM +0200, Christoph Hellwig wrote:
> Hi Shaohua, hi Neil,
> 
> this series contains a few updates to the raid5-cache feature.
> 
> The first patch just ports it to the post-4.2 block layer.  As part of that
> I noticed that it currently doesn't handle I/O errors - fixes for that will
> follow.
> 
> The second and third patch simplify the I/O unit state machine and reduce
> latency and memory usage for the I/O units.  The remainder are just a couple
> of cleanups in this area that I stumbled upon.

Hi Christoph,
Thanks for these work. Yes, I/O error handling is in the plan. We could
simplify panic (people here like this option) or report error and bypass
log. Any way an option is good.

For the patches, FUA write does simplify things a lot. However, I tried
it before, the performance is quite bad in SSD. FUA is off in SATA by
default, the emulation is farily slow because FLUSH request isn't NCQ
command. I tried to enable FUA in SATA too, FUA write is still slow in
the SSD I tested. Other than this one, other patches look good:

Reviewed-by: Shaohua Li <shli@fb.com>

^ permalink raw reply

* md127 auto created when use "-B" to build a legacy array without superblocks
From: Yi Zhang @ 2015-09-07 12:27 UTC (permalink / raw)
  To: NeilBrown; +Cc: linux-raid
In-Reply-To: <795255819.22273230.1441628546245.JavaMail.zimbra@redhat.com>

Hi Neil

When testing 00raid1, found the md127 auto created when use "-B" to build a legacy array without superblocks, is it reasonable?
pls check below detailed info:

+ mdadm -CR /dev/md0 --level=raid1 -n3 /dev/loop0 /dev/loop1 /dev/loop2
mdadm: /dev/loop0 appears to contain an ext2fs file system
       size=58368K  mtime=Thu Jan  1 08:00:00 1970
mdadm: Note: this array has metadata at the start and
    may not be suitable as a boot device.  If you plan to
    store '/boot' on this device please ensure that
    your boot-loader understands md/v1.x metadata, or use
    --metadata=0.90
mdadm: /dev/loop1 appears to contain an ext2fs file system
       size=38912K  mtime=Thu Jan  1 08:00:00 1970
mdadm: Defaulting to version 1.2 metadata
mdadm: array /dev/md0 started.
+ mdadm --wait /dev/md0
+ cat /proc/mdstat
Personalities : [raid6] [raid5] [raid4] [raid1] 
md0 : active raid1 loop2[2] loop1[1] loop0[0]
      19968 blocks super 1.2 [3/3] [UUU]
      
unused devices: <none>
+ mdadm -S /dev/md0
mdadm: stopped /dev/md0
+ mdadm -B /dev/md0 -l raid1 -n2 /dev/loop0 /dev/loop1
mdadm: array /dev/md0 built and started.
+ sleep 2
+ cat /proc/mdstat
Personalities : [raid6] [raid5] [raid4] [raid1] 
md127 : inactive md0[0](S)
      19968 blocks super 1.2
       
md0 : active raid1 loop1[1] loop0[0]
      20000 blocks super non-persistent [2/2] [UU]
      
unused devices: <none>
[root@dhcp-12-171 bug]# uname -r
4.2.0
[root@dhcp-12-171 bug]# mdadm -D /dev/md0 
/dev/md0:
        Version : 
  Creation Time : Mon Sep  7 20:21:20 2015
     Raid Level : raid1
     Array Size : 20000 (19.53 MiB 20.48 MB)
  Used Dev Size : 20000 (19.53 MiB 20.48 MB)
   Raid Devices : 2
  Total Devices : 2

          State : clean 
 Active Devices : 2
Working Devices : 2
 Failed Devices : 0
  Spare Devices : 0

    Number   Major   Minor   RaidDevice State
       0       7        0        0      active sync   /dev/loop0
       1       7        1        1      active sync   /dev/loop1
[root@dhcp-12-171 bug]# mdadm -D /dev/md127 
/dev/md127:
        Version : 1.2
     Raid Level : raid0
  Total Devices : 1
    Persistence : Superblock is persistent

          State : inactive

           Name : dhcp-12-171.nay.redhat.com:0  (local to host dhcp-12-171.nay.redhat.com)
           UUID : 40ace956:a9dd0793:f4984d2b:8431b92b
         Events : 17

    Number   Major   Minor   RaidDevice

       -       9        0        -        /dev/md0 

Best Regards,
  Yi Zhang



^ permalink raw reply

* RAID4/5/6 reshape grow stuck
From: Yi Zhang @ 2015-09-07 12:19 UTC (permalink / raw)
  To: NeilBrown; +Cc: linux-raid
In-Reply-To: <1007334923.22262755.1441626807144.JavaMail.zimbra@redhat.com>

Hi Neil

When testing 07revert-grow, found the RAID4/5/6 grow stuck issue, pls check below info:

Steps:
mdadm --quiet -CR --assume-clean /dev/md0 -l6 -n4 -x1 /dev/loop0 /dev/loop1 /dev/loop2 /dev/loop3 /dev/loop4
sleep 3
mdadm --wait /dev/md0
mdadm -G /dev/md0 -n 5

[root@dhcp-12-171 bug]# cat /proc/mdstat 
Personalities : [raid6] [raid5] [raid4] 
md0 : active raid6 loop4[4] loop3[3] loop2[2] loop1[1] loop0[0]
      38912 blocks super 1.2 level 6, 512k chunk, algorithm 2 [5/5] [UUUUU]
      [>....................]  reshape =  0.0% (0/19456) finish=3344.0min speed=0K/sec
      
unused devices: <none>
kernel: 4.2.0
dmesg:
[  563.248753] md: bind<loop0>
[  563.248825] md: bind<loop1>
[  563.248892] md: bind<loop2>
[  563.248954] md: bind<loop3>
[  563.251515] md: bind<loop4>
[  563.257778] md/raid:md0: device loop3 operational as raid disk 3
[  563.257784] md/raid:md0: device loop2 operational as raid disk 2
[  563.257787] md/raid:md0: device loop1 operational as raid disk 1
[  563.257789] md/raid:md0: device loop0 operational as raid disk 0
[  563.258543] md/raid:md0: allocated 4366kB
[  563.258613] md/raid:md0: raid level 6 active with 4 out of 4 devices, algorithm 2
[  563.258617] RAID conf printout:
[  563.258621]  --- level:6 rd:4 wd:4
[  563.258624]  disk 0, o:1, dev:loop0
[  563.258628]  disk 1, o:1, dev:loop1
[  563.258632]  disk 2, o:1, dev:loop2
[  563.258634]  disk 3, o:1, dev:loop3
[  563.258651] md/raid456: discard support disabled due to uncertainty.
[  563.258653] Set raid456.devices_handle_discard_safely=Y to override.
[  563.258694] md0: detected capacity change from 0 to 39845888
[  563.258814] RAID conf printout:
[  563.258821]  --- level:6 rd:4 wd:4
[  563.258825]  disk 0, o:1, dev:loop0
[  563.258829]  disk 1, o:1, dev:loop1
[  563.258833]  disk 2, o:1, dev:loop2
[  563.258838]  disk 3, o:1, dev:loop3
[  566.566752] RAID conf printout:
[  566.566758]  --- level:6 rd:5 wd:5
[  566.566762]  disk 0, o:1, dev:loop0
[  566.566765]  disk 1, o:1, dev:loop1
[  566.566767]  disk 2, o:1, dev:loop2
[  566.566770]  disk 3, o:1, dev:loop3
[  566.566772]  disk 4, o:1, dev:loop4
[  566.567770] md: reshape of RAID array md0
[  566.567776] md: minimum _guaranteed_  speed: 1000 KB/sec/disk.
[  566.567779] md: using maximum available idle IO bandwidth (but not more than 200000 KB/sec) for reshape.
[  566.567785] md: using 128k window, over a total of 19456k


Best Regards,
  Yi Zhang



^ permalink raw reply

* Disk failed on grow process and missing backup-file
From: Юрий Букин @ 2015-09-07 10:15 UTC (permalink / raw)
  To: linux-raid

Hello.

I run grow raid5 from 4 to 5 devices:

# mdadm --grow /dev/md5 -n5 --backup-file /root/md5-grow.backup

All OK. Process started. After sometime I receive message about problem:

-----------
TEMA:Fail event on /dev/md5
TEKCT:This is an automatically generated mail message from mdadm

A Fail event had been detected on md device /dev/md5.

Faithfully yours, etc.

P.S. The /proc/mdstat file currently contains the following:

Personalities : [raid1] [raid6] [raid5] [raid4]
md5 : active raid5 sda3[7] sdc3[5] sde3[3] sdb3[4] sdf3[6](F)
      5847406080 blocks super 1.2 level 5, 512k chunk, algorithm 2 [5/4] [U_UUU]
      [===================>.]  reshape = 98.0% (1911492608/1949135360)
finish=522.1min speed=1201K/sec

md6 : active raid6 sda2[3] sdb2[0] sdf2[4](F) sde2[1]
      7486080 blocks level 6, 64k chunk, algorithm 2 [4/3] [UU_U]

md2 : active raid1 sda1[3](S) sde1[0] sdb1[2] sdf1[1]
      160512 blocks [3/3] [UUU]
  --------
and server become offline.

Today I reboot server after crash but md5 can not started.
Then I run command:

# mdadm -A /dev/md5 --force
mdadm: /dev/md5 has been started with 4 drives (out of 5)

# cat /proc/mdstat
Personalities : [raid1] [raid6] [raid5] [raid4]
md5 : active (auto-read-only) raid5 sdc3[5] sda3[7] sde3[3] sdb3[4]
      5847406080 blocks super 1.2 level 5, 512k chunk, algorithm 2 [5/4] [U_UUU]

# mdadm --detail /dev/md5
/dev/md5:
        Version : 1.2
  Creation Time : Tue Apr 10 11:46:42 2012
     Raid Level : raid5
     Array Size : 5847406080 (5576.52 GiB 5987.74 GB)
  Used Dev Size : 1949135360 (1858.84 GiB 1995.91 GB)
   Raid Devices : 5
  Total Devices : 4
    Persistence : Superblock is persistent

    Update Time : Sat Sep  5 01:54:48 2015
          State : clean, degraded
 Active Devices : 4
Working Devices : 4
 Failed Devices : 0
  Spare Devices : 0

         Layout : left-symmetric
     Chunk Size : 512K

  Delta Devices : 1, (4->5)

           Name : tmn-rec1.tmn.teleartel.ru:5  (local to host
tmn-rec1.tmn.teleartel.ru)
           UUID : f77e074e:5b74a964:073b9c49:8dcbe5cd
         Events : 936072

    Number   Major   Minor   RaidDevice State
       5       8       35        0      active sync   /dev/sdc3
       1       0        0        1      removed
       4       8       19        2      active sync   /dev/sdb3
       3       8       67        3      active sync   /dev/sde3
       7       8        3        4      active sync   /dev/sda3

# mdadm --examine /dev/sdc3
/dev/sdc3:
          Magic : a92b4efc
        Version : 1.2
    Feature Map : 0x4
     Array UUID : f77e074e:5b74a964:073b9c49:8dcbe5cd
           Name : tmn-rec1.tmn.teleartel.ru:5  (local to host
tmn-rec1.tmn.teleartel.ru)
  Creation Time : Tue Apr 10 11:46:42 2012
     Raid Level : raid5
   Raid Devices : 5

 Avail Dev Size : 3898271744 (1858.84 GiB 1995.92 GB)
     Array Size : 7796541440 (7435.36 GiB 7983.66 GB)
  Used Dev Size : 3898270720 (1858.84 GiB 1995.91 GB)
    Data Offset : 2048 sectors
   Super Offset : 8 sectors
          State : clean
    Device UUID : 9020ef29:6c9ee6b1:76a80389:c3d1a1e8

  Reshape pos'n : 7645890560 (7291.69 GiB 7829.39 GB)
  Delta Devices : 1 (4->5)

    Update Time : Sat Sep  5 01:54:48 2015
       Checksum : e648496d - correct
         Events : 936072

         Layout : left-symmetric
     Chunk Size : 512K

   Device Role : Active device 0
   Array State : A.AAA ('A' == active, '.' == missing)

I try check fs on raid in read-only mode:

# fsck -C0 -fn /dev/vg_r5/archive
fsck from util-linux 2.20.1
e2fsck 1.42.5 (29-Jul-2012)
Warning: skipping journal recovery because doing a read-only filesystem check.
Pass 1: Checking inodes, blocks, and sizes
Pass 2: Checking directory structure
Pass 3: Checking directory connectivity
Pass 4: Checking reference counts
Pass 5: Checking group summary information
Free blocks count wrong (72425382, counted=72416375).
Fix? no

Free inodes count wrong (330815912, counted=330815899).
Fix? no

/dev/mapper/vg_r5-archive: 443992/331259904 files (49.0%
non-contiguous), 1252586586/1325011968 blocks

Probably all right. But:

# mount /dev/vg_r5/archive /mnt/tmp -o ro

stale and i get messages infinity:

INFO: task mount:9872 blocked for more than 120 seconds.

^ permalink raw reply

* [PATCH 10/10] raid5-cache: use bio chaining
From: Christoph Hellwig @ 2015-09-07  5:20 UTC (permalink / raw)
  To: Shaohua Li, neilb; +Cc: linux-raid, Kernel-team, dan.j.williams
In-Reply-To: <1441603250-5119-1-git-send-email-hch@lst.de>

Simplify the bio completion handler by using bio chaining and submitting
bios as soon as they are full.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/raid5-cache.c | 38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index af274e9..50ec1aa 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -91,8 +91,6 @@ struct r5l_io_unit {
 	struct page *meta_page; /* store meta block */
 	int meta_offset; /* current offset in meta_page */
 
-	struct bio_list bios;
-	atomic_t pending_io; /* pending bios not written to log yet */
 	struct bio *current_bio; /* current_bio accepting new data */
 
 	atomic_t pending_stripe; /* how many stripes not flushed to raid */
@@ -103,6 +101,7 @@ struct r5l_io_unit {
 	struct list_head stripe_list; /* stripes added to the io_unit */
 
 	int state;
+	bool need_split_bio;
 };
 
 /* r5l_io_unit state */
@@ -191,9 +190,6 @@ static void r5l_log_endio(struct bio *bio)
 
 	bio_put(bio);
 
-	if (!atomic_dec_and_test(&io->pending_io))
-		return;
-
 	spin_lock_irqsave(&log->io_list_lock, flags);
 	__r5l_set_io_unit_state(io, IO_UNIT_IO_END);
 	r5l_log_run_stripes(log);
@@ -204,7 +200,6 @@ static void r5l_submit_current_io(struct r5l_log *log)
 {
 	struct r5l_io_unit *io = log->current_io;
 	struct r5l_meta_block *block;
-	struct bio *bio;
 	unsigned long flags;
 	u32 crc;
 
@@ -221,22 +216,17 @@ static void r5l_submit_current_io(struct r5l_log *log)
 	__r5l_set_io_unit_state(io, IO_UNIT_IO_START);
 	spin_unlock_irqrestore(&log->io_list_lock, flags);
 
-	while ((bio = bio_list_pop(&io->bios)))
-		submit_bio(WRITE | REQ_FUA, bio);
+	submit_bio(WRITE | REQ_FUA, io->current_bio);
 }
 
-static struct bio *r5l_bio_alloc(struct r5l_log *log, struct r5l_io_unit *io)
+static struct bio *r5l_bio_alloc(struct r5l_log *log)
 {
 	struct bio *bio = bio_kmalloc(GFP_NOIO | __GFP_NOFAIL, BIO_MAX_PAGES);
 
 	bio->bi_rw = WRITE;
 	bio->bi_bdev = log->rdev->bdev;
 	bio->bi_iter.bi_sector = log->rdev->data_offset + log->log_start;
-	bio->bi_end_io = r5l_log_endio;
-	bio->bi_private = io;
 
-	bio_list_add(&io->bios, bio);
-	atomic_inc(&io->pending_io);
 	return bio;
 }
 
@@ -252,7 +242,7 @@ static void r5_reserve_log_entry(struct r5l_log *log, struct r5l_io_unit *io)
 	 * of BLOCK_SECTORS.
 	 */
 	if (log->log_start == 0)
-		io->current_bio = NULL;
+		io->need_split_bio = true;
 
 	io->log_end = log->log_start;
 }
@@ -265,7 +255,6 @@ static struct r5l_io_unit *r5l_new_meta(struct r5l_log *log)
 	/* We can't handle memory allocate failure so far */
 	io = kmem_cache_zalloc(log->io_kc, GFP_NOIO | __GFP_NOFAIL);
 	io->log = log;
-	bio_list_init(&io->bios);
 	INIT_LIST_HEAD(&io->log_sibling);
 	INIT_LIST_HEAD(&io->stripe_list);
 	io->state = IO_UNIT_RUNNING;
@@ -281,7 +270,9 @@ static struct r5l_io_unit *r5l_new_meta(struct r5l_log *log)
 	io->meta_offset = sizeof(struct r5l_meta_block);
 	io->seq = log->seq++;
 
-	io->current_bio = r5l_bio_alloc(log, io);
+	io->current_bio = r5l_bio_alloc(log);
+	io->current_bio->bi_end_io = r5l_log_endio;
+	io->current_bio->bi_private = io;
 	bio_add_page(io->current_bio, io->meta_page, PAGE_SIZE, 0);
 
 	r5_reserve_log_entry(log, io);
@@ -329,15 +320,18 @@ static void r5l_append_payload_page(struct r5l_log *log, struct page *page)
 {
 	struct r5l_io_unit *io = log->current_io;
 
-alloc_bio:
-	if (!io->current_bio) 
-		io->current_bio = r5l_bio_alloc(log, io);
+	if (io->need_split_bio) {
+		struct bio *prev = io->current_bio;
 
-	if (!bio_add_page(io->current_bio, page, PAGE_SIZE, 0)) {
-		io->current_bio = NULL;
-		goto alloc_bio;
+		io->current_bio = r5l_bio_alloc(log);
+		bio_chain(io->current_bio, prev);
+
+		submit_bio(WRITE, prev);
 	}
 
+	if (!bio_add_page(io->current_bio, page, PAGE_SIZE, 0))
+		BUG();
+
 	r5_reserve_log_entry(log, io);
 }
 
-- 
1.9.1


^ permalink raw reply related


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