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/