Re: [RFC PATCH v2 08/15] misc/arm-cla: Manage domain contexts

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] kthread: missing format argument in kthread_run_worker_on_cpu
- [Medium] sched: timeslice starvation via kthread_mod_delayed_work
--

commit 9e218cd8677c5cff035aa302edfc44906aa6b821
Author: Ryan Roberts <ryan.roberts@arm.com>
misc/arm-cla: Manage domain contexts

This commit introduces a domain scheduler for the CLA driver that queues
contexts for assignment with a fixed time slice. It runs hardware operations
through CPU-bound kthread workers to coordinate per-device and domain
switching. Failures on any device mark the entire domain as broken and
wake all waiters.

> diff --git a/drivers/misc/arm-cla/cla-init.c b/drivers/misc/arm-cla/cla-init.c
> index 58f37787a59c1..e1694f84e05d0 100644
> --- a/drivers/misc/arm-cla/cla-init.c
> +++ b/drivers/misc/arm-cla/cla-init.c
[ ... ]
> @@ -198,6 +201,37 @@ static void cla_dev_reinit(struct cla_dev *dev)
>  	cla_reg_write(dev, CLA_REG_LRESP, 0);
>  }
>  
> +static int cla_dev_worker_init(struct cla_dev *dev, int cpu)
> +{
> +	struct kthread_worker *worker;
> +
> +	worker = kthread_run_worker_on_cpu(cpu, 0, "cla-dev-worker/%u");

[Severity: Medium]
Is this missing a format argument for the %u specifier?

It looks like kthread_run_worker_on_cpu() is being called with the format
string "cla-dev-worker/%u", but without a corresponding unsigned int
argument. Will vsnprintf read uninitialized memory from the stack or a
register to satisfy the %u specifier, resulting in a garbage thread name?

> +	if (IS_ERR(worker))
> +		return PTR_ERR(worker);
> +
> +	WARN_ON(dev->worker);

[ ... ]

> diff --git a/drivers/misc/arm-cla/cla-sched.c b/drivers/misc/arm-cla/cla-sched.c
> new file mode 100644
> index 0000000000000..6b5b782ac33e9
> --- /dev/null
> +++ b/drivers/misc/arm-cla/cla-sched.c
[ ... ]
> +void cla_domain_schedule_reassignment(struct cla_domain *domain)
> +{
> +	struct cla_ctx *ctx = domain->assigned_ctx;
> +	unsigned long delay_jfs = 0;
> +
> +	lockdep_assert_held(&domain->lock);
> +
> +	if (domain->broken || ctx == &invalid_ctx)
> +		return;
> +
> +	if (ctx && !cla_ctx_is_dying(ctx))
> +		delay_jfs = msecs_to_jiffies(CLA_SLICE_MS);
> +
> +	kthread_mod_delayed_work(domain->worker, &domain->reassign, delay_jfs);

[Severity: Medium]
Could this lead to timeslice starvation?

By using kthread_mod_delayed_work(), the timer is reset to delay_jfs (100ms)
from the current time. If new contexts fault and are added to the queue,
this function will be called again.

Does each call push the timer back, effectively giving the currently
assigned context an infinite timeslice as long as new faults or wakeups
keep arriving, starving all other contexts in the queue?

Would kthread_queue_delayed_work() be more appropriate here for normal
timeslice tracking, since it does not push the timer back if it is already
pending?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015142552.4128028-1-ryan.roberts@arm.com?part=8