Re: [PATCH v3 1/2] firmware_loader: Add cancel helper for async requests

[Takashi Iwai]

On Fri, 01 May 2026 12:22:12 +0200,
Cássio Gabriel wrote:
> +static void firmware_work_free(struct firmware_work *fw_work)
> +{
> +	put_device(fw_work->device); /* taken in request_firmware_nowait() */
> +	module_put(fw_work->module);
> +	kfree_const(fw_work->name);
> +}
> +
>  static void request_firmware_work_func(struct work_struct *work)
>  {
>  	struct firmware_work *fw_work;
>  	const struct firmware *fw;
> +	unsigned long flags;
>  
>  	fw_work = container_of(work, struct firmware_work, work);
>  
>  	_request_firmware(&fw, fw_work->name, fw_work->device, NULL, 0, 0,
>  			  fw_work->opt_flags);
>  	fw_work->cont(fw, fw_work->context);
> -	put_device(fw_work->device); /* taken in request_firmware_nowait() */
>  
> -	module_put(fw_work->module);
> -	kfree_const(fw_work->name);
> -	kfree(fw_work);
> +	spin_lock_irqsave(&firmware_work_lock, flags);

This is a sleepable context, so you can use spin_lock_irq().

> +	if (!list_empty(&fw_work->list)) {
> +		list_del_init(&fw_work->list);
> +		spin_unlock_irqrestore(&firmware_work_lock, flags);
> +		firmware_work_free(fw_work);
> +		kfree(fw_work);

Can kfree(fw_work) be in firmware_work_free(), too?  All callers free
it in anyway.

(snip)
> +void request_firmware_nowait_cancel(struct device *device, void *context,
> +				    void (*cont)(const struct firmware *fw,
> +						 void *context))
> +{
> +	struct firmware_work *fw_work = NULL;
> +	struct firmware_work *tmp;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&firmware_work_lock, flags);

Here you can use spin_lock_irq() as well.


thanks,

Takashi