lkml.org 
[lkml]   [2008]   [Aug]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [GIT PULL]: firmware patches for building firmware into kernel
From
Date
Hello Dave,

On Thu, 2008-08-07 at 14:21 -0400, David Dillow wrote:

> I just looked at the tree; it still has locking issues, and needs
> further review. You must protect the list from modification while you
> iterate it looking for an match on the requested firmware.

So here is updated patch:

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 6074321..71ec20d 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -419,9 +419,9 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
return -EINVAL;

/* Return firmware pointer from firmware list if already allocated */
+ mutex_lock(&fw_lock);
list_for_each_entry(flst, &firmwarelist, list)
if (strcmp(name, flst->name) == 0) {
- mutex_lock(&fw_lock);
flst->count++;
*firmware_p = flst->fw;
mutex_unlock(&fw_lock);
@@ -429,6 +429,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
name, flst->count);
return 0;
}
+ mutex_unlock(&fw_lock);

*firmware_p = firmware = kzalloc(sizeof(*firmware), GFP_KERNEL);
if (!firmware)
@@ -568,19 +569,22 @@ void release_firmware(const struct firmware *fw)
{
struct firmware_list *flst;

+ mutex_lock(&fw_lock);
if (fw)
list_for_each_entry(flst, &firmwarelist, list)
if (fw == flst->fw) {
printk(KERN_INFO
"firmware: releasing %s count %d\n",
flst->name, flst->count);
- mutex_lock(&fw_lock);
flst->count--;
- mutex_unlock(&fw_lock);
- if (flst->count == 0)
- __release_firmware(fw, flst);
- return;
+ if (flst->count == 0) {
+ mutex_unlock(&fw_lock);
+ return __release_firmware(fw, flst);
+ }
+ goto out;
}
+out:
+ mutex_unlock(&fw_lock);
}

/*
@@ -598,6 +602,7 @@ void release_firmware_all(const struct firmware *fw)
{
struct firmware_list *flst;

+ mutex_lock(&fw_lock);
if (fw)
list_for_each_entry(flst, &firmwarelist, list)
if (fw == flst->fw) {
@@ -605,13 +610,14 @@ void release_firmware_all(const struct firmware *fw)
"firmware: release_all %s count %d\n",
flst->name, flst->count);
if (flst->count) {
- mutex_lock(&fw_lock);
flst->count = 0;
mutex_unlock(&fw_lock);
- __release_firmware(fw, flst);
+ return __release_firmware(fw, flst);
}
- return;
+ goto out;
}
+out:
+ mutex_unlock(&fw_lock);
}

/* Async support */

> Also, was it legal to call release_firmware() from an atomic context? It can now
> sleep, which may be an issue...
>

yes, release_firmware can sleep.
So now release_firmware also joined the family of request_firmware.

If you can do request_firmware you can also do release_firmware.

Any how release_firmware will be called below request_firmware or during
exit, I do not think this will make any issue.

Thank you,

Jaswinder Singh.




\
 
 \ /
  Last update: 2008-08-08 05:35    [W:0.059 / U:18.668 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site