lkml.org 
[lkml]   [2015]   [Jun]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 28/51] writeback, blkcg: restructure blk_{set|clear}_queue_congested()
    On Fri 22-05-15 17:13:42, Tejun Heo wrote:
    > blk_{set|clear}_queue_congested() take @q and set or clear,
    > respectively, the congestion state of its bdi's root wb. Because bdi
    > used to be able to handle congestion state only on the root wb, the
    > callers of those functions tested whether the congestion is on the
    > root blkcg and skipped if not.
    >
    > This is cumbersome and makes implementation of per cgroup
    > bdi_writeback congestion state propagation difficult. This patch
    > renames blk_{set|clear}_queue_congested() to
    > blk_{set|clear}_congested(), and makes them take request_list instead
    > of request_queue and test whether the specified request_list is the
    > root one before updating bdi_writeback congestion state. This makes
    > the tests in the callers unnecessary and simplifies them.
    >
    > As there are no external users of these functions, the definitions are
    > moved from include/linux/blkdev.h to block/blk-core.c.
    >
    > This patch doesn't introduce any noticeable behavior difference.

    Looks good. You can add:

    Reviewed-by: Jan Kara <jack@suse.com>

    BTW, I'd prefer if this was merged with the following patch. I was
    wondering for a while about the condition at the beginning of
    blk_clear_congested() only to learn it gets modified to the one I'd expect
    in the following patch :)

    Honza

    >
    > Signed-off-by: Tejun Heo <tj@kernel.org>
    > Cc: Jens Axboe <axboe@kernel.dk>
    > Cc: Jan Kara <jack@suse.cz>
    > Cc: Vivek Goyal <vgoyal@redhat.com>
    > ---
    > block/blk-core.c | 62 ++++++++++++++++++++++++++++++--------------------
    > include/linux/blkdev.h | 19 ----------------
    > 2 files changed, 37 insertions(+), 44 deletions(-)
    >
    > diff --git a/block/blk-core.c b/block/blk-core.c
    > index e0f726f..b457c4f 100644
    > --- a/block/blk-core.c
    > +++ b/block/blk-core.c
    > @@ -63,6 +63,28 @@ struct kmem_cache *blk_requestq_cachep;
    > */
    > static struct workqueue_struct *kblockd_workqueue;
    >
    > +static void blk_clear_congested(struct request_list *rl, int sync)
    > +{
    > + if (rl != &rl->q->root_rl)
    > + return;
    > +#ifdef CONFIG_CGROUP_WRITEBACK
    > + clear_wb_congested(rl->blkg->wb_congested, sync);
    > +#else
    > + clear_wb_congested(rl->q->backing_dev_info.wb.congested, sync);
    > +#endif
    > +}
    > +
    > +static void blk_set_congested(struct request_list *rl, int sync)
    > +{
    > + if (rl != &rl->q->root_rl)
    > + return;
    > +#ifdef CONFIG_CGROUP_WRITEBACK
    > + set_wb_congested(rl->blkg->wb_congested, sync);
    > +#else
    > + set_wb_congested(rl->q->backing_dev_info.wb.congested, sync);
    > +#endif
    > +}
    > +
    > void blk_queue_congestion_threshold(struct request_queue *q)
    > {
    > int nr;
    > @@ -841,13 +863,8 @@ static void __freed_request(struct request_list *rl, int sync)
    > {
    > struct request_queue *q = rl->q;
    >
    > - /*
    > - * bdi isn't aware of blkcg yet. As all async IOs end up root
    > - * blkcg anyway, just use root blkcg state.
    > - */
    > - if (rl == &q->root_rl &&
    > - rl->count[sync] < queue_congestion_off_threshold(q))
    > - blk_clear_queue_congested(q, sync);
    > + if (rl->count[sync] < queue_congestion_off_threshold(q))
    > + blk_clear_congested(rl, sync);
    >
    > if (rl->count[sync] + 1 <= q->nr_requests) {
    > if (waitqueue_active(&rl->wait[sync]))
    > @@ -880,25 +897,25 @@ static void freed_request(struct request_list *rl, unsigned int flags)
    > int blk_update_nr_requests(struct request_queue *q, unsigned int nr)
    > {
    > struct request_list *rl;
    > + int on_thresh, off_thresh;
    >
    > spin_lock_irq(q->queue_lock);
    > q->nr_requests = nr;
    > blk_queue_congestion_threshold(q);
    > + on_thresh = queue_congestion_on_threshold(q);
    > + off_thresh = queue_congestion_off_threshold(q);
    >
    > - /* congestion isn't cgroup aware and follows root blkcg for now */
    > - rl = &q->root_rl;
    > -
    > - if (rl->count[BLK_RW_SYNC] >= queue_congestion_on_threshold(q))
    > - blk_set_queue_congested(q, BLK_RW_SYNC);
    > - else if (rl->count[BLK_RW_SYNC] < queue_congestion_off_threshold(q))
    > - blk_clear_queue_congested(q, BLK_RW_SYNC);
    > + blk_queue_for_each_rl(rl, q) {
    > + if (rl->count[BLK_RW_SYNC] >= on_thresh)
    > + blk_set_congested(rl, BLK_RW_SYNC);
    > + else if (rl->count[BLK_RW_SYNC] < off_thresh)
    > + blk_clear_congested(rl, BLK_RW_SYNC);
    >
    > - if (rl->count[BLK_RW_ASYNC] >= queue_congestion_on_threshold(q))
    > - blk_set_queue_congested(q, BLK_RW_ASYNC);
    > - else if (rl->count[BLK_RW_ASYNC] < queue_congestion_off_threshold(q))
    > - blk_clear_queue_congested(q, BLK_RW_ASYNC);
    > + if (rl->count[BLK_RW_ASYNC] >= on_thresh)
    > + blk_set_congested(rl, BLK_RW_ASYNC);
    > + else if (rl->count[BLK_RW_ASYNC] < off_thresh)
    > + blk_clear_congested(rl, BLK_RW_ASYNC);
    >
    > - blk_queue_for_each_rl(rl, q) {
    > if (rl->count[BLK_RW_SYNC] >= q->nr_requests) {
    > blk_set_rl_full(rl, BLK_RW_SYNC);
    > } else {
    > @@ -1008,12 +1025,7 @@ static struct request *__get_request(struct request_list *rl, int rw_flags,
    > }
    > }
    > }
    > - /*
    > - * bdi isn't aware of blkcg yet. As all async IOs end up
    > - * root blkcg anyway, just use root blkcg state.
    > - */
    > - if (rl == &q->root_rl)
    > - blk_set_queue_congested(q, is_sync);
    > + blk_set_congested(rl, is_sync);
    > }
    >
    > /*
    > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
    > index 89bdef0..3d1065c 100644
    > --- a/include/linux/blkdev.h
    > +++ b/include/linux/blkdev.h
    > @@ -794,25 +794,6 @@ extern int sg_scsi_ioctl(struct request_queue *, struct gendisk *, fmode_t,
    >
    > extern void blk_queue_bio(struct request_queue *q, struct bio *bio);
    >
    > -/*
    > - * A queue has just exitted congestion. Note this in the global counter of
    > - * congested queues, and wake up anyone who was waiting for requests to be
    > - * put back.
    > - */
    > -static inline void blk_clear_queue_congested(struct request_queue *q, int sync)
    > -{
    > - clear_bdi_congested(&q->backing_dev_info, sync);
    > -}
    > -
    > -/*
    > - * A queue has just entered congestion. Flag that in the queue's VM-visible
    > - * state flags and increment the global gounter of congested queues.
    > - */
    > -static inline void blk_set_queue_congested(struct request_queue *q, int sync)
    > -{
    > - set_bdi_congested(&q->backing_dev_info, sync);
    > -}
    > -
    > extern void blk_start_queue(struct request_queue *q);
    > extern void blk_stop_queue(struct request_queue *q);
    > extern void blk_sync_queue(struct request_queue *q);
    > --
    > 2.4.0
    >
    --
    Jan Kara <jack@suse.cz>
    SUSE Labs, CR


    \
     
     \ /
      Last update: 2015-06-30 17:21    [W:4.186 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site