[sheepdog] [PATCH v4 6/8] object cache: reclaim cached objects when cache reaches the max size

[Liu Yuan]

On 07/27/2012 12:27 PM, levin li wrote:
> +static inline void mark_cache_flush(struct object_cache *cache)
> +{
> +	cache->in_flush = 1;
> +}
> +
> +static inline void end_cache_flush(struct object_cache *cache)
> +{
> +	cache->in_flush = 0;
> +}
> +

There is no need to add these two helpers, open code cache->in_flush
operation is more clear. Also better open code mark_entry_reclaim() into
plain code.

> +static int check_cache_status(struct object_cache *oc,
> +			      struct object_cache_entry *entry)
> +{
> +	int refcnt = uatomic_read(&entry->refcnt);
> +	/* If entry is being accessed, we don't reclaim it */
> +	if (refcnt > 0) {
> +		dprintf("cache object %" PRIx32 "(%08" PRIx32 ") "
> +			"can't be reclaimed, refcnt: %d\n",
> +			oc->vid, entry->idx, refcnt);
> +		return -1;
> +	}
> +
> +	return 0;
> +}

So you just check entry->refcnt... Then simply check if
(uatomic_read(&entry->refcnt > 0) in upper code is more clear. Open-code it.

> @@ -55,11 +67,16 @@ struct object_cache_entry {
>  	uint32_t idx;
>  	uint64_t bmap; /* each bit represents one dirty
>  			* block which should be flushed */
> -	struct rb_node rb;
> +	int refcnt;
> +	int flags;

We'll have lots of entries in memory, always try our best to save space.
No need to add extra flags. Use the reserved bits in 'idx'. do you
really need 32bits for refcnt?

> +retry:
> +	ret = object_cache_lookup(cache, idx, create);
> +	if (ret == SD_RES_NO_CACHE) {
>  		ret = object_cache_pull(cache, idx);
>  		if (ret != SD_RES_SUCCESS)
>  			return ret;
>  	}
>
> -	pthread_rwlock_rdlock(&cache->lock);
> -	entry = object_tree_search(&cache->object_tree, idx);

We will call find_cache_entry() twice for a cached object, this is the
hottest path! So I suggest:

	entry = get_cache_entry(cache, idx, create)
	if (!entry) {
		ret = object_cache_pull(cache, idx);
		if (ret != SD_RES_SUCCESS)
			goto err; /* call put_cache_entry */
		/* document ... */
		entry->refcnt++;
	}

Also in your code, you don't check -1 for object_cache_lookup().

Thanks,
Yuan