On Sun, Apr 15, 2012 at 11:04 PM, Liu Yuan <namei.unix at gmail.com> wrote: > 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() good catch. >> -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; >> } -- Yunkai Zhang Work at Taobao |