Linux RAID subsystem development
 help / color / mirror / Atom feed
* Re: block: fix blk_queue_split() resource exhaustion
From: Mike Snitzer @ 2016-06-24 15:15 UTC (permalink / raw)
  To: Lars Ellenberg
  Cc: Ming Lei, linux-block, Roland Kammerer, Jens Axboe, NeilBrown,
	Kent Overstreet, Shaohua Li, Alasdair Kergon,
	open list:DEVICE-MAPPER (LVM), Ingo Molnar, Peter Zijlstra,
	Takashi Iwai, Jiri Kosina, Zheng Liu, Keith Busch,
	Martin K. Petersen, Kirill A. Shutemov, Linux Kernel Mailing List,
	open list:BCACHE (BLOCK LAYER CACHE),
	open list:SOFTWARE RAID (Multiple Disks) SUPPORT
In-Reply-To: <20160624142711.GF3239@soda.linbit>

On Fri, Jun 24 2016 at 10:27am -0400,
Lars Ellenberg <lars.ellenberg@linbit.com> wrote:

> On Fri, Jun 24, 2016 at 07:36:57PM +0800, Ming Lei wrote:
> > >
> > > This is not a theoretical problem.
> > > At least int DRBD, and an unfortunately high IO concurrency wrt. the
> > > "max-buffers" setting, without this patch we have a reproducible deadlock.
> > 
> > Is there any log about the deadlock? And is there any lockdep warning
> > if it is enabled?
> 
> In DRBD, to avoid potentially very long internal queues as we wait for
> our replication peer device and local backend, we limit the number of
> in-flight bios we accept, and block in our ->make_request_fn() if that
> number exceeds a configured watermark ("max-buffers").
> 
> Works fine, as long as we could assume that once our make_request_fn()
> returns, any bios we "recursively" submitted against the local backend
> would be dispatched. Which used to be the case.

It'd be useful to know whether this patch fixes your issue:
https://patchwork.kernel.org/patch/7398411/

Ming Lei didn't like it due to concerns about I contexts changing
(whereby breaking merging that occurs via plugging).

But if it _does_ fix your issue then the case for the change is
increased; and we just need to focus on addressing Ming's concerns
(Mikulas has some ideas).

Conversely, and in parallel, Mikulas can look to see if your approach
fixes the observed dm-snapshot deadlock that he set out to fix.

^ permalink raw reply

* Re: [RFC] block: fix blk_queue_split() resource exhaustion
From: Lars Ellenberg @ 2016-06-24 14:27 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-block, Roland Kammerer, Jens Axboe, NeilBrown,
	Kent Overstreet, Shaohua Li, Alasdair Kergon, Mike Snitzer,
	open list:DEVICE-MAPPER (LVM), Ingo Molnar, Peter Zijlstra,
	Takashi Iwai, Jiri Kosina, Zheng Liu, Keith Busch,
	Martin K. Petersen, Kirill A. Shutemov, Linux Kernel Mailing List,
	open list:BCACHE (BLOCK LAYER CACHE),
	open list:SOFTWARE RAID (Multiple Disks) SUPPORT
In-Reply-To: <CACVXFVOtzcPpmg9Z_P6xN99dW=Sb_cCQNd-VcP0s5Nk0yHbwXQ@mail.gmail.com>

On Fri, Jun 24, 2016 at 07:36:57PM +0800, Ming Lei wrote:
> >
> > This is not a theoretical problem.
> > At least int DRBD, and an unfortunately high IO concurrency wrt. the
> > "max-buffers" setting, without this patch we have a reproducible deadlock.
> 
> Is there any log about the deadlock? And is there any lockdep warning
> if it is enabled?

In DRBD, to avoid potentially very long internal queues as we wait for
our replication peer device and local backend, we limit the number of
in-flight bios we accept, and block in our ->make_request_fn() if that
number exceeds a configured watermark ("max-buffers").

Works fine, as long as we could assume that once our make_request_fn()
returns, any bios we "recursively" submitted against the local backend
would be dispatched. Which used to be the case.

commit 54efd50 block: make generic_make_request handle arbitrarily sized bios
introduced blk_queue_split(), which will split any bio that is
violating the queue limits into a smaller piece to be processed
right away, and queue the "rest" on current->bio_list to be
processed by the iteration in generic_make_request() once the
current ->make_request_fn() returns.

Before that, any user was supposed to go through bio_add_page(),
which would call our merge bvec function, and that should already
be sufficient to not violate queue limits.

Previously, typically the next in line bio to be processed would
be the cloned one we passed down to our backend device, which in
case of further stacking devices (e.g. logical volume, raid1 or
similar) may again push further bios on that list.
All of which would simply be processed in turn.

Now, with blk_queue_split(), the next-in-line bio would be the
next piece of the "too big" original bio, so we end up calling
several times into our ->make_request_fn() without even
dispatching one bio to the backend.

With highly concurrent IO and/or very small "max-buffers" setting,
this can deadlock, where the current submit path would wait for
the completion of the bios supposedly already passed to the
backend, but which actually are not even dispatched yet, rotting
away on current->bio_list.

I could code around that in various ways inside the DRBD make_request_fn,
or always dispatch the backend bios to a different (thread or work_queue)
context, but I'd rather have the distinction between "recursively
submitted" bios and "pushed back part of originally passed in bio" as
implemented in this patch.

Thanks,

    Lars

> > I'm unsure if it could cause problems in md raid in some corner cases
> > or when a rebuild/scrub/... starts in a "bad" moment.
> >
> > It also may increase "stress" on any involved bio_set,
> > because it may increase the number of bios that are allocated
> > before the first of them is actually processed, causing more
> > frequent calls of punt_bios_to_rescuer().
> >
> > Alternatively,
> > a simple one-line change to generic_make_request() would also "fix" it,
> > by processing all recursive bios in LIFO order.
> > But that may change the order in which bios reach the "physical" layer,
> > in case they are in fact split into many smaller pieces, for example in
> > a striping setup, which may have other performance implications.
> >
> >  | diff --git a/block/blk-core.c b/block/blk-core.c
> >  | index 2475b1c7..a5623f6 100644
> >  | --- a/block/blk-core.c
> >  | +++ b/block/blk-core.c
> >  | @@ -2048,7 +2048,7 @@ blk_qc_t generic_make_request(struct bio *bio)
> >  |       * should be added at the tail
> >  |       */
> >  |      if (current->bio_list) {
> >  | -            bio_list_add(current->bio_list, bio);
> >  | +            bio_list_add_head(current->bio_list, bio);
> >  |              goto out;
> >  |      }

> > diff --git a/block/blk-core.c b/block/blk-core.c
> > index 2475b1c7..f03ff4c 100644
> > --- a/block/blk-core.c
> > +++ b/block/blk-core.c
> > @@ -2031,7 +2031,7 @@ end_io:
> >   */
> >  blk_qc_t generic_make_request(struct bio *bio)
> >  {
> > -       struct bio_list bio_list_on_stack;
> > +       struct recursion_to_iteration_bio_lists bio_lists_on_stack;
> >         blk_qc_t ret = BLK_QC_T_NONE;
> >
> >         if (!generic_make_request_checks(bio))
> > @@ -2040,15 +2040,18 @@ blk_qc_t generic_make_request(struct bio *bio)

> > -       if (current->bio_list) {
> > -               bio_list_add(current->bio_list, bio);
> > +       if (current->bio_lists) {
> > +               bio_list_add(&current->bio_lists->recursion, bio);
> >                 goto out;
> >         }
> >

> > @@ -2067,8 +2070,9 @@ blk_qc_t generic_make_request(struct bio *bio)
> >          * bio_list, and call into ->make_request() again.
> >          */
> >         BUG_ON(bio->bi_next);
> > -       bio_list_init(&bio_list_on_stack);
> > -       current->bio_list = &bio_list_on_stack;
> > +       bio_list_init(&bio_lists_on_stack.recursion);
> > +       bio_list_init(&bio_lists_on_stack.remainder);
> > +       current->bio_lists = &bio_lists_on_stack;
> >         do {
> >                 struct request_queue *q = bdev_get_queue(bio->bi_bdev);
> >
> > @@ -2076,16 +2080,14 @@ blk_qc_t generic_make_request(struct bio *bio)
> >                         ret = q->make_request_fn(q, bio);
> >
> >                         blk_queue_exit(q);
> > -
> > -                       bio = bio_list_pop(current->bio_list);
> >                 } else {
> > -                       struct bio *bio_next = bio_list_pop(current->bio_list);
> > -
> >                         bio_io_error(bio);
> > -                       bio = bio_next;
> >                 }
> > +               bio = bio_list_pop(&current->bio_lists->recursion);
> > +               if (!bio)
> > +                       bio = bio_list_pop(&current->bio_lists->remainder);
> >         } while (bio);
> > -       current->bio_list = NULL; /* deactivate */
> > +       current->bio_lists = NULL; /* deactivate */
> >
> >  out:
> >         return ret;
> > diff --git a/block/blk-merge.c b/block/blk-merge.c
> > index 2613531..8da0c22 100644
> > --- a/block/blk-merge.c
> > +++ b/block/blk-merge.c
> > @@ -172,6 +172,7 @@ void blk_queue_split(struct request_queue *q, struct bio **bio,
> >         struct bio *split, *res;
> >         unsigned nsegs;
> >
> > +       BUG_ON(!current->bio_lists);
> >         if ((*bio)->bi_rw & REQ_DISCARD)
> >                 split = blk_bio_discard_split(q, *bio, bs, &nsegs);
> >         else if ((*bio)->bi_rw & REQ_WRITE_SAME)
> > @@ -190,7 +191,7 @@ void blk_queue_split(struct request_queue *q, struct bio **bio,
> >
> >                 bio_chain(split, *bio);
> >                 trace_block_split(q, split, (*bio)->bi_iter.bi_sector);
> > -               generic_make_request(*bio);
> > +               bio_list_add_head(&current->bio_lists->remainder, *bio);
> >                 *bio = split;


^ permalink raw reply

* [PATCH] raid10: improve random reads performance
From: Tomasz Majchrzak @ 2016-06-24 12:20 UTC (permalink / raw)
  To: linux-raid

RAID10 random read performance is lower than expected due to excessive spinlock
utilisation which is required mostly for rebuild/resync. Simplify allow_barrier
as it's in IO path and encounters a lot of unnecessary congestion.

As lower_barrier just takes a lock in order to decrement a counter, convert
counter (nr_pending) into atomic variable and remove the spin lock. There is
also a congestion for wake_up (it uses lock internally) so call it only when
it's really needed. As wake_up is not called constantly anymore, ensure process
waiting to raise a barrier is notified when there are no more waiting IOs.

Signed-off-by: Tomasz Majchrzak <tomasz.majchrzak@intel.com>
---
 drivers/md/raid10.c | 21 ++++++++++++---------
 drivers/md/raid10.h |  3 ++-
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index e3fd725..cdbe504 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -913,7 +913,7 @@ static void raise_barrier(struct r10conf *conf, int force)
 
 	/* Now wait for all pending IO to complete */
 	wait_event_lock_irq(conf->wait_barrier,
-			    !conf->nr_pending && conf->barrier < RESYNC_DEPTH,
+			    !atomic_read(&conf->nr_pending) && conf->barrier < RESYNC_DEPTH,
 			    conf->resync_lock);
 
 	spin_unlock_irq(&conf->resync_lock);
@@ -944,23 +944,23 @@ static void wait_barrier(struct r10conf *conf)
 		 */
 		wait_event_lock_irq(conf->wait_barrier,
 				    !conf->barrier ||
-				    (conf->nr_pending &&
+				    (atomic_read(&conf->nr_pending) &&
 				     current->bio_list &&
 				     !bio_list_empty(current->bio_list)),
 				    conf->resync_lock);
 		conf->nr_waiting--;
+		if (!conf->nr_waiting)
+			wake_up(&conf->wait_barrier);
 	}
-	conf->nr_pending++;
+	atomic_inc(&conf->nr_pending);
 	spin_unlock_irq(&conf->resync_lock);
 }
 
 static void allow_barrier(struct r10conf *conf)
 {
-	unsigned long flags;
-	spin_lock_irqsave(&conf->resync_lock, flags);
-	conf->nr_pending--;
-	spin_unlock_irqrestore(&conf->resync_lock, flags);
-	wake_up(&conf->wait_barrier);
+	if ((atomic_dec_and_test(&conf->nr_pending)) ||
+			(conf->array_freeze_pending))
+		wake_up(&conf->wait_barrier);
 }
 
 static void freeze_array(struct r10conf *conf, int extra)
@@ -978,13 +978,15 @@ static void freeze_array(struct r10conf *conf, int extra)
 	 * we continue.
 	 */
 	spin_lock_irq(&conf->resync_lock);
+	conf->array_freeze_pending++;
 	conf->barrier++;
 	conf->nr_waiting++;
 	wait_event_lock_irq_cmd(conf->wait_barrier,
-				conf->nr_pending == conf->nr_queued+extra,
+				atomic_read(&conf->nr_pending) == conf->nr_queued+extra,
 				conf->resync_lock,
 				flush_pending_writes(conf));
 
+	conf->array_freeze_pending--;
 	spin_unlock_irq(&conf->resync_lock);
 }
 
@@ -3505,6 +3507,7 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 
 	spin_lock_init(&conf->resync_lock);
 	init_waitqueue_head(&conf->wait_barrier);
+	atomic_set(&conf->nr_pending, 0);
 
 	conf->thread = md_register_thread(raid10d, mddev, "raid10");
 	if (!conf->thread)
diff --git a/drivers/md/raid10.h b/drivers/md/raid10.h
index 6fc2c75..18ec1f7 100644
--- a/drivers/md/raid10.h
+++ b/drivers/md/raid10.h
@@ -64,10 +64,11 @@ struct r10conf {
 	int			pending_count;
 
 	spinlock_t		resync_lock;
-	int			nr_pending;
+	atomic_t		nr_pending;
 	int			nr_waiting;
 	int			nr_queued;
 	int			barrier;
+	int			array_freeze_pending;
 	sector_t		next_resync;
 	int			fullsync;  /* set to 1 if a full sync is needed,
 					    * (fresh device added).
-- 
1.8.3.1


^ permalink raw reply related

* Re: [RFC] block: fix blk_queue_split() resource exhaustion
From: Ming Lei @ 2016-06-24 11:36 UTC (permalink / raw)
  To: Lars Ellenberg
  Cc: linux-block, Roland Kammerer, Jens Axboe, NeilBrown,
	Kent Overstreet, Shaohua Li, Alasdair Kergon, Mike Snitzer,
	open list:DEVICE-MAPPER (LVM), Ingo Molnar, Peter Zijlstra,
	Takashi Iwai, Jiri Kosina, Zheng Liu, Keith Busch,
	Martin K. Petersen, Kirill A. Shutemov, Linux Kernel Mailing List,
	open list:BCACHE (BLOCK LAYER CACHE),
	open list:SOFTWARE RAID (Multiple Disks) SUPPORT
In-Reply-To: <1466583730-28595-1-git-send-email-lars.ellenberg@linbit.com>

On Wed, Jun 22, 2016 at 4:22 PM, Lars Ellenberg
<lars.ellenberg@linbit.com> wrote:
> For a long time, generic_make_request() converts recursion into
> iteration by queuing recursive arguments on current->bio_list.
>
> This is convenient for stacking drivers,
> the top-most driver would take the originally submitted bio,
> and re-submit a re-mapped version of it, or one or more clones,
> or one or more new allocated bios to its backend(s). Which
> are then simply processed in turn, and each can again queue
> more "backend-bios" until we reach the bottom of the driver stack,
> and actually dispatch to the real backend device.
>
> Any stacking driver ->make_request_fn() could expect that,
> once it returns, any backend-bios it submitted via recursive calls
> to generic_make_request() would now be processed and dispatched, before
> the current task would call into this driver again.
>
> This is changed by commit
>   54efd50 block: make generic_make_request handle arbitrarily sized bios
>
> Drivers may call blk_queue_split() inside their ->make_request_fn(),
> which may split the current bio into a front-part to be dealt with
> immediately, and a remainder-part, which may need to be split even
> further. That remainder-part will simply also be pushed to
> current->bio_list, and would end up being head-of-queue, in front
> of any backend-bios the current make_request_fn() might submit during
> processing of the fron-part.
>
> Which means the current task would immediately end up back in the same
> make_request_fn() of the same driver again, before any of its backend
> bios have even been processed.
>
> This can lead to resource starvation deadlock.

Could you explain it a bit what the resource is and how it is
exhausted?

> Drivers could avoid this by learning to not need blk_queue_split(),
> or by submitting their backend bios in a different context (dedicated
> kernel thread, work_queue context, ...). Or by playing funny re-ordering
> games with entries on current->bio_list.
>
> Instead, I suggest to distinguish between recursive calls to
> generic_make_request(), and pushing back the remainder part in
> blk_queue_split(), by pointing current->bio_lists to a
>         struct recursion_to_iteration_bio_lists {
>                 struct bio_list recursion;
>                 struct bio_list remainder;
>         }
>
> To have all bios targeted to drivers lower in the stack processed before
> processing the next piece of a bio targeted at the higher levels,
> as long as queued bios resulting from recursion are available,
> they will continue to be processed in FIFO order.
> Pushed back bio-parts resulting from blk_queue_split() will be processed
> in LIFO order, one-by-one, whenever the recursion list becomes empty.
>
> Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
> Signed-off-by: Roland Kammerer <roland.kammerer@linbit.com>
> ---
>
> This is not a theoretical problem.
> At least int DRBD, and an unfortunately high IO concurrency wrt. the
> "max-buffers" setting, without this patch we have a reproducible deadlock.

Is there any log about the deadlock? And is there any lockdep warning
if it is enabled?

Thanks,

>
> I'm unsure if it could cause problems in md raid in some corner cases
> or when a rebuild/scrub/... starts in a "bad" moment.
>
> It also may increase "stress" on any involved bio_set,
> because it may increase the number of bios that are allocated
> before the first of them is actually processed, causing more
> frequent calls of punt_bios_to_rescuer().
>
> Alternatively,
> a simple one-line change to generic_make_request() would also "fix" it,
> by processing all recursive bios in LIFO order.
> But that may change the order in which bios reach the "physical" layer,
> in case they are in fact split into many smaller pieces, for example in
> a striping setup, which may have other performance implications.
>
>  | diff --git a/block/blk-core.c b/block/blk-core.c
>  | index 2475b1c7..a5623f6 100644
>  | --- a/block/blk-core.c
>  | +++ b/block/blk-core.c
>  | @@ -2048,7 +2048,7 @@ blk_qc_t generic_make_request(struct bio *bio)
>  |       * should be added at the tail
>  |       */
>  |      if (current->bio_list) {
>  | -            bio_list_add(current->bio_list, bio);
>  | +            bio_list_add_head(current->bio_list, bio);
>  |              goto out;
>  |      }
>
> Any fix would need to go to stable 4.3.x and up.
> This patch does apply as is to 4.5 and up,
> with a trivial fixup (or a cherry-pick of cda2264) to 4.4,
> and with some more (also trivial) fixup to 4.3 because of
> GFP_WAIT -> GFP_RECLAIM and comment rewrap.
>
> Any input?
> How should I proceed?
>
> ---
>  block/bio.c               | 14 +++++++-------
>  block/blk-core.c          | 32 +++++++++++++++++---------------
>  block/blk-merge.c         |  3 ++-
>  drivers/md/bcache/btree.c | 12 ++++++------
>  drivers/md/dm-bufio.c     |  2 +-
>  drivers/md/md.h           |  7 +++++++
>  drivers/md/raid1.c        |  5 ++---
>  drivers/md/raid10.c       |  5 ++---
>  include/linux/bio.h       | 11 +++++++++++
>  include/linux/sched.h     |  4 ++--
>  10 files changed, 57 insertions(+), 38 deletions(-)
>
> diff --git a/block/bio.c b/block/bio.c
> index 0e4aa42..2ffcea0 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -368,10 +368,10 @@ static void punt_bios_to_rescuer(struct bio_set *bs)
>         bio_list_init(&punt);
>         bio_list_init(&nopunt);
>
> -       while ((bio = bio_list_pop(current->bio_list)))
> +       while ((bio = bio_list_pop(&current->bio_lists->recursion)))
>                 bio_list_add(bio->bi_pool == bs ? &punt : &nopunt, bio);
>
> -       *current->bio_list = nopunt;
> +       current->bio_lists->recursion = nopunt;
>
>         spin_lock(&bs->rescue_lock);
>         bio_list_merge(&bs->rescue_list, &punt);
> @@ -453,13 +453,13 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, int nr_iovecs, struct bio_set *bs)
>                  *
>                  * We solve this, and guarantee forward progress, with a rescuer
>                  * workqueue per bio_set. If we go to allocate and there are
> -                * bios on current->bio_list, we first try the allocation
> -                * without __GFP_DIRECT_RECLAIM; if that fails, we punt those
> -                * bios we would be blocking to the rescuer workqueue before
> -                * we retry with the original gfp_flags.
> +                * bios on current->bio_lists->recursion, we first try the
> +                * allocation without __GFP_DIRECT_RECLAIM; if that fails, we
> +                * punt those bios we would be blocking to the rescuer
> +                * workqueue before we retry with the original gfp_flags.
>                  */
>
> -               if (current->bio_list && !bio_list_empty(current->bio_list))
> +               if (current->bio_lists && !bio_list_empty(&current->bio_lists->recursion))
>                         gfp_mask &= ~__GFP_DIRECT_RECLAIM;
>
>                 p = mempool_alloc(bs->bio_pool, gfp_mask);
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 2475b1c7..f03ff4c 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -2031,7 +2031,7 @@ end_io:
>   */
>  blk_qc_t generic_make_request(struct bio *bio)
>  {
> -       struct bio_list bio_list_on_stack;
> +       struct recursion_to_iteration_bio_lists bio_lists_on_stack;
>         blk_qc_t ret = BLK_QC_T_NONE;
>
>         if (!generic_make_request_checks(bio))
> @@ -2040,15 +2040,18 @@ blk_qc_t generic_make_request(struct bio *bio)
>         /*
>          * We only want one ->make_request_fn to be active at a time, else
>          * stack usage with stacked devices could be a problem.  So use
> -        * current->bio_list to keep a list of requests submited by a
> -        * make_request_fn function.  current->bio_list is also used as a
> +        * current->bio_lists to keep a list of requests submited by a
> +        * make_request_fn function.  current->bio_lists is also used as a
>          * flag to say if generic_make_request is currently active in this
>          * task or not.  If it is NULL, then no make_request is active.  If
>          * it is non-NULL, then a make_request is active, and new requests
> -        * should be added at the tail
> +        * should be added at the tail of current->bio_lists->recursion;
> +        * remainder bios resulting from a call to bio_queue_split() from
> +        * within ->make_request_fn() should be added to the head of
> +        * current->bio_lists->remainder.
>          */
> -       if (current->bio_list) {
> -               bio_list_add(current->bio_list, bio);
> +       if (current->bio_lists) {
> +               bio_list_add(&current->bio_lists->recursion, bio);
>                 goto out;
>         }
>
> @@ -2057,7 +2060,7 @@ blk_qc_t generic_make_request(struct bio *bio)
>          * Before entering the loop, bio->bi_next is NULL (as all callers
>          * ensure that) so we have a list with a single bio.
>          * We pretend that we have just taken it off a longer list, so
> -        * we assign bio_list to a pointer to the bio_list_on_stack,
> +        * we assign bio_list to a pointer to the bio_lists_on_stack,
>          * thus initialising the bio_list of new bios to be
>          * added.  ->make_request() may indeed add some more bios
>          * through a recursive call to generic_make_request.  If it
> @@ -2067,8 +2070,9 @@ blk_qc_t generic_make_request(struct bio *bio)
>          * bio_list, and call into ->make_request() again.
>          */
>         BUG_ON(bio->bi_next);
> -       bio_list_init(&bio_list_on_stack);
> -       current->bio_list = &bio_list_on_stack;
> +       bio_list_init(&bio_lists_on_stack.recursion);
> +       bio_list_init(&bio_lists_on_stack.remainder);
> +       current->bio_lists = &bio_lists_on_stack;
>         do {
>                 struct request_queue *q = bdev_get_queue(bio->bi_bdev);
>
> @@ -2076,16 +2080,14 @@ blk_qc_t generic_make_request(struct bio *bio)
>                         ret = q->make_request_fn(q, bio);
>
>                         blk_queue_exit(q);
> -
> -                       bio = bio_list_pop(current->bio_list);
>                 } else {
> -                       struct bio *bio_next = bio_list_pop(current->bio_list);
> -
>                         bio_io_error(bio);
> -                       bio = bio_next;
>                 }
> +               bio = bio_list_pop(&current->bio_lists->recursion);
> +               if (!bio)
> +                       bio = bio_list_pop(&current->bio_lists->remainder);
>         } while (bio);
> -       current->bio_list = NULL; /* deactivate */
> +       current->bio_lists = NULL; /* deactivate */
>
>  out:
>         return ret;
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 2613531..8da0c22 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -172,6 +172,7 @@ void blk_queue_split(struct request_queue *q, struct bio **bio,
>         struct bio *split, *res;
>         unsigned nsegs;
>
> +       BUG_ON(!current->bio_lists);
>         if ((*bio)->bi_rw & REQ_DISCARD)
>                 split = blk_bio_discard_split(q, *bio, bs, &nsegs);
>         else if ((*bio)->bi_rw & REQ_WRITE_SAME)
> @@ -190,7 +191,7 @@ void blk_queue_split(struct request_queue *q, struct bio **bio,
>
>                 bio_chain(split, *bio);
>                 trace_block_split(q, split, (*bio)->bi_iter.bi_sector);
> -               generic_make_request(*bio);
> +               bio_list_add_head(&current->bio_lists->remainder, *bio);
>                 *bio = split;
>         }
>  }
> diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
> index eab505e..eb0b41a 100644
> --- a/drivers/md/bcache/btree.c
> +++ b/drivers/md/bcache/btree.c
> @@ -450,7 +450,7 @@ void __bch_btree_node_write(struct btree *b, struct closure *parent)
>
>         trace_bcache_btree_write(b);
>
> -       BUG_ON(current->bio_list);
> +       BUG_ON(current->bio_lists);
>         BUG_ON(b->written >= btree_blocks(b));
>         BUG_ON(b->written && !i->keys);
>         BUG_ON(btree_bset_first(b)->seq != i->seq);
> @@ -544,7 +544,7 @@ static void bch_btree_leaf_dirty(struct btree *b, atomic_t *journal_ref)
>
>         /* Force write if set is too big */
>         if (set_bytes(i) > PAGE_SIZE - 48 &&
> -           !current->bio_list)
> +           !current->bio_lists)
>                 bch_btree_node_write(b, NULL);
>  }
>
> @@ -889,7 +889,7 @@ static struct btree *mca_alloc(struct cache_set *c, struct btree_op *op,
>  {
>         struct btree *b;
>
> -       BUG_ON(current->bio_list);
> +       BUG_ON(current->bio_lists);
>
>         lockdep_assert_held(&c->bucket_lock);
>
> @@ -976,7 +976,7 @@ retry:
>         b = mca_find(c, k);
>
>         if (!b) {
> -               if (current->bio_list)
> +               if (current->bio_lists)
>                         return ERR_PTR(-EAGAIN);
>
>                 mutex_lock(&c->bucket_lock);
> @@ -2127,7 +2127,7 @@ static int bch_btree_insert_node(struct btree *b, struct btree_op *op,
>
>         return 0;
>  split:
> -       if (current->bio_list) {
> +       if (current->bio_lists) {
>                 op->lock = b->c->root->level + 1;
>                 return -EAGAIN;
>         } else if (op->lock <= b->c->root->level) {
> @@ -2209,7 +2209,7 @@ int bch_btree_insert(struct cache_set *c, struct keylist *keys,
>         struct btree_insert_op op;
>         int ret = 0;
>
> -       BUG_ON(current->bio_list);
> +       BUG_ON(current->bio_lists);
>         BUG_ON(bch_keylist_empty(keys));
>
>         bch_btree_op_init(&op.op, 0);
> diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c
> index cd77216..b64d4f0 100644
> --- a/drivers/md/dm-bufio.c
> +++ b/drivers/md/dm-bufio.c
> @@ -174,7 +174,7 @@ static inline int dm_bufio_cache_index(struct dm_bufio_client *c)
>  #define DM_BUFIO_CACHE(c)      (dm_bufio_caches[dm_bufio_cache_index(c)])
>  #define DM_BUFIO_CACHE_NAME(c) (dm_bufio_cache_names[dm_bufio_cache_index(c)])
>
> -#define dm_bufio_in_request()  (!!current->bio_list)
> +#define dm_bufio_in_request()  (!!current->bio_lists)
>
>  static void dm_bufio_lock(struct dm_bufio_client *c)
>  {
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index b5c4be7..7afc71c 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -663,6 +663,13 @@ static inline void rdev_dec_pending(struct md_rdev *rdev, struct mddev *mddev)
>         }
>  }
>
> +static inline bool current_has_pending_bios(void)
> +{
> +       return current->bio_lists && (
> +             !bio_list_empty(&current->bio_lists->recursion) ||
> +             !bio_list_empty(&current->bio_lists->remainder));
> +}
> +
>  extern struct md_cluster_operations *md_cluster_ops;
>  static inline int mddev_is_clustered(struct mddev *mddev)
>  {
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index c7c8cde..811f2c0 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -876,8 +876,7 @@ static sector_t wait_barrier(struct r1conf *conf, struct bio *bio)
>                                     (!conf->barrier ||
>                                      ((conf->start_next_window <
>                                        conf->next_resync + RESYNC_SECTORS) &&
> -                                     current->bio_list &&
> -                                     !bio_list_empty(current->bio_list))),
> +                                     current_has_pending_bios())),
>                                     conf->resync_lock);
>                 conf->nr_waiting--;
>         }
> @@ -1014,7 +1013,7 @@ static void raid1_unplug(struct blk_plug_cb *cb, bool from_schedule)
>         struct r1conf *conf = mddev->private;
>         struct bio *bio;
>
> -       if (from_schedule || current->bio_list) {
> +       if (from_schedule || current->bio_lists) {
>                 spin_lock_irq(&conf->device_lock);
>                 bio_list_merge(&conf->pending_bio_list, &plug->pending);
>                 conf->pending_count += plug->pending_cnt;
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index c7de2a5..2c70717 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -945,8 +945,7 @@ static void wait_barrier(struct r10conf *conf)
>                 wait_event_lock_irq(conf->wait_barrier,
>                                     !conf->barrier ||
>                                     (conf->nr_pending &&
> -                                    current->bio_list &&
> -                                    !bio_list_empty(current->bio_list)),
> +                                    current_has_pending_bios()),
>                                     conf->resync_lock);
>                 conf->nr_waiting--;
>         }
> @@ -1022,7 +1021,7 @@ static void raid10_unplug(struct blk_plug_cb *cb, bool from_schedule)
>         struct r10conf *conf = mddev->private;
>         struct bio *bio;
>
> -       if (from_schedule || current->bio_list) {
> +       if (from_schedule || current->bio_lists) {
>                 spin_lock_irq(&conf->device_lock);
>                 bio_list_merge(&conf->pending_bio_list, &plug->pending);
>                 conf->pending_count += plug->pending_cnt;
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index 9faebf7..57e45a0 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -598,6 +598,17 @@ struct bio_list {
>         struct bio *tail;
>  };
>
> +/* for generic_make_request() */
> +struct recursion_to_iteration_bio_lists {
> +       /* For stacking drivers submitting to their respective backend.
> +        * Added to tail, processed in FIFO order. */
> +       struct bio_list recursion;
> +       /* For pushing back the "remainder" part resulting from calling
> +        * blk_queue_split(). Added to head, processed in LIFO order,
> +        * once the "recursion" list has been drained. */
> +       struct bio_list remainder;
> +};
> +
>  static inline int bio_list_empty(const struct bio_list *bl)
>  {
>         return bl->head == NULL;
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 6e42ada..146eedc 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -128,7 +128,7 @@ struct sched_attr {
>
>  struct futex_pi_state;
>  struct robust_list_head;
> -struct bio_list;
> +struct recursion_to_iteration_bio_lists;
>  struct fs_struct;
>  struct perf_event_context;
>  struct blk_plug;
> @@ -1727,7 +1727,7 @@ struct task_struct {
>         void *journal_info;
>
>  /* stacked block device info */
> -       struct bio_list *bio_list;
> +       struct recursion_to_iteration_bio_lists *bio_lists;
>
>  #ifdef CONFIG_BLOCK
>  /* stack plugging */
> --
> 1.9.1
>

^ permalink raw reply

* Re: [PATCH v2 1/3] block: add QUEUE_FLAG_DAX for devices to advertise their DAX support
From: Mike Snitzer @ 2016-06-23 21:11 UTC (permalink / raw)
  To: Kani, Toshimitsu
  Cc: yigal@plexistor.com, axboe@kernel.dk, linux-s390@vger.kernel.org,
	linux-nvdimm@lists.01.org, heiko.carstens@de.ibm.com,
	linux-kernel@vger.kernel.org, linux-raid@vger.kernel.org,
	dm-devel@redhat.com, viro@zeniv.linux.org.uk,
	schwidefsky@de.ibm.com, dan.j.williams@intel.com, agk@redhat.com
In-Reply-To: <1466702773.3504.363.camel@hpe.com>

On Thu, Jun 23 2016 at  1:36pm -0400,
Kani, Toshimitsu <toshi.kani@hpe.com> wrote:

> On Thu, 2016-06-23 at 19:31 +0300, Yigal Korman wrote:
> > On Thu, Jun 23, 2016 at 2:54 AM, Toshi Kani <toshi.kani@hpe.com> wrote:
> > > 
> > > 
> > > Currently, presence of direct_access() in block_device_operations
> > > indicates support of DAX on its block device.  Because
> > > block_device_operations is instantiated with 'const', this DAX
> > > capablity may not be enabled conditinally.
> > > 
> > > In preparation for supporting DAX to device-mapper devices, add
> > > QUEUE_FLAG_DAX to request_queue flags to advertise their DAX
> > > support.  This will allow to set the DAX capability based on how
> > > mapped device is composed.
> > 
> > Hi Toshi,
> > This patch is very helpful!
> > I think QUEUE_FLAG_DAX can also help with identifying dax devices in
> > userspace.
> > Perhaps you'd be willing to squash the patch below with this one or
> > add it to your submission?
> 
> Hi Yigal,
> 
> Good idea.  Mike can probably take it into his tree, but I will include it
> into the series if I needed to submit v3.
> 
> Acked-by: Toshi Kani <toshi.kani@hpe.com>

As you can see I sent out v3 of this set.  Jens will need to pick up the
"block:" patches but I'll pick up the DM patches once Jens does so.
--
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 v2 1/3] block: add QUEUE_FLAG_DAX for devices to advertise their DAX support
From: Dan Williams @ 2016-06-23 18:10 UTC (permalink / raw)
  To: Yigal Korman
  Cc: axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org,
	linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org,
	heiko.carstens-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-raid-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org,
	schwidefsky-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org,
	agk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <CACTTzNY-sQYGGy-NhGP2hCUH2ynpDD-g74sJt4zY9c9RvGGFKw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Thu, Jun 23, 2016 at 11:05 AM, Yigal Korman <yigal-/8YdC2HfS5554TAoqtyWWQ@public.gmane.org> wrote:
> On Thu, Jun 23, 2016 at 8:36 PM, Kani, Toshimitsu <toshi.kani-ZPxbGqLxI0U@public.gmane.org> wrote:
>> On Thu, 2016-06-23 at 19:31 +0300, Yigal Korman wrote:
>>> On Thu, Jun 23, 2016 at 2:54 AM, Toshi Kani <toshi.kani-ZPxbGqLxI0U@public.gmane.org> wrote:
>>> >
>>> >
>>> > Currently, presence of direct_access() in block_device_operations
>>> > indicates support of DAX on its block device.  Because
>>> > block_device_operations is instantiated with 'const', this DAX
>>> > capablity may not be enabled conditinally.
>>> >
>>> > In preparation for supporting DAX to device-mapper devices, add
>>> > QUEUE_FLAG_DAX to request_queue flags to advertise their DAX
>>> > support.  This will allow to set the DAX capability based on how
>>> > mapped device is composed.
>>>
>>> Hi Toshi,
>>> This patch is very helpful!
>>> I think QUEUE_FLAG_DAX can also help with identifying dax devices in
>>> userspace.
>>> Perhaps you'd be willing to squash the patch below with this one or
>>> add it to your submission?
>>
>> Hi Yigal,
>>
>> Good idea.  Mike can probably take it into his tree, but I will include it
>> into the series if I needed to submit v3.
>>
>> Acked-by: Toshi Kani <toshi.kani-ZPxbGqLxI0U@public.gmane.org>
>
> Great, Thanks!
> I'll follow the thread to see if it goes through or needs to be resubmitted.
>
>>
>> I have one minor comment below.
>
> My reply below as well.
>
>>
>>> [PATCH] block: expose QUEUE_FLAG_DAX in sysfs
>>>
>>> There's currently no way to identify DAX enabled devices in userspace.
>>>
>>> Signed-off-by: Yigal Korman <yigal-/8YdC2HfS5554TAoqtyWWQ@public.gmane.org>
>>> ---
>>>  block/blk-sysfs.c | 15 +++++++++++++++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
>>> index 9920596..d55126d 100644
>>> --- a/block/blk-sysfs.c
>>> +++ b/block/blk-sysfs.c
>>> @@ -379,6 +379,15 @@ static ssize_t queue_wc_store(struct
>>> request_queue *q, const char *page,
>>>         return count;
>>>  }
>>>
>>> +static ssize_t queue_dax_show(struct request_queue *q, char *page)
>>> +{
>>> +       int bit;
>>> +
>>> +       bit = test_bit(QUEUE_FLAG_DAX, &q->queue_flags);
>>> +
>>> +       return queue_var_show(bit, page);
>>
>> This can be:
>>         return queue_var_show(blk_queue_dax(q), page);
>
> Oh, sure, of course you're right. thanks.
>
> Subject: [PATCH] block: expose QUEUE_FLAG_DAX in sysfs
>
> There's currently no way to identify DAX enabled devices in userspace.
>
> Signed-off-by: Yigal Korman <yigal-/8YdC2HfS5554TAoqtyWWQ@public.gmane.org>
> Acked-by: Toshi Kani <toshi.kani-ZPxbGqLxI0U@public.gmane.org>

Great!

Acked-by: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

^ permalink raw reply

* Re: [PATCH v2 1/3] block: add QUEUE_FLAG_DAX for devices to advertise their DAX support
From: Yigal Korman @ 2016-06-23 18:05 UTC (permalink / raw)
  To: Kani, Toshimitsu
  Cc: axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org,
	linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org,
	heiko.carstens-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-raid-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org,
	schwidefsky-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org,
	agk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <1466702773.3504.363.camel-ZPxbGqLxI0U@public.gmane.org>

On Thu, Jun 23, 2016 at 8:36 PM, Kani, Toshimitsu <toshi.kani-ZPxbGqLxI0U@public.gmane.org> wrote:
> On Thu, 2016-06-23 at 19:31 +0300, Yigal Korman wrote:
>> On Thu, Jun 23, 2016 at 2:54 AM, Toshi Kani <toshi.kani-ZPxbGqLxI0U@public.gmane.org> wrote:
>> >
>> >
>> > Currently, presence of direct_access() in block_device_operations
>> > indicates support of DAX on its block device.  Because
>> > block_device_operations is instantiated with 'const', this DAX
>> > capablity may not be enabled conditinally.
>> >
>> > In preparation for supporting DAX to device-mapper devices, add
>> > QUEUE_FLAG_DAX to request_queue flags to advertise their DAX
>> > support.  This will allow to set the DAX capability based on how
>> > mapped device is composed.
>>
>> Hi Toshi,
>> This patch is very helpful!
>> I think QUEUE_FLAG_DAX can also help with identifying dax devices in
>> userspace.
>> Perhaps you'd be willing to squash the patch below with this one or
>> add it to your submission?
>
> Hi Yigal,
>
> Good idea.  Mike can probably take it into his tree, but I will include it
> into the series if I needed to submit v3.
>
> Acked-by: Toshi Kani <toshi.kani-ZPxbGqLxI0U@public.gmane.org>

Great, Thanks!
I'll follow the thread to see if it goes through or needs to be resubmitted.

>
> I have one minor comment below.

My reply below as well.

>
>> [PATCH] block: expose QUEUE_FLAG_DAX in sysfs
>>
>> There's currently no way to identify DAX enabled devices in userspace.
>>
>> Signed-off-by: Yigal Korman <yigal-/8YdC2HfS5554TAoqtyWWQ@public.gmane.org>
>> ---
>>  block/blk-sysfs.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
>> index 9920596..d55126d 100644
>> --- a/block/blk-sysfs.c
>> +++ b/block/blk-sysfs.c
>> @@ -379,6 +379,15 @@ static ssize_t queue_wc_store(struct
>> request_queue *q, const char *page,
>>         return count;
>>  }
>>
>> +static ssize_t queue_dax_show(struct request_queue *q, char *page)
>> +{
>> +       int bit;
>> +
>> +       bit = test_bit(QUEUE_FLAG_DAX, &q->queue_flags);
>> +
>> +       return queue_var_show(bit, page);
>
> This can be:
>         return queue_var_show(blk_queue_dax(q), page);

Oh, sure, of course you're right. thanks.

Subject: [PATCH] block: expose QUEUE_FLAG_DAX in sysfs

There's currently no way to identify DAX enabled devices in userspace.

Signed-off-by: Yigal Korman <yigal-/8YdC2HfS5554TAoqtyWWQ@public.gmane.org>
Acked-by: Toshi Kani <toshi.kani-ZPxbGqLxI0U@public.gmane.org>
---
 block/blk-sysfs.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 9920596..f87a7e7 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -379,6 +379,11 @@ static ssize_t queue_wc_store(struct
request_queue *q, const char *page,
        return count;
 }

+static ssize_t queue_dax_show(struct request_queue *q, char *page)
+{
+       return queue_var_show(blk_queue_dax(q), page);
+}
+
 static struct queue_sysfs_entry queue_requests_entry = {
        .attr = {.name = "nr_requests", .mode = S_IRUGO | S_IWUSR },
        .show = queue_requests_show,
@@ -516,6 +521,11 @@ static struct queue_sysfs_entry queue_wc_entry = {
        .store = queue_wc_store,
 };

+static struct queue_sysfs_entry queue_dax_entry = {
+       .attr = {.name = "dax", .mode = S_IRUGO },
+       .show = queue_dax_show,
+};
+
 static struct attribute *default_attrs[] = {
        &queue_requests_entry.attr,
        &queue_ra_entry.attr,
@@ -542,6 +552,7 @@ static struct attribute *default_attrs[] = {
        &queue_random_entry.attr,
        &queue_poll_entry.attr,
        &queue_wc_entry.attr,
+       &queue_dax_entry.attr,
        NULL,
 };

--
1.9.3

Thanks,
Yigal

>
> Thanks,
> -Toshi
>
>> +}
>> +
>>  static struct queue_sysfs_entry queue_requests_entry = {
>>         .attr = {.name = "nr_requests", .mode = S_IRUGO | S_IWUSR },
>>         .show = queue_requests_show,
>> @@ -516,6 +525,11 @@ static struct queue_sysfs_entry queue_wc_entry = {
>>         .store = queue_wc_store,
>>  };
>>
>> +static struct queue_sysfs_entry queue_dax_entry = {
>> +       .attr = {.name = "dax", .mode = S_IRUGO },
>> +       .show = queue_dax_show,
>> +};
>> +
>>  static struct attribute *default_attrs[] = {
>>         &queue_requests_entry.attr,
>>         &queue_ra_entry.attr,
>> @@ -542,6 +556,7 @@ static struct attribute *default_attrs[] = {
>>         &queue_random_entry.attr,
>>         &queue_poll_entry.attr,
>>         &queue_wc_entry.attr,
>> +       &queue_dax_entry.attr,
>>         NULL,
>>  };
>>

^ permalink raw reply related

* Re: [PATCH v2 1/3] block: add QUEUE_FLAG_DAX for devices to advertise their DAX support
From: Kani, Toshimitsu @ 2016-06-23 17:36 UTC (permalink / raw)
  To: yigal-/8YdC2HfS5554TAoqtyWWQ@public.gmane.org
  Cc: axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org,
	linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org,
	heiko.carstens-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-raid-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org,
	schwidefsky-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org,
	agk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <CACTTzNYAxtY-d8+8Vo=Y2WpuXVaSkJk9PC7DcHXKJuU=gqvXFw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Thu, 2016-06-23 at 19:31 +0300, Yigal Korman wrote:
> On Thu, Jun 23, 2016 at 2:54 AM, Toshi Kani <toshi.kani@hpe.com> wrote:
> > 
> > 
> > Currently, presence of direct_access() in block_device_operations
> > indicates support of DAX on its block device.  Because
> > block_device_operations is instantiated with 'const', this DAX
> > capablity may not be enabled conditinally.
> > 
> > In preparation for supporting DAX to device-mapper devices, add
> > QUEUE_FLAG_DAX to request_queue flags to advertise their DAX
> > support.  This will allow to set the DAX capability based on how
> > mapped device is composed.
> 
> Hi Toshi,
> This patch is very helpful!
> I think QUEUE_FLAG_DAX can also help with identifying dax devices in
> userspace.
> Perhaps you'd be willing to squash the patch below with this one or
> add it to your submission?

Hi Yigal,

Good idea.  Mike can probably take it into his tree, but I will include it
into the series if I needed to submit v3.

Acked-by: Toshi Kani <toshi.kani@hpe.com>

I have one minor comment below.

> [PATCH] block: expose QUEUE_FLAG_DAX in sysfs
> 
> There's currently no way to identify DAX enabled devices in userspace.
> 
> Signed-off-by: Yigal Korman <yigal@plexistor.com>
> ---
>  block/blk-sysfs.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index 9920596..d55126d 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -379,6 +379,15 @@ static ssize_t queue_wc_store(struct
> request_queue *q, const char *page,
>         return count;
>  }
> 
> +static ssize_t queue_dax_show(struct request_queue *q, char *page)
> +{
> +       int bit;
> +
> +       bit = test_bit(QUEUE_FLAG_DAX, &q->queue_flags);
> +
> +       return queue_var_show(bit, page);

This can be:
	return queue_var_show(blk_queue_dax(q), page);

Thanks,
-Toshi

> +}
> +
>  static struct queue_sysfs_entry queue_requests_entry = {
>         .attr = {.name = "nr_requests", .mode = S_IRUGO | S_IWUSR },
>         .show = queue_requests_show,
> @@ -516,6 +525,11 @@ static struct queue_sysfs_entry queue_wc_entry = {
>         .store = queue_wc_store,
>  };
> 
> +static struct queue_sysfs_entry queue_dax_entry = {
> +       .attr = {.name = "dax", .mode = S_IRUGO },
> +       .show = queue_dax_show,
> +};
> +
>  static struct attribute *default_attrs[] = {
>         &queue_requests_entry.attr,
>         &queue_ra_entry.attr,
> @@ -542,6 +556,7 @@ static struct attribute *default_attrs[] = {
>         &queue_random_entry.attr,
>         &queue_poll_entry.attr,
>         &queue_wc_entry.attr,
> +       &queue_dax_entry.attr,
>         NULL,
>  };
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply

* Re: [PATCH v2 1/3] block: add QUEUE_FLAG_DAX for devices to advertise their DAX support
From: Yigal Korman @ 2016-06-23 16:31 UTC (permalink / raw)
  To: Toshi Kani
  Cc: Jens Axboe, linux-s390-u79uwXL29TY76Z2rM5mHXA,
	snitzer-H+wXaHxf7aLQT0dZR+AlfA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Heiko Carstens,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA, Alexander Viro,
	Martin Schwidefsky, agk-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <1466639694-25394-2-git-send-email-toshi.kani-ZPxbGqLxI0U@public.gmane.org>

On Thu, Jun 23, 2016 at 2:54 AM, Toshi Kani <toshi.kani-ZPxbGqLxI0U@public.gmane.org> wrote:
>
> Currently, presence of direct_access() in block_device_operations
> indicates support of DAX on its block device.  Because
> block_device_operations is instantiated with 'const', this DAX
> capablity may not be enabled conditinally.
>
> In preparation for supporting DAX to device-mapper devices, add
> QUEUE_FLAG_DAX to request_queue flags to advertise their DAX
> support.  This will allow to set the DAX capability based on how
> mapped device is composed.


Hi Toshi,
This patch is very helpful!
I think QUEUE_FLAG_DAX can also help with identifying dax devices in userspace.
Perhaps you'd be willing to squash the patch below with this one or
add it to your submission?

Thanks,
Yigal


[PATCH] block: expose QUEUE_FLAG_DAX in sysfs

There's currently no way to identify DAX enabled devices in userspace.

Signed-off-by: Yigal Korman <yigal-/8YdC2HfS5554TAoqtyWWQ@public.gmane.org>
---
 block/blk-sysfs.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 9920596..d55126d 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -379,6 +379,15 @@ static ssize_t queue_wc_store(struct
request_queue *q, const char *page,
        return count;
 }

+static ssize_t queue_dax_show(struct request_queue *q, char *page)
+{
+       int bit;
+
+       bit = test_bit(QUEUE_FLAG_DAX, &q->queue_flags);
+
+       return queue_var_show(bit, page);
+}
+
 static struct queue_sysfs_entry queue_requests_entry = {
        .attr = {.name = "nr_requests", .mode = S_IRUGO | S_IWUSR },
        .show = queue_requests_show,
@@ -516,6 +525,11 @@ static struct queue_sysfs_entry queue_wc_entry = {
        .store = queue_wc_store,
 };

+static struct queue_sysfs_entry queue_dax_entry = {
+       .attr = {.name = "dax", .mode = S_IRUGO },
+       .show = queue_dax_show,
+};
+
 static struct attribute *default_attrs[] = {
        &queue_requests_entry.attr,
        &queue_ra_entry.attr,
@@ -542,6 +556,7 @@ static struct attribute *default_attrs[] = {
        &queue_random_entry.attr,
        &queue_poll_entry.attr,
        &queue_wc_entry.attr,
+       &queue_dax_entry.attr,
        NULL,
 };

--
1.9.3

>
>
> Signed-off-by: Toshi Kani <toshi.kani-ZPxbGqLxI0U@public.gmane.org>
> Cc: Jens Axboe <axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
> Cc: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: Ross Zwisler <ross.zwisler-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Cc: Martin Schwidefsky <schwidefsky-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> Cc: Heiko Carstens <heiko.carstens-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> Cc: <linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> ---
>  drivers/block/brd.c          |    4 +++-
>  drivers/nvdimm/pmem.c        |    1 +
>  drivers/s390/block/dcssblk.c |    1 +
>  fs/block_dev.c               |    5 +++--
>  include/linux/blkdev.h       |    2 ++
>  5 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/brd.c b/drivers/block/brd.c
> index f5b0d6f..dd96a93 100644
> --- a/drivers/block/brd.c
> +++ b/drivers/block/brd.c
> @@ -509,7 +509,9 @@ static struct brd_device *brd_alloc(int i)
>         blk_queue_max_discard_sectors(brd->brd_queue, UINT_MAX);
>         brd->brd_queue->limits.discard_zeroes_data = 1;
>         queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, brd->brd_queue);
> -
> +#ifdef CONFIG_BLK_DEV_RAM_DAX
> +       queue_flag_set_unlocked(QUEUE_FLAG_DAX, brd->brd_queue);
> +#endif
>         disk = brd->brd_disk = alloc_disk(max_part);
>         if (!disk)
>                 goto out_free_queue;
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index 608fc44..53b701b 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -283,6 +283,7 @@ static int pmem_attach_disk(struct device *dev,
>         blk_queue_max_hw_sectors(q, UINT_MAX);
>         blk_queue_bounce_limit(q, BLK_BOUNCE_ANY);
>         queue_flag_set_unlocked(QUEUE_FLAG_NONROT, q);
> +       queue_flag_set_unlocked(QUEUE_FLAG_DAX, q);
>         q->queuedata = pmem;
>
>         disk = alloc_disk_node(0, nid);
> diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
> index bed53c4..093e9e1 100644
> --- a/drivers/s390/block/dcssblk.c
> +++ b/drivers/s390/block/dcssblk.c
> @@ -618,6 +618,7 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
>         dev_info->gd->driverfs_dev = &dev_info->dev;
>         blk_queue_make_request(dev_info->dcssblk_queue, dcssblk_make_request);
>         blk_queue_logical_block_size(dev_info->dcssblk_queue, 4096);
> +       queue_flag_set_unlocked(QUEUE_FLAG_DAX, dev_info->dcssblk_queue);
>
>         seg_byte_size = (dev_info->end - dev_info->start + 1);
>         set_capacity(dev_info->gd, seg_byte_size >> 9); // size in sectors
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 71ccab1..d012be4 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -493,7 +493,7 @@ long bdev_direct_access(struct block_device *bdev, struct blk_dax_ctl *dax)
>
>         if (size < 0)
>                 return size;
> -       if (!ops->direct_access)
> +       if (!blk_queue_dax(bdev_get_queue(bdev)) || !ops->direct_access)
>                 return -EOPNOTSUPP;
>         if ((sector + DIV_ROUND_UP(size, 512)) >
>                                         part_nr_sects_read(bdev->bd_part))
> @@ -1287,7 +1287,8 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
>                 bdev->bd_disk = disk;
>                 bdev->bd_queue = disk->queue;
>                 bdev->bd_contains = bdev;
> -               if (IS_ENABLED(CONFIG_BLK_DEV_DAX) && disk->fops->direct_access)
> +               if (IS_ENABLED(CONFIG_BLK_DEV_DAX) &&
> +                   blk_queue_dax(disk->queue))
>                         bdev->bd_inode->i_flags = S_DAX;
>                 else
>                         bdev->bd_inode->i_flags = 0;
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 9746d22..1493ab3 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -505,6 +505,7 @@ struct request_queue {
>  #define QUEUE_FLAG_WC         23       /* Write back caching */
>  #define QUEUE_FLAG_FUA        24       /* device supports FUA writes */
>  #define QUEUE_FLAG_FLUSH_NQ    25      /* flush not queueuable */
> +#define QUEUE_FLAG_DAX         26      /* device supports DAX */
>
>  #define QUEUE_FLAG_DEFAULT     ((1 << QUEUE_FLAG_IO_STAT) |            \
>                                  (1 << QUEUE_FLAG_STACKABLE)    |       \
> @@ -594,6 +595,7 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
>  #define blk_queue_discard(q)   test_bit(QUEUE_FLAG_DISCARD, &(q)->queue_flags)
>  #define blk_queue_secdiscard(q)        (blk_queue_discard(q) && \
>         test_bit(QUEUE_FLAG_SECDISCARD, &(q)->queue_flags))
> +#define blk_queue_dax(q)       test_bit(QUEUE_FLAG_DAX, &(q)->queue_flags)
>
>  #define blk_noretry_request(rq) \
>         ((rq)->cmd_flags & (REQ_FAILFAST_DEV|REQ_FAILFAST_TRANSPORT| \
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related

* Re: [PATCH] Documentation: fix wrong value in md.txt
From: Jonathan Corbet @ 2016-06-23 14:08 UTC (permalink / raw)
  To: Tiezhu Yang; +Cc: shli, linux-doc, linux-raid, linux-kernel
In-Reply-To: <2b0cf4c2.21fa.1555c03d3e9.Coremail.kernelpatch@126.com>

On Fri, 17 Jun 2016 09:40:02 +0800 (CST)
"Tiezhu Yang" <kernelpatch@126.com> wrote:

> I read drivers/md/raid5.c and find the following related code:
> 1) in function 'raid5_set_cache_size':
> if (size <= 16 || size > 32768)
> 	return -EINVAL;
> 2) #define NR_STRIPES		256
> 
> So the lower limit value of stripe_cache_size should be 17 and
> the default value should be 256.

Applied to the docs tree, thanks.

jon

^ permalink raw reply

* [PATCH] Fix kernel module refcount handling
From: Alexey Obitotskiy @ 2016-06-23 10:11 UTC (permalink / raw)
  To: linux-raid; +Cc: shli, Aleksey Obitotskiy

md loads raidX modules and increments module refcount each time level
has changed but does not decrement it. You are unable to unload raid0
module after reshape because raid0 reshape changes level to raid4
and back to raid0.

Signed-off-by: Aleksey Obitotskiy <aleksey.obitotskiy@intel.com>
---
 drivers/md/md.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 0963edc..c515e76 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -3590,6 +3590,8 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
 			mddev->to_remove = &md_redundancy_group;
 	}
 
+	module_put(oldpers->owner);
+
 	rdev_for_each(rdev, mddev) {
 		if (rdev->raid_disk < 0)
 			continue;
-- 
2.7.4


^ permalink raw reply related

* [PATCH v2 3/3] dm linear: add DAX support
From: Toshi Kani @ 2016-06-22 23:54 UTC (permalink / raw)
  To: snitzer, axboe, dan.j.williams
  Cc: agk, ross.zwisler, viro, toshi.kani, linux-nvdimm, dm-devel,
	linux-raid, linux-kernel
In-Reply-To: <1466639694-25394-1-git-send-email-toshi.kani@hpe.com>

Change dm-linear to implement direct_access function,
linear_direct_access(), which maps sector and calls direct_access
function of its physical target device.

Change dm-linear to sets 'dax_supported' when its target physical device
supports DAX.

Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Alasdair Kergon <agk@redhat.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 drivers/md/dm-linear.c |   23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
index 05c35aa..c7fdd1d 100644
--- a/drivers/md/dm-linear.c
+++ b/drivers/md/dm-linear.c
@@ -59,6 +59,8 @@ static int linear_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	ti->num_flush_bios = 1;
 	ti->num_discard_bios = 1;
 	ti->num_write_same_bios = 1;
+	if (blk_queue_dax(lc->dev->bdev->bd_queue))
+		ti->dax_supported = true;
 	ti->private = lc;
 	return 0;
 
@@ -141,9 +143,27 @@ static int linear_iterate_devices(struct dm_target *ti,
 	return fn(ti, lc->dev, lc->start, ti->len, data);
 }
 
+static long linear_direct_access(struct dm_target *ti, sector_t sector,
+				 void __pmem **kaddr, pfn_t *pfn, long size)
+{
+	struct linear_c *lc = ti->private;
+	struct block_device *bdev = lc->dev->bdev;
+	struct blk_dax_ctl dax = {
+		.sector = linear_map_sector(ti, sector),
+		.size = size,
+	};
+	long ret;
+
+	ret = bdev_direct_access(bdev, &dax);
+	*kaddr = dax.addr;
+	*pfn = dax.pfn;
+
+	return ret;
+}
+
 static struct target_type linear_target = {
 	.name   = "linear",
-	.version = {1, 2, 1},
+	.version = {1, 3, 0},
 	.module = THIS_MODULE,
 	.ctr    = linear_ctr,
 	.dtr    = linear_dtr,
@@ -151,6 +171,7 @@ static struct target_type linear_target = {
 	.status = linear_status,
 	.prepare_ioctl = linear_prepare_ioctl,
 	.iterate_devices = linear_iterate_devices,
+	.direct_access = linear_direct_access,
 };
 
 int __init dm_linear_init(void)

^ permalink raw reply related

* [PATCH v2 2/3] dm: add infrastructure for DAX support
From: Toshi Kani @ 2016-06-22 23:54 UTC (permalink / raw)
  To: snitzer-H+wXaHxf7aLQT0dZR+AlfA, axboe-tSWWG44O7X1aa/9Udqfwiw,
	dan.j.williams-ral2JQCrhuEAvxtiuMwx3w
  Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, agk-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <1466639694-25394-1-git-send-email-toshi.kani-ZPxbGqLxI0U@public.gmane.org>

Change mapped device to implement direct_access function,
dm_blk_direct_access(), which calls a target direct_access function.
'struct target_type' is extended to have target direct_access interface.
This function limits direct accessible size to the dm_target's limit
with max_io_len().

Extend 'struct dm_target' to have dax_supported bit, which allows
dm-table to check if a dm-target supports DAX.

Add a new dm type, DM_TYPE_DAX_BIO_BASED, which indicates that mapped
device supports DAX and is bio based.  This new type is used to assure
that all target devices have DAX support and remain that way after
QUEUE_FLAG_DAX is set in mapped device.

At initial table load, QUEUE_FLAG_DAX is set to mapped device when setting
DM_TYPE_DAX_BIO_BASED to the type.  Any subsequent table load to the
mapped device must have the same type, or else it fails per the check in
table_load().

Signed-off-by: Toshi Kani <toshi.kani-ZPxbGqLxI0U@public.gmane.org>
Cc: Alasdair Kergon <agk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Ross Zwisler <ross.zwisler-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/md/dm-table.c         |   23 ++++++++++++++++++++---
 drivers/md/dm.c               |   36 +++++++++++++++++++++++++++++++++++-
 drivers/md/dm.h               |    1 +
 include/linux/device-mapper.h |   16 ++++++++++++++++
 4 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 88f0174..aa14fc4 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -827,6 +827,12 @@ void dm_consume_args(struct dm_arg_set *as, unsigned num_args)
 }
 EXPORT_SYMBOL(dm_consume_args);
 
+static bool __table_type_bio_based(unsigned table_type)
+{
+	return (table_type == DM_TYPE_BIO_BASED ||
+		table_type == DM_TYPE_DAX_BIO_BASED);
+}
+
 static bool __table_type_request_based(unsigned table_type)
 {
 	return (table_type == DM_TYPE_REQUEST_BASED ||
@@ -842,7 +848,7 @@ EXPORT_SYMBOL_GPL(dm_table_set_type);
 static int dm_table_determine_type(struct dm_table *t)
 {
 	unsigned i;
-	unsigned bio_based = 0, request_based = 0, hybrid = 0;
+	unsigned bio_based = 0, request_based = 0, hybrid = 0, num_dax = 0;
 	bool verify_blk_mq = false;
 	struct dm_target *tgt;
 	struct dm_dev_internal *dd;
@@ -865,6 +871,9 @@ static int dm_table_determine_type(struct dm_table *t)
 		else
 			bio_based = 1;
 
+		if (tgt->dax_supported)
+			num_dax++;
+
 		if (bio_based && request_based) {
 			DMWARN("Inconsistent table: different target types"
 			       " can't be mixed up");
@@ -886,7 +895,10 @@ static int dm_table_determine_type(struct dm_table *t)
 
 	if (bio_based) {
 		/* We must use this table as bio-based */
-		t->type = DM_TYPE_BIO_BASED;
+		if (num_dax && num_dax == t->num_targets)
+			t->type = DM_TYPE_DAX_BIO_BASED;
+		else
+			t->type = DM_TYPE_BIO_BASED;
 		return 0;
 	}
 
@@ -979,6 +991,11 @@ struct dm_target *dm_table_get_wildcard_target(struct dm_table *t)
 	return NULL;
 }
 
+bool dm_table_bio_based(struct dm_table *t)
+{
+	return __table_type_bio_based(dm_table_get_type(t));
+}
+
 bool dm_table_request_based(struct dm_table *t)
 {
 	return __table_type_request_based(dm_table_get_type(t));
@@ -1001,7 +1018,7 @@ static int dm_table_alloc_md_mempools(struct dm_table *t, struct mapped_device *
 		return -EINVAL;
 	}
 
-	if (type == DM_TYPE_BIO_BASED)
+	if (__table_type_bio_based(type))
 		for (i = 0; i < t->num_targets; i++) {
 			tgt = t->targets + i;
 			per_io_data_size = max(per_io_data_size, tgt->per_io_data_size);
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 2c907bc..17cd25a 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -905,6 +905,34 @@ int dm_set_target_max_io_len(struct dm_target *ti, sector_t len)
 }
 EXPORT_SYMBOL_GPL(dm_set_target_max_io_len);
 
+static long dm_blk_direct_access(struct block_device *bdev, sector_t sector,
+				 void __pmem **kaddr, pfn_t *pfn, long size)
+{
+	struct mapped_device *md = bdev->bd_disk->private_data;
+	struct dm_table *map;
+	struct dm_target *ti;
+	int srcu_idx;
+	long len, ret = -EIO;
+
+	map = dm_get_live_table(md, &srcu_idx);
+	if (!map)
+		return ret;
+
+	ti = dm_table_find_target(map, sector);
+	if (!dm_target_is_valid(ti))
+		goto out;
+
+	len = max_io_len(sector, ti) << SECTOR_SHIFT;
+	size = min(len, size);
+
+	if (ti->type->direct_access)
+		ret = ti->type->direct_access(ti, sector, kaddr, pfn, size);
+out:
+	dm_put_live_table(md, srcu_idx);
+
+	return min(ret, size);
+}
+
 /*
  * A target may call dm_accept_partial_bio only from the map routine.  It is
  * allowed for all bio types except REQ_PREFLUSH.
@@ -1548,7 +1576,7 @@ static void __bind_mempools(struct mapped_device *md, struct dm_table *t)
 
 	if (md->bs) {
 		/* The md already has necessary mempools. */
-		if (dm_table_get_type(t) == DM_TYPE_BIO_BASED) {
+		if (dm_table_bio_based(t)) {
 			/*
 			 * Reload bioset because front_pad may have changed
 			 * because a different table was loaded.
@@ -1715,6 +1743,9 @@ void dm_set_md_type(struct mapped_device *md, unsigned type)
 {
 	BUG_ON(!mutex_is_locked(&md->type_lock));
 	md->type = type;
+
+	if (type == DM_TYPE_DAX_BIO_BASED)
+		queue_flag_set_unlocked(QUEUE_FLAG_DAX, md->queue);
 }
 
 unsigned dm_get_md_type(struct mapped_device *md)
@@ -1761,6 +1792,7 @@ int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t)
 		}
 		break;
 	case DM_TYPE_BIO_BASED:
+	case DM_TYPE_DAX_BIO_BASED:
 		dm_init_normal_md_queue(md);
 		blk_queue_make_request(md->queue, dm_make_request);
 		/*
@@ -2465,6 +2497,7 @@ struct dm_md_mempools *dm_alloc_md_mempools(struct mapped_device *md, unsigned t
 
 	switch (type) {
 	case DM_TYPE_BIO_BASED:
+	case DM_TYPE_DAX_BIO_BASED:
 		cachep = _io_cache;
 		pool_size = dm_get_reserved_bio_based_ios();
 		front_pad = roundup(per_io_data_size, __alignof__(struct dm_target_io)) + offsetof(struct dm_target_io, clone);
@@ -2641,6 +2674,7 @@ static const struct block_device_operations dm_blk_dops = {
 	.open = dm_blk_open,
 	.release = dm_blk_close,
 	.ioctl = dm_blk_ioctl,
+	.direct_access = dm_blk_direct_access,
 	.getgeo = dm_blk_getgeo,
 	.pr_ops = &dm_pr_ops,
 	.owner = THIS_MODULE
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index 2e0e4a5..f0aad08 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -68,6 +68,7 @@ unsigned dm_table_get_type(struct dm_table *t);
 struct target_type *dm_table_get_immutable_target_type(struct dm_table *t);
 struct dm_target *dm_table_get_immutable_target(struct dm_table *t);
 struct dm_target *dm_table_get_wildcard_target(struct dm_table *t);
+bool dm_table_bio_based(struct dm_table *t);
 bool dm_table_request_based(struct dm_table *t);
 bool dm_table_all_blk_mq_devices(struct dm_table *t);
 void dm_table_free_md_mempools(struct dm_table *t);
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 2ce3392..1f1a6de 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -26,6 +26,7 @@ struct bio_vec;
 #define DM_TYPE_BIO_BASED		1
 #define DM_TYPE_REQUEST_BASED		2
 #define DM_TYPE_MQ_REQUEST_BASED	3
+#define DM_TYPE_DAX_BIO_BASED		4
 
 typedef enum { STATUSTYPE_INFO, STATUSTYPE_TABLE } status_type_t;
 
@@ -124,6 +125,15 @@ typedef void (*dm_io_hints_fn) (struct dm_target *ti,
  */
 typedef int (*dm_busy_fn) (struct dm_target *ti);
 
+/*
+ * Returns:
+ *  < 0 : error
+ * >= 0 : the number of bytes accessible at the address
+ */
+typedef long (*dm_direct_access_fn) (struct dm_target *ti, sector_t sector,
+				     void __pmem **kaddr, pfn_t *pfn,
+				     long size);
+
 void dm_error(const char *message);
 
 struct dm_dev {
@@ -170,6 +180,7 @@ struct target_type {
 	dm_busy_fn busy;
 	dm_iterate_devices_fn iterate_devices;
 	dm_io_hints_fn io_hints;
+	dm_direct_access_fn direct_access;
 
 	/* For internal device-mapper use. */
 	struct list_head list;
@@ -288,6 +299,11 @@ struct dm_target {
 	 * Set if this target does not return zeroes on discarded blocks.
 	 */
 	bool discard_zeroes_data_unsupported:1;
+
+	/*
+	 * Set if the target supports DAX (direct access).
+	 */
+	bool dax_supported:1;
 };
 
 /* Each target can link one of these into the table */

^ permalink raw reply related

* [PATCH v2 1/3] block: add QUEUE_FLAG_DAX for devices to advertise their DAX support
From: Toshi Kani @ 2016-06-22 23:54 UTC (permalink / raw)
  To: snitzer-H+wXaHxf7aLQT0dZR+AlfA, axboe-tSWWG44O7X1aa/9Udqfwiw,
	dan.j.williams-ral2JQCrhuEAvxtiuMwx3w
  Cc: linux-s390-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Heiko Carstens,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, Martin Schwidefsky,
	agk-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <1466639694-25394-1-git-send-email-toshi.kani-ZPxbGqLxI0U@public.gmane.org>

Currently, presence of direct_access() in block_device_operations
indicates support of DAX on its block device.  Because
block_device_operations is instantiated with 'const', this DAX
capablity may not be enabled conditinally.

In preparation for supporting DAX to device-mapper devices, add
QUEUE_FLAG_DAX to request_queue flags to advertise their DAX
support.  This will allow to set the DAX capability based on how
mapped device is composed.

Signed-off-by: Toshi Kani <toshi.kani-ZPxbGqLxI0U@public.gmane.org>
Cc: Jens Axboe <axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
Cc: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Ross Zwisler <ross.zwisler-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: Martin Schwidefsky <schwidefsky-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
Cc: Heiko Carstens <heiko.carstens-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
Cc: <linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
 drivers/block/brd.c          |    4 +++-
 drivers/nvdimm/pmem.c        |    1 +
 drivers/s390/block/dcssblk.c |    1 +
 fs/block_dev.c               |    5 +++--
 include/linux/blkdev.h       |    2 ++
 5 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index f5b0d6f..dd96a93 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -509,7 +509,9 @@ static struct brd_device *brd_alloc(int i)
 	blk_queue_max_discard_sectors(brd->brd_queue, UINT_MAX);
 	brd->brd_queue->limits.discard_zeroes_data = 1;
 	queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, brd->brd_queue);
-
+#ifdef CONFIG_BLK_DEV_RAM_DAX
+	queue_flag_set_unlocked(QUEUE_FLAG_DAX, brd->brd_queue);
+#endif
 	disk = brd->brd_disk = alloc_disk(max_part);
 	if (!disk)
 		goto out_free_queue;
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 608fc44..53b701b 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -283,6 +283,7 @@ static int pmem_attach_disk(struct device *dev,
 	blk_queue_max_hw_sectors(q, UINT_MAX);
 	blk_queue_bounce_limit(q, BLK_BOUNCE_ANY);
 	queue_flag_set_unlocked(QUEUE_FLAG_NONROT, q);
+	queue_flag_set_unlocked(QUEUE_FLAG_DAX, q);
 	q->queuedata = pmem;
 
 	disk = alloc_disk_node(0, nid);
diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index bed53c4..093e9e1 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -618,6 +618,7 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
 	dev_info->gd->driverfs_dev = &dev_info->dev;
 	blk_queue_make_request(dev_info->dcssblk_queue, dcssblk_make_request);
 	blk_queue_logical_block_size(dev_info->dcssblk_queue, 4096);
+	queue_flag_set_unlocked(QUEUE_FLAG_DAX, dev_info->dcssblk_queue);
 
 	seg_byte_size = (dev_info->end - dev_info->start + 1);
 	set_capacity(dev_info->gd, seg_byte_size >> 9); // size in sectors
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 71ccab1..d012be4 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -493,7 +493,7 @@ long bdev_direct_access(struct block_device *bdev, struct blk_dax_ctl *dax)
 
 	if (size < 0)
 		return size;
-	if (!ops->direct_access)
+	if (!blk_queue_dax(bdev_get_queue(bdev)) || !ops->direct_access)
 		return -EOPNOTSUPP;
 	if ((sector + DIV_ROUND_UP(size, 512)) >
 					part_nr_sects_read(bdev->bd_part))
@@ -1287,7 +1287,8 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
 		bdev->bd_disk = disk;
 		bdev->bd_queue = disk->queue;
 		bdev->bd_contains = bdev;
-		if (IS_ENABLED(CONFIG_BLK_DEV_DAX) && disk->fops->direct_access)
+		if (IS_ENABLED(CONFIG_BLK_DEV_DAX) &&
+		    blk_queue_dax(disk->queue))
 			bdev->bd_inode->i_flags = S_DAX;
 		else
 			bdev->bd_inode->i_flags = 0;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 9746d22..1493ab3 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -505,6 +505,7 @@ struct request_queue {
 #define QUEUE_FLAG_WC	       23	/* Write back caching */
 #define QUEUE_FLAG_FUA	       24	/* device supports FUA writes */
 #define QUEUE_FLAG_FLUSH_NQ    25	/* flush not queueuable */
+#define QUEUE_FLAG_DAX         26	/* device supports DAX */
 
 #define QUEUE_FLAG_DEFAULT	((1 << QUEUE_FLAG_IO_STAT) |		\
 				 (1 << QUEUE_FLAG_STACKABLE)	|	\
@@ -594,6 +595,7 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
 #define blk_queue_discard(q)	test_bit(QUEUE_FLAG_DISCARD, &(q)->queue_flags)
 #define blk_queue_secdiscard(q)	(blk_queue_discard(q) && \
 	test_bit(QUEUE_FLAG_SECDISCARD, &(q)->queue_flags))
+#define blk_queue_dax(q)	test_bit(QUEUE_FLAG_DAX, &(q)->queue_flags)
 
 #define blk_noretry_request(rq) \
 	((rq)->cmd_flags & (REQ_FAILFAST_DEV|REQ_FAILFAST_TRANSPORT| \

^ permalink raw reply related

* [PATCH v2 0/3] Support DAX for device-mapper dm-linear devices
From: Toshi Kani @ 2016-06-22 23:54 UTC (permalink / raw)
  To: snitzer-H+wXaHxf7aLQT0dZR+AlfA, axboe-tSWWG44O7X1aa/9Udqfwiw,
	dan.j.williams-ral2JQCrhuEAvxtiuMwx3w
  Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, agk-H+wXaHxf7aLQT0dZR+AlfA

This patch-set adds DAX support to device-mapper dm-linear devices
used by LVM.  It works with LVM commands as follows:
 - Creation of a logical volume with all DAX capable devices (such
   as pmem) sets the logical volume DAX capable as well.
 - Once a logical volume is set to DAX capable, the volume may not
   be extended with non-DAX capable devices.

The direct_access interface is added to dm and dm-linear to map
a request to a target device.

The patches replace v1 series at:
http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip

v2:
 - Fix issue with partitioning.
 - Change to add DAX flag to a request queue.

---
Toshi Kani (3):
 1/3 block: add QUEUE_FLAG_DAX for devices to advertise their DAX support
 2/3 dm: add infrastructure for DAX support
 3/3 dm linear: add DAX support

---
 drivers/block/brd.c           |  4 +++-
 drivers/md/dm-linear.c        | 23 ++++++++++++++++++++++-
 drivers/md/dm-table.c         | 23 ++++++++++++++++++++---
 drivers/md/dm.c               | 36 +++++++++++++++++++++++++++++++++++-
 drivers/md/dm.h               |  1 +
 drivers/nvdimm/pmem.c         |  1 +
 drivers/s390/block/dcssblk.c  |  1 +
 fs/block_dev.c                |  5 +++--
 include/linux/blkdev.h        |  2 ++
 include/linux/device-mapper.h | 16 ++++++++++++++++
 10 files changed, 104 insertions(+), 8 deletions(-)

^ permalink raw reply

* Re: [PATCH 0/6] Support DAX for device-mapper dm-linear devices
From: Kani, Toshimitsu @ 2016-06-22 22:59 UTC (permalink / raw)
  To: snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
  Cc: axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org,
	sandeen-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	axboe-b10kYP2dOMg@public.gmane.org,
	linux-nvdimm-y27Ovi1pjclAfugRpC6u6w@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-raid-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org,
	agk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <20160622223842.GA34512-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Wed, 2016-06-22 at 18:38 -0400, Mike Snitzer wrote:
> On Wed, Jun 22 2016 at  4:16P -0400,
> Kani, Toshimitsu <toshi.kani@hpe.com> wrote:
> > 
> > On Wed, 2016-06-22 at 12:15 -0700, Dan Williams wrote:
> > > 
> > > On Wed, Jun 22, 2016 at 10:44 AM, Kani, Toshimitsu <toshi.kani@hpe.com>
> > > wrote:
> > > > 
> > > > On Tue, 2016-06-21 at 14:17 -0400, Mike Snitzer wrote:
> > > > > 
> > > > > On Tue, Jun 21 2016 at 11:44am -0400,
> > > > > Kani, Toshimitsu <toshi.kani@hpe.com> wrote:
> > > > > > 
> > > > > > On Tue, 2016-06-21 at 09:41 -0400, Mike Snitzer wrote:
> > > > > > > 
> > > > > The devices in question have a request_queue.  All bio-based device
> > > > > have a request_queue.
> > > > 
> > > > DAX-capable devices have two operation modes, bio-based and DAX.  I
> > > > agree that bio-based operation is associated with a request queue, and
> > > > its capabilities should be set to it.  DAX, on the other hand, is
> > > > rather independent from a request queue.
> > > > 
> > > > > 
> > > > > I don't have a big problem with GENHD_FL_DAX.  Just wanted to point
> > > > > out that such block device capabilities are generally advertised in
> > > > > terms of a QUEUE_FLAG.
> > > > 
> > > > I do not have a strong opinion, but feel a bit odd to associate DAX to
> > > > a request queue.
> > >
> > > Given that we do not support dax to a raw block device [1] it seems a
> > > gendisk flag is more misleading than request_queue flag that specifies
> > > what requests can be made of the device.
> > > 
> > > [1]: acc93d30d7d4 Revert "block: enable dax for raw block devices"
> >
> > Oh, I see.  I will change to use request_queue flag.
>
> I implemented the block patch for this yesterday but based on your
> feedback I stopped there (didn't carry the change through to the DM core
> and DM linear -- can easily do so tomorrow though).
> 
> Feel free to use this as a starting point and fix/extend and add a
> proper header:

Thanks Mike! I made similar changes as well. I will take yours and finish up
the rest. :)
-Toshi
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply

* Re: [PATCH 0/6] Support DAX for device-mapper dm-linear devices
From: Mike Snitzer @ 2016-06-22 22:38 UTC (permalink / raw)
  To: Kani, Toshimitsu
  Cc: dan.j.williams@intel.com, linux-kernel@vger.kernel.org,
	sandeen@redhat.com, linux-nvdimm@ml01.01.org, agk@redhat.com,
	linux-raid@vger.kernel.org, viro@zeniv.linux.org.uk,
	axboe@kernel.dk, axboe@fb.com, ross.zwisler@linux.intel.com,
	dm-devel@redhat.com
In-Reply-To: <1466625958.3504.340.camel@hpe.com>

On Wed, Jun 22 2016 at  4:16P -0400,
Kani, Toshimitsu <toshi.kani@hpe.com> wrote:

> On Wed, 2016-06-22 at 12:15 -0700, Dan Williams wrote:
> > On Wed, Jun 22, 2016 at 10:44 AM, Kani, Toshimitsu <toshi.kani@hpe.com>
> > wrote:
> > > On Tue, 2016-06-21 at 14:17 -0400, Mike Snitzer wrote:
> > > > 
> > > > On Tue, Jun 21 2016 at 11:44am -0400,
> > > > Kani, Toshimitsu <toshi.kani@hpe.com> wrote:
> > > > > 
> > > > > On Tue, 2016-06-21 at 09:41 -0400, Mike Snitzer wrote:
> > > > > > On Mon, Jun 20 2016 at  6:22pm -0400,
> > > > > > Mike Snitzer <snitzer@redhat.com> wrote:
> > > > > > I'm now wondering if we'd be better off setting a new QUEUE_FLAG_DAX
> > > > > > rather than establish GENHD_FL_DAX on the genhd?
> > > > > > 
> > > > > > It'd be quite a bit easier to allow upper layers (e.g. XFS and ext4)
> > > > > > to check for a queue flag.
> > > > > 
> > > > > I think GENHD_FL_DAX is more appropriate since DAX does not use a
> > > > > request queue, except for protecting the underlining device being
> > > > > disabled while direct_access() is called (b2e0d1625e19).
> > > > 
> > > > The devices in question have a request_queue.  All bio-based device have
> > > > a request_queue.
> > >
> > > DAX-capable devices have two operation modes, bio-based and DAX.  I agree
> > > that bio-based operation is associated with a request queue, and its
> > > capabilities should be set to it.  DAX, on the other hand, is rather
> > > independent from a request queue.
> > > 
> > > > I don't have a big problem with GENHD_FL_DAX.  Just wanted to point out
> > > > that such block device capabilities are generally advertised in terms of
> > > > a QUEUE_FLAG.
> > >
> > > I do not have a strong opinion, but feel a bit odd to associate DAX to a
> > > request queue.
> >
> > Given that we do not support dax to a raw block device [1] it seems a
> > gendisk flag is more misleading than request_queue flag that specifies
> > what requests can be made of the device.
> > 
> > [1]: acc93d30d7d4 Revert "block: enable dax for raw block devices"
> 
> Oh, I see.  I will change to use request_queue flag.

I implemented the block patch for this yesterday but based on your
feedback I stopped there (didn't carry the change through to the DM core
and DM linear -- can easily do so tomorrow though).

Feel free to use this as a starting point and fix/extend and add a
proper header:

From e88736ce322f248157da6c7d402e940adafffa1e Mon Sep 17 00:00:00 2001
From: Mike Snitzer <snitzer@redhat.com>
Date: Tue, 21 Jun 2016 12:23:29 -0400
Subject: [PATCH] block: add QUEUE_FLAG_DAX for devices to advertise their DAX support

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/block/brd.c          | 3 +++
 drivers/nvdimm/pmem.c        | 1 +
 drivers/s390/block/dcssblk.c | 1 +
 fs/block_dev.c               | 5 +++--
 include/linux/blkdev.h       | 2 ++
 5 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index f5b0d6f..13eee12 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -508,6 +508,9 @@ static struct brd_device *brd_alloc(int i)
 	brd->brd_queue->limits.discard_granularity = PAGE_SIZE;
 	blk_queue_max_discard_sectors(brd->brd_queue, UINT_MAX);
 	brd->brd_queue->limits.discard_zeroes_data = 1;
+#ifdef CONFIG_BLK_DEV_RAM_DAX
+	queue_flag_set_unlocked(QUEUE_FLAG_DAX, brd->brd_queue);
+#endif
 	queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, brd->brd_queue);
 
 	disk = brd->brd_disk = alloc_disk(max_part);
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 608fc44..53b701b 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -283,6 +283,7 @@ static int pmem_attach_disk(struct device *dev,
 	blk_queue_max_hw_sectors(q, UINT_MAX);
 	blk_queue_bounce_limit(q, BLK_BOUNCE_ANY);
 	queue_flag_set_unlocked(QUEUE_FLAG_NONROT, q);
+	queue_flag_set_unlocked(QUEUE_FLAG_DAX, q);
 	q->queuedata = pmem;
 
 	disk = alloc_disk_node(0, nid);
diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index bed53c4..093e9e1 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -618,6 +618,7 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
 	dev_info->gd->driverfs_dev = &dev_info->dev;
 	blk_queue_make_request(dev_info->dcssblk_queue, dcssblk_make_request);
 	blk_queue_logical_block_size(dev_info->dcssblk_queue, 4096);
+	queue_flag_set_unlocked(QUEUE_FLAG_DAX, dev_info->dcssblk_queue);
 
 	seg_byte_size = (dev_info->end - dev_info->start + 1);
 	set_capacity(dev_info->gd, seg_byte_size >> 9); // size in sectors
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 71ccab1..9bcb3a9 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -484,6 +484,7 @@ long bdev_direct_access(struct block_device *bdev, struct blk_dax_ctl *dax)
 	sector_t sector = dax->sector;
 	long avail, size = dax->size;
 	const struct block_device_operations *ops = bdev->bd_disk->fops;
+	struct request_queue *q = bdev_get_queue(bdev);
 
 	/*
 	 * The device driver is allowed to sleep, in order to make the
@@ -493,7 +494,7 @@ long bdev_direct_access(struct block_device *bdev, struct blk_dax_ctl *dax)
 
 	if (size < 0)
 		return size;
-	if (!ops->direct_access)
+	if (!blk_queue_dax(q) || !ops->direct_access)
 		return -EOPNOTSUPP;
 	if ((sector + DIV_ROUND_UP(size, 512)) >
 					part_nr_sects_read(bdev->bd_part))
@@ -1287,7 +1288,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
 		bdev->bd_disk = disk;
 		bdev->bd_queue = disk->queue;
 		bdev->bd_contains = bdev;
-		if (IS_ENABLED(CONFIG_BLK_DEV_DAX) && disk->fops->direct_access)
+		if (IS_ENABLED(CONFIG_BLK_DEV_DAX) && blk_queue_dax(disk->queue))
 			bdev->bd_inode->i_flags = S_DAX;
 		else
 			bdev->bd_inode->i_flags = 0;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 9746d22..d5cb326 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -505,6 +505,7 @@ struct request_queue {
 #define QUEUE_FLAG_WC	       23	/* Write back caching */
 #define QUEUE_FLAG_FUA	       24	/* device supports FUA writes */
 #define QUEUE_FLAG_FLUSH_NQ    25	/* flush not queueuable */
+#define QUEUE_FLAG_DAX	       26	/* device supports DAX */
 
 #define QUEUE_FLAG_DEFAULT	((1 << QUEUE_FLAG_IO_STAT) |		\
 				 (1 << QUEUE_FLAG_STACKABLE)	|	\
@@ -594,6 +595,7 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
 #define blk_queue_discard(q)	test_bit(QUEUE_FLAG_DISCARD, &(q)->queue_flags)
 #define blk_queue_secdiscard(q)	(blk_queue_discard(q) && \
 	test_bit(QUEUE_FLAG_SECDISCARD, &(q)->queue_flags))
+#define blk_queue_dax(q)	test_bit(QUEUE_FLAG_DAX, &(q)->queue_flags)
 
 #define blk_noretry_request(rq) \
 	((rq)->cmd_flags & (REQ_FAILFAST_DEV|REQ_FAILFAST_TRANSPORT| \
-- 
2.7.4 (Apple Git-66)

^ permalink raw reply related

* Re: [PATCH 0/6] Support DAX for device-mapper dm-linear devices
From: Kani, Toshimitsu @ 2016-06-22 20:16 UTC (permalink / raw)
  To: dan.j.williams@intel.com
  Cc: linux-kernel@vger.kernel.org, sandeen@redhat.com,
	linux-nvdimm@ml01.01.org, agk@redhat.com,
	linux-raid@vger.kernel.org, snitzer@redhat.com,
	viro@zeniv.linux.org.uk, axboe@kernel.dk, axboe@fb.com,
	ross.zwisler@linux.intel.com, dm-devel@redhat.com
In-Reply-To: <CAPcyv4gqiNQ-FRqCV3WxBzjUBZNY6eZVA9ioc0q+Lm=oG8bWAg@mail.gmail.com>

On Wed, 2016-06-22 at 12:15 -0700, Dan Williams wrote:
> On Wed, Jun 22, 2016 at 10:44 AM, Kani, Toshimitsu <toshi.kani@hpe.com>
> wrote:
> > On Tue, 2016-06-21 at 14:17 -0400, Mike Snitzer wrote:
> > > 
> > > On Tue, Jun 21 2016 at 11:44am -0400,
> > > Kani, Toshimitsu <toshi.kani@hpe.com> wrote:
> > > > 
> > > > On Tue, 2016-06-21 at 09:41 -0400, Mike Snitzer wrote:
> > > > > On Mon, Jun 20 2016 at  6:22pm -0400,
> > > > > Mike Snitzer <snitzer@redhat.com> wrote:
> > > > > I'm now wondering if we'd be better off setting a new QUEUE_FLAG_DAX
> > > > > rather than establish GENHD_FL_DAX on the genhd?
> > > > > 
> > > > > It'd be quite a bit easier to allow upper layers (e.g. XFS and ext4)
> > > > > to check for a queue flag.
> > > > 
> > > > I think GENHD_FL_DAX is more appropriate since DAX does not use a
> > > > request queue, except for protecting the underlining device being
> > > > disabled while direct_access() is called (b2e0d1625e19).
> > > 
> > > The devices in question have a request_queue.  All bio-based device have
> > > a request_queue.
> >
> > DAX-capable devices have two operation modes, bio-based and DAX.  I agree
> > that bio-based operation is associated with a request queue, and its
> > capabilities should be set to it.  DAX, on the other hand, is rather
> > independent from a request queue.
> > 
> > > I don't have a big problem with GENHD_FL_DAX.  Just wanted to point out
> > > that such block device capabilities are generally advertised in terms of
> > > a QUEUE_FLAG.
> >
> > I do not have a strong opinion, but feel a bit odd to associate DAX to a
> > request queue.
>
> Given that we do not support dax to a raw block device [1] it seems a
> gendisk flag is more misleading than request_queue flag that specifies
> what requests can be made of the device.
> 
> [1]: acc93d30d7d4 Revert "block: enable dax for raw block devices"

Oh, I see.  I will change to use request_queue flag.


> > > > About protecting direct_access, this patch assumes that the
> > > > underlining device cannot be disabled until dtr() is called.  Is this
> > > > correct?  If not, I will need to call dax_map_atomic().
> > >
> > > One of the big design considerations for DM that a DM device can be
> > > suspended (with or without flush) and any new IO will be blocked until
> > > the DM device is resumed.
> > > 
> > > So ideally DM should be able to have the same capability even if using
> > > DAX.
> >
> > Supporting suspend for DAX is challenging since it allows user
> > applications to access a device directly.  Once a device range is mmap'd,
> > there is no kernel intervention to access the range, unless we invalidate
> > user mappings.  This isn't done today even after a driver is unbind'd from
> > a device.
> > 
> > > But that is different than what commit b2e0d1625e19 is addressing.  For
> > > DM, I wouldn't think you'd need the extra protections that
> > > dax_map_atomic() is providing given that the underlying block device
> > > lifetime is managed via DM core's dm_get_device/dm_put_device (see also:
> > > dm.c:open_table_device/close_table_device).
> >
> > I thought so as well.  But I realized that there is (almost) nothing that
> > can prevent the unbind operation.  It cannot fail, either.  This unbind
> > proceeds even when a device is in-use.  In case of a pmem device, it is
> > only protected by pmem_release_queue(), which is called when a pmem device
> > is being deleted and calls blk_cleanup_queue() to serialize a critical
> > section between
> > blk_queue_enter() and blk_queue_exit() per b2e0d1625e19.  This prevents
> > from a kernel DTLB fault, but does not prevent a device disappeared while
> > in-use.
> > 
> > Protecting DM's underlining device with blk_queue_enter() (or something
> > similar) requires more thoughts...  blk_queue_enter() to a DM device
> > cannot be redirected to its underlining device.  So, this is TBD for
> > now.  But I do not think this is a blocker issue since doing unbind to a
> > underlining device is quite harmful no matter what we do - even if it is
> > protected with blk_queue_enter().
>
> I still have the "block device removed" notification patches on my
> todo list.  It's not a blocker, but there are scenarios where we can
> keep accessing memory via dax of a disabled device leading to memory
> corruption.  

Right, I noticed that user applications can access mmap'd ranges on a disabled
device.

> I'll bump that up in my queue now that we are looking at
> additional scenarios where letting DAX mappings leak past the
> reconfiguration of a block device could lead to trouble.

Great.  With DM, removing a underlining device while in-use can lead to
trouble, esp. with RAID0.  Users need to remove a device from DM first...

Thanks,
-Toshi

^ permalink raw reply

* Re: [PATCH 0/6] Support DAX for device-mapper dm-linear devices
From: Dan Williams @ 2016-06-22 19:15 UTC (permalink / raw)
  To: Kani, Toshimitsu
  Cc: axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org,
	sandeen-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	axboe-b10kYP2dOMg@public.gmane.org,
	linux-nvdimm-y27Ovi1pjclAfugRpC6u6w@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-raid-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org,
	agk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <1466616868.3504.320.camel-ZPxbGqLxI0U@public.gmane.org>

On Wed, Jun 22, 2016 at 10:44 AM, Kani, Toshimitsu <toshi.kani-ZPxbGqLxI0U@public.gmane.org> wrote:
> On Tue, 2016-06-21 at 14:17 -0400, Mike Snitzer wrote:
>> On Tue, Jun 21 2016 at 11:44am -0400,
>> Kani, Toshimitsu <toshi.kani-ZPxbGqLxI0U@public.gmane.org> wrote:
>> >
>> > On Tue, 2016-06-21 at 09:41 -0400, Mike Snitzer wrote:
>> > >
>> > > On Mon, Jun 20 2016 at  6:22pm -0400,
>> > > Mike Snitzer <snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>  :
>> > > I'm now wondering if we'd be better off setting a new QUEUE_FLAG_DAX
>> > > rather than establish GENHD_FL_DAX on the genhd?
>> > >
>> > > It'd be quite a bit easier to allow upper layers (e.g. XFS and ext4) to
>> > > check for a queue flag.
>> >
>> > I think GENHD_FL_DAX is more appropriate since DAX does not use a request
>> > queue, except for protecting the underlining device being disabled while
>> > direct_access() is called (b2e0d1625e19).
>>
>> The devices in question have a request_queue.  All bio-based device have
>> a request_queue.
>
> DAX-capable devices have two operation modes, bio-based and DAX.  I agree that
> bio-based operation is associated with a request queue, and its capabilities
> should be set to it.  DAX, on the other hand, is rather independent from a
> request queue.
>
>> I don't have a big problem with GENHD_FL_DAX.  Just wanted to point out
>> that such block device capabilities are generally advertised in terms of
>> a QUEUE_FLAG.
>
> I do not have a strong opinion, but feel a bit odd to associate DAX to a
> request queue.

Given that we do not support dax to a raw block device [1] it seems a
gendisk flag is more misleading than request_queue flag that specifies
what requests can be made of the device.

[1]: acc93d30d7d4 Revert "block: enable dax for raw block devices"


>> > About protecting direct_access, this patch assumes that the underlining
>> > device cannot be disabled until dtr() is called.  Is this correct?  If
>> > not, I will need to call dax_map_atomic().
>>
>> One of the big design considerations for DM that a DM device can be
>> suspended (with or without flush) and any new IO will be blocked until
>> the DM device is resumed.
>>
>> So ideally DM should be able to have the same capability even if using
>> DAX.
>
> Supporting suspend for DAX is challenging since it allows user applications to
> access a device directly.  Once a device range is mmap'd, there is no kernel
> intervention to access the range, unless we invalidate user mappings.  This
> isn't done today even after a driver is unbind'd from a device.
>
>> But that is different than what commit b2e0d1625e19 is addressing.  For
>> DM, I wouldn't think you'd need the extra protections that
>> dax_map_atomic() is providing given that the underlying block device
>> lifetime is managed via DM core's dm_get_device/dm_put_device (see also:
>> dm.c:open_table_device/close_table_device).
>
> I thought so as well.  But I realized that there is (almost) nothing that can
> prevent the unbind operation.  It cannot fail, either.  This unbind proceeds
> even when a device is in-use.  In case of a pmem device, it is only protected
> by pmem_release_queue(), which is called when a pmem device is being deleted
> and calls blk_cleanup_queue() to serialize a critical section between
> blk_queue_enter() and blk_queue_exit() per b2e0d1625e19.  This prevents from a
> kernel DTLB fault, but does not prevent a device disappeared while in-use.
>
> Protecting DM's underlining device with blk_queue_enter() (or something
> similar) requires more thoughts...  blk_queue_enter() to a DM device cannot be
> redirected to its underlining device.  So, this is TBD for now.  But I do not
> think this is a blocker issue since doing unbind to a underlining device is
> quite harmful no matter what we do - even if it is protected with
> blk_queue_enter().

I still have the "block device removed" notification patches on my
todo list.  It's not a blocker, but there are scenarios where we can
keep accessing memory via dax of a disabled device leading to memory
corruption.  I'll bump that up in my queue now that we are looking at
additional scenarios where letting DAX mappings leak past the
reconfiguration of a block device could lead to trouble.

^ permalink raw reply

* Re: [PATCH 0/6] Support DAX for device-mapper dm-linear devices
From: Kani, Toshimitsu @ 2016-06-22 17:44 UTC (permalink / raw)
  To: snitzer@redhat.com
  Cc: axboe@kernel.dk, sandeen@redhat.com, axboe@fb.com,
	linux-nvdimm@ml01.01.org, linux-kernel@vger.kernel.org,
	linux-raid@vger.kernel.org, dm-devel@redhat.com,
	viro@zeniv.linux.org.uk, dan.j.williams@intel.com,
	ross.zwisler@linux.intel.com, agk@redhat.com
In-Reply-To: <20160621181728.GA27821@redhat.com>

On Tue, 2016-06-21 at 14:17 -0400, Mike Snitzer wrote:
> On Tue, Jun 21 2016 at 11:44am -0400,
> Kani, Toshimitsu <toshi.kani@hpe.com> wrote:
> > 
> > On Tue, 2016-06-21 at 09:41 -0400, Mike Snitzer wrote:
> > > 
> > > On Mon, Jun 20 2016 at  6:22pm -0400,
> > > Mike Snitzer <snitzer@redhat.com> wrote:
 :
> > > I'm now wondering if we'd be better off setting a new QUEUE_FLAG_DAX
> > > rather than establish GENHD_FL_DAX on the genhd?
> > > 
> > > It'd be quite a bit easier to allow upper layers (e.g. XFS and ext4) to
> > > check for a queue flag.
> >
> > I think GENHD_FL_DAX is more appropriate since DAX does not use a request
> > queue, except for protecting the underlining device being disabled while
> > direct_access() is called (b2e0d1625e19).  
>
> The devices in question have a request_queue.  All bio-based device have
> a request_queue.

DAX-capable devices have two operation modes, bio-based and DAX.  I agree that
bio-based operation is associated with a request queue, and its capabilities
should be set to it.  DAX, on the other hand, is rather independent from a
request queue.

> I don't have a big problem with GENHD_FL_DAX.  Just wanted to point out
> that such block device capabilities are generally advertised in terms of
> a QUEUE_FLAG.

I do not have a strong opinion, but feel a bit odd to associate DAX to a
request queue. 
 
> > About protecting direct_access, this patch assumes that the underlining
> > device cannot be disabled until dtr() is called.  Is this correct?  If
> > not, I will need to call dax_map_atomic().
>
> One of the big design considerations for DM that a DM device can be
> suspended (with or without flush) and any new IO will be blocked until
> the DM device is resumed.
> 
> So ideally DM should be able to have the same capability even if using
> DAX.

Supporting suspend for DAX is challenging since it allows user applications to
access a device directly.  Once a device range is mmap'd, there is no kernel
intervention to access the range, unless we invalidate user mappings.  This
isn't done today even after a driver is unbind'd from a device.

> But that is different than what commit b2e0d1625e19 is addressing.  For
> DM, I wouldn't think you'd need the extra protections that
> dax_map_atomic() is providing given that the underlying block device
> lifetime is managed via DM core's dm_get_device/dm_put_device (see also:
> dm.c:open_table_device/close_table_device).

I thought so as well.  But I realized that there is (almost) nothing that can
prevent the unbind operation.  It cannot fail, either.  This unbind proceeds
even when a device is in-use.  In case of a pmem device, it is only protected
by pmem_release_queue(), which is called when a pmem device is being deleted
and calls blk_cleanup_queue() to serialize a critical section between
blk_queue_enter() and blk_queue_exit() per b2e0d1625e19.  This prevents from a
kernel DTLB fault, but does not prevent a device disappeared while in-use.

Protecting DM's underlining device with blk_queue_enter() (or something
similar) requires more thoughts...  blk_queue_enter() to a DM device cannot be
redirected to its underlining device.  So, this is TBD for now.  But I do not
think this is a blocker issue since doing unbind to a underlining device is
quite harmful no matter what we do - even if it is protected with
blk_queue_enter().

Thanks,
-Toshi

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

^ permalink raw reply

* [RFC] block: fix blk_queue_split() resource exhaustion
From: Lars Ellenberg @ 2016-06-22  8:22 UTC (permalink / raw)
  To: linux-block
  Cc: Jens Axboe, linux-raid, linux-kernel, Martin K. Petersen,
	Mike Snitzer, Peter Zijlstra, Jiri Kosina, Ming Lei, linux-bcache,
	NeilBrown, Zheng Liu, Keith Busch, Takashi Iwai, dm-devel,
	Ingo Molnar, Kirill A. Shutemov, Lars Ellenberg, Shaohua Li,
	Kent Overstreet, Alasdair Kergon, Roland Kammerer

For a long time, generic_make_request() converts recursion into
iteration by queuing recursive arguments on current->bio_list.

This is convenient for stacking drivers,
the top-most driver would take the originally submitted bio,
and re-submit a re-mapped version of it, or one or more clones,
or one or more new allocated bios to its backend(s). Which
are then simply processed in turn, and each can again queue
more "backend-bios" until we reach the bottom of the driver stack,
and actually dispatch to the real backend device.

Any stacking driver ->make_request_fn() could expect that,
once it returns, any backend-bios it submitted via recursive calls
to generic_make_request() would now be processed and dispatched, before
the current task would call into this driver again.

This is changed by commit
  54efd50 block: make generic_make_request handle arbitrarily sized bios

Drivers may call blk_queue_split() inside their ->make_request_fn(),
which may split the current bio into a front-part to be dealt with
immediately, and a remainder-part, which may need to be split even
further. That remainder-part will simply also be pushed to
current->bio_list, and would end up being head-of-queue, in front
of any backend-bios the current make_request_fn() might submit during
processing of the fron-part.

Which means the current task would immediately end up back in the same
make_request_fn() of the same driver again, before any of its backend
bios have even been processed.

This can lead to resource starvation deadlock.
Drivers could avoid this by learning to not need blk_queue_split(),
or by submitting their backend bios in a different context (dedicated
kernel thread, work_queue context, ...). Or by playing funny re-ordering
games with entries on current->bio_list.

Instead, I suggest to distinguish between recursive calls to
generic_make_request(), and pushing back the remainder part in
blk_queue_split(), by pointing current->bio_lists to a
	struct recursion_to_iteration_bio_lists {
		struct bio_list recursion;
		struct bio_list remainder;
	}

To have all bios targeted to drivers lower in the stack processed before
processing the next piece of a bio targeted at the higher levels,
as long as queued bios resulting from recursion are available,
they will continue to be processed in FIFO order.
Pushed back bio-parts resulting from blk_queue_split() will be processed
in LIFO order, one-by-one, whenever the recursion list becomes empty.

Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
Signed-off-by: Roland Kammerer <roland.kammerer@linbit.com>
---

This is not a theoretical problem.
At least int DRBD, and an unfortunately high IO concurrency wrt. the
"max-buffers" setting, without this patch we have a reproducible deadlock.

I'm unsure if it could cause problems in md raid in some corner cases
or when a rebuild/scrub/... starts in a "bad" moment.

It also may increase "stress" on any involved bio_set,
because it may increase the number of bios that are allocated
before the first of them is actually processed, causing more
frequent calls of punt_bios_to_rescuer().

Alternatively,
a simple one-line change to generic_make_request() would also "fix" it,
by processing all recursive bios in LIFO order.
But that may change the order in which bios reach the "physical" layer,
in case they are in fact split into many smaller pieces, for example in
a striping setup, which may have other performance implications.

 | diff --git a/block/blk-core.c b/block/blk-core.c
 | index 2475b1c7..a5623f6 100644
 | --- a/block/blk-core.c
 | +++ b/block/blk-core.c
 | @@ -2048,7 +2048,7 @@ blk_qc_t generic_make_request(struct bio *bio)
 |  	 * should be added at the tail
 |  	 */
 |  	if (current->bio_list) {
 | -		bio_list_add(current->bio_list, bio);
 | +		bio_list_add_head(current->bio_list, bio);
 |  		goto out;
 |  	}

Any fix would need to go to stable 4.3.x and up.
This patch does apply as is to 4.5 and up,
with a trivial fixup (or a cherry-pick of cda2264) to 4.4,
and with some more (also trivial) fixup to 4.3 because of
GFP_WAIT -> GFP_RECLAIM and comment rewrap.

Any input?
How should I proceed?

---
 block/bio.c               | 14 +++++++-------
 block/blk-core.c          | 32 +++++++++++++++++---------------
 block/blk-merge.c         |  3 ++-
 drivers/md/bcache/btree.c | 12 ++++++------
 drivers/md/dm-bufio.c     |  2 +-
 drivers/md/md.h           |  7 +++++++
 drivers/md/raid1.c        |  5 ++---
 drivers/md/raid10.c       |  5 ++---
 include/linux/bio.h       | 11 +++++++++++
 include/linux/sched.h     |  4 ++--
 10 files changed, 57 insertions(+), 38 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 0e4aa42..2ffcea0 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -368,10 +368,10 @@ static void punt_bios_to_rescuer(struct bio_set *bs)
 	bio_list_init(&punt);
 	bio_list_init(&nopunt);
 
-	while ((bio = bio_list_pop(current->bio_list)))
+	while ((bio = bio_list_pop(&current->bio_lists->recursion)))
 		bio_list_add(bio->bi_pool == bs ? &punt : &nopunt, bio);
 
-	*current->bio_list = nopunt;
+	current->bio_lists->recursion = nopunt;
 
 	spin_lock(&bs->rescue_lock);
 	bio_list_merge(&bs->rescue_list, &punt);
@@ -453,13 +453,13 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, int nr_iovecs, struct bio_set *bs)
 		 *
 		 * We solve this, and guarantee forward progress, with a rescuer
 		 * workqueue per bio_set. If we go to allocate and there are
-		 * bios on current->bio_list, we first try the allocation
-		 * without __GFP_DIRECT_RECLAIM; if that fails, we punt those
-		 * bios we would be blocking to the rescuer workqueue before
-		 * we retry with the original gfp_flags.
+		 * bios on current->bio_lists->recursion, we first try the
+		 * allocation without __GFP_DIRECT_RECLAIM; if that fails, we
+		 * punt those bios we would be blocking to the rescuer
+		 * workqueue before we retry with the original gfp_flags.
 		 */
 
-		if (current->bio_list && !bio_list_empty(current->bio_list))
+		if (current->bio_lists && !bio_list_empty(&current->bio_lists->recursion))
 			gfp_mask &= ~__GFP_DIRECT_RECLAIM;
 
 		p = mempool_alloc(bs->bio_pool, gfp_mask);
diff --git a/block/blk-core.c b/block/blk-core.c
index 2475b1c7..f03ff4c 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2031,7 +2031,7 @@ end_io:
  */
 blk_qc_t generic_make_request(struct bio *bio)
 {
-	struct bio_list bio_list_on_stack;
+	struct recursion_to_iteration_bio_lists bio_lists_on_stack;
 	blk_qc_t ret = BLK_QC_T_NONE;
 
 	if (!generic_make_request_checks(bio))
@@ -2040,15 +2040,18 @@ blk_qc_t generic_make_request(struct bio *bio)
 	/*
 	 * We only want one ->make_request_fn to be active at a time, else
 	 * stack usage with stacked devices could be a problem.  So use
-	 * current->bio_list to keep a list of requests submited by a
-	 * make_request_fn function.  current->bio_list is also used as a
+	 * current->bio_lists to keep a list of requests submited by a
+	 * make_request_fn function.  current->bio_lists is also used as a
 	 * flag to say if generic_make_request is currently active in this
 	 * task or not.  If it is NULL, then no make_request is active.  If
 	 * it is non-NULL, then a make_request is active, and new requests
-	 * should be added at the tail
+	 * should be added at the tail of current->bio_lists->recursion;
+	 * remainder bios resulting from a call to bio_queue_split() from
+	 * within ->make_request_fn() should be added to the head of
+	 * current->bio_lists->remainder.
 	 */
-	if (current->bio_list) {
-		bio_list_add(current->bio_list, bio);
+	if (current->bio_lists) {
+		bio_list_add(&current->bio_lists->recursion, bio);
 		goto out;
 	}
 
@@ -2057,7 +2060,7 @@ blk_qc_t generic_make_request(struct bio *bio)
 	 * Before entering the loop, bio->bi_next is NULL (as all callers
 	 * ensure that) so we have a list with a single bio.
 	 * We pretend that we have just taken it off a longer list, so
-	 * we assign bio_list to a pointer to the bio_list_on_stack,
+	 * we assign bio_list to a pointer to the bio_lists_on_stack,
 	 * thus initialising the bio_list of new bios to be
 	 * added.  ->make_request() may indeed add some more bios
 	 * through a recursive call to generic_make_request.  If it
@@ -2067,8 +2070,9 @@ blk_qc_t generic_make_request(struct bio *bio)
 	 * bio_list, and call into ->make_request() again.
 	 */
 	BUG_ON(bio->bi_next);
-	bio_list_init(&bio_list_on_stack);
-	current->bio_list = &bio_list_on_stack;
+	bio_list_init(&bio_lists_on_stack.recursion);
+	bio_list_init(&bio_lists_on_stack.remainder);
+	current->bio_lists = &bio_lists_on_stack;
 	do {
 		struct request_queue *q = bdev_get_queue(bio->bi_bdev);
 
@@ -2076,16 +2080,14 @@ blk_qc_t generic_make_request(struct bio *bio)
 			ret = q->make_request_fn(q, bio);
 
 			blk_queue_exit(q);
-
-			bio = bio_list_pop(current->bio_list);
 		} else {
-			struct bio *bio_next = bio_list_pop(current->bio_list);
-
 			bio_io_error(bio);
-			bio = bio_next;
 		}
+		bio = bio_list_pop(&current->bio_lists->recursion);
+		if (!bio)
+			bio = bio_list_pop(&current->bio_lists->remainder);
 	} while (bio);
-	current->bio_list = NULL; /* deactivate */
+	current->bio_lists = NULL; /* deactivate */
 
 out:
 	return ret;
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 2613531..8da0c22 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -172,6 +172,7 @@ void blk_queue_split(struct request_queue *q, struct bio **bio,
 	struct bio *split, *res;
 	unsigned nsegs;
 
+	BUG_ON(!current->bio_lists);
 	if ((*bio)->bi_rw & REQ_DISCARD)
 		split = blk_bio_discard_split(q, *bio, bs, &nsegs);
 	else if ((*bio)->bi_rw & REQ_WRITE_SAME)
@@ -190,7 +191,7 @@ void blk_queue_split(struct request_queue *q, struct bio **bio,
 
 		bio_chain(split, *bio);
 		trace_block_split(q, split, (*bio)->bi_iter.bi_sector);
-		generic_make_request(*bio);
+		bio_list_add_head(&current->bio_lists->remainder, *bio);
 		*bio = split;
 	}
 }
diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index eab505e..eb0b41a 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -450,7 +450,7 @@ void __bch_btree_node_write(struct btree *b, struct closure *parent)
 
 	trace_bcache_btree_write(b);
 
-	BUG_ON(current->bio_list);
+	BUG_ON(current->bio_lists);
 	BUG_ON(b->written >= btree_blocks(b));
 	BUG_ON(b->written && !i->keys);
 	BUG_ON(btree_bset_first(b)->seq != i->seq);
@@ -544,7 +544,7 @@ static void bch_btree_leaf_dirty(struct btree *b, atomic_t *journal_ref)
 
 	/* Force write if set is too big */
 	if (set_bytes(i) > PAGE_SIZE - 48 &&
-	    !current->bio_list)
+	    !current->bio_lists)
 		bch_btree_node_write(b, NULL);
 }
 
@@ -889,7 +889,7 @@ static struct btree *mca_alloc(struct cache_set *c, struct btree_op *op,
 {
 	struct btree *b;
 
-	BUG_ON(current->bio_list);
+	BUG_ON(current->bio_lists);
 
 	lockdep_assert_held(&c->bucket_lock);
 
@@ -976,7 +976,7 @@ retry:
 	b = mca_find(c, k);
 
 	if (!b) {
-		if (current->bio_list)
+		if (current->bio_lists)
 			return ERR_PTR(-EAGAIN);
 
 		mutex_lock(&c->bucket_lock);
@@ -2127,7 +2127,7 @@ static int bch_btree_insert_node(struct btree *b, struct btree_op *op,
 
 	return 0;
 split:
-	if (current->bio_list) {
+	if (current->bio_lists) {
 		op->lock = b->c->root->level + 1;
 		return -EAGAIN;
 	} else if (op->lock <= b->c->root->level) {
@@ -2209,7 +2209,7 @@ int bch_btree_insert(struct cache_set *c, struct keylist *keys,
 	struct btree_insert_op op;
 	int ret = 0;
 
-	BUG_ON(current->bio_list);
+	BUG_ON(current->bio_lists);
 	BUG_ON(bch_keylist_empty(keys));
 
 	bch_btree_op_init(&op.op, 0);
diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c
index cd77216..b64d4f0 100644
--- a/drivers/md/dm-bufio.c
+++ b/drivers/md/dm-bufio.c
@@ -174,7 +174,7 @@ static inline int dm_bufio_cache_index(struct dm_bufio_client *c)
 #define DM_BUFIO_CACHE(c)	(dm_bufio_caches[dm_bufio_cache_index(c)])
 #define DM_BUFIO_CACHE_NAME(c)	(dm_bufio_cache_names[dm_bufio_cache_index(c)])
 
-#define dm_bufio_in_request()	(!!current->bio_list)
+#define dm_bufio_in_request()	(!!current->bio_lists)
 
 static void dm_bufio_lock(struct dm_bufio_client *c)
 {
diff --git a/drivers/md/md.h b/drivers/md/md.h
index b5c4be7..7afc71c 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -663,6 +663,13 @@ static inline void rdev_dec_pending(struct md_rdev *rdev, struct mddev *mddev)
 	}
 }
 
+static inline bool current_has_pending_bios(void)
+{
+	return current->bio_lists && (
+	      !bio_list_empty(&current->bio_lists->recursion) ||
+	      !bio_list_empty(&current->bio_lists->remainder));
+}
+
 extern struct md_cluster_operations *md_cluster_ops;
 static inline int mddev_is_clustered(struct mddev *mddev)
 {
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index c7c8cde..811f2c0 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -876,8 +876,7 @@ static sector_t wait_barrier(struct r1conf *conf, struct bio *bio)
 				    (!conf->barrier ||
 				     ((conf->start_next_window <
 				       conf->next_resync + RESYNC_SECTORS) &&
-				      current->bio_list &&
-				      !bio_list_empty(current->bio_list))),
+				      current_has_pending_bios())),
 				    conf->resync_lock);
 		conf->nr_waiting--;
 	}
@@ -1014,7 +1013,7 @@ static void raid1_unplug(struct blk_plug_cb *cb, bool from_schedule)
 	struct r1conf *conf = mddev->private;
 	struct bio *bio;
 
-	if (from_schedule || current->bio_list) {
+	if (from_schedule || current->bio_lists) {
 		spin_lock_irq(&conf->device_lock);
 		bio_list_merge(&conf->pending_bio_list, &plug->pending);
 		conf->pending_count += plug->pending_cnt;
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index c7de2a5..2c70717 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -945,8 +945,7 @@ static void wait_barrier(struct r10conf *conf)
 		wait_event_lock_irq(conf->wait_barrier,
 				    !conf->barrier ||
 				    (conf->nr_pending &&
-				     current->bio_list &&
-				     !bio_list_empty(current->bio_list)),
+				     current_has_pending_bios()),
 				    conf->resync_lock);
 		conf->nr_waiting--;
 	}
@@ -1022,7 +1021,7 @@ static void raid10_unplug(struct blk_plug_cb *cb, bool from_schedule)
 	struct r10conf *conf = mddev->private;
 	struct bio *bio;
 
-	if (from_schedule || current->bio_list) {
+	if (from_schedule || current->bio_lists) {
 		spin_lock_irq(&conf->device_lock);
 		bio_list_merge(&conf->pending_bio_list, &plug->pending);
 		conf->pending_count += plug->pending_cnt;
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 9faebf7..57e45a0 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -598,6 +598,17 @@ struct bio_list {
 	struct bio *tail;
 };
 
+/* for generic_make_request() */
+struct recursion_to_iteration_bio_lists {
+	/* For stacking drivers submitting to their respective backend.
+	 * Added to tail, processed in FIFO order. */
+	struct bio_list recursion;
+	/* For pushing back the "remainder" part resulting from calling
+	 * blk_queue_split(). Added to head, processed in LIFO order,
+	 * once the "recursion" list has been drained. */
+	struct bio_list remainder;
+};
+
 static inline int bio_list_empty(const struct bio_list *bl)
 {
 	return bl->head == NULL;
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6e42ada..146eedc 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -128,7 +128,7 @@ struct sched_attr {
 
 struct futex_pi_state;
 struct robust_list_head;
-struct bio_list;
+struct recursion_to_iteration_bio_lists;
 struct fs_struct;
 struct perf_event_context;
 struct blk_plug;
@@ -1727,7 +1727,7 @@ struct task_struct {
 	void *journal_info;
 
 /* stacked block device info */
-	struct bio_list *bio_list;
+	struct recursion_to_iteration_bio_lists *bio_lists;
 
 #ifdef CONFIG_BLOCK
 /* stack plugging */
-- 
1.9.1

^ permalink raw reply related

* [PATCH] dm: Check kthread_run's return value
From: Minfei Huang @ 2016-06-22  2:32 UTC (permalink / raw)
  To: agk, snitzer, shli
  Cc: dm-devel, linux-raid, linux-kernel, minfei.hmf, Minfei Huang

kthread function is used to process kthread_work. And there is no return
value checking during create this thread. Add this checking to fix this
issue.

Signed-off-by: Minfei Huang <mnghuan@gmail.com>
Signed-off-by: Minfei Huang <minfei.hmf@alibaba-inc.com>
---
 drivers/md/dm.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 1b2f962..d68b9d2 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2654,12 +2654,15 @@ struct queue_limits *dm_get_queue_limits(struct mapped_device *md)
 }
 EXPORT_SYMBOL_GPL(dm_get_queue_limits);
 
-static void dm_old_init_rq_based_worker_thread(struct mapped_device *md)
+static int dm_old_init_rq_based_worker_thread(struct mapped_device *md)
 {
 	/* Initialize the request-based DM worker thread */
 	init_kthread_worker(&md->kworker);
 	md->kworker_task = kthread_run(kthread_worker_fn, &md->kworker,
 				       "kdmwork-%s", dm_device_name(md));
+	if (IS_ERR(md->kworker_task))
+		return -ENOMEM;
+	return 0;
 }
 
 /*
@@ -2667,6 +2670,8 @@ static void dm_old_init_rq_based_worker_thread(struct mapped_device *md)
  */
 static int dm_old_init_request_queue(struct mapped_device *md)
 {
+	int ret;
+
 	/* Fully initialize the queue */
 	if (!blk_init_allocated_queue(md->queue, dm_request_fn, NULL))
 		return -EINVAL;
@@ -2678,7 +2683,9 @@ static int dm_old_init_request_queue(struct mapped_device *md)
 	blk_queue_softirq_done(md->queue, dm_softirq_done);
 	blk_queue_prep_rq(md->queue, dm_old_prep_fn);
 
-	dm_old_init_rq_based_worker_thread(md);
+	ret = dm_old_init_rq_based_worker_thread(md);
+	if (ret < 0)
+		return ret;
 
 	elv_register_queue(md->queue);
 
-- 
2.7.4 (Apple Git-66)


^ permalink raw reply related

* [RESEND PATCH 3/3] bcache: Remove redundant block_size assignment
From: Yijing Wang @ 2016-06-22  2:12 UTC (permalink / raw)
  To: axboe, Kent Overstreet
  Cc: Eric Wheeler, Coly Li, linux-bcache, linux-raid, linux-kernel,
	Yijing Wang

We have assigned sb->block_size before the switch,
so remove the redundant one.

Reviewed-by: Coly Li <colyli@suse.de>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Acked-by: Eric Wheeler <bcache@lists.ewheeler.net>
---
 drivers/md/bcache/super.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index aecaace..bf4b100 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -134,7 +134,6 @@ static const char *read_super(struct cache_sb *sb, struct block_device *bdev,
 	case BCACHE_SB_VERSION_CDEV:
 	case BCACHE_SB_VERSION_CDEV_WITH_UUID:
 		sb->nbuckets	= le64_to_cpu(s->nbuckets);
-		sb->block_size	= le16_to_cpu(s->block_size);
 		sb->bucket_size	= le16_to_cpu(s->bucket_size);
 
 		sb->nr_in_set	= le16_to_cpu(s->nr_in_set);
-- 
1.7.1

^ permalink raw reply related

* [RESEND PATCH 2/3] bcache: update document info
From: Yijing Wang @ 2016-06-22  2:12 UTC (permalink / raw)
  To: axboe, Kent Overstreet
  Cc: Eric Wheeler, Coly Li, linux-bcache, linux-raid, linux-kernel,
	Yijing Wang

There is no return in continue_at(), update the documentation.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/md/bcache/closure.c |    2 +-
 drivers/md/bcache/closure.h |    3 ---
 2 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c
index 9eaf1d6..864e673 100644
--- a/drivers/md/bcache/closure.c
+++ b/drivers/md/bcache/closure.c
@@ -112,7 +112,7 @@ bool closure_wait(struct closure_waitlist *waitlist, struct closure *cl)
 EXPORT_SYMBOL(closure_wait);
 
 /**
- * closure_sync - sleep until a closure a closure has nothing left to wait on
+ * closure_sync - sleep until a closure has nothing left to wait on
  *
  * Sleeps until the refcount hits 1 - the thread that's running the closure owns
  * the last refcount.
diff --git a/drivers/md/bcache/closure.h b/drivers/md/bcache/closure.h
index 782cc2c..f51188d 100644
--- a/drivers/md/bcache/closure.h
+++ b/drivers/md/bcache/closure.h
@@ -31,9 +31,6 @@
  * passing it, as you might expect, the function to run when nothing is pending
  * and the workqueue to run that function out of.
  *
- * continue_at() also, critically, is a macro that returns the calling function.
- * There's good reason for this.
- *
  * To use safely closures asynchronously, they must always have a refcount while
  * they are running owned by the thread that is running them. Otherwise, suppose
  * you submit some bios and wish to have a function run when they all complete:
-- 
1.7.1

^ permalink raw reply related

* [RESEND PATCH 1/3] bcache: Remove redundant parameter for cache_alloc()
From: Yijing Wang @ 2016-06-22  2:10 UTC (permalink / raw)
  To: axboe, Kent Overstreet
  Cc: Eric Wheeler, Coly Li, linux-bcache, linux-raid, linux-kernel,
	Yijing Wang

Cache_sb is not used in cache_alloc, and we have copied
sb info to cache->sb already, remove it.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/md/bcache/super.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index f5dbb4e..aecaace 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -1803,7 +1803,7 @@ void bch_cache_release(struct kobject *kobj)
 	module_put(THIS_MODULE);
 }
 
-static int cache_alloc(struct cache_sb *sb, struct cache *ca)
+static int cache_alloc(struct cache *ca)
 {
 	size_t free;
 	struct bucket *b;
@@ -1858,7 +1858,7 @@ static int register_cache(struct cache_sb *sb, struct page *sb_page,
 	if (blk_queue_discard(bdev_get_queue(ca->bdev)))
 		ca->discard = CACHE_DISCARD(&ca->sb);
 
-	ret = cache_alloc(sb, ca);
+	ret = cache_alloc(ca);
 	if (ret != 0)
 		goto err;
 
-- 
1.7.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