lkml.org 
[lkml]   [2014]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 1/9] block: Stop abusing csd.list for fifo_time
    On Mon, Dec 23, 2013 at 09:39:22PM +0100, Jan Kara wrote:
    > Block layer currently abuses rq->csd.list.next for storing fifo_time.
    > That is a terrible hack and completely unnecessary as well. Union
    > achieves the same space saving in a cleaner way.
    >
    > Signed-off-by: Jan Kara <jack@suse.cz>

    Hi Jan,

    Taken as is, the patch is fine and it builds.
    But later when I finally get rid of csd->list in a subsequent patch,
    rq_fifo_clear() callers break the build.

    This is because rq_fifo_clear() initialize the csd->list and I'm not
    sure how to fix that leftover because I am not clear about the purpose
    of that INIT_LIST_HEAD(): is it to reset fifo time or to prepare for
    an IPI to be queued?

    All in one it looks buggy because if this is to prepare for the IPI,
    it's useless as csd.list is not a list head but just a node. Otherwise if it
    is to reset fifo_time it's wrong because INIT_LIST_HEAD doesn't initialize
    to 0.

    Anyway so I did a braindead fix by removing the whole INIT_LIST_HEAD()
    from rq_fifo_clear(), see the patch below. Now to be honest I have no idea
    what I'm doing.

    ---
    From 112bcbb73076dd1374541eec9b554410dd0e73e0 Mon Sep 17 00:00:00 2001
    From: Jan Kara <jack@suse.cz>
    Date: Mon, 23 Dec 2013 21:39:22 +0100
    Subject: [PATCH] block: Stop abusing csd.list for fifo_time

    Block layer currently abuses rq->csd.list.next for storing fifo_time.
    That is a terrible hack and completely unnecessary as well. Union
    achieves the same space saving in a cleaner way.

    Signed-off-by: Jan Kara <jack@suse.cz>
    Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
    ---
    block/cfq-iosched.c | 8 ++++----
    block/deadline-iosched.c | 8 ++++----
    include/linux/blkdev.h | 1 +
    include/linux/elevator.h | 12 ++----------
    4 files changed, 11 insertions(+), 18 deletions(-)

    diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
    index 744833b..5873e4a 100644
    --- a/block/cfq-iosched.c
    +++ b/block/cfq-iosched.c
    @@ -2367,10 +2367,10 @@ cfq_merged_requests(struct request_queue *q, struct request *rq,
    * reposition in fifo if next is older than rq
    */
    if (!list_empty(&rq->queuelist) && !list_empty(&next->queuelist) &&
    - time_before(rq_fifo_time(next), rq_fifo_time(rq)) &&
    + time_before(next->fifo_time, rq->fifo_time) &&
    cfqq == RQ_CFQQ(next)) {
    list_move(&rq->queuelist, &next->queuelist);
    - rq_set_fifo_time(rq, rq_fifo_time(next));
    + rq->fifo_time = next->fifo_time;
    }

    if (cfqq->next_rq == next)
    @@ -2814,7 +2814,7 @@ static struct request *cfq_check_fifo(struct cfq_queue *cfqq)
    return NULL;

    rq = rq_entry_fifo(cfqq->fifo.next);
    - if (time_before(jiffies, rq_fifo_time(rq)))
    + if (time_before(jiffies, rq->fifo_time))
    rq = NULL;

    cfq_log_cfqq(cfqq->cfqd, cfqq, "fifo=%p", rq);
    @@ -3927,7 +3927,7 @@ static void cfq_insert_request(struct request_queue *q, struct request *rq)
    cfq_log_cfqq(cfqd, cfqq, "insert_request");
    cfq_init_prio_data(cfqq, RQ_CIC(rq));

    - rq_set_fifo_time(rq, jiffies + cfqd->cfq_fifo_expire[rq_is_sync(rq)]);
    + rq->fifo_time = jiffies + cfqd->cfq_fifo_expire[rq_is_sync(rq)];
    list_add_tail(&rq->queuelist, &cfqq->fifo);
    cfq_add_rq_rb(rq);
    cfqg_stats_update_io_add(RQ_CFQG(rq), cfqd->serving_group,
    diff --git a/block/deadline-iosched.c b/block/deadline-iosched.c
    index 9ef6640..a753df2 100644
    --- a/block/deadline-iosched.c
    +++ b/block/deadline-iosched.c
    @@ -106,7 +106,7 @@ deadline_add_request(struct request_queue *q, struct request *rq)
    /*
    * set expire time and add to fifo list
    */
    - rq_set_fifo_time(rq, jiffies + dd->fifo_expire[data_dir]);
    + rq->fifo_time = jiffies + dd->fifo_expire[data_dir];
    list_add_tail(&rq->queuelist, &dd->fifo_list[data_dir]);
    }

    @@ -174,9 +174,9 @@ deadline_merged_requests(struct request_queue *q, struct request *req,
    * and move into next position (next will be deleted) in fifo
    */
    if (!list_empty(&req->queuelist) && !list_empty(&next->queuelist)) {
    - if (time_before(rq_fifo_time(next), rq_fifo_time(req))) {
    + if (time_before(next->fifo_time, req->fifo_time)) {
    list_move(&req->queuelist, &next->queuelist);
    - rq_set_fifo_time(req, rq_fifo_time(next));
    + req->fifo_time = next->fifo_time;
    }
    }

    @@ -230,7 +230,7 @@ static inline int deadline_check_fifo(struct deadline_data *dd, int ddir)
    /*
    * rq is expired!
    */
    - if (time_after_eq(jiffies, rq_fifo_time(rq)))
    + if (time_after_eq(jiffies, rq->fifo_time))
    return 1;

    return 0;
    diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
    index 8678c43..dda98e3 100644
    --- a/include/linux/blkdev.h
    +++ b/include/linux/blkdev.h
    @@ -99,6 +99,7 @@ struct request {
    union {
    struct call_single_data csd;
    struct work_struct mq_flush_data;
    + unsigned long fifo_time;
    };

    struct request_queue *q;
    diff --git a/include/linux/elevator.h b/include/linux/elevator.h
    index 306dd8c..0b87f4e 100644
    --- a/include/linux/elevator.h
    +++ b/include/linux/elevator.h
    @@ -202,17 +202,9 @@ enum {
    #define rq_end_sector(rq) (blk_rq_pos(rq) + blk_rq_sectors(rq))
    #define rb_entry_rq(node) rb_entry((node), struct request, rb_node)

    -/*
    - * Hack to reuse the csd.list list_head as the fifo time holder while
    - * the request is in the io scheduler. Saves an unsigned long in rq.
    - */
    -#define rq_fifo_time(rq) ((unsigned long) (rq)->csd.list.next)
    -#define rq_set_fifo_time(rq,exp) ((rq)->csd.list.next = (void *) (exp))
    #define rq_entry_fifo(ptr) list_entry((ptr), struct request, queuelist)
    -#define rq_fifo_clear(rq) do { \
    - list_del_init(&(rq)->queuelist); \
    - INIT_LIST_HEAD(&(rq)->csd.list); \
    - } while (0)
    +//CHECKME twice
    +#define rq_fifo_clear(rq) list_del_init(&(rq)->queuelist);

    #else /* CONFIG_BLOCK */

    --
    1.8.3.1


    \
     
     \ /
      Last update: 2014-02-01 18:01    [W:4.259 / U:0.168 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site