lkml.org 
[lkml]   [2002]   [Mar]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] 2.5.6-pre3 Fast Walk Dcache

--On Friday, March 08, 2002 13:28:23 -0800 Paul Menage <pmenage@ensim.com> wrote:

>
> 1) You're missing parentheses in cached_lookup_nd() and path_lookup():
>
Oops. Operator precedence strikes again. They are in
cached_lookup_nd() and link_path_walk(). I have made
the changes.

> 2) Since cached_lookup_nd() calls __d_lookup() and hence
> __dget_locked(), it's not clear how you actually avoid incrementing the
> d_count values of the dentries, other than the root/cwd dentries. Can
> you explain the logic in a little more detail?

The frequency with which root/cwd dentries are in any given path
probably cause the majority of the cacheline bouncing of d_count.
So the logic there is clear. However, you make a good point and
I'm looking at this.

> 3) If you replace walk_init_root() and path_lookup() with something
> like the following, you can pull the ugliness of walk_init_root() out
> of path_lookup(). Basically, make walk_init_root() recognise
> LOOKUP_LOCKED and take the dcache_lock rather than grabbing refcounts.
> walk_init_root() drops the LOOKUP_LOCKED flag if necessary while
> calling __emul_lookup_dentry() to avoid additional complexity. If
> walk_init_root() returns 0, then the dcache lock wasn't taken,
> regardless of whether the nd.flags had LOOKUP_LOCKED set.
>
> static inline int
> walk_init_root(const char *name, struct nameidata *nd)
> {
> unsigned int flags = nd->flags;
> read_lock(&current->fs->lock);
> if (current->fs->altroot && !(nd->flags & LOOKUP_NOALT)) {
>
> if(flags & LOOKUP_LOCKED)
> nd->flags &= ~LOOKUP_LOCKED;
>
> nd->mnt = mntget(current->fs->altrootmnt);
> nd->dentry = dget(current->fs->altroot);

The first reaction I have is that it breaks the consistancy
between a flag and what the flag represents. Having the dcache_lock
held without the LOOKUP_LOCKED flag in this part of the code might
cause deadlocks or lead to hard-to-maintain code.

I appreciate you taking the time to provide such thoughtful and
deatailed comments and I will look at the whole patch again with
these comments in mind. Next week expect a new and improved
version!

Thanks.

Hanna





-
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: 2005-03-22 13:24    [W:0.051 / U:0.336 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site