On Mon, Feb 08, 2016 at 02:06:36AM -0800, Christoph Hellwig wrote:
> > --- a/include/cache.h
> > +++ b/include/cache.h
> > @@ -51,6 +51,7 @@ enum {
> > #define CACHE_BASE_PRIORITY 0
> > #define CACHE_PREFETCH_PRIORITY 8
> > #define CACHE_MAX_PRIORITY 15
> > +#define CACHE_DIRTY_PRIORITY (CACHE_MAX_PRIORITY + 1)
>
> Sizing arrays based on, and iterating up to CACHE_DIRTY_PRIORITY seems
> rather odd. Maybe add a new
>
> #define CACHE_NR_PRIORITIES CACHE_DIRTY_PRIORITY
>
> and a comment explaining the magic to make it more obvious?
Ok.
> > +cache_move_to_dirty_mru(
> > + struct cache *cache,
> > + struct cache_node *node)
> > +{
> > + struct cache_mru *mru;
> > +
> > + mru = &cache->c_mrus[CACHE_DIRTY_PRIORITY];
> > +
> > + pthread_mutex_lock(&mru->cm_mutex);
> > + node->cn_priority = CACHE_DIRTY_PRIORITY;
> > + list_move(&node->cn_mru, &mru->cm_list);
> > + mru->cm_count++;
> > + pthread_mutex_unlock(&mru->cm_mutex);
> > +}
>
> Maybe it would better to just do a list_add here and leave the
> list_del to the caller to avoid needing to nest two different
> cm_mutex instances.
I'll have a look at it.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
|