Looks good:
Reviewed-by: Christoph Hellwig <hch@xxxxxx>
Follow up comments below:
> + rcu_read_lock();
> +
> + if (!bio->bi_blkg) {
> + char b[BDEVNAME_SIZE];
> +
> + WARN_ONCE(1,
> + "no blkg associated for bio on block-device: %s\n",
> + bio_devname(bio, b));
> + bio_associate_blkg(bio);
> + }
> +
> + blkg = bio->bi_blkg;
We now always assign a bi_blkg, so as a follow on patch we should
probab;y remove this check and assign blkg at the time of declaration.
> +
> + throtl = blk_throtl_bio(q, blkg, bio);
> +
> + if (!throtl) {
The empty line hurts my feelings :)
> + struct blkg_iostat_set *bis;
> + int rwd, cpu;
> +
> + if (op_is_discard(bio->bi_opf))
> + rwd = BLKG_IOSTAT_DISCARD;
> + else if (op_is_write(bio->bi_opf))
> + rwd = BLKG_IOSTAT_WRITE;
> + else
> + rwd = BLKG_IOSTAT_READ;
> +
> + cpu = get_cpu();
> + bis = per_cpu_ptr(blkg->iostat_cpu, cpu);
> + u64_stats_update_begin(&bis->sync);
> +
> + /*
> + * If the bio is flagged with BIO_QUEUE_ENTERED it means this
> + * is a split bio and we would have already accounted for the
> + * size of the bio.
> + */
> + if (!bio_flagged(bio, BIO_QUEUE_ENTERED))
> + bis->cur.bytes[rwd] += bio->bi_iter.bi_size;
> + bis->cur.ios[rwd]++;
> +
> + u64_stats_update_end(&bis->sync);
> + if (cgroup_subsys_on_dfl(io_cgrp_subsys))
> + cgroup_rstat_updated(blkg->blkcg->css.cgroup, cpu);
> + put_cpu();
As-is this will clash with my BIO_QUEUE_ENTERED cleanup.
> @@ -666,6 +609,7 @@ static inline void blkcg_clear_delay(struct blkcg_gq *blkg)
> }
> }
>
> +bool blkcg_bio_issue_check(struct request_queue *q, struct bio *bio);
It might be worth to just throw a IS_ENABLED(CONFIG_BLK_CGROUP) into
the caller and avoid the need for a stub in the header.
