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

[MORITA Kazutaka]

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?

> 
> > 
> >> +
> >> +	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.

Thanks,

Kazutaka