woops forgot the patch
Shaya Potter wrote:
here's a patch, based on the algorithm, shrinks
unionfs_rename_whiteout() tremendously.
small tests seem to work correctly, the regression suite obviously fails
as the algorithm has changed (i.e. if not leftmost, we copyup, even
branch is rw.
Josef Sipek wrote:
On Fri, Jan 19, 2007 at 02:52:12PM -0500, Shaya Potter wrote:
why not
if (branch(old_dentry) != left_most)
copyup();
vfs_rename(old_dentry, new_dentry);
create_whiteout(old_dentry);
I'm thinking of something more like this:
if (is_ro_branch(old))
copyup_to_left_most();
vfs_rename(old,new);
create_wh(old);
This way, we copyup only when we have to.
would seemingly simplify it a lot, negative is that a rename() would
create a copy even if source is on a rw branch.
Just changing the condition can prevent a number of copyups, while
keeping
the code sane.
The question is rename() more similar to a "file modification" or a
file creation/unlink. if it's the former, the current hoops make
some sense, if it's the latter, the simplified version would
seemingly make more sense.
I have no problem trying this out in the lkml code. I'm not quite
ready to
break the 1.x semantics as some people depend on them.
Shaya, If you can make a patch, I'll add it to the code to push to
Andrew.
I'm not sure when I'll get to do it myself...classes just started
again, and
I'm trying to battle the page cache :)
Jeff.
_______________________________________________
unionfs mailing list
[email protected]
http://www.fsl.cs.sunysb.edu/mailman/listinfo/unionfs
Index: rename.c
===================================================================
RCS file: /home/spotter/unionfs/2/unionfs/unionfs/rename.c,v
retrieving revision 1.47
diff -u -r1.47 rename.c
--- rename.c 5 Aug 2006 01:28:46 -0000 1.47
+++ rename.c 23 Jan 2007 17:49:21 -0000
@@ -163,14 +163,12 @@
struct dentry *new_dentry)
{
int err = 0;
- int bindex, bwh_old;
+ int bindex = 0, bwh_old;
int old_bstart, old_bend;
int new_bstart, new_bend;
int do_copyup = -1;
struct dentry *parent_dentry;
int local_err = 0;
- int eio = 0;
- int revert = 0;
struct dentry *wh_old = NULL;
print_entry_location();
@@ -183,58 +181,9 @@
new_bstart = dbstart(new_dentry);
new_bend = dbend(new_dentry);
- /* Rename source to destination. */
- err = do_rename(old_dir, old_dentry, new_dir, new_dentry, old_bstart,
- &wh_old);
- if (err) {
- if (!IS_COPYUP_ERR(err)) {
- goto out;
- }
- do_copyup = old_bstart - 1;
- } else {
- revert = 1;
- }
-
- /* Unlink all instances of destination that exist to the left of
- * bstart of source. On error, revert back, goto out.
- */
- for (bindex = old_bstart - 1; bindex >= new_bstart; bindex--) {
- struct dentry *unlink_dentry;
- struct dentry *unlink_dir_dentry;
-
- unlink_dentry = dtohd_index(new_dentry, bindex);
- if (!unlink_dentry) {
- continue;
- }
-
- unlink_dir_dentry = lock_parent(unlink_dentry);
- if (!(err = is_robranch_super(old_dir->i_sb, bindex))) {
- err =
- vfs_unlink(unlink_dir_dentry->d_inode,
- unlink_dentry);
- }
-
- fist_copy_attr_times(new_dentry->d_parent->d_inode,
- unlink_dir_dentry->d_inode);
- /* propagate number of hard-links */
- new_dentry->d_parent->d_inode->i_nlink =
- get_nlinks(new_dentry->d_parent->d_inode);
-
- unlock_dir(unlink_dir_dentry);
- if (!err) {
- if (bindex != new_bstart) {
- DPUT(unlink_dentry);
- set_dtohd_index(new_dentry, bindex, NULL);
- }
- } else if (IS_COPYUP_ERR(err)) {
- do_copyup = bindex - 1;
- } else if (revert) {
- DPUT(wh_old);
- goto revert;
- }
- }
-
- if (do_copyup != -1) {
+ /* 1. if !leftmost, do copyup */
+ if (old_bstart != 0) {
+ do_copyup = old_bstart-1;
for (bindex = do_copyup; bindex >= 0; bindex--) {
/* copyup the file into some left directory, so that
you can rename it */
err =
@@ -242,21 +191,26 @@
old_dentry, old_bstart, bindex, NULL,
old_dentry->d_inode->i_size);
if (!err) {
- DPUT(wh_old);
+ /* 1a. if copyup worked, need to know where to
+ whiteout */
bwh_old = bindex;
- err =
- do_rename(old_dir, old_dentry, new_dir,
- new_dentry, bindex, &wh_old);
break;
}
}
}
+ /* 2. do actual rename, this will remove "dest" whiteout if exists */
+ err = do_rename(old_dir, old_dentry, new_dir, new_dentry, bindex,
+ &wh_old);
+ if (err)
+ goto out;
+
/* make it opaque */
if (S_ISDIR(old_dentry->d_inode->i_mode)) {
err = make_dir_opaque(old_dentry, dbstart(old_dentry));
- if (err)
- goto revert;
+ if (err) {
+ dprint(PRINT_DEBUG, "rename: error creating directory
override entry: %d\n", err);
+ }
}
/* Create whiteout for source, only if:
@@ -285,62 +239,6 @@
DPUT(wh_old);
print_exit_status(err);
return err;
-
- revert:
- /* Do revert here. */
- local_err = unionfs_refresh_hidden_dentry(new_dentry, old_bstart);
- if (local_err) {
- printk(KERN_WARNING
- "Revert failed in rename: the new refresh failed.\n");
- eio = -EIO;
- }
-
- local_err = unionfs_refresh_hidden_dentry(old_dentry, old_bstart);
- if (local_err) {
- printk(KERN_WARNING
- "Revert failed in rename: the old refresh failed.\n");
- eio = -EIO;
- goto revert_out;
- }
-
- if (!dtohd_index(new_dentry, bindex)
- || !dtohd_index(new_dentry, bindex)->d_inode) {
- printk(KERN_WARNING
- "Revert failed in rename: the object disappeared from
under us!\n");
- eio = -EIO;
- goto revert_out;
- }
-
- if (dtohd_index(old_dentry, bindex)
- && dtohd_index(old_dentry, bindex)->d_inode) {
- printk(KERN_WARNING
- "Revert failed in rename: the object was created
underneath us!\n");
- eio = -EIO;
- goto revert_out;
- }
-
- local_err =
- do_rename(new_dir, new_dentry, old_dir, old_dentry, old_bstart,
- NULL);
-
- /* If we can't fix it, then we cop-out with -EIO. */
- if (local_err) {
- printk(KERN_WARNING "Revert failed in rename!\n");
- eio = -EIO;
- }
-
- local_err = unionfs_refresh_hidden_dentry(new_dentry, bindex);
- if (local_err)
- eio = -EIO;
- local_err = unionfs_refresh_hidden_dentry(old_dentry, bindex);
- if (local_err)
- eio = -EIO;
-
- revert_out:
- if (eio)
- err = eio;
- print_exit_status(err);
- return err;
}
/*
_______________________________________________
unionfs mailing list
[email protected]
http://www.fsl.cs.sunysb.edu/mailman/listinfo/unionfs
