Re: [GIT PULL]: firmware patches for building firmware into kernel
From: Jaswinder Singh
Date: Thu Aug 07 2008 - 23:33:07 EST
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/