Re: [PATCH v2 3/3] sched_ext: Add cmask mask ops

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scx_cmask_and() and scx_cmask_copy() fail to clear destination bits outside the source mask's range, breaking expected set semantics.
--

commit 009e14b778a21e3b2cdd8abbf60a8256e3cacb80
Author: Tejun Heo <tj@kernel.org>

sched_ext: Add cmask mask ops

This commit introduces operations on cmasks, including mutating operations
like and/or/copy/andnot and predicates like subset/intersects/empty. It
implements cmask_walk_op2() to process the intersection of two mask ranges.

> diff --git a/kernel/sched/ext_cid.c b/kernel/sched/ext_cid.c
> index 44dd47a877090..0c91b951fd33c 100644
> --- a/kernel/sched/ext_cid.c
> +++ b/kernel/sched/ext_cid.c
[ ... ]
> +/*
> + * Set ops on cmasks. cmask_walk_op2() shares one walk across mutating
> + * (and/or/copy/andnot) and predicate (subset/intersects) two-cmask forms;
> + * cmask_walk_op1() does the same shape over a single cmask range. Every public
> + * entry passes a compile-time-constant @op; cmask_walk_op{1,2}() and
> + * cmask_word_op{1,2}() are __always_inline so the inner switch collapses to the
> + * selected op and cmask_op2_is_pred() folds the predicate early-exit out of
> + * mutating ops.
> + *
> + * Two-cmask ops only touch @dst bits inside the intersection of the two ranges;
> + * bits outside stay untouched. In particular, scx_cmask_copy() does NOT zero
> + * @dst bits that lie outside @src's range.
> + *
> + * The _RACY variants are otherwise identical to their non-racy counterpart but
> + * read @src word-by-word via data_race(). Memory ordering with concurrent
> + * writers is the caller's responsibility.
> + */
[ ... ]
> +static __always_inline bool cmask_walk_op2(u64 *a_bits, u32 a_base, u32 a_nr_cids,
> +					   const u64 *b_bits, u32 b_base, u32 b_nr_cids,
> +					   const enum cmask_op2 op)
> +{
> +	u32 lo = max(a_base, b_base);
> +	u32 hi = min(a_base + a_nr_cids, b_base + b_nr_cids);
[ ... ]
> +void scx_cmask_and(struct scx_cmask *dst, const struct scx_cmask *src)
> +{
> +	cmask_walk_op2(dst->bits, dst->base, dst->nr_cids,
> +		       src->bits, src->base, src->nr_cids, CMASK_OP2_AND);
> +}
[ ... ]
> +void scx_cmask_copy(struct scx_cmask *dst, const struct scx_cmask *src)
> +{
> +	cmask_walk_op2(dst->bits, dst->base, dst->nr_cids,
> +		       src->bits, src->base, src->nr_cids, CMASK_OP2_COPY);
> +}

While the comment documents that bits outside the intersection stay untouched,
could exposing these as standard scx_cmask_and() and scx_cmask_copy()
operations cause regressions for callers expecting standard set semantics?

For scx_cmask_and(), any bits in dst outside src's bounds implicitly evaluate
to 0 in src, but since cmask_walk_op2() restricts its bounds to the
intersection, those bits in dst are left unmodified rather than being cleared.

Similarly for scx_cmask_copy(), dst retains any previously set bits that fall
outside src's bounds.

Could callers relying on standard bitwise AND or COPY semantics end up with
stale bits in dst, leading to unexpected behavior in the scheduler?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519075838.2706712-1-tj@kernel.org?part=3