Re: [PATCH v2 3/9] xfsrestore: cache path lookups

Alex Elder
2010-11-12T23:24:36Z

On Fri, 2010-11-05 at 11:35 -0500, wkendall@sgi.com wrote:
>
> In order to resolve a pathname, xfsrestore must work from an inode
> number (from the dump) and recurse up the directory entry tree that it
> has constructed. Each level of recursion requires a seek and read to
> get the name of the dirent, and possibly a mmap of a section of the
> directory entry tree if it is not already mapped (and in that case,
> possibly a munmap of another section). It's quite common to resolve
> pathnames in the same directory consecutively, so simply caching the
> parent directory pathname from the previous lookup saves quite a bit
> of overhead.
>
> Signed-off-by: Bill Kendall <wkendall@sgi.com>

I think an assertion would do in a place I show below,
but it's nothing to worry about.

Reviewed-by: Alex Elder <aelder@sgi.com>

> ---
> restore/tree.c | 41 ++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 36 insertions(+), 5 deletions(-)
>
> Index: xfsdump-kernel.org/restore/tree.c
> ===================================================================
> --- xfsdump-kernel.org.orig/restore/tree.c
> +++ xfsdump-kernel.org/restore/tree.c

. . .

> @@ -3475,6 +3486,13 @@ Node2path_recurse( nh_t nh, char *buf, i
> return bufsz;
> }
>
> + /* if we have a cache hit, no need to recurse any further
> + */
> + if ( nh == cache.nh && bufsz > cache.len ) {

Since a nh basically encodes the entire path,
the check that the buffer is big enough should
not be necessary, and could probably be inserted
instead:
ASSERT(bufsz > cache.len);

> + strcpy( buf, cache.buf );
> + return bufsz - cache.len;
> + }
> +
> /* extract useful node members
> */
> np = Node_map( nh );

. . .