lkml.org 
[lkml]   [2022]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.18 734/879] bfq: Get rid of __bio_blkcg() usage
    Date
    From: Jan Kara <jack@suse.cz>

    commit 4e54a2493e582361adc3bfbf06c7d50d19d18837 upstream.

    BFQ usage of __bio_blkcg() is a relict from the past. Furthermore if bio
    would not be associated with any blkcg, the usage of __bio_blkcg() in
    BFQ is prone to races with the task being migrated between cgroups as
    __bio_blkcg() calls at different places could return different blkcgs.

    Convert BFQ to the new situation where bio->bi_blkg is initialized in
    bio_set_dev() and thus practically always valid. This allows us to save
    blkcg_gq lookup and noticeably simplify the code.

    CC: stable@vger.kernel.org
    Fixes: 0fe061b9f03c ("blkcg: fix ref count issue with bio_blkcg() using task_css")
    Tested-by: "yukuai (C)" <yukuai3@huawei.com>
    Signed-off-by: Jan Kara <jack@suse.cz>
    Reviewed-by: Christoph Hellwig <hch@lst.de>
    Link: https://lore.kernel.org/r/20220401102752.8599-8-jack@suse.cz
    Signed-off-by: Jens Axboe <axboe@kernel.dk>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    block/bfq-cgroup.c | 63 ++++++++++++++++++----------------------------------
    block/bfq-iosched.c | 11 ---------
    block/bfq-iosched.h | 3 --
    3 files changed, 25 insertions(+), 52 deletions(-)

    --- a/block/bfq-cgroup.c
    +++ b/block/bfq-cgroup.c
    @@ -586,27 +586,11 @@ static void bfq_group_set_parent(struct
    entity->sched_data = &parent->sched_data;
    }

    -static struct bfq_group *bfq_lookup_bfqg(struct bfq_data *bfqd,
    - struct blkcg *blkcg)
    +static void bfq_link_bfqg(struct bfq_data *bfqd, struct bfq_group *bfqg)
    {
    - struct blkcg_gq *blkg;
    -
    - blkg = blkg_lookup(blkcg, bfqd->queue);
    - if (likely(blkg))
    - return blkg_to_bfqg(blkg);
    - return NULL;
    -}
    -
    -struct bfq_group *bfq_find_set_group(struct bfq_data *bfqd,
    - struct blkcg *blkcg)
    -{
    - struct bfq_group *bfqg, *parent;
    + struct bfq_group *parent;
    struct bfq_entity *entity;

    - bfqg = bfq_lookup_bfqg(bfqd, blkcg);
    - if (unlikely(!bfqg))
    - return NULL;
    -
    /*
    * Update chain of bfq_groups as we might be handling a leaf group
    * which, along with some of its relatives, has not been hooked yet
    @@ -623,8 +607,15 @@ struct bfq_group *bfq_find_set_group(str
    bfq_group_set_parent(curr_bfqg, parent);
    }
    }
    +}

    - return bfqg;
    +struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio)
    +{
    + struct blkcg_gq *blkg = bio->bi_blkg;
    +
    + if (!blkg)
    + return bfqd->root_group;
    + return blkg_to_bfqg(blkg);
    }

    /**
    @@ -714,25 +705,15 @@ void bfq_bfqq_move(struct bfq_data *bfqd
    * Move bic to blkcg, assuming that bfqd->lock is held; which makes
    * sure that the reference to cgroup is valid across the call (see
    * comments in bfq_bic_update_cgroup on this issue)
    - *
    - * NOTE: an alternative approach might have been to store the current
    - * cgroup in bfqq and getting a reference to it, reducing the lookup
    - * time here, at the price of slightly more complex code.
    */
    -static struct bfq_group *__bfq_bic_change_cgroup(struct bfq_data *bfqd,
    - struct bfq_io_cq *bic,
    - struct blkcg *blkcg)
    +static void *__bfq_bic_change_cgroup(struct bfq_data *bfqd,
    + struct bfq_io_cq *bic,
    + struct bfq_group *bfqg)
    {
    struct bfq_queue *async_bfqq = bic_to_bfqq(bic, 0);
    struct bfq_queue *sync_bfqq = bic_to_bfqq(bic, 1);
    - struct bfq_group *bfqg;
    struct bfq_entity *entity;

    - bfqg = bfq_find_set_group(bfqd, blkcg);
    -
    - if (unlikely(!bfqg))
    - bfqg = bfqd->root_group;
    -
    if (async_bfqq) {
    entity = &async_bfqq->entity;

    @@ -784,20 +765,24 @@ static struct bfq_group *__bfq_bic_chang
    void bfq_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio)
    {
    struct bfq_data *bfqd = bic_to_bfqd(bic);
    - struct bfq_group *bfqg = NULL;
    + struct bfq_group *bfqg = bfq_bio_bfqg(bfqd, bio);
    uint64_t serial_nr;

    - rcu_read_lock();
    - serial_nr = __bio_blkcg(bio)->css.serial_nr;
    + serial_nr = bfqg_to_blkg(bfqg)->blkcg->css.serial_nr;

    /*
    * Check whether blkcg has changed. The condition may trigger
    * spuriously on a newly created cic but there's no harm.
    */
    if (unlikely(!bfqd) || likely(bic->blkcg_serial_nr == serial_nr))
    - goto out;
    + return;

    - bfqg = __bfq_bic_change_cgroup(bfqd, bic, __bio_blkcg(bio));
    + /*
    + * New cgroup for this process. Make sure it is linked to bfq internal
    + * cgroup hierarchy.
    + */
    + bfq_link_bfqg(bfqd, bfqg);
    + __bfq_bic_change_cgroup(bfqd, bic, bfqg);
    /*
    * Update blkg_path for bfq_log_* functions. We cache this
    * path, and update it here, for the following
    @@ -850,8 +835,6 @@ void bfq_bic_update_cgroup(struct bfq_io
    */
    blkg_path(bfqg_to_blkg(bfqg), bfqg->blkg_path, sizeof(bfqg->blkg_path));
    bic->blkcg_serial_nr = serial_nr;
    -out:
    - rcu_read_unlock();
    }

    /**
    @@ -1469,7 +1452,7 @@ void bfq_end_wr_async(struct bfq_data *b
    bfq_end_wr_async_queues(bfqd, bfqd->root_group);
    }

    -struct bfq_group *bfq_find_set_group(struct bfq_data *bfqd, struct blkcg *blkcg)
    +struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio)
    {
    return bfqd->root_group;
    }
    --- a/block/bfq-iosched.c
    +++ b/block/bfq-iosched.c
    @@ -5730,14 +5730,7 @@ static struct bfq_queue *bfq_get_queue(s
    struct bfq_queue *bfqq;
    struct bfq_group *bfqg;

    - rcu_read_lock();
    -
    - bfqg = bfq_find_set_group(bfqd, __bio_blkcg(bio));
    - if (!bfqg) {
    - bfqq = &bfqd->oom_bfqq;
    - goto out;
    - }
    -
    + bfqg = bfq_bio_bfqg(bfqd, bio);
    if (!is_sync) {
    async_bfqq = bfq_async_queue_prio(bfqd, bfqg, ioprio_class,
    ioprio);
    @@ -5783,8 +5776,6 @@ out:

    if (bfqq != &bfqd->oom_bfqq && is_sync && !respawn)
    bfqq = bfq_do_or_sched_stable_merge(bfqd, bfqq, bic);
    -
    - rcu_read_unlock();
    return bfqq;
    }

    --- a/block/bfq-iosched.h
    +++ b/block/bfq-iosched.h
    @@ -1010,8 +1010,7 @@ void bfq_bfqq_move(struct bfq_data *bfqd
    void bfq_init_entity(struct bfq_entity *entity, struct bfq_group *bfqg);
    void bfq_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio);
    void bfq_end_wr_async(struct bfq_data *bfqd);
    -struct bfq_group *bfq_find_set_group(struct bfq_data *bfqd,
    - struct blkcg *blkcg);
    +struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio);
    struct blkcg_gq *bfqg_to_blkg(struct bfq_group *bfqg);
    struct bfq_group *bfqq_group(struct bfq_queue *bfqq);
    struct bfq_group *bfq_create_group_hierarchy(struct bfq_data *bfqd, int node);

    \
     
     \ /
      Last update: 2022-06-08 02:23    [W:3.085 / U:0.416 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site