On 2012年07月27日 19:17, MORITA Kazutaka wrote:
> At Fri, 27 Jul 2012 19:01:03 +0800,
> levin li wrote:
>>
>>>> +static void reclaim_work(struct work *work)
>>>> +{
>>>> + struct object_cache_entry *entry, *n;
>>>> + int ret;
>>>> +
>>>> + if (node_in_recovery())
>>>> + return;
>>>
>>> Can you comment briefly why we cannot recaim caches while recovering?
>>> I think it will help newcomer to understand the code.
>>>
>>
>> I did it because it may makes things more complicated, just as we don't
>> do flush while recovering, maybe later we make sure that there's no problem
>> to do reclaim in recovery, then we can remove this.
>
> Then, how about adding it as a TODO comment?
>
OK
>>
>>>
>>>> +
>>>> + list_for_each_entry_revert_safe_rcu(entry, n,
>>>> + &sys_cache.cache_lru_list, lru_list) {
>>>> + unsigned data_length;
>>>> + /* Reclaim cache to 80% of max size */
>>>> + if (uatomic_read(&sys_cache.cache_size) <=
>>>> + sys->cache_size * 8 / 10)
>>>> + break;
>>>> +
>>>> + ret = reclaim_object(entry);
>>>> + if (ret != SD_RES_SUCCESS)
>>>> + continue;
>>>> + if (idx_has_vdi_bit(entry->idx))
>>>> + data_length = SD_INODE_SIZE;
>>>> + else
>>>> + data_length = SD_DATA_OBJ_SIZE;
>>>> +
>>>> + uatomic_sub(&sys_cache.cache_size, data_length);
>>>> + free(entry);
>>>> + }
>>>> +
>>>> + dprintf("cache reclaim complete\n");
>>>> +}
>>>> +
>>>> +static void reclaim_done(struct work *work)
>>>> +{
>>>> + uatomic_set(&sys_cache.reclaiming, 0);
>>>> + free(work);
>>>> +}
>>>> +
>>>> static struct object_cache_entry *
>>>> dirty_tree_insert(struct object_cache *oc, uint32_t idx,
>>>> uint64_t bmap, int create)
>>>> @@ -171,18 +530,21 @@ dirty_tree_insert(struct object_cache *oc, uint32_t
>>>> idx,
>>>> struct rb_node **p = &oc->dirty_tree.rb_node;
>>>> struct rb_node *parent = NULL;
>>>> struct object_cache_entry *entry;
>>>> + idx = idx_mask(idx);
>>>>
>>>> while (*p) {
>>>> parent = *p;
>>>> entry = rb_entry(parent, struct object_cache_entry, dirty_node);
>>>>
>>>> - if (idx < entry->idx)
>>>> + if (idx < idx_mask(entry->idx))
>>>> p = &(*p)->rb_left;
>>>> - else if (idx > entry->idx)
>>>> + else if (idx > idx_mask(entry->idx))
>>>> p = &(*p)->rb_right;
>>>> else {
>>>> /* already has this entry, merge bmap */
>>>> entry->bmap |= bmap;
>>>> + if (create)
>>>> + entry->idx |= CACHE_CREATE_BIT;
>>>> return entry;
>>>> }
>>>> }
>>>> @@ -192,7 +554,8 @@ dirty_tree_insert(struct object_cache *oc, uint32_t
>>>> idx,
>>>> return NULL;
>>>>
>>>> entry->bmap |= bmap;
>>>> - entry->flags |= ENTRY_CREATE_BIT;
>>>> + if (create)
>>>> + entry->idx |= CACHE_CREATE_BIT;
>>>> rb_link_node(&entry->dirty_node, parent, p);
>>>> rb_insert_color(&entry->dirty_node, &oc->dirty_tree);
>>>> list_add(&entry->list, &oc->dirty_list);
>>>> @@ -200,26 +563,6 @@ dirty_tree_insert(struct object_cache *oc, uint32_t
>>>> idx,
>>>> return entry;
>>>> }
>>>>
>>>> -static struct object_cache_entry *dirty_tree_search(struct rb_root *root,
>>>> - uint32_t idx)
>>>> -{
>>>> - struct rb_node *n = root->rb_node;
>>>> - struct object_cache_entry *t;
>>>> -
>>>> - while (n) {
>>>> - t = rb_entry(n, struct object_cache_entry, dirty_node);
>>>> -
>>>> - if (idx < t->idx)
>>>> - n = n->rb_left;
>>>> - else if (idx > t->idx)
>>>> - n = n->rb_right;
>>>> - else
>>>> - return t; /* found it */
>>>> - }
>>>> -
>>>> - return NULL;
>>>> -}
>>>> -
>>>> static int create_dir_for(uint32_t vid)
>>>> {
>>>> int ret = 0;
>>>> @@ -257,6 +600,7 @@ not_found:
>>>> if (create) {
>>>> cache = xzalloc(sizeof(*cache));
>>>> cache->vid = vid;
>>>> + cache->object_tree = RB_ROOT;
>>>> create_dir_for(vid);
>>>>
>>>> cache->dirty_tree = RB_ROOT;
>>>> @@ -271,14 +615,6 @@ out:
>>>> return cache;
>>>> }
>>>>
>>>> -static inline void
>>>> -del_from_dirty_tree_and_list(struct object_cache_entry *entry,
>>>> - struct rb_root *dirty_tree)
>>>> -{
>>>> - rb_erase(&entry->dirty_node, dirty_tree);
>>>> - list_del(&entry->list);
>>>> -}
>>>> -
>>>> /* Caller should hold the oc->lock */
>>>> static inline void
>>>> add_to_dirty_tree_and_list(struct object_cache *oc, uint32_t idx,
>>>> @@ -289,6 +625,9 @@ add_to_dirty_tree_and_list(struct object_cache *oc,
>>>> uint32_t idx,
>>>> if (!entry)
>>>> panic("Can not find object entry %" PRIx32 "\n", idx);
>>>>
>>>> + if (cache_in_reclaim(0))
>>>> + return;
>>>> +
>>>> /* If cache isn't in reclaiming, move it
>>>> * to the head of lru list */
>>>> cds_list_del_rcu(&entry->lru_list);
>>>> @@ -312,15 +651,18 @@ static void add_to_object_cache(struct object_cache
>>>> *oc, uint32_t idx)
>>>> uint32_t data_length;
>>>>
>>>> if (idx_has_vdi_bit(idx))
>>>> - data_length = SD_INODE_SIZE / 1024;
>>>> + data_length = SD_INODE_SIZE;
>>>> else
>>>> - data_length = SD_DATA_OBJ_SIZE / 1024;
>>>> + data_length = SD_DATA_OBJ_SIZE;
>>>
>>> What's the intention of this change? And if you want to use the value
>>> 1024, please use a macro and specify what it is.
>>>
>>
>> Well, I will not use the magic number 1024 any more, because the 64 bits
>> cache_size
>> it big enough.
>
> Then, please remove the code from the 2nd patch.
>
Well, sorry for this, I just didn't want to rebase, I'd resend this patchset.
thanks,
levin
> Thanks,
>
> Kazutaka
>
--
sheepdog mailing list
[email protected]
http://lists.wpkg.org/mailman/listinfo/sheepdog
