Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1575901 > unrolled thread

[WIP PATCHSET 0/4] WIP branch for bfq-mq

Back to article view | Back to linux.kernel

Contents

  [WIP PATCHSET 0/4] WIP branch for bfq-mq Paolo Valente <paolo.valente@linaro.org> - 2017-02-07 18:30 +0100
    [WIP PATCHSET 1/4] blk-mq: pass bio to blk_mq_sched_get_rq_priv Paolo Valente <paolo.valente@linaro.org> - 2017-02-07 18:30 +0100
      Re: [WIP PATCHSET 1/4] blk-mq: pass bio to blk_mq_sched_get_rq_priv Jens Axboe <axboe@kernel.dk> - 2017-02-10 17:20 +0100
    [WIP PATCHSET 3/4] Embed bfq-ioc.c and add locking on request queue Paolo Valente <paolo.valente@linaro.org> - 2017-02-07 18:30 +0100
    [WIP PATCHSET 2/4] Move thinktime from bic to bfqq Paolo Valente <paolo.valente@linaro.org> - 2017-02-07 18:30 +0100
    Re: [WIP PATCHSET 0/4] WIP branch for bfq-mq Bart Van Assche <Bart.VanAssche@sandisk.com> - 2017-02-10 17:20 +0100
      Re: [WIP PATCHSET 0/4] WIP branch for bfq-mq Paolo Valente <paolo.valente@linaro.org> - 2017-02-10 18:00 +0100
    Re: [WIP PATCHSET 0/4] WIP branch for bfq-mq Bart Van Assche <Bart.VanAssche@sandisk.com> - 2017-02-10 17:50 +0100
      Re: [WIP PATCHSET 0/4] WIP branch for bfq-mq Paolo Valente <paolo.valente@linaro.org> - 2017-02-10 18:00 +0100
        Re: [WIP PATCHSET 0/4] WIP branch for bfq-mq Bart Van Assche <bart.vanassche@sandisk.com> - 2017-02-10 19:30 +0100
          Re: [WIP PATCHSET 0/4] WIP branch for bfq-mq Paolo Valente <paolo.valente@linaro.org> - 2017-02-10 21:00 +0100
            Re: [WIP PATCHSET 0/4] WIP branch for bfq-mq Paolo Valente <paolo.valente@linaro.org> - 2017-02-13 22:10 +0100
              Re: [WIP PATCHSET 0/4] WIP branch for bfq-mq Bart Van Assche <Bart.VanAssche@sandisk.com> - 2017-02-13 23:10 +0100
                Re: [WIP PATCHSET 0/4] WIP branch for bfq-mq Bart Van Assche <Bart.VanAssche@sandisk.com> - 2017-02-13 23:40 +0100
                  Re: [WIP PATCHSET 0/4] WIP branch for bfq-mq Paolo Valente <paolo.valente@linaro.org> - 2017-02-22 22:40 +0100
                    Re: [WIP PATCHSET 0/4] WIP branch for bfq-mq Paolo Valente <paolo.valente@linaro.org> - 2017-02-25 18:50 +0100
    Re: [WIP PATCHSET 0/4] WIP branch for bfq-mq Paolo Valente <paolo.valente@linaro.org> - 2017-02-10 20:40 +0100
    Re: [WIP PATCHSET 0/4] WIP branch for bfq-mq Bart Van Assche <Bart.VanAssche@sandisk.com> - 2017-02-10 20:40 +0100
    Re: [WIP PATCHSET 0/4] WIP branch for bfq-mq Paolo Valente <paolo.valente@linaro.org> - 2017-02-13 22:10 +0100
      Re: [WIP PATCHSET 0/4] WIP branch for bfq-mq Jens Axboe <axboe@kernel.dk> - 2017-02-13 23:30 +0100

#1575901 — [WIP PATCHSET 0/4] WIP branch for bfq-mq

Hi,

I have finally pushed here [1] the current WIP branch of bfq for
blk-mq, which I have tentatively named bfq-mq.

This branch *IS NOT* meant for merging into mainline and contain code
that mau easily violate code style, and not only, in many
places. Commits implement the following main steps:
1) Add the last version of bfq for blk
2) Clone bfq source files into identical bfq-mq source files
3) Modify bfq-mq files to get a working version of bfq for blk-mq
(cgroups support not yet functional)

In my intentions, the main goals of this branch are:

1) Show, as soon as I could, the changes I made to let bfq-mq comply
with blk-mq-sched framework. I though this could be particularly
useful for Jens, being BFQ identical to CFQ in terms of hook
interfaces and io-context handling, and almost identical in terms
request-merging.

2) Enable people to test this first version bfq-mq. Code is purposely
overfull of log messages and invariant checks that halt the system on
failure (lock assertions, BUG_ONs, ...).

To make it easier to revise commits, I'm sending the patches that
transform bfq into bfq-mq (last four patches in the branch [1]). They
work on two files, bfq-mq-iosched.c and bfq-mq.h, which, at the
beginning, are just copies of bfq-iosched.c and bfq.h.

Thanks,
Paolo

[1] https://github.com/Algodev-github/bfq-mq

Paolo Valente (4):
  blk-mq: pass bio to blk_mq_sched_get_rq_priv
  Move thinktime from bic to bfqq
  Embed bfq-ioc.c and add locking on request queue
  Modify interface and operation to comply with blk-mq-sched

 block/bfq-cgroup.c       |   4 -
 block/bfq-mq-iosched.c   | 852 +++++++++++++++++++++++++++++------------------
 block/bfq-mq.h           |  65 ++--
 block/blk-mq-sched.c     |   8 +-
 block/blk-mq-sched.h     |   5 +-
 include/linux/elevator.h |   2 +-
 6 files changed, 567 insertions(+), 369 deletions(-)

--
2.10.0

[toc] | [next] | [standalone]

#1575902 — [WIP PATCHSET 1/4] blk-mq: pass bio to blk_mq_sched_get_rq_priv

bio is used in bfq-mq's get_rq_priv, to get the request group. We could
pass directly the group here, but I thought that passing the bio was
more general, giving the possibility to get other pieces of information
if needed.

Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
 block/blk-mq-sched.c     | 8 +++++---
 block/blk-mq-sched.h     | 5 +++--
 include/linux/elevator.h | 2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index ee455e7..314a8ed 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -68,7 +68,9 @@ int blk_mq_sched_init_hctx_data(struct request_queue *q, size_t size,
 EXPORT_SYMBOL_GPL(blk_mq_sched_init_hctx_data);
 
 static void __blk_mq_sched_assign_ioc(struct request_queue *q,
-				      struct request *rq, struct io_context *ioc)
+				      struct request *rq,
+				      struct bio *bio,
+				      struct io_context *ioc)
 {
 	struct io_cq *icq;
 
@@ -83,7 +85,7 @@ static void __blk_mq_sched_assign_ioc(struct request_queue *q,
 	}
 
 	rq->elv.icq = icq;
-	if (!blk_mq_sched_get_rq_priv(q, rq)) {
+	if (!blk_mq_sched_get_rq_priv(q, rq, bio)) {
 		rq->rq_flags |= RQF_ELVPRIV;
 		get_io_context(icq->ioc);
 		return;
@@ -99,7 +101,7 @@ static void blk_mq_sched_assign_ioc(struct request_queue *q,
 
 	ioc = rq_ioc(bio);
 	if (ioc)
-		__blk_mq_sched_assign_ioc(q, rq, ioc);
+		__blk_mq_sched_assign_ioc(q, rq, bio, ioc);
 }
 
 struct request *blk_mq_sched_get_request(struct request_queue *q,
diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h
index 5954859..7b5f3b9 100644
--- a/block/blk-mq-sched.h
+++ b/block/blk-mq-sched.h
@@ -49,12 +49,13 @@ blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio)
 }
 
 static inline int blk_mq_sched_get_rq_priv(struct request_queue *q,
-					   struct request *rq)
+					   struct request *rq,
+					   struct bio *bio)
 {
 	struct elevator_queue *e = q->elevator;
 
 	if (e && e->type->ops.mq.get_rq_priv)
-		return e->type->ops.mq.get_rq_priv(q, rq);
+		return e->type->ops.mq.get_rq_priv(q, rq, bio);
 
 	return 0;
 }
diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index b5825c4..ce14ede 100644
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -99,7 +99,7 @@ struct elevator_mq_ops {
 	void (*requeue_request)(struct request *);
 	struct request *(*former_request)(struct request_queue *, struct request *);
 	struct request *(*next_request)(struct request_queue *, struct request *);
-	int (*get_rq_priv)(struct request_queue *, struct request *);
+	int (*get_rq_priv)(struct request_queue *, struct request *, struct bio *);
 	void (*put_rq_priv)(struct request_queue *, struct request *);
 	void (*init_icq)(struct io_cq *);
 	void (*exit_icq)(struct io_cq *);
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1578593 — Re: [WIP PATCHSET 1/4] blk-mq: pass bio to blk_mq_sched_get_rq_priv

On 02/07/2017 10:24 AM, Paolo Valente wrote:
> bio is used in bfq-mq's get_rq_priv, to get the request group. We could
> pass directly the group here, but I thought that passing the bio was
> more general, giving the possibility to get other pieces of information
> if needed.

I applied this one, thanks Paolo.

-- 
Jens Axboe

[toc] | [prev] | [next] | [standalone]

#1575907 — [WIP PATCHSET 3/4] Embed bfq-ioc.c and add locking on request queue

The version of bfq-ioc.c for bfq-iosched.c is not correct any more for
bfq-mq, because, in bfq-mq, the request queue lock is not being held
when bfq_bic_lookup is invoked. That function must then take that look
on its own. This commit removes the inclusion of bfq-ioc.c, copies the
content of bfq-ioc.c into bfq-mq-iosched.c, and adds the grabbing of
the lock.

Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
 block/bfq-mq-iosched.c | 39 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/block/bfq-mq-iosched.c b/block/bfq-mq-iosched.c
index 7e3f429..a8679de 100644
--- a/block/bfq-mq-iosched.c
+++ b/block/bfq-mq-iosched.c
@@ -190,7 +190,39 @@ static int device_speed_thresh[2];
 
 static void bfq_schedule_dispatch(struct bfq_data *bfqd);
 
-#include "bfq-ioc.c"
+/**
+ * icq_to_bic - convert iocontext queue structure to bfq_io_cq.
+ * @icq: the iocontext queue.
+ */
+static struct bfq_io_cq *icq_to_bic(struct io_cq *icq)
+{
+	/* bic->icq is the first member, %NULL will convert to %NULL */
+	return container_of(icq, struct bfq_io_cq, icq);
+}
+
+/**
+ * bfq_bic_lookup - search into @ioc a bic associated to @bfqd.
+ * @bfqd: the lookup key.
+ * @ioc: the io_context of the process doing I/O.
+ * @q: the request queue.
+ */
+static struct bfq_io_cq *bfq_bic_lookup(struct bfq_data *bfqd,
+					struct io_context *ioc,
+					struct request_queue *q)
+{
+	if (ioc) {
+		struct bfq_io_cq *icq;
+
+		spin_lock_irq(q->queue_lock);
+		icq = icq_to_bic(ioc_lookup_icq(ioc, q));
+		spin_unlock_irq(q->queue_lock);
+
+		return icq;
+	}
+
+	return NULL;
+}
+
 #include "bfq-sched.c"
 #include "bfq-cgroup.c"
 
@@ -1480,13 +1512,14 @@ static void bfq_add_request(struct request *rq)
 }
 
 static struct request *bfq_find_rq_fmerge(struct bfq_data *bfqd,
-					  struct bio *bio)
+					  struct bio *bio,
+					  struct request_queue *q)
 {
 	struct task_struct *tsk = current;
 	struct bfq_io_cq *bic;
 	struct bfq_queue *bfqq;
 
-	bic = bfq_bic_lookup(bfqd, tsk->io_context);
+	bic = bfq_bic_lookup(bfqd, tsk->io_context, q);
 	if (!bic)
 		return NULL;
 
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1575910 — [WIP PATCHSET 2/4] Move thinktime from bic to bfqq

Prep change to make it possible to protect this field with a
scheduler lock.

Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
 block/bfq-mq-iosched.c | 28 ++++++++++++++--------------
 block/bfq-mq.h         | 30 ++++++++++++++++--------------
 2 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/block/bfq-mq-iosched.c b/block/bfq-mq-iosched.c
index 0a7da4e..7e3f429 100644
--- a/block/bfq-mq-iosched.c
+++ b/block/bfq-mq-iosched.c
@@ -670,6 +670,7 @@ bfq_bfqq_resume_state(struct bfq_queue *bfqq, struct bfq_io_cq *bic)
 	else
 		bfq_clear_bfqq_IO_bound(bfqq);
 
+	bfqq->ttime = bic->saved_ttime;
 	bfqq->wr_coeff = bic->saved_wr_coeff;
 	bfqq->wr_start_at_switch_to_srt = bic->saved_wr_start_at_switch_to_srt;
 	BUG_ON(time_is_after_jiffies(bfqq->wr_start_at_switch_to_srt));
@@ -1247,7 +1248,7 @@ static void bfq_bfqq_handle_idle_busy_switch(struct bfq_data *bfqd,
 		 * details on the usage of the next variable.
 		 */
 		arrived_in_time =  ktime_get_ns() <=
-			RQ_BIC(rq)->ttime.last_end_request +
+			bfqq->ttime.last_end_request +
 			bfqd->bfq_slice_idle * 3;
 
 	bfq_log_bfqq(bfqd, bfqq,
@@ -2003,6 +2004,7 @@ static void bfq_bfqq_save_state(struct bfq_queue *bfqq)
 	if (!bic)
 		return;
 
+	bic->saved_ttime = bfqq->ttime;
 	bic->saved_idle_window = bfq_bfqq_idle_window(bfqq);
 	bic->saved_IO_bound = bfq_bfqq_IO_bound(bfqq);
 	bic->saved_in_large_burst = bfq_bfqq_in_large_burst(bfqq);
@@ -3870,11 +3872,6 @@ static void bfq_exit_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq)
 	bfq_put_queue(bfqq);
 }
 
-static void bfq_init_icq(struct io_cq *icq)
-{
-	icq_to_bic(icq)->ttime.last_end_request = ktime_get_ns() - (1ULL<<32);
-}
-
 static void bfq_exit_icq(struct io_cq *icq)
 {
 	struct bfq_io_cq *bic = icq_to_bic(icq);
@@ -4000,6 +3997,9 @@ static void bfq_init_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq,
 		bfq_mark_bfqq_just_created(bfqq);
 	} else
 		bfq_clear_bfqq_sync(bfqq);
+
+	bfqq->ttime.last_end_request = ktime_get_ns() - (1ULL<<32);
+
 	bfq_mark_bfqq_IO_bound(bfqq);
 
 	/* Tentative initial value to trade off between thr and lat */
@@ -4107,14 +4107,14 @@ static struct bfq_queue *bfq_get_queue(struct bfq_data *bfqd,
 }
 
 static void bfq_update_io_thinktime(struct bfq_data *bfqd,
-				    struct bfq_io_cq *bic)
+				    struct bfq_queue *bfqq)
 {
-	struct bfq_ttime *ttime = &bic->ttime;
-	u64 elapsed = ktime_get_ns() - bic->ttime.last_end_request;
+	struct bfq_ttime *ttime = &bfqq->ttime;
+	u64 elapsed = ktime_get_ns() - bfqq->ttime.last_end_request;
 
 	elapsed = min_t(u64, elapsed, 2 * bfqd->bfq_slice_idle);
 
-	ttime->ttime_samples = (7*bic->ttime.ttime_samples + 256) / 8;
+	ttime->ttime_samples = (7*bfqq->ttime.ttime_samples + 256) / 8;
 	ttime->ttime_total = div_u64(7*ttime->ttime_total + 256*elapsed,  8);
 	ttime->ttime_mean = div64_ul(ttime->ttime_total + 128,
 				     ttime->ttime_samples);
@@ -4157,8 +4157,8 @@ static void bfq_update_idle_window(struct bfq_data *bfqd,
 		(bfqd->hw_tag && BFQQ_SEEKY(bfqq) &&
 			bfqq->wr_coeff == 1))
 		enable_idle = 0;
-	else if (bfq_sample_valid(bic->ttime.ttime_samples)) {
-		if (bic->ttime.ttime_mean > bfqd->bfq_slice_idle &&
+	else if (bfq_sample_valid(bfqq->ttime.ttime_samples)) {
+		if (bfqq->ttime.ttime_mean > bfqd->bfq_slice_idle &&
 			bfqq->wr_coeff == 1)
 			enable_idle = 0;
 		else
@@ -4185,7 +4185,7 @@ static void bfq_rq_enqueued(struct bfq_data *bfqd, struct bfq_queue *bfqq,
 	if (rq->cmd_flags & REQ_META)
 		bfqq->meta_pending++;
 
-	bfq_update_io_thinktime(bfqd, bic);
+	bfq_update_io_thinktime(bfqd, bfqq);
 	bfq_update_io_seektime(bfqd, bfqq, rq);
 	if (bfqq->entity.service > bfq_max_budget(bfqd) / 8 ||
 	    !BFQQ_SEEKY(bfqq))
@@ -4354,7 +4354,7 @@ static void bfq_completed_request(struct request_queue *q, struct request *rq)
 
 	now_ns = ktime_get_ns();
 
-	RQ_BIC(rq)->ttime.last_end_request = now_ns;
+	bfqq->ttime.last_end_request = now_ns;
 
 	/*
 	 * Using us instead of ns, to get a reasonable precision in
diff --git a/block/bfq-mq.h b/block/bfq-mq.h
index f28feb6..c6acee2 100644
--- a/block/bfq-mq.h
+++ b/block/bfq-mq.h
@@ -199,6 +199,18 @@ struct bfq_entity {
 struct bfq_group;
 
 /**
+ * struct bfq_ttime - per process thinktime stats.
+ */
+struct bfq_ttime {
+	u64 last_end_request; /* completion time of last request */
+
+	u64 ttime_total; /* total process thinktime */
+	unsigned long ttime_samples; /* number of thinktime samples */
+	u64 ttime_mean; /* average process thinktime */
+
+};
+
+/**
  * struct bfq_queue - leaf schedulable entity.
  *
  * A bfq_queue is a leaf request queue; it can be associated with an
@@ -259,6 +271,9 @@ struct bfq_queue {
 	/* node for active/idle bfqq list inside parent bfqd */
 	struct list_head bfqq_list;
 
+	/* associated @bfq_ttime struct */
+	struct bfq_ttime ttime;
+
 	/* bit vector: a 1 for each seeky requests in history */
 	u32 seek_history;
 
@@ -322,18 +337,6 @@ struct bfq_queue {
 };
 
 /**
- * struct bfq_ttime - per process thinktime stats.
- */
-struct bfq_ttime {
-	u64 last_end_request; /* completion time of last request */
-
-	u64 ttime_total; /* total process thinktime */
-	unsigned long ttime_samples; /* number of thinktime samples */
-	u64 ttime_mean; /* average process thinktime */
-
-};
-
-/**
  * struct bfq_io_cq - per (request_queue, io_context) structure.
  */
 struct bfq_io_cq {
@@ -341,8 +344,6 @@ struct bfq_io_cq {
 	struct io_cq icq; /* must be the first member */
 	/* array of two process queues, the sync and the async */
 	struct bfq_queue *bfqq[2];
-	/* associated @bfq_ttime struct */
-	struct bfq_ttime ttime;
 	/* per (request_queue, blkcg) ioprio */
 	int ioprio;
 #ifdef BFQ_GROUP_IOSCHED_ENABLED
@@ -379,6 +380,7 @@ struct bfq_io_cq {
 	unsigned long saved_last_wr_start_finish;
 	unsigned long saved_wr_start_at_switch_to_srt;
 	unsigned int saved_wr_cur_max_time;
+	struct bfq_ttime saved_ttime;
 };
 
 enum bfq_device_speed {
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1578581

On Tue, 2017-02-07 at 18:24 +0100, Paolo Valente wrote:
> [1] https://github.com/Algodev-github/bfq-mq

Hello Paolo,

That branch includes two changes of the version suffix (EXTRAVERSION in Makefile).
Please don't do that but set CONFIG_LOCALVERSION in .config to add a suffix to
the kernel version string.

Thanks,

Bart.

[toc] | [prev] | [next] | [standalone]

#1578620

> Il giorno 10 feb 2017, alle ore 17:08, Bart Van Assche <bart.vanassche@sandisk.com> ha scritto:
> 
> On Tue, 2017-02-07 at 18:24 +0100, Paolo Valente wrote:
>> [1] https://github.com/Algodev-github/bfq-mq
> 
> Hello Paolo,
> 
> That branch includes two changes of the version suffix (EXTRAVERSION in Makefile).
> Please don't do that but set CONFIG_LOCALVERSION in .config to add a suffix to
> the kernel version string.
> 

I know it, thanks. Unfortunately, many other irregular things you will probably find in that sort of private branch (as for that suffix, for some reason it was handy for me to have it tracked by git).

Thanks,
Paolo

> Thanks,
> 
> Bart.

[toc] | [prev] | [next] | [standalone]

#1578609

On Tue, 2017-02-07 at 18:24 +0100, Paolo Valente wrote:
> 2) Enable people to test this first version bfq-mq.

Hello Paolo,

I installed this version of bfq-mq on a server that boots from a SATA
disk. That server boots fine with kernel v4.10-rc7 but not with this
tree. The first 30 seconds of the boot process seem to proceed normally
but after that time the messages on the console stop scrolling and
about another 30 seconds later the server reboots. I haven't found
anything useful in the system log. I configured the block layer as
follows:

$ grep '^C.*_MQ_' .config
CONFIG_BLK_MQ_PCI=y
CONFIG_MQ_IOSCHED_BFQ=y
CONFIG_MQ_IOSCHED_DEADLINE=y
CONFIG_MQ_IOSCHED_NONE=y
CONFIG_DEFAULT_MQ_BFQ_MQ=y
CONFIG_DEFAULT_MQ_IOSCHED="bfq-mq"
CONFIG_SCSI_MQ_DEFAULT=y
CONFIG_DM_MQ_DEFAULT=y

Bart.

[toc] | [prev] | [next] | [standalone]

#1578617

> Il giorno 10 feb 2017, alle ore 17:45, Bart Van Assche <Bart.VanAssche@sandisk.com> ha scritto:
> 
> On Tue, 2017-02-07 at 18:24 +0100, Paolo Valente wrote:
>> 2) Enable people to test this first version bfq-mq.
> 
> Hello Paolo,
> 
> I installed this version of bfq-mq on a server that boots from a SATA
> disk. That server boots fine with kernel v4.10-rc7 but not with this
> tree. The first 30 seconds of the boot process seem to proceed normally
> but after that time the messages on the console stop scrolling and
> about another 30 seconds later the server reboots. I haven't found
> anything useful in the system log. I configured the block layer as
> follows:
> 
> $ grep '^C.*_MQ_' .config
> CONFIG_BLK_MQ_PCI=y
> CONFIG_MQ_IOSCHED_BFQ=y
> CONFIG_MQ_IOSCHED_DEADLINE=y
> CONFIG_MQ_IOSCHED_NONE=y
> CONFIG_DEFAULT_MQ_BFQ_MQ=y
> CONFIG_DEFAULT_MQ_IOSCHED="bfq-mq"
> CONFIG_SCSI_MQ_DEFAULT=y
> CONFIG_DM_MQ_DEFAULT=y
> 

Could you reconfigure with none or mq-deadline as default, check
whether the system boots, and, it it does, switch manually to bfq-mq,
check what happens, and, in the likely case of a failure, try to get
the oops?

Thank you very much,
Paolo

> Bart.
> Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:
> 
> This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.
> 

[toc] | [prev] | [next] | [standalone]

#1578683

On 02/10/2017 08:49 AM, Paolo Valente wrote:
>> $ grep '^C.*_MQ_' .config
>> CONFIG_BLK_MQ_PCI=y
>> CONFIG_MQ_IOSCHED_BFQ=y
>> CONFIG_MQ_IOSCHED_DEADLINE=y
>> CONFIG_MQ_IOSCHED_NONE=y
>> CONFIG_DEFAULT_MQ_BFQ_MQ=y
>> CONFIG_DEFAULT_MQ_IOSCHED="bfq-mq"
>> CONFIG_SCSI_MQ_DEFAULT=y
>> CONFIG_DM_MQ_DEFAULT=y
>>
> 
> Could you reconfigure with none or mq-deadline as default, check
> whether the system boots, and, it it does, switch manually to bfq-mq,
> check what happens, and, in the likely case of a failure, try to get
> the oops?

Hello Paolo,

I just finished performing that test with the following kernel config:
$ grep '^C.*_MQ_' .config
CONFIG_BLK_MQ_PCI=y
CONFIG_MQ_IOSCHED_BFQ=y
CONFIG_MQ_IOSCHED_DEADLINE=y
CONFIG_MQ_IOSCHED_NONE=y
CONFIG_DEFAULT_MQ_DEADLINE=y
CONFIG_DEFAULT_MQ_IOSCHED="mq-deadline"
CONFIG_SCSI_MQ_DEFAULT=y
CONFIG_DM_MQ_DEFAULT=y

After the system came up I logged in, switched to the bfq-mq scheduler
and ran several I/O tests against the boot disk. Sorry but nothing
interesting appeared in the kernel log.

Bart.

[toc] | [prev] | [next] | [standalone]

#1578765

> Il giorno 10 feb 2017, alle ore 19:13, Bart Van Assche <bart.vanassche@sandisk.com> ha scritto:
> 
> On 02/10/2017 08:49 AM, Paolo Valente wrote:
>>> $ grep '^C.*_MQ_' .config
>>> CONFIG_BLK_MQ_PCI=y
>>> CONFIG_MQ_IOSCHED_BFQ=y
>>> CONFIG_MQ_IOSCHED_DEADLINE=y
>>> CONFIG_MQ_IOSCHED_NONE=y
>>> CONFIG_DEFAULT_MQ_BFQ_MQ=y
>>> CONFIG_DEFAULT_MQ_IOSCHED="bfq-mq"
>>> CONFIG_SCSI_MQ_DEFAULT=y
>>> CONFIG_DM_MQ_DEFAULT=y
>>> 
>> 
>> Could you reconfigure with none or mq-deadline as default, check
>> whether the system boots, and, it it does, switch manually to bfq-mq,
>> check what happens, and, in the likely case of a failure, try to get
>> the oops?
> 
> Hello Paolo,
> 
> I just finished performing that test with the following kernel config:
> $ grep '^C.*_MQ_' .config
> CONFIG_BLK_MQ_PCI=y
> CONFIG_MQ_IOSCHED_BFQ=y
> CONFIG_MQ_IOSCHED_DEADLINE=y
> CONFIG_MQ_IOSCHED_NONE=y
> CONFIG_DEFAULT_MQ_DEADLINE=y
> CONFIG_DEFAULT_MQ_IOSCHED="mq-deadline"
> CONFIG_SCSI_MQ_DEFAULT=y
> CONFIG_DM_MQ_DEFAULT=y
> 
> After the system came up I logged in, switched to the bfq-mq scheduler
> and ran several I/O tests against the boot disk.

Without any failure, right?

Unfortunately, as you can imagine, no boot failure occurred on
any of my test systems so far :(

This version of bfq-mq can be configured to print all its activity in
the kernel log, by just defining a macro.  This will of course slow
down the system so much to make it probably unusable, if bfq-mq is
active from boot.  Yet, the failure may still occur so early to make
this approach useful to discover where bfq-mq gets stuck.  As of now I
have no better ideas.  Any suggestion is welcome.

Thanks,
Paolo

> Sorry but nothing
> interesting appeared in the kernel log.
> 
> Bart.
> Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:
> 
> This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.
> 

[toc] | [prev] | [next] | [standalone]

#1580071

> Il giorno 10 feb 2017, alle ore 20:49, Paolo Valente <paolo.valente@linaro.org> ha scritto:
> 
>> 
>> Il giorno 10 feb 2017, alle ore 19:13, Bart Van Assche <bart.vanassche@sandisk.com> ha scritto:
>> 
>> On 02/10/2017 08:49 AM, Paolo Valente wrote:
>>>> $ grep '^C.*_MQ_' .config
>>>> CONFIG_BLK_MQ_PCI=y
>>>> CONFIG_MQ_IOSCHED_BFQ=y
>>>> CONFIG_MQ_IOSCHED_DEADLINE=y
>>>> CONFIG_MQ_IOSCHED_NONE=y
>>>> CONFIG_DEFAULT_MQ_BFQ_MQ=y
>>>> CONFIG_DEFAULT_MQ_IOSCHED="bfq-mq"
>>>> CONFIG_SCSI_MQ_DEFAULT=y
>>>> CONFIG_DM_MQ_DEFAULT=y
>>>> 
>>> 
>>> Could you reconfigure with none or mq-deadline as default, check
>>> whether the system boots, and, it it does, switch manually to bfq-mq,
>>> check what happens, and, in the likely case of a failure, try to get
>>> the oops?
>> 
>> Hello Paolo,
>> 
>> I just finished performing that test with the following kernel config:
>> $ grep '^C.*_MQ_' .config
>> CONFIG_BLK_MQ_PCI=y
>> CONFIG_MQ_IOSCHED_BFQ=y
>> CONFIG_MQ_IOSCHED_DEADLINE=y
>> CONFIG_MQ_IOSCHED_NONE=y
>> CONFIG_DEFAULT_MQ_DEADLINE=y
>> CONFIG_DEFAULT_MQ_IOSCHED="mq-deadline"
>> CONFIG_SCSI_MQ_DEFAULT=y
>> CONFIG_DM_MQ_DEFAULT=y
>> 
>> After the system came up I logged in, switched to the bfq-mq scheduler
>> and ran several I/O tests against the boot disk.
> 
> Without any failure, right?
> 
> Unfortunately, as you can imagine, no boot failure occurred on
> any of my test systems so far :(
> 
> This version of bfq-mq can be configured to print all its activity in
> the kernel log, by just defining a macro.  This will of course slow
> down the system so much to make it probably unusable, if bfq-mq is
> active from boot.  Yet, the failure may still occur so early to make
> this approach useful to discover where bfq-mq gets stuck.  As of now I
> have no better ideas.  Any suggestion is welcome.
> 

Hi Bart,
I have found a machine crashing at boot, yet not only when bfq-mq is
chosen, but also when mq-deadline is chosen as the default
scheduler.  I have found and just reported the cause of the failure,
together with a fix.  Probably this is not the cause of your failure,
but what do you think about trying this fix?  BTW, I have rebased the
branch [1] against the new commits in Jens for-4.11/next.

Otherwise, if you have no news or suggestions, would you be willing to
try my micro-logging proposal?

Thanks,
Paolo

[1] https://github.com/Algodev-github/bfq-mq

> Thanks,
> Paolo
> 
>> Sorry but nothing
>> interesting appeared in the kernel log.
>> 
>> Bart.
>> Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:
>> 
>> This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.

[toc] | [prev] | [next] | [standalone]

#1580101

On Mon, 2017-02-13 at 22:07 +0100, Paolo Valente wrote:
> but what do you think about trying this fix?

Sorry but with ... the same server I used for the previous test still
didn't boot up properly. A screenshot is available at
https://goo.gl/photos/Za9QVGCNe2BJBwxVA.

> Otherwise, if you have no news or suggestions, would you be willing to
> try my micro-logging proposal https://github.com/Algodev-github/bfq-mq?

Sorry but it's not clear to me what logging mechanism you are referring
to and how to enable it? Are you perhaps referring to
CONFIG_BFQ_REDIRECT_TO_CONSOLE?

Thanks,

Bart.

[toc] | [prev] | [next] | [standalone]

#1580125

On Mon, 2017-02-13 at 22:07 +0000, Bart Van Assche wrote:
> On Mon, 2017-02-13 at 22:07 +0100, Paolo Valente wrote:
> > but what do you think about trying this fix?
> 
> Sorry but with ... the same server I used for the previous test still
> didn't boot up properly. A screenshot is available at
> https://goo.gl/photos/Za9QVGCNe2BJBwxVA.
> 
> > Otherwise, if you have no news or suggestions, would you be willing to
> > try my micro-logging proposal https://github.com/Algodev-github/bfq-mq?
> 
> Sorry but it's not clear to me what logging mechanism you are referring
> to and how to enable it? Are you perhaps referring to
> CONFIG_BFQ_REDIRECT_TO_CONSOLE?

Anyway, a second screenshot has been added to the same album after I had
applied the following patch:

diff --git a/block/Makefile b/block/Makefile
index 1c04fe19e825..bf472ac82c08 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -2,6 +2,8 @@
 # Makefile for the kernel block layer
 #
  
+KBUILD_CFLAGS += -DCONFIG_BFQ_REDIRECT_TO_CONSOLE
+

 obj-$(CONFIG_BLOCK) := bio.o elevator.o blk-core.o blk-tag.o blk-sysfs.o \
                        blk-flush.o blk-settings.o blk-ioc.o blk-map.o \
                        blk-exec.o blk-merge.o blk-softirq.o blk-timeout.o \

Bart.

[toc] | [prev] | [next] | [standalone]

#1586507

> Il giorno 13 feb 2017, alle ore 23:38, Bart Van Assche <bart.vanassche@sandisk.com> ha scritto:
> 
> On Mon, 2017-02-13 at 22:07 +0000, Bart Van Assche wrote:
>> On Mon, 2017-02-13 at 22:07 +0100, Paolo Valente wrote:
>>> but what do you think about trying this fix?
>> 
>> Sorry but with ... the same server I used for the previous test still
>> didn't boot up properly. A screenshot is available at
>> https://goo.gl/photos/Za9QVGCNe2BJBwxVA.
>> 
>>> Otherwise, if you have no news or suggestions, would you be willing to
>>> try my micro-logging proposal https://github.com/Algodev-github/bfq-mq?
>> 
>> Sorry but it's not clear to me what logging mechanism you are referring
>> to and how to enable it? Are you perhaps referring to
>> CONFIG_BFQ_REDIRECT_TO_CONSOLE?
> 
> Anyway, a second screenshot has been added to the same album after I had
> applied the following patch:
> 

Hi Bart,
thanks for this second attempt of yours.  Although, unfortunately, not
providing some clear indication of the exact cause of your hang (apart
from a possible deadlock), your log helped me notice another bug.

At any rate, as I have just written to Jens, I have pushed a new
version of the branch [1] (not just added new commits, but also
integrated some old commit with new changes, to make it more quickly).
The branch now contains both a fix for the above bug, and, more
importantly, a fix for the circular dependencies that were still
lurking around.  Could you please test it?

Crossing my fingers,
Paolo

[1] https://github.com/Algodev-github/bfq-mq

> diff --git a/block/Makefile b/block/Makefile
> index 1c04fe19e825..bf472ac82c08 100644
> --- a/block/Makefile
> +++ b/block/Makefile
> @@ -2,6 +2,8 @@
> # Makefile for the kernel block layer
> #
>  
> +KBUILD_CFLAGS += -DCONFIG_BFQ_REDIRECT_TO_CONSOLE
> +
> 
> obj-$(CONFIG_BLOCK) := bio.o elevator.o blk-core.o blk-tag.o blk-sysfs.o \
>                        blk-flush.o blk-settings.o blk-ioc.o blk-map.o \
>                        blk-exec.o blk-merge.o blk-softirq.o blk-timeout.o \
> 
> Bart.
> Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:
> 
> This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.
> 

[toc] | [prev] | [next] | [standalone]

#1588180

> Il giorno 24 feb 2017, alle ore 19:44, Bart Van Assche <bart.vanassche@sandisk.com> ha scritto:
> 
> On Wed, 2017-02-22 at 22:29 +0100, Paolo Valente wrote:
>> thanks for this second attempt of yours.  Although, unfortunately, not
>> providing some clear indication of the exact cause of your hang (apart
>> from a possible deadlock), your log helped me notice another bug.
>> 
>> At any rate, as I have just written to Jens, I have pushed a new
>> version of the branch [1] (not just added new commits, but also
>> integrated some old commit with new changes, to make it more quickly).
>> The branch now contains both a fix for the above bug, and, more
>> importantly, a fix for the circular dependencies that were still
>> lurking around.  Could you please test it?
> 
> Hello Paolo,
> 

Hi

> I have good news: the same test system boots normally with the same
> kernel config I used during my previous tests and with the latest
> bfq-mq code (commit a965d19585c0) merged with kernel v4.10.
> 

Whew, I was longing for you reply, thanks :)

Should you want to have a look at it, I have just finished completing
cgroups support too, as you have probably already read from my
previous email.

Thanks,
Paolo

> Thanks,
> 
> Bart.

[toc] | [prev] | [next] | [standalone]

#1578753

> Il giorno 10 feb 2017, alle ore 19:34, Bart Van Assche <bart.vanassche@sandisk.com> ha scritto:
> 
> On Tue, 2017-02-07 at 18:24 +0100, Paolo Valente wrote:
>> (lock assertions, BUG_ONs, ...).
> 
> Hello Paolo,
> 
> If you are using BUG_ON(), does that mean that you are not aware of Linus'
> opinion about BUG_ON()? Please read https://lkml.org/lkml/2016/10/4/1.
> 

I am, thanks.  But this is a testing version, overfull of assertions
as a form of hysteric defensive programming.  I will of course remove
all halting assertions in the submission for merging.

Thanks,
Paolo

> Thanks,
> 
> Bart.
> Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:
> 
> This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.
> 

[toc] | [prev] | [next] | [standalone]

#1578755

On Tue, 2017-02-07 at 18:24 +0100, Paolo Valente wrote:
> (lock assertions, BUG_ONs, ...).

Hello Paolo,

If you are using BUG_ON(), does that mean that you are not aware of Linus'
opinion about BUG_ON()? Please read https://lkml.org/lkml/2016/10/4/1.

Thanks,

Bart.

[toc] | [prev] | [next] | [standalone]

#1580070

> Il giorno 07 feb 2017, alle ore 18:24, Paolo Valente <paolo.valente@linaro.org> ha scritto:
> 
> Hi,
> 
> I have finally pushed here [1] the current WIP branch of bfq for
> blk-mq, which I have tentatively named bfq-mq.
> 
> This branch *IS NOT* meant for merging into mainline and contain code
> that mau easily violate code style, and not only, in many
> places. Commits implement the following main steps:
> 1) Add the last version of bfq for blk
> 2) Clone bfq source files into identical bfq-mq source files
> 3) Modify bfq-mq files to get a working version of bfq for blk-mq
> (cgroups support not yet functional)
> 
> In my intentions, the main goals of this branch are:
> 
> 1) Show, as soon as I could, the changes I made to let bfq-mq comply
> with blk-mq-sched framework. I though this could be particularly
> useful for Jens, being BFQ identical to CFQ in terms of hook
> interfaces and io-context handling, and almost identical in terms
> request-merging.
> 
> 2) Enable people to test this first version bfq-mq. Code is purposely
> overfull of log messages and invariant checks that halt the system on
> failure (lock assertions, BUG_ONs, ...).
> 
> To make it easier to revise commits, I'm sending the patches that
> transform bfq into bfq-mq (last four patches in the branch [1]). They
> work on two files, bfq-mq-iosched.c and bfq-mq.h, which, at the
> beginning, are just copies of bfq-iosched.c and bfq.h.
> 

Hi,
this is just to inform that, as I just wrote to Bart, I have rebase
the branch [1] against the current content of for-4.11/next.

Jens, Omar, did you find the time to have a look at the main commits
or to run some test?

Thanks,
Paolo

[1] https://github.com/Algodev-github/bfq-mq

> Thanks,
> Paolo
> 
> [1] https://github.com/Algodev-github/bfq-mq
> 
> Paolo Valente (4):
>  blk-mq: pass bio to blk_mq_sched_get_rq_priv
>  Move thinktime from bic to bfqq
>  Embed bfq-ioc.c and add locking on request queue
>  Modify interface and operation to comply with blk-mq-sched
> 
> block/bfq-cgroup.c       |   4 -
> block/bfq-mq-iosched.c   | 852 +++++++++++++++++++++++++++++------------------
> block/bfq-mq.h           |  65 ++--
> block/blk-mq-sched.c     |   8 +-
> block/blk-mq-sched.h     |   5 +-
> include/linux/elevator.h |   2 +-
> 6 files changed, 567 insertions(+), 369 deletions(-)
> 
> --
> 2.10.0

[toc] | [prev] | [next] | [standalone]

#1580122

On 02/13/2017 02:09 PM, Paolo Valente wrote:
> 
>> Il giorno 07 feb 2017, alle ore 18:24, Paolo Valente <paolo.valente@linaro.org> ha scritto:
>>
>> Hi,
>>
>> I have finally pushed here [1] the current WIP branch of bfq for
>> blk-mq, which I have tentatively named bfq-mq.
>>
>> This branch *IS NOT* meant for merging into mainline and contain code
>> that mau easily violate code style, and not only, in many
>> places. Commits implement the following main steps:
>> 1) Add the last version of bfq for blk
>> 2) Clone bfq source files into identical bfq-mq source files
>> 3) Modify bfq-mq files to get a working version of bfq for blk-mq
>> (cgroups support not yet functional)
>>
>> In my intentions, the main goals of this branch are:
>>
>> 1) Show, as soon as I could, the changes I made to let bfq-mq comply
>> with blk-mq-sched framework. I though this could be particularly
>> useful for Jens, being BFQ identical to CFQ in terms of hook
>> interfaces and io-context handling, and almost identical in terms
>> request-merging.
>>
>> 2) Enable people to test this first version bfq-mq. Code is purposely
>> overfull of log messages and invariant checks that halt the system on
>> failure (lock assertions, BUG_ONs, ...).
>>
>> To make it easier to revise commits, I'm sending the patches that
>> transform bfq into bfq-mq (last four patches in the branch [1]). They
>> work on two files, bfq-mq-iosched.c and bfq-mq.h, which, at the
>> beginning, are just copies of bfq-iosched.c and bfq.h.
>>
> 
> Hi,
> this is just to inform that, as I just wrote to Bart, I have rebase
> the branch [1] against the current content of for-4.11/next.
> 
> Jens, Omar, did you find the time to have a look at the main commits
> or to run some test?

I only looked at the core change you proposed for passing in the
bio as well, and Omar fixed up the icq exit part and I also applied
that patch. I haven't look at any of the bfq-mq patches at all yet.
Not sure what I can do with those, I don't think those are
particularly useful to anyone but you.

Might make more sense to post the conversion for review as a
whole.

-- 
Jens Axboe

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web