[sheepdog] [RFC PATCH 1/2] make list iterator functions safe against removal of entry
MORITA Kazutaka
morita.kazutaka at gmail.com
Mon Aug 19 10:36:16 CEST 2013
From: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
Removing entries in non-safe list iterators causes severe bugs and
they are usually difficult to find. This makes all the iterator
functions safe against removal of list entries in exchange for a very
small overhead.
Signed-off-by: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
---
dog/farm/farm.c | 4 ++--
dog/farm/object_tree.c | 4 ++--
dog/treeview.c | 4 ++--
include/list.h | 47 +++++++++++++++------------------------------
sheep/cluster/zookeeper.c | 4 ++--
sheep/group.c | 10 +++++-----
sheep/object_cache.c | 16 +++++++--------
sheep/object_list_cache.c | 4 ++--
sheep/request.c | 20 +++++++++----------
9 files changed, 49 insertions(+), 64 deletions(-)
diff --git a/dog/farm/farm.c b/dog/farm/farm.c
index 1969927..14f6278 100644
--- a/dog/farm/farm.c
+++ b/dog/farm/farm.c
@@ -101,8 +101,8 @@ static int create_active_vdis(void)
static void free_vdi_list(void)
{
- struct vdi_entry *vdi, *next;
- list_for_each_entry_safe(vdi, next, &last_vdi_list, list)
+ struct vdi_entry *vdi;
+ list_for_each_entry(vdi, &last_vdi_list, list)
free(vdi);
}
diff --git a/dog/farm/object_tree.c b/dog/farm/object_tree.c
index 00cad2d..93ee9a1 100644
--- a/dog/farm/object_tree.c
+++ b/dog/farm/object_tree.c
@@ -91,8 +91,8 @@ void object_tree_print(void)
void object_tree_free(void)
{
- struct object_tree_entry *entry, *next;
- list_for_each_entry_safe(entry, next, &tree.list, list)
+ struct object_tree_entry *entry;
+ list_for_each_entry(entry, &tree.list, list)
free(entry);
free(cached_entry);
diff --git a/dog/treeview.c b/dog/treeview.c
index baf0f00..fe7c7e6 100644
--- a/dog/treeview.c
+++ b/dog/treeview.c
@@ -87,9 +87,9 @@ void add_vdi_tree(const char *name, const char *label, uint32_t vid,
static void compaction(struct vdi_tree *parent)
{
- struct vdi_tree *vdi, *e, *new_parent;
+ struct vdi_tree *vdi, *new_parent;
- list_for_each_entry_safe(vdi, e, &parent->children, siblings) {
+ list_for_each_entry(vdi, &parent->children, siblings) {
new_parent = find_vdi(root, vdi->pvid, vdi->name);
if (new_parent && parent != new_parent)
list_move_tail(&vdi->siblings, &new_parent->children);
diff --git a/include/list.h b/include/list.h
index 4b9d9a7..e37b37e 100644
--- a/include/list.h
+++ b/include/list.h
@@ -36,18 +36,17 @@ static inline int list_empty(const struct list_head *head)
container_of(ptr, type, member)
#define list_for_each(pos, head) \
- for (pos = (head)->next; pos != (head); pos = pos->next)
+ pos = (head)->next; \
+ for (typeof(pos) __n = pos->next; pos != (head); \
+ pos = __n, __n = pos->next)
#define list_for_each_entry(pos, head, member) \
- for (pos = list_entry((head)->next, typeof(*pos), member); \
+ pos = list_entry((head)->next, typeof(*pos), member); \
+ for (typeof(pos) __n = list_entry(pos->member.next, typeof(*pos), \
+ member); \
&pos->member != (head); \
- pos = list_entry(pos->member.next, typeof(*pos), member))
-
-#define list_for_each_entry_safe(pos, n, head, member) \
- for (pos = list_entry((head)->next, typeof(*pos), member), \
- n = list_entry(pos->member.next, typeof(*pos), member); \
- &pos->member != (head); \
- pos = n, n = list_entry(n->member.next, typeof(*n), member))
+ pos = __n, __n = list_entry(__n->member.next, typeof(*__n), \
+ member))
static inline void __list_add(struct list_head *new,
struct list_head *prev,
@@ -218,11 +217,10 @@ static inline void hlist_add_after(struct hlist_node *n,
#define hlist_entry(ptr, type, member) container_of(ptr, type, member)
-#define hlist_for_each(pos, head) \
- for (pos = (head)->first; pos ; pos = pos->next)
-
-#define hlist_for_each_safe(pos, n, head) \
- for (pos = (head)->first; pos && ({ n = pos->next; 1; }); pos = n)
+#define hlist_for_each(pos, head) \
+ pos = (head)->first; \
+ for (typeof(pos) __n; pos && ({ __n = pos->next; 1; }); \
+ pos = __n) \
/*
* hlist_for_each_entry - iterate over list of given type
@@ -232,24 +230,11 @@ static inline void hlist_add_after(struct hlist_node *n,
* @member: the name of the hlist_node within the struct.
*/
#define hlist_for_each_entry(tpos, pos, head, member) \
- for (pos = (head)->first; \
- pos && ({ tpos = hlist_entry(pos, typeof(*tpos), member); 1; }); \
- pos = pos->next)
-
-/*
- * hlist_for_each_entry_safe - iterate over list of given type safe against
- * removal of list entry
- * @tpos: the type * to use as a loop cursor.
- * @pos: the &struct hlist_node to use as a loop cursor.
- * @n: another &struct hlist_node to use as temporary storage
- * @head: the head for your list.
- * @member: the name of the hlist_node within the struct.
- */
-#define hlist_for_each_entry_safe(tpos, pos, n, head, member) \
- for (pos = (head)->first; \
- pos && ({ n = pos->next; 1; }) && \
+ pos = (head)->first; \
+ for (typeof(pos) __n; \
+ pos && ({ __n = pos->next; 1; }) && \
({ tpos = hlist_entry(pos, typeof(*tpos), member); 1; }); \
- pos = n)
+ pos = __n)
void list_sort(void *priv, struct list_head *head,
int (*cmp)(void *priv, struct list_head *a,
diff --git a/sheep/cluster/zookeeper.c b/sheep/cluster/zookeeper.c
index 5088968..8a9804d 100644
--- a/sheep/cluster/zookeeper.c
+++ b/sheep/cluster/zookeeper.c
@@ -924,9 +924,9 @@ static void kick_block_event(void)
static void block_event_list_del(struct zk_node *n)
{
- struct zk_node *ev, *t;
+ struct zk_node *ev;
- list_for_each_entry_safe(ev, t, &zk_block_list, list) {
+ list_for_each_entry(ev, &zk_block_list, list) {
if (node_eq(&ev->node, &n->node)) {
list_del(&ev->list);
free(ev);
diff --git a/sheep/group.c b/sheep/group.c
index acd6f41..587e982 100644
--- a/sheep/group.c
+++ b/sheep/group.c
@@ -774,12 +774,12 @@ static int send_join_request(struct sd_node *ent)
static void requeue_cluster_request(void)
{
- struct request *req, *p;
+ struct request *req;
struct vdi_op_message *msg;
size_t size;
- list_for_each_entry_safe(req, p, main_thread_get(pending_notify_list),
- pending_list) {
+ list_for_each_entry(req, main_thread_get(pending_notify_list),
+ pending_list) {
/*
* ->notify() was called and succeeded but after that
* this node session-timeouted and sd_notify_handler
@@ -794,8 +794,8 @@ static void requeue_cluster_request(void)
free(msg);
}
- list_for_each_entry_safe(req, p, main_thread_get(pending_block_list),
- pending_list) {
+ list_for_each_entry(req, main_thread_get(pending_block_list),
+ pending_list) {
switch (req->status) {
case REQUEST_INIT:
/* this request has never been executed, re-queue it */
diff --git a/sheep/object_cache.c b/sheep/object_cache.c
index 1f8822e..4d22e28 100644
--- a/sheep/object_cache.c
+++ b/sheep/object_cache.c
@@ -538,12 +538,12 @@ out:
#define HIGH_WATERMARK (sys->object_cache_size * 9 / 10)
static void do_reclaim_object(struct object_cache *oc)
{
- struct object_cache_entry *entry, *t;
+ struct object_cache_entry *entry;
uint64_t oid;
uint32_t cap;
write_lock_cache(oc);
- list_for_each_entry_safe(entry, t, &oc->lru_head, lru_list) {
+ list_for_each_entry(entry, &oc->lru_head, lru_list) {
oid = idx_to_oid(oc->vid, entry_idx(entry));
if (entry_in_use(entry)) {
sd_debug("%"PRIx64" is in use, skip...", oid);
@@ -927,7 +927,7 @@ static void push_object_done(struct work *work)
*/
static int object_cache_push(struct object_cache *oc)
{
- struct object_cache_entry *entry, *t;
+ struct object_cache_entry *entry;
write_lock_cache(oc);
if (list_empty(&oc->dirty_head)) {
@@ -936,7 +936,7 @@ static int object_cache_push(struct object_cache *oc)
}
uatomic_set(&oc->push_count, uatomic_read(&oc->dirty_count));
- list_for_each_entry_safe(entry, t, &oc->dirty_head, dirty_list) {
+ list_for_each_entry(entry, &oc->dirty_head, dirty_list) {
struct push_work *pw;
get_cache_entry(entry);
@@ -972,7 +972,7 @@ void object_cache_delete(uint32_t vid)
{
struct object_cache *cache;
int h = hash(vid);
- struct object_cache_entry *entry, *t;
+ struct object_cache_entry *entry;
char path[PATH_MAX];
cache = find_object_cache(vid, false);
@@ -985,7 +985,7 @@ void object_cache_delete(uint32_t vid)
sd_unlock(&hashtable_lock[h]);
write_lock_cache(cache);
- list_for_each_entry_safe(entry, t, &cache->lru_head, lru_list) {
+ list_for_each_entry(entry, &cache->lru_head, lru_list) {
free_cache_entry(entry);
uatomic_sub(&gcache.capacity, CACHE_OBJECT_SIZE);
}
@@ -1391,12 +1391,12 @@ err:
void object_cache_format(void)
{
struct object_cache *cache;
- struct hlist_node *node, *t;
+ struct hlist_node *node;
int i;
for (i = 0; i < HASH_SIZE; i++) {
struct hlist_head *head = cache_hashtable + i;
- hlist_for_each_entry_safe(cache, node, t, head, hash) {
+ hlist_for_each_entry(cache, node, head, hash) {
object_cache_delete(cache->vid);
}
}
diff --git a/sheep/object_list_cache.c b/sheep/object_list_cache.c
index 1710522..f572544 100644
--- a/sheep/object_list_cache.c
+++ b/sheep/object_list_cache.c
@@ -163,7 +163,7 @@ static void objlist_deletion_work(struct work *work)
{
struct objlist_deletion_work *ow =
container_of(work, struct objlist_deletion_work, work);
- struct objlist_cache_entry *entry, *t;
+ struct objlist_cache_entry *entry;
uint32_t vid = ow->vid, entry_vid;
/*
@@ -180,7 +180,7 @@ static void objlist_deletion_work(struct work *work)
}
sd_write_lock(&obj_list_cache.lock);
- list_for_each_entry_safe(entry, t, &obj_list_cache.entry_list, list) {
+ list_for_each_entry(entry, &obj_list_cache.entry_list, list) {
entry_vid = oid_to_vid(entry->oid);
if (entry_vid != vid)
continue;
diff --git a/sheep/request.c b/sheep/request.c
index dbb13cf..27ccb72 100644
--- a/sheep/request.c
+++ b/sheep/request.c
@@ -197,12 +197,12 @@ static bool request_in_recovery(struct request *req)
/* Wakeup requests because of epoch mismatch */
void wakeup_requests_on_epoch(void)
{
- struct request *req, *t;
+ struct request *req;
LIST_HEAD(pending_list);
list_splice_init(&sys->req_wait_queue, &pending_list);
- list_for_each_entry_safe(req, t, &pending_list, request_list) {
+ list_for_each_entry(req, &pending_list, request_list) {
switch (req->rp.result) {
case SD_RES_OLD_NODE_VER:
/*
@@ -234,12 +234,12 @@ void wakeup_requests_on_epoch(void)
/* Wakeup the requests on the oid that was previously being recoverred */
void wakeup_requests_on_oid(uint64_t oid)
{
- struct request *req, *t;
+ struct request *req;
LIST_HEAD(pending_list);
list_splice_init(&sys->req_wait_queue, &pending_list);
- list_for_each_entry_safe(req, t, &pending_list, request_list) {
+ list_for_each_entry(req, &pending_list, request_list) {
if (req->local_oid != oid)
continue;
sd_debug("retry %" PRIx64, req->local_oid);
@@ -250,12 +250,12 @@ void wakeup_requests_on_oid(uint64_t oid)
void wakeup_all_requests(void)
{
- struct request *req, *n;
+ struct request *req;
LIST_HEAD(pending_list);
list_splice_init(&sys->req_wait_queue, &pending_list);
- list_for_each_entry_safe(req, n, &pending_list, request_list) {
+ list_for_each_entry(req, &pending_list, request_list) {
sd_debug("%"PRIx64, req->rq.obj.oid);
del_requeue_request(req);
}
@@ -640,11 +640,11 @@ static void destroy_client(struct client_info *ci)
static void clear_client_info(struct client_info *ci)
{
- struct request *req, *t;
+ struct request *req;
sd_debug("connection seems to be dead");
- list_for_each_entry_safe(req, t, &ci->done_reqs, request_list) {
+ list_for_each_entry(req, &ci->done_reqs, request_list) {
list_del(&req->request_list);
free_request(req);
}
@@ -809,7 +809,7 @@ int init_unix_domain_socket(const char *dir)
static void local_req_handler(int listen_fd, int events, void *data)
{
- struct request *req, *t;
+ struct request *req;
LIST_HEAD(pending_list);
if (events & EPOLLERR)
@@ -821,7 +821,7 @@ static void local_req_handler(int listen_fd, int events, void *data)
list_splice_init(&sys->local_req_queue, &pending_list);
pthread_mutex_unlock(&sys->local_req_lock);
- list_for_each_entry_safe(req, t, &pending_list, request_list) {
+ list_for_each_entry(req, &pending_list, request_list) {
list_del(&req->request_list);
queue_request(req);
}
--
1.7.9.5
More information about the sheepdog
mailing list