* [GIT PULL -mm] 0/3 Unionfs updates/fixes/cleanups
@ 2008-01-03 5:57 Erez Zadok
2008-01-03 5:57 ` [PATCH 1/3] Unionfs: use printk KERN_CONT for debugging messages Erez Zadok
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Erez Zadok @ 2008-01-03 5:57 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel, linux-fsdevel, viro, hch
The following is a series of patchsets related to Unionfs. This is the
third set of patchsets resulting from an lkml review of the entire unionfs
code base. The most significant change here is a locking/race bugfix during
dentry revalidation.
These patches were tested (where appropriate) on Linus's 2.6.24 latest code
(as of v2.6.24-rc6-179-gb8c9a18), MM, as well as the backports to
2.6.{23,22,21,20,19,18,9} on ext2/3/4, xfs, reiserfs, nfs2/3/4, jffs2,
ramfs, tmpfs, cramfs, and squashfs (where available). Also tested with
LTP-full. See http://unionfs.filesystems.org/ to download back-ported
unionfs code.
Please pull from the 'master' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/ezk/unionfs.git
to receive the following:
Erez Zadok (3):
Unionfs: use printk KERN_CONT for debugging messages
Unionfs: locking fixes
Unionfs: use VFS helpers to manipulate i_nlink
debug.c | 50 ++++++++++++++++++++++++++------------------------
dentry.c | 13 ++++++++++++-
fanout.h | 3 ++-
unlink.c | 2 +-
4 files changed, 41 insertions(+), 27 deletions(-)
---
Erez Zadok
ezk@cs.sunysb.edu
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] Unionfs: use printk KERN_CONT for debugging messages
2008-01-03 5:57 [GIT PULL -mm] 0/3 Unionfs updates/fixes/cleanups Erez Zadok
@ 2008-01-03 5:57 ` Erez Zadok
2008-01-03 6:26 ` Joe Perches
2008-01-03 5:57 ` [PATCH 2/3] Unionfs: locking fixes Erez Zadok
2008-01-03 5:57 ` [PATCH 3/3] Unionfs: use VFS helpers to manipulate i_nlink Erez Zadok
2 siblings, 1 reply; 6+ messages in thread
From: Erez Zadok @ 2008-01-03 5:57 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel, linux-fsdevel, viro, hch, Erez Zadok
Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
---
fs/unionfs/debug.c | 50 ++++++++++++++++++++++++++------------------------
1 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/fs/unionfs/debug.c b/fs/unionfs/debug.c
index c2b8b58..5f1d887 100644
--- a/fs/unionfs/debug.c
+++ b/fs/unionfs/debug.c
@@ -456,9 +456,10 @@ void __show_branch_counts(const struct super_block *sb,
mnt = UNIONFS_D(sb->s_root)->lower_paths[i].mnt;
else
mnt = NULL;
- pr_debug("%d:", (mnt ? atomic_read(&mnt->mnt_count) : -99));
+ printk(KERN_CONT "%d:",
+ (mnt ? atomic_read(&mnt->mnt_count) : -99));
}
- pr_debug("%s:%s:%d\n", file, fxn, line);
+ printk(KERN_CONT "%s:%s:%d\n", file, fxn, line);
}
void __show_inode_times(const struct inode *inode,
@@ -472,15 +473,15 @@ void __show_inode_times(const struct inode *inode,
if (unlikely(!lower_inode))
continue;
pr_debug("IT(%lu:%d): ", inode->i_ino, bindex);
- pr_debug("%s:%s:%d ", file, fxn, line);
- pr_debug("um=%lu/%lu lm=%lu/%lu ",
- inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
- lower_inode->i_mtime.tv_sec,
- lower_inode->i_mtime.tv_nsec);
- pr_debug("uc=%lu/%lu lc=%lu/%lu\n",
- inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
- lower_inode->i_ctime.tv_sec,
- lower_inode->i_ctime.tv_nsec);
+ printk(KERN_CONT "%s:%s:%d ", file, fxn, line);
+ printk(KERN_CONT "um=%lu/%lu lm=%lu/%lu ",
+ inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
+ lower_inode->i_mtime.tv_sec,
+ lower_inode->i_mtime.tv_nsec);
+ printk(KERN_CONT "uc=%lu/%lu lc=%lu/%lu\n",
+ inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
+ lower_inode->i_ctime.tv_sec,
+ lower_inode->i_ctime.tv_nsec);
}
}
@@ -497,15 +498,15 @@ void __show_dinode_times(const struct dentry *dentry,
continue;
pr_debug("DT(%s:%lu:%d): ", dentry->d_name.name, inode->i_ino,
bindex);
- pr_debug("%s:%s:%d ", file, fxn, line);
- pr_debug("um=%lu/%lu lm=%lu/%lu ",
- inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
- lower_inode->i_mtime.tv_sec,
- lower_inode->i_mtime.tv_nsec);
- pr_debug("uc=%lu/%lu lc=%lu/%lu\n",
- inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
- lower_inode->i_ctime.tv_sec,
- lower_inode->i_ctime.tv_nsec);
+ printk(KERN_CONT "%s:%s:%d ", file, fxn, line);
+ printk(KERN_CONT "um=%lu/%lu lm=%lu/%lu ",
+ inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
+ lower_inode->i_mtime.tv_sec,
+ lower_inode->i_mtime.tv_nsec);
+ printk(KERN_CONT "uc=%lu/%lu lc=%lu/%lu\n",
+ inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
+ lower_inode->i_ctime.tv_sec,
+ lower_inode->i_ctime.tv_nsec);
}
}
@@ -524,9 +525,10 @@ void __show_inode_counts(const struct inode *inode,
lower_inode = unionfs_lower_inode_idx(inode, bindex);
if (unlikely(!lower_inode))
continue;
- pr_debug("SIC(%lu:%d:%d): ", inode->i_ino, bindex,
- atomic_read(&(inode)->i_count));
- pr_debug("lc=%d ", atomic_read(&(lower_inode)->i_count));
- pr_debug("%s:%s:%d\n", file, fxn, line);
+ printk(KERN_CONT "SIC(%lu:%d:%d): ", inode->i_ino, bindex,
+ atomic_read(&(inode)->i_count));
+ printk(KERN_CONT "lc=%d ",
+ atomic_read(&(lower_inode)->i_count));
+ printk(KERN_CONT "%s:%s:%d\n", file, fxn, line);
}
}
--
1.5.2.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] Unionfs: locking fixes
2008-01-03 5:57 [GIT PULL -mm] 0/3 Unionfs updates/fixes/cleanups Erez Zadok
2008-01-03 5:57 ` [PATCH 1/3] Unionfs: use printk KERN_CONT for debugging messages Erez Zadok
@ 2008-01-03 5:57 ` Erez Zadok
2008-01-03 5:57 ` [PATCH 3/3] Unionfs: use VFS helpers to manipulate i_nlink Erez Zadok
2 siblings, 0 replies; 6+ messages in thread
From: Erez Zadok @ 2008-01-03 5:57 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel, linux-fsdevel, viro, hch, Erez Zadok
Lock parent dentries during revalidation.
Reduce total number of lockdep classes used.
Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
---
fs/unionfs/dentry.c | 13 ++++++++++++-
fs/unionfs/fanout.h | 3 ++-
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 0369d93..7646828 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -42,6 +42,7 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry,
memset(&lowernd, 0, sizeof(struct nameidata));
verify_locked(dentry);
+ verify_locked(dentry->d_parent);
/* if the dentry is unhashed, do NOT revalidate */
if (d_deleted(dentry))
@@ -351,7 +352,10 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd,
* to child order.
*/
for (i = 0; i < chain_len; i++) {
- unionfs_lock_dentry(chain[i], UNIONFS_DMUTEX_REVAL+i);
+ unionfs_lock_dentry(chain[i], UNIONFS_DMUTEX_REVAL_CHILD);
+ if (chain[i] != chain[i]->d_parent)
+ unionfs_lock_dentry(chain[i]->d_parent,
+ UNIONFS_DMUTEX_REVAL_PARENT);
saved_bstart = dbstart(chain[i]);
saved_bend = dbend(chain[i]);
sbgen = atomic_read(&UNIONFS_SB(dentry->d_sb)->generation);
@@ -366,6 +370,8 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd,
bindex++)
unionfs_mntput(chain[i], bindex);
}
+ if (chain[i] != chain[i]->d_parent)
+ unionfs_unlock_dentry(chain[i]->d_parent);
unionfs_unlock_dentry(chain[i]);
if (unlikely(!valid))
@@ -376,6 +382,9 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd,
out_this:
/* finally, lock this dentry and revalidate it */
verify_locked(dentry);
+ if (dentry != dentry->d_parent)
+ unionfs_lock_dentry(dentry->d_parent,
+ UNIONFS_DMUTEX_REVAL_PARENT);
dgen = atomic_read(&UNIONFS_D(dentry)->generation);
if (unlikely(is_newer_lower(dentry))) {
@@ -394,6 +403,8 @@ out_this:
purge_inode_data(dentry->d_inode);
}
valid = __unionfs_d_revalidate_one(dentry, nd);
+ if (dentry != dentry->d_parent)
+ unionfs_unlock_dentry(dentry->d_parent);
/*
* If __unionfs_d_revalidate_one() succeeded above, then it will
diff --git a/fs/unionfs/fanout.h b/fs/unionfs/fanout.h
index 5f31015..4d9a45f 100644
--- a/fs/unionfs/fanout.h
+++ b/fs/unionfs/fanout.h
@@ -290,7 +290,8 @@ enum unionfs_dentry_lock_class {
UNIONFS_DMUTEX_PARENT,
UNIONFS_DMUTEX_CHILD,
UNIONFS_DMUTEX_WHITEOUT,
- UNIONFS_DMUTEX_REVAL, /* for file/dentry revalidate */
+ UNIONFS_DMUTEX_REVAL_PARENT, /* for file/dentry revalidate */
+ UNIONFS_DMUTEX_REVAL_CHILD, /* for file/dentry revalidate */
};
static inline void unionfs_lock_dentry(struct dentry *d,
--
1.5.2.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] Unionfs: use VFS helpers to manipulate i_nlink
2008-01-03 5:57 [GIT PULL -mm] 0/3 Unionfs updates/fixes/cleanups Erez Zadok
2008-01-03 5:57 ` [PATCH 1/3] Unionfs: use printk KERN_CONT for debugging messages Erez Zadok
2008-01-03 5:57 ` [PATCH 2/3] Unionfs: locking fixes Erez Zadok
@ 2008-01-03 5:57 ` Erez Zadok
2 siblings, 0 replies; 6+ messages in thread
From: Erez Zadok @ 2008-01-03 5:57 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel, linux-fsdevel, viro, hch, Erez Zadok
Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
---
fs/unionfs/unlink.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/unionfs/unlink.c b/fs/unionfs/unlink.c
index a1c82b6..1e370a1 100644
--- a/fs/unionfs/unlink.c
+++ b/fs/unionfs/unlink.c
@@ -79,7 +79,7 @@ static int unionfs_unlink_whiteout(struct inode *dir, struct dentry *dentry)
out:
if (!err)
- dentry->d_inode->i_nlink--;
+ inode_dec_link_count(dentry->d_inode);
/* We don't want to leave negative leftover dentries for revalidate. */
if (!err && (dbopaque(dentry) != -1))
--
1.5.2.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] Unionfs: use printk KERN_CONT for debugging messages
2008-01-03 5:57 ` [PATCH 1/3] Unionfs: use printk KERN_CONT for debugging messages Erez Zadok
@ 2008-01-03 6:26 ` Joe Perches
2008-01-03 18:36 ` Erez Zadok
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2008-01-03 6:26 UTC (permalink / raw)
To: Erez Zadok; +Cc: akpm, linux-kernel, linux-fsdevel, viro, hch
On Thu, 2008-01-03 at 00:57 -0500, Erez Zadok wrote:
> diff --git a/fs/unionfs/debug.c b/fs/unionfs/debug.c
> index c2b8b58..5f1d887 100644
> --- a/fs/unionfs/debug.c
> +++ b/fs/unionfs/debug.c
> void __show_inode_times(const struct inode *inode,
> @@ -472,15 +473,15 @@ void __show_inode_times(const struct inode *inode,
> if (unlikely(!lower_inode))
> continue;
> pr_debug("IT(%lu:%d): ", inode->i_ino, bindex);
> - pr_debug("%s:%s:%d ", file, fxn, line);
> - pr_debug("um=%lu/%lu lm=%lu/%lu ",
> - inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
> - lower_inode->i_mtime.tv_sec,
> - lower_inode->i_mtime.tv_nsec);
> - pr_debug("uc=%lu/%lu lc=%lu/%lu\n",
> - inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
> - lower_inode->i_ctime.tv_sec,
> - lower_inode->i_ctime.tv_nsec);
> + printk(KERN_CONT "%s:%s:%d ", file, fxn, line);
> + printk(KERN_CONT "um=%lu/%lu lm=%lu/%lu ",
> + inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
> + lower_inode->i_mtime.tv_sec,
> + lower_inode->i_mtime.tv_nsec);
> + printk(KERN_CONT "uc=%lu/%lu lc=%lu/%lu\n",
> + inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
> + lower_inode->i_ctime.tv_sec,
> + lower_inode->i_ctime.tv_nsec);
> }
> }
>
I think printks should be single statements and
KERN_CONT should be used as sparingly as possible.
Perhaps:
pr_debug("IT(%lu:%d): %s:%s:%d "
"um=%lu/%lu lm=%lu/%lu "
"uc=%lu/%lu lc=%lu/%lu\n",
inode->i_ino, bindex, file, fnx, line,
inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
lower_inode->i_mtime.tv_sec,
lower_inode->i_mtime.tv_nsec
inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
lower_inode->i_ctime.tv_sec,
lower_inode->i_ctime.tv_nsec);
> @@ -497,15 +498,15 @@ void __show_dinode_times(const struct dentry *dentry,
> continue;
> pr_debug("DT(%s:%lu:%d): ", dentry->d_name.name, inode->i_ino,
> bindex);
> - pr_debug("%s:%s:%d ", file, fxn, line);
> - pr_debug("um=%lu/%lu lm=%lu/%lu ",
> - inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
> - lower_inode->i_mtime.tv_sec,
> - lower_inode->i_mtime.tv_nsec);
> - pr_debug("uc=%lu/%lu lc=%lu/%lu\n",
> - inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
> - lower_inode->i_ctime.tv_sec,
> - lower_inode->i_ctime.tv_nsec);
> + printk(KERN_CONT "%s:%s:%d ", file, fxn, line);
> + printk(KERN_CONT "um=%lu/%lu lm=%lu/%lu ",
> + inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
> + lower_inode->i_mtime.tv_sec,
> + lower_inode->i_mtime.tv_nsec);
> + printk(KERN_CONT "uc=%lu/%lu lc=%lu/%lu\n",
> + inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
> + lower_inode->i_ctime.tv_sec,
> + lower_inode->i_ctime.tv_nsec);
> }
> }
>
and
pr_debug("DT(%s:%lu:%d): %s:%s:%d "
"um=%lu/%lu lm=%lu/%lu "
"uc=%lu/%lu lc=%lu/%lu\n",
dentry->d_name.name, inode->i_ino, bindex,
file, fnx, line,
inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
lower_inode->i_mtime.tv_sec,
lower_inode->i_mtime.tv_nsec
inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
lower_inode->i_ctime.tv_sec,
lower_inode->i_ctime.tv_nsec);
>
> @@ -524,9 +525,10 @@ void __show_inode_counts(const struct inode *inode,
> lower_inode = unionfs_lower_inode_idx(inode, bindex);
> if (unlikely(!lower_inode))
> continue;
> - pr_debug("SIC(%lu:%d:%d): ", inode->i_ino, bindex,
> - atomic_read(&(inode)->i_count));
> - pr_debug("lc=%d ", atomic_read(&(lower_inode)->i_count));
> - pr_debug("%s:%s:%d\n", file, fxn, line);
> + printk(KERN_CONT "SIC(%lu:%d:%d): ", inode->i_ino, bindex,
> + atomic_read(&(inode)->i_count));
> + printk(KERN_CONT "lc=%d ",
> + atomic_read(&(lower_inode)->i_count));
> + printk(KERN_CONT "%s:%s:%d\n", file, fxn, line);
> }
> }
and
pr_debug("SIC(%lu:%d:%d): lc=%d %s:%s:%d\n",
inode->i_ino, bindex,
atomic_read(&(inode)->i_count),
atomic_read(&(lower_inode)->i_count),
file, fxn, line);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] Unionfs: use printk KERN_CONT for debugging messages
2008-01-03 6:26 ` Joe Perches
@ 2008-01-03 18:36 ` Erez Zadok
0 siblings, 0 replies; 6+ messages in thread
From: Erez Zadok @ 2008-01-03 18:36 UTC (permalink / raw)
To: Joe Perches; +Cc: Erez Zadok, akpm, linux-kernel, linux-fsdevel, viro, hch
In message <1199341581.6615.39.camel@localhost>, Joe Perches writes:
> On Thu, 2008-01-03 at 00:57 -0500, Erez Zadok wrote:
> I think printks should be single statements and
> KERN_CONT should be used as sparingly as possible.
[...]
KERN_CONT is documented as not being SMP safe, but I figured it was harmless
for just some debugging message. Still, I like your way better. Thanks
Joe.
> Perhaps:
> pr_debug("IT(%lu:%d): %s:%s:%d "
> "um=%lu/%lu lm=%lu/%lu "
> "uc=%lu/%lu lc=%lu/%lu\n",
> inode->i_ino, bindex, file, fnx, line,
> inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
> lower_inode->i_mtime.tv_sec,
> lower_inode->i_mtime.tv_nsec
> inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
> lower_inode->i_ctime.tv_sec,
> lower_inode->i_ctime.tv_nsec);
[...]
Erez.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-01-03 18:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-01-03 5:57 [GIT PULL -mm] 0/3 Unionfs updates/fixes/cleanups Erez Zadok
2008-01-03 5:57 ` [PATCH 1/3] Unionfs: use printk KERN_CONT for debugging messages Erez Zadok
2008-01-03 6:26 ` Joe Perches
2008-01-03 18:36 ` Erez Zadok
2008-01-03 5:57 ` [PATCH 2/3] Unionfs: locking fixes Erez Zadok
2008-01-03 5:57 ` [PATCH 3/3] Unionfs: use VFS helpers to manipulate i_nlink Erez Zadok
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox