lkml.org 
[lkml]   [2021]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] blk-cgroup: check blkcg policy is enabled in blkg_create()
    From
    Date
    On 2021/10/12 1:16, Tejun Heo wrote:
    > On Fri, Oct 08, 2021 at 03:27:20PM +0800, Yu Kuai wrote:
    >> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
    >> index eb48090eefce..00e1d97621ea 100644
    >> --- a/block/blk-cgroup.c
    >> +++ b/block/blk-cgroup.c
    >> @@ -226,6 +226,20 @@ struct blkcg_gq *blkg_lookup_slowpath(struct blkcg *blkcg,
    >> }
    >> EXPORT_SYMBOL_GPL(blkg_lookup_slowpath);
    >>
    >> +static void blkg_check_pd(struct request_queue *q, struct blkcg_gq *blkg)
    >> +{
    >> + int i;
    >> +
    >> + for (i = 0; i < BLKCG_MAX_POLS; i++) {
    >> + struct blkcg_policy *pol = blkcg_policy[i];
    >> +
    >> + if (blkg->pd[i] && !blkcg_policy_enabled(q, pol)) {
    >> + pol->pd_free_fn(blkg->pd[i]);
    >> + blkg->pd[i] = NULL;
    >> + }
    >> + }
    >> +}
    >> +
    >> /*
    >> * If @new_blkg is %NULL, this function tries to allocate a new one as
    >> * necessary using %GFP_NOWAIT. @new_blkg is always consumed on return.
    >> @@ -252,6 +266,9 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg,
    >> goto err_free_blkg;
    >> }
    >>
    >> + if (new_blkg)
    >> + blkg_check_pd(q, new_blkg);
    >> +
    >
    > Can't this happen the other way around too? ie. Linking a pd which doesn't
    > have an entry for a policy which got enabled inbetween? And what if an
    > existing policy was de-registered and another policy got the policy id
    > inbetween? I think the correct solution here would be synchronizing alloc -
    > create blocks against policy deactivation rather than trying to patch an
    > allocated blkg later. Deactivation being a really slow path, there are
    > plenty of options. The main challenge would making it difficult to make
    > mistakes with, I guess.

    For the case policy was de-registered, I think there won't be a problem
    because pd_init_fn() is not called yet, and the blkg is not at
    blkg_list, it's fine to use this blkg for the new policy.

    For the case policy got enabled inbetween, the problem is that the pd
    still doesn't have an entry for the policy, perhaps we can call
    pd_alloc_fn() additionally in blkg_create?

    If checking the blkg in blkg_create() is not a good solution, and we
    decide to synchronize alloc-create blkg against policy deactivation.
    Since only bfq policy can be deactivated or activated while queue is
    not dying, and queue is freezed during activation and deactivation,
    can we get a q->q_usage_counter and put it after blkg_create() is done
    to prevent concurrent bfq policy activation and deactivation?

    Thanks,
    Kuai
    >
    > Thanks.
    >

    \
     
     \ /
      Last update: 2021-10-12 03:40    [W:9.112 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site