lkml.org 
[lkml]   [2013]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/4] firmware: Refactoring for splitting user-mode helper code
At Thu, 31 Jan 2013 17:48:34 +0800,
Ming Lei wrote:
>
(snip)
> > +/* called from request_firmware() and request_firmware_work_func() */
> > +static int
> > +_request_firmware(const struct firmware **firmware_p, const char *name,
> > + struct device *device, bool uevent, bool nowait)
> > +{
> > + struct firmware *fw;
> > + long timeout;
> > + int ret;
> > +
> > + if (!firmware_p)
> > + return -EINVAL;
> > +
> > + ret = _request_firmware_prepare(&fw, name, device);
> > + if (ret <= 0) /* error or already assigned */
> > + goto out;
> > +
> > + ret = 0;
> > + timeout = firmware_loading_timeout();
>
> The above line may be moved below the line of 'if (nowait) '.

I thought of that, too, but later found that the timeout is used in
the call of fw_load_from_user_helper() below.

> > + if (nowait) {
> > + timeout = usermodehelper_read_lock_wait(timeout);
> > + if (!timeout) {
> > + dev_dbg(device, "firmware: %s loading timed out\n",
> > + name);
> > + ret = -EBUSY;
> > + }
> > + } else {
> > + ret = usermodehelper_read_trylock();
> > + if (WARN_ON(ret)) {
> > + dev_err(device, "firmware: %s will not be loaded\n",
> > + name);
> > + }
> > + }
> > +
> > + if (!ret) {
> > + if (!fw_get_filesystem_firmware(device, fw->priv))
> > + ret = fw_load_from_user_helper(fw, name, device,
> > + uevent, nowait,
> > + timeout);
> > + if (!ret)
> > + ret = assign_firmware_buf(fw, device);
> > + }
> > +
> > + usermodehelper_read_unlock();
>
> The above should be move inside the above 'if (!ret) {...}'

Yep, but it would make the code less understandable.
I'll make rather the error path of usermodehelper lock goto's.

>
> Except for the above two comments, looks the patch v4 is fine.
>
> Please CC greg-kh when you send v5, and you may add my ack
> for the patchset.
>
> Acked-by: Ming Lei <ming.lei@canonical.com>

OK, thanks!


Takashi


\
 
 \ /
  Last update: 2013-01-31 11:44    [W:0.070 / U:0.308 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site