> + struct list_head *freeable = arg;
> + struct dentry *dentry = container_of(item, struct dentry, d_lru);
> +
> +
> + /*
double empty line.
> + * we are inverting the lru lock/dentry->d_lock here,
> + * so use a trylock. If we fail to get the lock, just skip
> + * it
> + */
> + if (!spin_trylock(&dentry->d_lock))
> + return 2;
> +
> + /*
> + * Referenced dentries are still in use. If they have active
> + * counts, just remove them from the LRU. Otherwise give them
> + * another pass through the LRU.
> + */
> + if (dentry->d_count) {
> + list_del_init(&dentry->d_lru);
> + spin_unlock(&dentry->d_lock);
> + return 0;
> + }
> +
> + if (dentry->d_flags & DCACHE_REFERENCED) {
The comment aove seems odd, given that it doesn't match the code.
I'd rather have something like:
/*
* Used dentry, remove it from the LRU.
*/
in its place, and a second one above the DCACHE_REFERENCED check:
/*
* Referenced dentry, give it another pass through the LRU.
*/
> + dentry->d_flags &= ~DCACHE_REFERENCED;
> + spin_unlock(&dentry->d_lock);
> +
> + /*
> + * XXX: this list move should be be done under d_lock. Need to
> + * determine if it is safe just to do it under the lru lock.
> + */
> + return 1;
> + }
> +
> + list_move_tail(&dentry->d_lru, freeable);
Another odd comment. It talks about doing a list_move in the branch
that doesn't do the list_move, and the list_move outside the branch
actually has the d_lock, thus disagreeing with the comment.
> + this_cpu_dec(nr_dentry_unused);
> + spin_unlock(&dentry->d_lock);
No need to decrement the per-cpu counter while still having the lock
held.
> @@ -1094,11 +1069,10 @@ resume:
> /*
> * move only zero ref count dentries to the dispose list.
> */
> + dentry_lru_del(dentry);
> if (!dentry->d_count) {
> - dentry_lru_move_list(dentry, dispose);
> + list_add_tail(&dentry->d_lru, dispose);
> found++;
> - } else {
> - dentry_lru_del(dentry);
I'd rather move this hunk to the previous patch, as it fits into the
logical change done there.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
