Re: [RFC PATCH v2 7/8] bfq: avoid blkg lookup from locked cgroup update

Hi,

在 2026/7/29 15:33, Tao Cui 写道:
>
> 在 2026/7/24 20:30, Yu Kuai 写道:
>> From: Yu Kuai <[email protected]>
>>
>> bfq_bio_bfqg() is called while bfqd->lock is held from the merge and
>> request insertion paths.  It walks bio->bi_blkg and its parent chain to
>> find the closest online BFQ group, and re-associates the bio by looking
>> the chosen blkg up again through bio_associate_blkg_from_css().
>>
>> Now that blkg creation runs under q->blkcg_mutex from the sleepable submit
>> path, bio_associate_blkg_from_css() can sleep on a lookup miss, which BFQ
>> must not do while holding bfqd->lock.  The blkg BFQ wants is already in
>> hand from the ancestry walk, so update bio->bi_blkg by swapping references
>> to that existing blkg directly instead of looking it up again by css.
>>
> Just checking the new bfq helper that swaps a bio's blkg: it does the
> blkg_put() without the NULL guard the old helper had.  Is there any path
> where BFQ could see a bio that hasn't been associated yet (bi_blkg == NULL)?
> The fallback branch calls it unconditionally, so if such a path exists it
> would crash.  My reading is the bio is always associated before it reaches
> BFQ, so it can't happen -- but wanted to ask in case I'm missing a path.

You're not missing. Bio must associate a blkg before submitting, you can see
lots of places will deference bio's blkg directly.

>
> Thanks,
> Tao
>
>> Signed-off-by: Yu Kuai <[email protected]>
>> ---
>>   block/bfq-cgroup.c | 16 +++++++++++++---
>>   1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
>> index 42614aa78cd4..8a3ff9510386 100644
>> --- a/block/bfq-cgroup.c
>> +++ b/block/bfq-cgroup.c
>> @@ -604,6 +604,16 @@ static void bfq_link_bfqg(struct bfq_data *bfqd, struct bfq_group *bfqg)
>>   	}
>>   }
>>   
>> +static void bfq_bio_update_blkg(struct bio *bio, struct blkcg_gq *blkg)
>> +{
>> +	if (bio->bi_blkg == blkg)
>> +		return;
>> +
>> +	blkg_get(blkg);
>> +	blkg_put(bio->bi_blkg);
>> +	bio->bi_blkg = blkg;
>> +}
>> +
>>   struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio)
>>   {
>>   	struct blkcg_gq *blkg = bio->bi_blkg;
>> @@ -616,13 +626,13 @@ struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio)
>>   		}
>>   		bfqg = blkg_to_bfqg(blkg);
>>   		if (bfqg->pd.online) {
>> -			bio_associate_blkg_from_css(bio, &blkg->blkcg->css);
>> +			bfq_bio_update_blkg(bio, blkg);
>>   			return bfqg;
>>   		}
>>   		blkg = blkg->parent;
>>   	}
>> -	bio_associate_blkg_from_css(bio,
>> -				&bfqg_to_blkg(bfqd->root_group)->blkcg->css);
>> +	blkg = bfqg_to_blkg(bfqd->root_group);
>> +	bfq_bio_update_blkg(bio, blkg);
>>   	return bfqd->root_group;
>>   }
>>   

-- 
Thanks,
Kuai