Hi Jordi,
Jordi Pujol <jordipujolp@xxxxxxxxx> writes:
> - to improve copy_up performance I have redesigned the procedure,
> it traverses the tree looking for the topmost lower dentry, anotating the
> concerning dentries and locking them; after, for each dentry from top to
> bottom, it copies the inodes and releases the dentry.
Looks good, but I'm only willing to look at performance issues after the
bugs have been fixed.
> - a repeated line is found in overlayfs.h
Yeah, patch applied. Thanks.
> - a few days ago we talked about an error when repetitive sed instructions
> were executed, also it happens in other processes, like mantaining list of
> installed files for actual packages,
> that is solved in ovl_rename issuing a dget for the old-dentry and parent
> dentries. (This code is inspired on rename.c from unionfs)
>
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -512,12 +512,17 @@ static int ovl_rename(struct inode *oldd
> old_upperdir = ovl_dentry_upper(old->d_parent);
> new_upperdir = ovl_dentry_upper(new->d_parent);
>
> + (void) dget(old_upperdir);
> + (void) dget(new_upperdir);
> +
This should not be necessary. vfs_rename() locks old and new parents
which means taking an extra ref is totally superfluous. If this does
make some difference then there's something very sinister going on an we
need to investigate further.
> trap = lock_rename(new_upperdir, old_upperdir);
>
> olddentry = ovl_dentry_upper(old);
> - newdentry = ovl_dentry_upper(new);
> - if (newdentry) {
> - dget(newdentry);
> + (void) dget(olddentry);
Again, while we have a ref on old, we should have a ref on olddentry as
well, so this shouldn't make a difference.
> +
> + if (!ovl_is_whiteout(new) &&
> + (newdentry = ovl_dentry_upper(new))) {
> + (void) dget(newdentry);
This shouldn't make a difference either, ovl_dentry_upper() will return
NULL for whiteouts.
Can you try introducing the above changes one by one to see which one,
if any of them, makes a difference to your test case?
Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
