[sheepdog] [PATCH UPDATE] object cache: reclaim cached objects when cache reaches the max size

[levin li]

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
>