On 04/15/2012 09:53 PM, Yunkai Zhang wrote: > int object_cache_push(struct object_cache *oc) > { > struct object_cache_entry *entry, *t; > + struct rb_root *inactive_dirty_tree; > + struct list_head *inactive_dirty_list; > int ret = SD_RES_SUCCESS; > > if (node_in_recovery()) > /* We don't do flushing in recovery */ > return SD_RES_SUCCESS; > > - list_for_each_entry_safe(entry, t, &oc->dirty_list, list) { > + pthread_mutex_lock(&oc->lock); > + switch_dirty_tree_and_list(oc, > + &inactive_dirty_tree, > + &inactive_dirty_list); > + pthread_mutex_unlock(&oc->lock); > + > + /* 1. for async flush, there is only one worker > + * 2. for sync flush, Guest assure us of that only one sync > + * request is issued in one of gateway worker threads > + * So we need not to protect inactive dirty tree and list */ > + list_for_each_entry_safe(entry, t, inactive_dirty_list, list) { > ret = push_cache_object(oc->vid, entry->idx, entry->create); > - if (ret != SD_RES_SUCCESS) > - goto out; > - pthread_mutex_lock(&oc->lock); > - rb_erase(&entry->rb, &oc->dirty_rb); > - list_del(&entry->list); > - pthread_mutex_unlock(&oc->lock); > + if (ret != SD_RES_SUCCESS) { > + goto push_failed; > + } > + del_from_dirty_tree_and_list(entry, > + inactive_dirty_tree, > + inactive_dirty_list); > free(entry); > } right here I think we need return ret; to avoid unnecessary call of merge_dirty_tree_and_list() > -out: > +push_failed: > + pthread_mutex_lock(&oc->lock); > + merge_dirty_tree_and_list(oc, > + inactive_dirty_tree, > + inactive_dirty_list); > + pthread_mutex_unlock(&oc->lock); > return ret; > } |