Jan Blunck wrote:
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);
}
Yes, I think it is.
I think the condition below should be an else if
No. We always lower the reference count in d_genocide.
Yep, good catch
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;
Is this true for all paths? d_genocide() might actually not return
+
+ return 0;
+}
+
+typedef int (*walker_t)(struct dentry * dentry, int * return);
+
Will there be a different type of walker as well? Is it going to be too different?
+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;
+}
The overall code looks good.
--
Balbir Singh,
Linux Technology Center,
IBM Software Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
