Messages in this thread | | | Date | Wed, 25 Jul 2012 18:04:25 +0200 | From | Borislav Petkov <> | Subject | Re: [RFC PATCH 08/13] driver core: firmware loader: fix device lifetime |
| |
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.lei@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
| |