lkml.org 
[lkml]   [2006]   [Jun]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/5] vfs: d_genocide() doesnt add dentries to unused list
On Mon, Jun 19, Balbir Singh wrote:

> > this_parent = dentry;
> > goto repeat;
> > }
> >- atomic_dec(&dentry->d_count);
> >+ if (!list_empty(&dentry->d_lru)) {
> >+ dentry_stat.nr_unused--;
> >+ list_del_init(&dentry->d_lru);
> >+ }
> >+ if (atomic_dec_and_test(&dentry->d_count)) {
> >+ list_add(&dentry->d_lru, dentry_unused.prev);
> >+ dentry_stat.nr_unused++;
> >+ }
>
> We could have dentries on the LRU list with non-zero d_count. If
> we have a dentry on the LRU list with a count of 1, then the code
> will remove it from LRU list and then add it back subsequently.
>

So you think this is better?

if (atomic_dec_and_test(&dentry->d_count)) {
if (!list_empty(&dentry_d_lru))
list_move_tail(&dentry->d_lru, dentry_unused);
} else
if (!list_empty(&dentry->d_lru)) {
dentry_stat.nr_unused--;
list_del_init(&dentry->d_lru);
}

> I think the condition below should be an else if
>

No. We always lower the reference count in d_genocide.

>
> d_genocide() now almost looks like select_parent(). I think we can share a
> lot
> of code between the two.
>

Hmm, interesting idea. This would save the dentry-tree walking code in
have_submounts too. Maybe something like this:

+static int select_parent_walker(struct dentry * dentry, int * found)
+{
+ if (!list_empty(&dentry->d_lru)) {
+ dentry_stat.nr_unused--;
+ list_del_init(&dentry->d_lru);
+ }
+
+ /*
+ * move only zero ref count dentries to the end
+ * of the unused list for prune_dcache
+ */
+ if (!atomic_read(&dentry->d_count)) {
+ list_add(&dentry->d_lru, dentry_unused.prev);
+ dentry_stat.nr_unused++;
+ *found++;
+ }
+
+ /*
+ * We can return to the caller if we have found some (this
+ * ensures forward progress). We'll be coming back to find
+ * the rest.
+ */
+ if (*found && need_resched())
+ return -1;
+
+ return 0;
+}
+
+typedef int (*walker_t)(struct dentry * dentry, int * return);
+
+static int dentry_tree_walk(struct dentry * parent, walker_t walker)
+{
+ struct dentry *this_parent = parent;
+ struct list_head *next;
+ int ret = 0;
+
+ spin_lock(&dcache_lock);
+repeat:
+ next = this_parent->d_subdirs.next;
+resume:
+ while (next != &this_parent->d_subdirs) {
+ struct list_head *tmp = next;
+ struct dentry *dentry = list_entry(tmp, struct dentry,
+ d_u.d_child);
+ next = tmp->next;
+
+ if (walker(dentry, &ret))
+ goto out;
+
+ /*
+ * Descend a level if the d_subdirs list is non-empty.
+ */
+ if (!list_empty(&dentry->d_subdirs)) {
+ this_parent = dentry;
+ goto repeat;
+ }
+ }
+ /*
+ * All done at this level ... ascend and resume the search.
+ */
+ if (this_parent != parent) {
+ next = this_parent->d_u.d_child.next;
+ this_parent = this_parent->d_parent;
+ goto resume;
+ }
+out:
+ spin_unlock(&dcache_lock);
+ return ret;
+}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2006-06-19 11:25    [from the cache]
©2003-2011 Jasper Spaans