Re: [PATCH v2 1/2] block: add BPF kfuncs to read blkcg io.stat

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Concurrent execution of `blkcg_fill_root_iostats()` corrupts the `u64_stats_sync` sequence counter, leading to reader deadlocks.
--

--- Patch [1]: [PATCH v2 1/2] block: add BPF kfuncs to read blkcg io.stat ---
commit bdaf96b752ce960b924c3be337708d377f83207c
Author: Ziyang Men <[email protected]>

block: add BPF kfuncs to read blkcg io.stat

This commit introduces BPF kfuncs to allow reading per-device block I/O
controller statistics. It exposes blkcg_fill_root_iostats and provides
bpf_blkcg_flush_stats and iterator functions for blkg.

> diff --git a/block/bpf_blkcg.c b/block/bpf_blkcg.c
> new file mode 100644
> index 0000000000000..25c809f5091c2
> --- /dev/null
> +++ b/block/bpf_blkcg.c
[ ... ]
> +__bpf_kfunc void bpf_blkcg_flush_stats(struct cgroup *cgrp)
> +{
> +	struct cgroup_subsys_state *css;
> +
> +	/* Pin the css for the sleepable flush. */
> +	rcu_read_lock();
> +	css = cgroup_css(cgrp, &io_cgrp_subsys);
> +	if (css && !css_tryget(css))
> +		css = NULL;
> +	rcu_read_unlock();
> +
> +	if (!css)
> +		return;
> +
> +	if (!css->parent)
> +		blkcg_fill_root_iostats();

[Severity: High]
This isn't a bug introduced by this patch, but can concurrent executions
here lead to sequence counter corruption and reader deadlocks on 32-bit
systems?

In block/blk-cgroup.c, blkcg_fill_root_iostats() loops over block devices
and updates statistics without an overarching writer lock:

blkcg_fill_root_iostats() {
    ...
    flags = u64_stats_update_begin_irqsave(&blkg->iostat.sync);
    blkg_iostat_set(&blkg->iostat.cur, &tmp);
    u64_stats_update_end_irqrestore(&blkg->iostat.sync, flags);
    ...
}

On 32-bit architectures, u64_stats_update_begin_irqsave() maps to
write_seqcount_begin(), which requires external mutual exclusion to prevent
concurrent writers from corrupting the sequence counter. If the counter
becomes permanently odd, readers using read_seqcount_begin() will spin
infinitely.

While this bug existed previously via concurrent reads of the root cgroup's
io.stat sysfs file, exposing blkcg_fill_root_iostats() via this sleepable
BPF kfunc allows BPF programs to easily and repeatedly trigger this lockless
code path simultaneously across multiple CPUs.

> +	else
> +		css_rstat_flush(css);
> +
> +	css_put(css);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1