On 2026-05-08 at 09:19:08, Ratheesh Kannoth ([email protected]) wrote:
> CN20K NPC MCAM is split into 32 subbanks that are searched in a
> predefined order during allocation. Lower-numbered subbanks have
> higher priority than higher-numbered ones.
pw-bot: changes-requested
> + for (sb_idx = 0; sb_idx < cnt; sb_idx++) {
>> + sb = &npc_priv.sb[sb_idx];
>> +
>> + xa = &npc_priv.xa_sb_free;
>> + if (sb->flags & NPC_SUBBANK_FLAG_USED)
>> + xa = &npc_priv.xa_sb_used;
>> +
>> + sb->arr_idx = narr[sb_idx];
>> +
>> + rc = xa_err(xa_store(xa, sb->arr_idx,
>> + xa_mk_value(sb_idx), GFP_KERNEL));
>If the new index assignment swaps indices between a free subbank and a used
>subbank, is the old index in the original xarray ever overwritten?
>Since xa_erase() is not called on the old arr_idx, this might leave the
>subbank mapped at both its old and new arr_idx if they don't overwrite each
>other.
>Can this result in stale duplicate entries?
No. npc_cn20k_search_order_set() is only rearranging indexes in free and used
subbank.
indexes wont move from free subbank to used subbank.
>If xa_store() fails (for example, due to -ENOMEM from GFP_KERNEL) and returns
>an error, the code jumps to the fail label. Are the successful modifications
>made to previously processed subbanks rolled back?
>If not, could this leave the system in a torn state where restrict_valid is
>false, and some subbanks have updated indices while others keep their old
>ones?
ACK, but, There is no way, we can gracefully rollback, as rollback can also
fail due to memory
pressurem while reinserting old index using xa_store. This is fatal error, will
add
a err print to alert user.
>> + union devlink_param_value *val,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct rvu_devlink *rvu_dl = devlink_priv(devlink);
>> + struct rvu *rvu = rvu_dl->rvu;
>> + bool restricted_order;
>> + unsigned long w = 0;
>> + u64 *arr;
>> + u32 sz;
>> +
>> + npc_cn20k_search_order_get(&restricted_order, &sz);
>> + if (sz != val->u64arr.size) {
>> + dev_err(rvu->dev,
>> + "Wrong size %llu, should be %u\n",
>> + val->u64arr.size, sz);
>The size field in devlink array values is a 32-bit unsigned integer (u32),
>but the error message uses the %llu format specifier.
>On some architectures, could this cause printk to consume 8 bytes from the
>arguments and read garbage from adjacent memory?
>> + return -EINVAL;
>> + }
No. %llu is used for val->u64arr.size, which is 64bit.
