Re: dm review needed, was: Re: [PATCH v3 2/3] blk-cgroup: store blkcg in bio instead of blkg

Hi,

在 2026/9/18 15:16, Christoph Hellwig 写道:
> On Fri, Sep 18, 2026 at 12:17:43PM +0800, Yu Kuai wrote:
>>   static inline void dm_bio_record(struct dm_bio_details *bd, struct bio *bio)
>>   {
>>   	bd->bi_bdev = bio->bi_bdev;
>> -	bd->bi_flags = bio->bi_flags;
>> +	/*
>> +	 * Restoring BIO_BLKG_REF would not recreate its associated reference.
>> +	 */
>> +	bd->bi_flags = bio->bi_flags & ~(1U << BIO_BLKG_REF);
> But do we need to restore the reference?  I have to admit that
> this surrounding code is very confusing and I would really like
> to have the dm maintainers look over it.

dm_bio_record() will store the bi_flags for current bio, later the flag is restored
by dm_bio_restore(). For example like dm-raid1 read error path:

issue path, if blk-throttle is enabled, flag can be set before bio issuing to dm-raid1

mirror_map
  dm_bio_record
   bd->bi_flags = bio->bi_flags
   // contain the new flag

bio_endio
  bio_clear_blkcg
  // new flag is cleared
  clone_endio
   mirror_end_io
   // if this IO failed and there is another mirror to read
    dm_bio_restore
     bio->bi_flags = bd->bi_flags
     // new flag is set without get a blkg reference

So I think we don't need to restore the reference, BTW, there is another approach to
avoid such problem in blk-throttle I don't take for now:

For bio entering blk-throttle, we can clear the new flag and blkg reference directly
after the bio is throttled and before this bio is issuing to disk, because bio will
not enter blk-throttle in the completion path. And since blk-throttle is the only rq_qos
policy that can be enabled for bio-based disk, then dm_bio_record() will never see a
bio with the new flag set.

-- 
Thanks,
Kuai