diff options
author | Stefan Richter <stefanr@s5r6.in-berlin.de> | 2007-09-15 12:50:25 (GMT) |
---|---|---|
committer | Stefan Richter <stefanr@s5r6.in-berlin.de> | 2007-10-16 21:59:59 (GMT) |
commit | 17a19b795e9187d65b6e45cb22797725d50f7edb (patch) | |
tree | 60db3734335f3956378063c2f8126c036b83bfed | |
parent | 638d5bb8167c2c88552257d5af23f7f65ab4defd (diff) | |
download | linux-fsl-qoriq-17a19b795e9187d65b6e45cb22797725d50f7edb.tar.xz |
ieee1394: csr1212: proper refcounting
At least since nodemgr got rid of coarse global locking, accesses to
struct csr1212_keyval's reference counter should be atomic and coupled
with proper barriers. Also, calls to csr1212_keep_keyval(kv) should
occur before kv is being used.
(We probably should convert refcnt to struct kref, but how to keep
csr1212_destroy_keyval's implementation non-recursively then?)
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
-rw-r--r-- | drivers/ieee1394/csr1212.c | 57 | ||||
-rw-r--r-- | drivers/ieee1394/csr1212.h | 6 | ||||
-rw-r--r-- | drivers/ieee1394/nodemgr.c | 4 |
3 files changed, 36 insertions, 31 deletions
diff --git a/drivers/ieee1394/csr1212.c b/drivers/ieee1394/csr1212.c index d08166b..e8122de 100644 --- a/drivers/ieee1394/csr1212.c +++ b/drivers/ieee1394/csr1212.c @@ -218,12 +218,10 @@ static struct csr1212_keyval *csr1212_new_keyval(u8 type, u8 key) if (!kv) return NULL; + atomic_set(&kv->refcnt, 1); kv->key.type = type; kv->key.id = key; - kv->associate = NULL; - kv->refcnt = 1; - kv->next = NULL; kv->prev = NULL; kv->offset = 0; @@ -326,12 +324,13 @@ void csr1212_associate_keyval(struct csr1212_keyval *kv, if (kv->associate) csr1212_release_keyval(kv->associate); - associate->refcnt++; + csr1212_keep_keyval(associate); kv->associate = associate; } -int csr1212_attach_keyval_to_directory(struct csr1212_keyval *dir, - struct csr1212_keyval *kv) +static int __csr1212_attach_keyval_to_directory(struct csr1212_keyval *dir, + struct csr1212_keyval *kv, + bool keep_keyval) { struct csr1212_dentry *dentry; @@ -341,10 +340,10 @@ int csr1212_attach_keyval_to_directory(struct csr1212_keyval *dir, if (!dentry) return -ENOMEM; + if (keep_keyval) + csr1212_keep_keyval(kv); dentry->kv = kv; - kv->refcnt++; - dentry->next = NULL; dentry->prev = dir->value.directory.dentries_tail; @@ -358,6 +357,12 @@ int csr1212_attach_keyval_to_directory(struct csr1212_keyval *dir, return CSR1212_SUCCESS; } +int csr1212_attach_keyval_to_directory(struct csr1212_keyval *dir, + struct csr1212_keyval *kv) +{ + return __csr1212_attach_keyval_to_directory(dir, kv, true); +} + #define CSR1212_DESCRIPTOR_LEAF_DATA(kv) \ (&((kv)->value.leaf.data[1])) @@ -483,15 +488,18 @@ void csr1212_detach_keyval_from_directory(struct csr1212_keyval *dir, /* This function is used to free the memory taken by a keyval. If the given * keyval is a directory type, then any keyvals contained in that directory - * will be destroyed as well if their respective refcnts are 0. By means of + * will be destroyed as well if noone holds a reference on them. By means of * list manipulation, this routine will descend a directory structure in a * non-recursive manner. */ -static void csr1212_destroy_keyval(struct csr1212_keyval *kv) +void csr1212_release_keyval(struct csr1212_keyval *kv) { struct csr1212_keyval *k, *a; struct csr1212_dentry dentry; struct csr1212_dentry *head, *tail; + if (!atomic_dec_and_test(&kv->refcnt)) + return; + dentry.kv = kv; dentry.next = NULL; dentry.prev = NULL; @@ -503,9 +511,8 @@ static void csr1212_destroy_keyval(struct csr1212_keyval *kv) k = head->kv; while (k) { - k->refcnt--; - - if (k->refcnt > 0) + /* must not dec_and_test kv->refcnt again */ + if (k != kv && !atomic_dec_and_test(&k->refcnt)) break; a = k->associate; @@ -536,14 +543,6 @@ static void csr1212_destroy_keyval(struct csr1212_keyval *kv) } } -void csr1212_release_keyval(struct csr1212_keyval *kv) -{ - if (kv->refcnt > 1) - kv->refcnt--; - else - csr1212_destroy_keyval(kv); -} - void csr1212_destroy_csr(struct csr1212_csr *csr) { struct csr1212_csr_rom_cache *c, *oc; @@ -1126,6 +1125,7 @@ csr1212_parse_dir_entry(struct csr1212_keyval *dir, u32 ki, u32 kv_pos) int ret = CSR1212_SUCCESS; struct csr1212_keyval *k = NULL; u32 offset; + bool keep_keyval = true; switch (CSR1212_KV_KEY_TYPE(ki)) { case CSR1212_KV_TYPE_IMMEDIATE: @@ -1135,8 +1135,8 @@ csr1212_parse_dir_entry(struct csr1212_keyval *dir, u32 ki, u32 kv_pos) ret = -ENOMEM; goto out; } - - k->refcnt = 0; /* Don't keep local reference when parsing. */ + /* Don't keep local reference when parsing. */ + keep_keyval = false; break; case CSR1212_KV_TYPE_CSR_OFFSET: @@ -1146,7 +1146,8 @@ csr1212_parse_dir_entry(struct csr1212_keyval *dir, u32 ki, u32 kv_pos) ret = -ENOMEM; goto out; } - k->refcnt = 0; /* Don't keep local reference when parsing. */ + /* Don't keep local reference when parsing. */ + keep_keyval = false; break; default: @@ -1174,8 +1175,10 @@ csr1212_parse_dir_entry(struct csr1212_keyval *dir, u32 ki, u32 kv_pos) ret = -ENOMEM; goto out; } - k->refcnt = 0; /* Don't keep local reference when parsing. */ - k->valid = 0; /* Contents not read yet so it's not valid. */ + /* Don't keep local reference when parsing. */ + keep_keyval = false; + /* Contents not read yet so it's not valid. */ + k->valid = 0; k->offset = offset; k->prev = dir; @@ -1183,7 +1186,7 @@ csr1212_parse_dir_entry(struct csr1212_keyval *dir, u32 ki, u32 kv_pos) dir->next->prev = k; dir->next = k; } - ret = csr1212_attach_keyval_to_directory(dir, k); + ret = __csr1212_attach_keyval_to_directory(dir, k, keep_keyval); out: if (ret != CSR1212_SUCCESS && k != NULL) free_keyval(k); diff --git a/drivers/ieee1394/csr1212.h b/drivers/ieee1394/csr1212.h index df909ce..043039f 100644 --- a/drivers/ieee1394/csr1212.h +++ b/drivers/ieee1394/csr1212.h @@ -32,6 +32,7 @@ #include <linux/types.h> #include <linux/slab.h> +#include <asm/atomic.h> #define CSR1212_MALLOC(size) kmalloc((size), GFP_KERNEL) #define CSR1212_FREE(ptr) kfree(ptr) @@ -149,7 +150,7 @@ struct csr1212_keyval { struct csr1212_directory directory; } value; struct csr1212_keyval *associate; - int refcnt; + atomic_t refcnt; /* used in generating and/or parsing CSR image */ struct csr1212_keyval *next, *prev; /* flat list of CSR elements */ @@ -350,7 +351,8 @@ csr1212_get_keyval(struct csr1212_csr *csr, struct csr1212_keyval *kv); * need for code to retain a keyval that has been parsed. */ static inline void csr1212_keep_keyval(struct csr1212_keyval *kv) { - kv->refcnt++; + atomic_inc(&kv->refcnt); + smp_mb__after_atomic_inc(); } diff --git a/drivers/ieee1394/nodemgr.c b/drivers/ieee1394/nodemgr.c index ec8edd2f..90dc75b 100644 --- a/drivers/ieee1394/nodemgr.c +++ b/drivers/ieee1394/nodemgr.c @@ -1014,13 +1014,13 @@ static struct unit_directory *nodemgr_process_unit_directory CSR1212_TEXTUAL_DESCRIPTOR_LEAF_LANGUAGE(kv) == 0) { switch (last_key_id) { case CSR1212_KV_ID_VENDOR: - ud->vendor_name_kv = kv; csr1212_keep_keyval(kv); + ud->vendor_name_kv = kv; break; case CSR1212_KV_ID_MODEL: - ud->model_name_kv = kv; csr1212_keep_keyval(kv); + ud->model_name_kv = kv; break; } |