Greg KH [off-list ref] wrote:

On Wed, Oct 03, 2012 at 10:32:08AM -0700, Linus Torvalds wrote:

quoted

On Wed, Oct 3, 2012 at 10:09 AM, Al Viro [off-list ref]

wrote:

quoted

quoted

+       if (!S_ISREG(inode->i_mode))
+               return false;
+       size = i_size_read(inode);

Probably better to do vfs_getattr() and check mode and size in

kstat; if

quoted

quoted

it's sufficiently hot for that to hurt, we are fucked anyway.

+               file = filp_open(path, O_RDONLY, 0);
+               if (IS_ERR(file))
+                       continue;
+printk("from file '%s' ", path);
+               success = fw_read_file_contents(file, fw);
+               filp_close(file, NULL);

fput(file), please.  We have enough misuses of filp_close() as it

is...

quoted

Ok, like this?

This looks good to me.  Having udev do firmware loading and tieing it
to
the driver model may have not been such a good idea so many years ago.
Doing it this way makes more sense.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-media"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

I agree that not calling out to userspace for firmware load is better.

Here is an old, unresolved bug about Oops on firmware loading race condition

https://bugzilla.kernel.org/show_bug.cgi?id=15294

The firmware loading timeout in the kernel was cleaning things up, just as udev what trying to say "I'm done loading the firmware" via sysfs; and then *boom*.

Regards,
Andy