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    [W:0.032 / U:0.004 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site