On Wed, Jul 25, 2012 at 01:00:08AM +0800, Ming Lei wrote:
> Callers of request_firmware* must hold the reference count of
> @device, otherwise it is easy to trigger oops since the firmware
> loader device is the child of @device.
>
> This patch adds comments about the usage. In fact, most of drivers
> call request_firmware* in its probe() or open(), so the constraint
> should be reasonable and satisfied easily.
>
> Also this patch holds the reference cound of @device before
count
> schedule_work() in request_firmware_nowait() to avoid that
> the @device dies after request_firmware_nowait returns and before
> the work is scheduled.
>
> Also request_firmware_nowait should be called in atomic context now,
> so fix the obsolete comments.
>
> Signed-off-by: Ming Lei <ming....@canonical.com>
> ---
> drivers/base/firmware_class.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 674cb11..540b2e1 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -717,6 +717,8 @@ err_put_dev:
> * @name will be used as $FIRMWARE in the uevent environment and
> * should be distinctive enough not to be confused with any other
> * firmware image for this or any other device.
> + *
> + * Caller must hold the reference count of @device.
> **/
> int
> request_firmware(const struct firmware **firmware_p, const char *name,
> @@ -798,6 +800,7 @@ static void request_firmware_work_func(struct work_struct
> *work)
>
> out:
> fw_work->cont(fw, fw_work->context);
> + put_device(fw_work->device);
>
> module_put(fw_work->module);
> kfree(fw_work);
> @@ -816,9 +819,10 @@ static void request_firmware_work_func(struct
> work_struct *work)
> * @cont: function will be called asynchronously when the firmware
> * request is over.
> *
> + * Caller must hold the reference count of @device.
> + *
> * Asynchronous variant of request_firmware() for user contexts where
> - * it is not possible to sleep for long time. It can't be called
> - * in atomic contexts.
> + * it is not possible to sleep for long time.
Let's state it explicitly:
"it is not allowed to sleep for it is called in atomic context."
> **/
> int
> request_firmware_nowait(
> @@ -844,6 +848,7 @@ request_firmware_nowait(
> return -EFAULT;
> }
>
> + get_device(fw_work->device);
> INIT_WORK(&fw_work->work, request_firmware_work_func);
> schedule_work(&fw_work->work);
> return 0;
Thanks.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
