2007-09-26 03:27:35

by Erez Zadok

Subject: [GIT PULL -mm] 00/25 Unionfs updates/cleanups/fixes

The following is a series of patches related to Unionfs. Aside from a few
minor cleanups/fixes, the two main changes are (1) lower nameidata support
so we can stack on nfsv4, and (2) un/likely optimizations. These patches
were tested (where appropriate) on our 2.6.23-rc8 latest code, as well as
the backports to 2.6.{22,21,20,19,18,9} on ext2/3/4, xfs, reiserfs,
nfs2/3/4, jffs2, ramfs, tmpfs, cramfs, and squashfs (where available). See
http://unionfs.filesystems.org/ to download backported 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 (22):
Unionfs: display informational messages only if debug is on
Unionfs: cast page->index loff_t before shifting
Unionfs: minor coding style updates
Unionfs: add lower nameidata debugging support
Unionfs: lower nameidata support for nfsv4
Unionfs: add un/likely conditionals on common fileops
Unionfs: add un/likely conditionals on copyup ops
Unionfs: add un/likely conditionals on debug ops
Unionfs: add un/likely conditionals on dentry ops
Unionfs: add un/likely conditionals on dir ops
Unionfs: add un/likely conditionals on headers
Unionfs: add un/likely conditionals on fileops
Unionfs: add un/likely conditionals on inode ops
Unionfs: add un/likely conditionals on lookup ops
Unionfs: add un/likely conditionals on super ops
Unionfs: add un/likely conditionals on mmap ops
Unionfs: add un/likely conditionals on rename ops
Unionfs: add un/likely conditionals on readdir ops
Unionfs: add un/likely conditionals on common subr
Unionfs: add un/likely conditionals on unlink ops
Unionfs: add un/likely conditionals on xattr ops
Unionfs: use poison.h for safe poison pointers

Josef 'Jeff' Sipek (2):
Unionfs: Simplify unionfs_get_nlinks
Unionfs: Remove unused #defines

Olivier Blin (1):
Unionfs: cache-coherency fixes

commonfops.c | 98 +++++++++++++++----------------
copyup.c | 102 ++++++++++++++++----------------
debug.c | 140 ++++++++++++++++++++++++++------------------
dentry.c | 87 +++++++++++++++------------
dirfops.c | 22 +++---
dirhelper.c | 30 ++++-----
fanout.h | 13 ++--
file.c | 38 ++++++------
inode.c | 186 +++++++++++++++++++++++++++++++----------------------------
lookup.c | 60 +++++++++++--------
main.c | 102 ++++++++++++++++----------------
mmap.c | 33 +++++-----
rdstate.c | 15 ++--
rename.c | 96 +++++++++++++++---------------
sioq.c | 4 -
subr.c | 67 ++++++---------------
super.c | 90 ++++++++++++++--------------
union.h | 19 +++---
unlink.c | 32 +++++-----
xattr.c | 12 +--
20 files changed, 647 insertions(+), 599 deletions(-)

---
Erez Zadok
[email protected]

2007-09-26 03:28:19

by Erez Zadok

Subject: [PATCH 05/25] Unionfs: cast page->index loff_t before shifting

Fixes bugs in number promotion/demotion computation, as per
<http://lkml.org/lkml/2007/9/20/17>

Signed-off-by: Erez Zadok <[email protected]>
Acked-by: Josef 'Jeff' Sipek <[email protected]>
---
fs/unionfs/mmap.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
index 88ef6a6..37af979 100644
--- a/fs/unionfs/mmap.c
+++ b/fs/unionfs/mmap.c
@@ -179,7 +179,8 @@ static int unionfs_do_readpage(struct file *file, struct page *page)
* may be a little slower, but a lot safer, as the VFS does a lot of
* the necessary magic for us.
*/
- offset = lower_file->f_pos = (page->index << PAGE_CACHE_SHIFT);
+ offset = lower_file->f_pos =
+ ((loff_t) page->index << PAGE_CACHE_SHIFT);
old_fs = get_fs();
set_fs(KERNEL_DS);
err = vfs_read(lower_file, page_data, PAGE_CACHE_SIZE,
@@ -289,7 +290,7 @@ static int unionfs_commit_write(struct file *file, struct page *page,
BUG_ON(lower_file == NULL);

page_data = (char *)kmap(page);
- lower_file->f_pos = (page->index << PAGE_CACHE_SHIFT) + from;
+ lower_file->f_pos = ((loff_t) page->index << PAGE_CACHE_SHIFT) + from;

/*
* SP: I use vfs_write instead of copying page data and the
--
1.5.2.2

2007-09-26 03:28:33

by Erez Zadok

Subject: [PATCH 24/25] Unionfs: add un/likely conditionals on xattr ops

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/xattr.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/unionfs/xattr.c b/fs/unionfs/xattr.c
index 7f77d7d..bd2de06 100644
--- a/fs/unionfs/xattr.c
+++ b/fs/unionfs/xattr.c
@@ -23,14 +23,14 @@ void *unionfs_xattr_alloc(size_t size, size_t limit)
{
void *ptr;

- if (size > limit)
+ if (unlikely(size > limit))
return ERR_PTR(-E2BIG);

if (!size) /* size request, no buffer is needed */
return NULL;

ptr = kmalloc(size, GFP_KERNEL);
- if (!ptr)
+ if (unlikely(!ptr))
return ERR_PTR(-ENOMEM);
return ptr;
}
@@ -48,7 +48,7 @@ ssize_t unionfs_getxattr(struct dentry *dentry, const char *name, void *value,
unionfs_read_lock(dentry->d_sb);
unionfs_lock_dentry(dentry);

- if (!__unionfs_d_revalidate_chain(dentry, NULL, false)) {
+ if (unlikely(!__unionfs_d_revalidate_chain(dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
@@ -77,7 +77,7 @@ int unionfs_setxattr(struct dentry *dentry, const char *name,
unionfs_read_lock(dentry->d_sb);
unionfs_lock_dentry(dentry);

- if (!__unionfs_d_revalidate_chain(dentry, NULL, false)) {
+ if (unlikely(!__unionfs_d_revalidate_chain(dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
@@ -106,7 +106,7 @@ int unionfs_removexattr(struct dentry *dentry, const char *name)
unionfs_read_lock(dentry->d_sb);
unionfs_lock_dentry(dentry);

- if (!__unionfs_d_revalidate_chain(dentry, NULL, false)) {
+ if (unlikely(!__unionfs_d_revalidate_chain(dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
@@ -135,7 +135,7 @@ ssize_t unionfs_listxattr(struct dentry *dentry, char *list, size_t size)
unionfs_read_lock(dentry->d_sb);
unionfs_lock_dentry(dentry);

- if (!__unionfs_d_revalidate_chain(dentry, NULL, false)) {
+ if (unlikely(!__unionfs_d_revalidate_chain(dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
--
1.5.2.2

2007-09-26 03:28:50

by Erez Zadok

Subject: [PATCH 14/25] Unionfs: add un/likely conditionals on headers

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/fanout.h | 13 ++++++++-----
fs/unionfs/union.h | 4 ++--
2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/fs/unionfs/fanout.h b/fs/unionfs/fanout.h
index 51aa0de..6405399 100644
--- a/fs/unionfs/fanout.h
+++ b/fs/unionfs/fanout.h
@@ -308,17 +308,20 @@ static inline void unionfs_copy_attr_times(struct inode *upper)
int bindex;
struct inode *lower;

- if (!upper || ibstart(upper) < 0)
+ if (unlikely(!upper || ibstart(upper) < 0))
return;
for (bindex=ibstart(upper); bindex <= ibend(upper); bindex++) {
lower = unionfs_lower_inode_idx(upper, bindex);
- if (!lower)
+ if (unlikely(!lower))
continue; /* not all lower dir objects may exist */
- if (timespec_compare(&upper->i_mtime, &lower->i_mtime) < 0)
+ if (unlikely(timespec_compare(&upper->i_mtime,
+ &lower->i_mtime) < 0))
upper->i_mtime = lower->i_mtime;
- if (timespec_compare(&upper->i_ctime, &lower->i_ctime) < 0)
+ if (likely(timespec_compare(&upper->i_ctime,
+ &lower->i_ctime) < 0))
upper->i_ctime = lower->i_ctime;
- if (timespec_compare(&upper->i_atime, &lower->i_atime) < 0)
+ if (likely(timespec_compare(&upper->i_atime,
+ &lower->i_atime) < 0))
upper->i_atime = lower->i_atime;
}
}
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index d27844d..8df44a9 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -472,7 +472,7 @@ static inline struct vfsmount *unionfs_mntget(struct dentry *dentry,

mnt = mntget(unionfs_lower_mnt_idx(dentry, bindex));
#ifdef CONFIG_UNION_FS_DEBUG
- if (!mnt)
+ if (unlikely(!mnt))
printk(KERN_DEBUG "unionfs_mntget: mnt=%p bindex=%d\n",
mnt, bindex);
#endif /* CONFIG_UNION_FS_DEBUG */
@@ -484,7 +484,7 @@ static inline void unionfs_mntput(struct dentry *dentry, int bindex)
{
struct vfsmount *mnt;

- if (!dentry && bindex < 0)
+ if (unlikely(!dentry && bindex < 0))
return;
BUG_ON(!dentry || bindex < 0);

--
1.5.2.2

2007-09-26 03:29:18

by Erez Zadok

Subject: [PATCH 03/25] Unionfs: display informational messages only if debug is on

This is to avoid filling the console/logs with messages that are primarily
of debugging use.

Signed-off-by: Erez Zadok <[email protected]>
Acked-by: Josef 'Jeff' Sipek <[email protected]>
---
fs/unionfs/commonfops.c | 4 ++--
fs/unionfs/dentry.c | 6 +++---
fs/unionfs/union.h | 4 ++++
3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 87cbb09..e69ccf6 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -394,8 +394,8 @@ int unionfs_file_revalidate(struct file *file, bool willwrite)
if (willwrite && IS_WRITE_FLAG(file->f_flags) &&
!IS_WRITE_FLAG(unionfs_lower_file(file)->f_flags) &&
is_robranch(dentry)) {
- printk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n",
- dentry->d_name.name);
+ dprintk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n",
+ dentry->d_name.name);
err = do_delayed_copyup(file);
}

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 9e0742d..08b5722 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -46,9 +46,9 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry,

/* if the dentry is unhashed, do NOT revalidate */
if (d_deleted(dentry)) {
- printk(KERN_DEBUG "unionfs: unhashed dentry being "
- "revalidated: %*s\n",
- dentry->d_name.len, dentry->d_name.name);
+ dprintk(KERN_DEBUG "unionfs: unhashed dentry being "
+ "revalidated: %*s\n",
+ dentry->d_name.len, dentry->d_name.name);
goto out;
}

diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 140b8ae..5e9843b 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -507,6 +507,8 @@ static inline void unionfs_mntput(struct dentry *dentry, int bindex)

#ifdef CONFIG_UNION_FS_DEBUG

+#define dprintk(args...) printk(args)
+
/* useful for tracking code reachability */
#define UDBG printk("DBG:%s:%s:%d\n",__FILE__,__FUNCTION__,__LINE__)

@@ -543,6 +545,8 @@ extern void __show_inode_counts(const struct inode *inode,

#else /* not CONFIG_UNION_FS_DEBUG */

+#define dprintk(args...) do { } while (0)
+
/* we leave useful hooks for these check functions throughout the code */
#define unionfs_check_inode(i) do { } while(0)
#define unionfs_check_dentry(d) do { } while(0)
--
1.5.2.2

2007-09-26 03:29:34

by Erez Zadok

Subject: [PATCH 21/25] Unionfs: add un/likely conditionals on readdir ops

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/rdstate.c | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/unionfs/rdstate.c b/fs/unionfs/rdstate.c
index 0a18d5c..7ec7f95 100644
--- a/fs/unionfs/rdstate.c
+++ b/fs/unionfs/rdstate.c
@@ -45,7 +45,7 @@ int unionfs_init_filldir_cache(void)

void unionfs_destroy_filldir_cache(void)
{
- if (unionfs_filldir_cachep)
+ if (likely(unionfs_filldir_cachep))
kmem_cache_destroy(unionfs_filldir_cachep);
}

@@ -72,7 +72,8 @@ static int guesstimate_hash_size(struct inode *inode)
return UNIONFS_I(inode)->hashsize;

for (bindex = ibstart(inode); bindex <= ibend(inode); bindex++) {
- if (!(lower_inode = unionfs_lower_inode_idx(inode, bindex)))
+ lower_inode = unionfs_lower_inode_idx(inode, bindex);
+ if (unlikely(!lower_inode))
continue;

if (lower_inode->i_size == DENTPAGE)
@@ -136,7 +137,7 @@ struct unionfs_dir_state *alloc_rdstate(struct inode *inode, int bindex)
sizeof(struct list_head);

rdstate = kmalloc(mallocsize, GFP_KERNEL);
- if (!rdstate)
+ if (unlikely(!rdstate))
return NULL;

spin_lock(&UNIONFS_I(inode)->rdlock);
@@ -217,7 +218,7 @@ struct filldir_node *find_filldir_node(struct unionfs_dir_state *rdstate,
* if the duplicate is in this branch, then the file
* system is corrupted.
*/
- if (cursor->bindex == rdstate->bindex) {
+ if (unlikely(cursor->bindex == rdstate->bindex)) {
printk(KERN_DEBUG "unionfs: filldir: possible "
"I/O error: a file is duplicated "
"in the same branch %d: %s\n",
@@ -227,7 +228,7 @@ struct filldir_node *find_filldir_node(struct unionfs_dir_state *rdstate,
}
}

- if (!found)
+ if (unlikely(!found))
cursor = NULL;

return cursor;
@@ -249,7 +250,7 @@ int add_filldir_node(struct unionfs_dir_state *rdstate, const char *name,
head = &(rdstate->list[index]);

new = kmem_cache_alloc(unionfs_filldir_cachep, GFP_KERNEL);
- if (!new) {
+ if (unlikely(!new)) {
err = -ENOMEM;
goto out;
}
@@ -264,7 +265,7 @@ int add_filldir_node(struct unionfs_dir_state *rdstate, const char *name,
new->name = new->iname;
else {
new->name = kmalloc(namelen + 1, GFP_KERNEL);
- if (!new->name) {
+ if (unlikely(!new->name)) {
kmem_cache_free(unionfs_filldir_cachep, new);
new = NULL;
goto out;
--
1.5.2.2

2007-09-26 03:29:51

by Erez Zadok

Subject: [PATCH 19/25] Unionfs: add un/likely conditionals on mmap ops

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/mmap.c | 28 ++++++++++++++--------------
1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
index 37af979..1cea075 100644
--- a/fs/unionfs/mmap.c
+++ b/fs/unionfs/mmap.c
@@ -84,7 +84,7 @@ static int unionfs_writepage(struct page *page, struct writeback_control *wbc)
* resort to RAIF's page pointer flipping trick.)
*/
lower_page = find_lock_page(lower_inode->i_mapping, page->index);
- if (!lower_page) {
+ if (unlikely(!lower_page)) {
err = AOP_WRITEPAGE_ACTIVATE;
set_page_dirty(page);
goto out;
@@ -102,7 +102,7 @@ static int unionfs_writepage(struct page *page, struct writeback_control *wbc)
BUG_ON(!lower_inode->i_mapping->a_ops->writepage);

/* workaround for some lower file systems: see big comment on top */
- if (wbc->for_writepages && !wbc->fs_private)
+ if (unlikely(wbc->for_writepages && !wbc->fs_private))
wbc->for_writepages = 0;

/* call lower writepage (expects locked page) */
@@ -111,12 +111,12 @@ static int unionfs_writepage(struct page *page, struct writeback_control *wbc)
wbc->for_writepages = saved_for_writepages; /* restore value */

/* b/c find_lock_page locked it and ->writepage unlocks on success */
- if (err)
+ if (unlikely(err))
unlock_page(lower_page);
/* b/c grab_cache_page increased refcnt */
page_cache_release(lower_page);

- if (err < 0) {
+ if (unlikely(err < 0)) {
ClearPageUptodate(page);
goto out;
}
@@ -160,7 +160,7 @@ static int unionfs_do_readpage(struct file *file, struct page *page)
char *page_data = NULL;
loff_t offset;

- if (!UNIONFS_F(file)) {
+ if (unlikely(!UNIONFS_F(file))) {
err = -ENOENT;
goto out;
}
@@ -189,7 +189,7 @@ static int unionfs_do_readpage(struct file *file, struct page *page)

kunmap(page);

- if (err < 0)
+ if (unlikely(err < 0))
goto out;
err = 0;

@@ -199,7 +199,7 @@ static int unionfs_do_readpage(struct file *file, struct page *page)
flush_dcache_page(page);

out:
- if (err == 0)
+ if (likely(err == 0))
SetPageUptodate(page);
else
ClearPageUptodate(page);
@@ -212,13 +212,13 @@ static int unionfs_readpage(struct file *file, struct page *page)
int err;

unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, false)))
+ if (unlikely((err = unionfs_file_revalidate(file, false))))
goto out;
unionfs_check_file(file);

err = unionfs_do_readpage(file, page);

- if (!err) {
+ if (likely(!err)) {
touch_atime(unionfs_lower_mnt(file->f_path.dentry),
unionfs_lower_dentry(file->f_path.dentry));
unionfs_copy_attr_times(file->f_path.dentry->d_inode);
@@ -276,14 +276,14 @@ static int unionfs_commit_write(struct file *file, struct page *page,
BUG_ON(file == NULL);

unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, true)))
+ if (unlikely((err = unionfs_file_revalidate(file, true))))
goto out;
unionfs_check_file(file);

inode = page->mapping->host;
lower_inode = unionfs_lower_inode(inode);

- if (UNIONFS_F(file) != NULL)
+ if (likely(UNIONFS_F(file) != NULL))
lower_file = unionfs_lower_file(file);

/* FIXME: is this assertion right here? */
@@ -307,7 +307,7 @@ static int unionfs_commit_write(struct file *file, struct page *page,

kunmap(page);

- if (err < 0)
+ if (unlikely(err < 0))
goto out;

inode->i_blocks = lower_inode->i_blocks;
@@ -320,7 +320,7 @@ static int unionfs_commit_write(struct file *file, struct page *page,
mark_inode_dirty_sync(inode);

out:
- if (err < 0)
+ if (unlikely(err < 0))
ClearPageUptodate(page);

unionfs_read_unlock(file->f_path.dentry->d_sb);
@@ -347,7 +347,7 @@ static void unionfs_sync_page(struct page *page)
* do is ensure that pending I/O gets done.
*/
lower_page = find_lock_page(lower_inode->i_mapping, page->index);
- if (!lower_page) {
+ if (unlikely(!lower_page)) {
printk(KERN_DEBUG "unionfs: find_lock_page failed\n");
goto out;
}
--
1.5.2.2

2007-09-26 03:30:17

by Erez Zadok

Subject: [PATCH 08/25] Unionfs: lower nameidata support for nfsv4

Pass nameidata structures as needed to the lower file system, support
LOOKUP_ACCESS/OPEN intents. This makes unionfs work on top of nfsv4.

Signed-off-by: Erez Zadok <[email protected]>
Acked-by: Josef 'Jeff' Sipek <[email protected]>
---
fs/unionfs/dentry.c | 11 +++++++++--
fs/unionfs/inode.c | 8 +++++++-
fs/unionfs/lookup.c | 20 +++++++++++++++++---
3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index b21f1e3..52bcb18 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -156,8 +156,15 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry,
if (!lower_dentry || !lower_dentry->d_op
|| !lower_dentry->d_op->d_revalidate)
continue;
- if (!lower_dentry->d_op->d_revalidate(lower_dentry,
- &lowernd))
+ /*
+ * Don't pass nameidata to lower file system, because we
+ * don't want an arbitrary lower file being opened or
+ * returned to us: it may be useless to us because of the
+ * fanout nature of unionfs (cf. file/directory open-file
+ * invariants). We will open lower files as and when needed
+ * later on.
+ */
+ if (!lower_dentry->d_op->d_revalidate(lower_dentry, NULL))
valid = false;
}

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index f8b2c88..7ee4760 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -27,6 +27,7 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
struct dentry *lower_parent_dentry = NULL;
char *name = NULL;
int valid = 0;
+ struct nameidata lower_nd;

unionfs_read_lock(dentry->d_sb);
unionfs_lock_dentry(dentry);
@@ -113,7 +114,12 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
goto out;
}

- err = vfs_create(lower_parent_dentry->d_inode, lower_dentry, mode, nd);
+ err = init_lower_nd(&lower_nd, LOOKUP_CREATE);
+ if (err < 0)
+ goto out;
+ err = vfs_create(lower_parent_dentry->d_inode, lower_dentry, mode,
+ &lower_nd);
+ release_lower_nd(&lower_nd, err);

if (!err) {
err = PTR_ERR(unionfs_interpose(dentry, parent->i_sb, 0));
diff --git a/fs/unionfs/lookup.c b/fs/unionfs/lookup.c
index 963d622..2109714 100644
--- a/fs/unionfs/lookup.c
+++ b/fs/unionfs/lookup.c
@@ -583,6 +583,11 @@ void update_bstart(struct dentry *dentry)
* Inside that nd structure, this function may also return an allocated
* struct file (for open intents). The caller, when done with this nd, must
* kfree the intent file (using release_lower_nd).
+ *
+ * XXX: this code, and the callers of this code, should be redone using
+ * vfs_path_lookup() when (1) the nameidata structure is refactored into a
+ * separate intent-structure, and (2) open_namei() is broken into a VFS-only
+ * function and a method that other file systems can call.
*/
int init_lower_nd(struct nameidata *nd, unsigned int flags)
{
@@ -597,11 +602,16 @@ int init_lower_nd(struct nameidata *nd, unsigned int flags)
#endif /* ALLOC_LOWER_ND_FILE */

memset(nd, 0, sizeof(struct nameidata));
+ if (!flags)
+ return err;

switch (flags) {
case LOOKUP_CREATE:
- nd->flags = LOOKUP_CREATE;
- nd->intent.open.flags = FMODE_READ | FMODE_WRITE | O_CREAT;
+ nd->intent.open.flags |= O_CREAT;
+ /* fall through: shared code for create/open cases */
+ case LOOKUP_OPEN:
+ nd->flags = flags;
+ nd->intent.open.flags |= (FMODE_READ | FMODE_WRITE);
#ifdef ALLOC_LOWER_ND_FILE
file = kzalloc(sizeof(struct file), GFP_KERNEL);
if (!file) {
@@ -611,11 +621,15 @@ int init_lower_nd(struct nameidata *nd, unsigned int flags)
nd->intent.open.file = file;
#endif /* ALLOC_LOWER_ND_FILE */
break;
+ case LOOKUP_ACCESS:
+ nd->flags = flags;
+ break;
default:
/*
* We should never get here, for now.
* We can add new cases here later on.
*/
+ dprintk("unionfs: unknown nameidata flag 0x%x\n", flags);
BUG();
break;
}
@@ -627,7 +641,7 @@ void release_lower_nd(struct nameidata *nd, int err)
{
if (!nd->intent.open.file)
return;
- if (!err)
+ else if (!err)
release_open_intent(nd);
#ifdef ALLOC_LOWER_ND_FILE
kfree(nd->intent.open.file);
--
1.5.2.2

2007-09-26 03:30:45

by Erez Zadok

Subject: [PATCH 07/25] Unionfs: add lower nameidata debugging support

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/debug.c | 20 ++++++++++++++++++++
fs/unionfs/dentry.c | 4 +++-
fs/unionfs/inode.c | 8 +++++++-
fs/unionfs/union.h | 4 ++++
4 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/fs/unionfs/debug.c b/fs/unionfs/debug.c
index 2d15fb0..9546a41 100644
--- a/fs/unionfs/debug.c
+++ b/fs/unionfs/debug.c
@@ -415,6 +415,26 @@ void __unionfs_check_file(const struct file *file,
__unionfs_check_dentry(dentry,fname,fxn,line);
}

+void __unionfs_check_nd(const struct nameidata *nd,
+ const char *fname, const char *fxn, int line)
+{
+ struct file *file;
+ int printed_caller = 0;
+
+ if (!nd)
+ return;
+ if (nd->flags & LOOKUP_OPEN) {
+ file = nd->intent.open.file;
+ if (file->f_path.dentry &&
+ strcmp(file->f_dentry->d_sb->s_type->name, "unionfs")) {
+ PRINT_CALLER(fname, fxn, line);
+ printk(" CND1: lower_file of type %s\n",
+ file->f_path.dentry->d_sb->s_type->name);
+ BUG();
+ }
+ }
+}
+
/* useful to track vfsmount leaks that could cause EBUSY on unmount */
void __show_branch_counts(const struct super_block *sb,
const char *file, const char *fxn, int line)
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index d9bb199..b21f1e3 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -418,8 +418,10 @@ static int unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
unionfs_lock_dentry(dentry);
err = __unionfs_d_revalidate_chain(dentry, nd, false);
unionfs_unlock_dentry(dentry);
- if (err > 0) /* true==1: dentry is valid */
+ if (err > 0) { /* true==1: dentry is valid */
unionfs_check_dentry(dentry);
+ unionfs_check_nd(nd);
+ }

unionfs_read_unlock(dentry->d_sb);

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index de78e26..f8b2c88 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -138,8 +138,10 @@ out:
unionfs_read_unlock(dentry->d_sb);

unionfs_check_inode(parent);
- if (!err)
+ if (!err) {
unionfs_check_dentry(dentry->d_parent);
+ unionfs_check_nd(nd);
+ }
unionfs_check_dentry(dentry);
return err;
}
@@ -186,6 +188,7 @@ static struct dentry *unionfs_lookup(struct inode *parent,
unionfs_check_inode(parent);
unionfs_check_dentry(dentry);
unionfs_check_dentry(dentry->d_parent);
+ unionfs_check_nd(nd);
unionfs_read_unlock(dentry->d_sb);

return ret;
@@ -856,6 +859,7 @@ static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd)

out:
unionfs_check_dentry(dentry);
+ unionfs_check_nd(nd);
unionfs_read_unlock(dentry->d_sb);
return ERR_PTR(err);
}
@@ -872,6 +876,7 @@ static void unionfs_put_link(struct dentry *dentry, struct nameidata *nd,
unionfs_unlock_dentry(dentry);

unionfs_check_dentry(dentry);
+ unionfs_check_nd(nd);
kfree(nd_get_link(nd));
unionfs_read_unlock(dentry->d_sb);
}
@@ -1002,6 +1007,7 @@ static int unionfs_permission(struct inode *inode, int mask,

out:
unionfs_check_inode(inode);
+ unionfs_check_nd(nd);
return err;
}

diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 755bc25..d27844d 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -518,6 +518,8 @@ static inline void unionfs_mntput(struct dentry *dentry, int bindex)
__FILE__,__FUNCTION__,__LINE__)
#define unionfs_check_file(f) __unionfs_check_file((f), \
__FILE__,__FUNCTION__,__LINE__)
+#define unionfs_check_nd(n) __unionfs_check_nd((n), \
+ __FILE__,__FUNCTION__,__LINE__)
#define show_branch_counts(sb) __show_branch_counts((sb), \
__FILE__,__FUNCTION__,__LINE__)
#define show_inode_times(i) __show_inode_times((i), \
@@ -534,6 +536,8 @@ extern void __unionfs_check_dentry(const struct dentry *dentry,
int line);
extern void __unionfs_check_file(const struct file *file,
const char *fname, const char *fxn, int line);
+extern void __unionfs_check_nd(const struct nameidata *nd,
+ const char *fname, const char *fxn, int line);
extern void __show_branch_counts(const struct super_block *sb,
const char *file, const char *fxn, int line);
extern void __show_inode_times(const struct inode *inode,
--
1.5.2.2

2007-09-26 03:31:08

by Erez Zadok

Subject: [PATCH 22/25] Unionfs: add un/likely conditionals on common subr

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/sioq.c | 4 ++--
fs/unionfs/subr.c | 26 +++++++++++++-------------
2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/fs/unionfs/sioq.c b/fs/unionfs/sioq.c
index 2a8c88e..35d9fc3 100644
--- a/fs/unionfs/sioq.c
+++ b/fs/unionfs/sioq.c
@@ -28,7 +28,7 @@ int __init init_sioq(void)
int err;

superio_workqueue = create_workqueue("unionfs_siod");
- if (!IS_ERR(superio_workqueue))
+ if (unlikely(!IS_ERR(superio_workqueue)))
return 0;

err = PTR_ERR(superio_workqueue);
@@ -39,7 +39,7 @@ int __init init_sioq(void)

void stop_sioq(void)
{
- if (superio_workqueue)
+ if (likely(superio_workqueue))
destroy_workqueue(superio_workqueue);
}

diff --git a/fs/unionfs/subr.c b/fs/unionfs/subr.c
index 6b93b64..6067d65 100644
--- a/fs/unionfs/subr.c
+++ b/fs/unionfs/subr.c
@@ -40,7 +40,7 @@ int create_whiteout(struct dentry *dentry, int start)

/* create dentry's whiteout equivalent */
name = alloc_whname(dentry->d_name.name, dentry->d_name.len);
- if (IS_ERR(name)) {
+ if (unlikely(IS_ERR(name))) {
err = PTR_ERR(name);
goto out;
}
@@ -60,7 +60,7 @@ int create_whiteout(struct dentry *dentry, int start)
dentry,
dentry->d_name.name,
bindex);
- if (!lower_dentry || IS_ERR(lower_dentry)) {
+ if (unlikely(!lower_dentry || IS_ERR(lower_dentry))) {
printk(KERN_DEBUG "unionfs: create_parents "
"failed for bindex = %d\n", bindex);
continue;
@@ -70,7 +70,7 @@ int create_whiteout(struct dentry *dentry, int start)
lower_wh_dentry =
lookup_one_len(name, lower_dentry->d_parent,
dentry->d_name.len + UNIONFS_WHLEN);
- if (IS_ERR(lower_wh_dentry))
+ if (unlikely(IS_ERR(lower_wh_dentry)))
continue;

/*
@@ -84,7 +84,7 @@ int create_whiteout(struct dentry *dentry, int start)
}

err = init_lower_nd(&nd, LOOKUP_CREATE);
- if (err < 0)
+ if (unlikely(err < 0))
goto out;
lower_dir_dentry = lock_parent(lower_wh_dentry);
if (!(err = is_robranch_super(dentry->d_sb, bindex)))
@@ -96,12 +96,12 @@ int create_whiteout(struct dentry *dentry, int start)
dput(lower_wh_dentry);
release_lower_nd(&nd, err);

- if (!err || !IS_COPYUP_ERR(err))
+ if (unlikely(!err || !IS_COPYUP_ERR(err)))
break;
}

/* set dbopaque so that lookup will not proceed after this branch */
- if (!err)
+ if (likely(!err))
set_dbopaque(dentry, bindex);

out:
@@ -129,7 +129,7 @@ int unionfs_refresh_lower_dentry(struct dentry *dentry, int bindex)

lower_dentry = lookup_one_len(dentry->d_name.name, lower_parent,
dentry->d_name.len);
- if (IS_ERR(lower_dentry)) {
+ if (unlikely(IS_ERR(lower_dentry))) {
err = PTR_ERR(lower_dentry);
goto out;
}
@@ -138,7 +138,7 @@ int unionfs_refresh_lower_dentry(struct dentry *dentry, int bindex)
iput(unionfs_lower_inode_idx(dentry->d_inode, bindex));
unionfs_set_lower_inode_idx(dentry->d_inode, bindex, NULL);

- if (!lower_dentry->d_inode) {
+ if (unlikely(!lower_dentry->d_inode)) {
dput(lower_dentry);
unionfs_set_lower_dentry_idx(dentry, bindex, NULL);
} else {
@@ -166,17 +166,17 @@ int make_dir_opaque(struct dentry *dentry, int bindex)
mutex_lock(&lower_dir->i_mutex);
diropq = lookup_one_len(UNIONFS_DIR_OPAQUE, lower_dentry,
sizeof(UNIONFS_DIR_OPAQUE) - 1);
- if (IS_ERR(diropq)) {
+ if (unlikely(IS_ERR(diropq))) {
err = PTR_ERR(diropq);
goto out;
}

err = init_lower_nd(&nd, LOOKUP_CREATE);
- if (err < 0)
+ if (unlikely(err < 0))
goto out;
if (!diropq->d_inode)
err = vfs_create(lower_dir, diropq, S_IRUGO, &nd);
- if (!err)
+ if (likely(!err))
set_dbopaque(dentry, bindex);
release_lower_nd(&nd, err);

@@ -193,7 +193,7 @@ out:
int unionfs_get_nlinks(const struct inode *inode)
{
/* don't bother to do all the work since we're unlinked */
- if (inode->i_nlink == 0)
+ if (unlikely(inode->i_nlink == 0))
return 0;

if (!S_ISDIR(inode->i_mode))
@@ -213,7 +213,7 @@ char *alloc_whname(const char *name, int len)
char *buf;

buf = kmalloc(len + UNIONFS_WHLEN + 1, GFP_KERNEL);
- if (!buf)
+ if (unlikely(!buf))
return ERR_PTR(-ENOMEM);

strcpy(buf, UNIONFS_WHPFX);
--
1.5.2.2

2007-09-26 03:31:30

by Erez Zadok

Subject: [PATCH 06/25] Unionfs: minor coding style updates

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/debug.c | 6 ++++--
fs/unionfs/dentry.c | 2 +-
fs/unionfs/inode.c | 14 ++++++++------
fs/unionfs/main.c | 4 ++--
fs/unionfs/union.h | 2 +-
5 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/fs/unionfs/debug.c b/fs/unionfs/debug.c
index f678534..2d15fb0 100644
--- a/fs/unionfs/debug.c
+++ b/fs/unionfs/debug.c
@@ -467,7 +467,8 @@ void __show_dinode_times(const struct dentry *dentry,
lower_inode = unionfs_lower_inode_idx(inode, bindex);
if (!lower_inode)
continue;
- printk("DT(%s:%lu:%d): ", dentry->d_name.name, inode->i_ino, bindex);
+ printk("DT(%s:%lu:%d): ", dentry->d_name.name, inode->i_ino,
+ bindex);
printk("%s:%s:%d ",file,fxn,line);
printk("um=%lu/%lu lm=%lu/%lu ",
inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
@@ -490,7 +491,8 @@ void __show_inode_counts(const struct inode *inode,
printk("SiC: Null inode\n");
return;
}
- for (bindex=sbstart(inode->i_sb); bindex <= sbend(inode->i_sb); bindex++) {
+ for (bindex=sbstart(inode->i_sb); bindex <= sbend(inode->i_sb);
+ bindex++) {
lower_inode = unionfs_lower_inode_idx(inode, bindex);
if (!lower_inode)
continue;
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 08b5722..d9bb199 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -26,7 +26,7 @@
* Returns true if valid, false otherwise.
*/
static bool __unionfs_d_revalidate_one(struct dentry *dentry,
- struct nameidata *nd)
+ struct nameidata *nd)
{
bool valid = true; /* default is valid */
struct dentry *lower_dentry;
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 9638b64..de78e26 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -99,7 +99,8 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
* if lower_dentry is NULL, create the entire
* dentry directory structure in branch 0.
*/
- lower_dentry = create_parents(parent, dentry, dentry->d_name.name, 0);
+ lower_dentry = create_parents(parent, dentry,
+ dentry->d_name.name, 0);
if (IS_ERR(lower_dentry)) {
err = PTR_ERR(lower_dentry);
goto out;
@@ -447,9 +448,8 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry,

if (!(err = is_robranch_super(dentry->d_sb, bindex))) {
mode = S_IALLUGO;
- err =
- vfs_symlink(lower_dir_dentry->d_inode,
- lower_dentry, symname, mode);
+ err = vfs_symlink(lower_dir_dentry->d_inode,
+ lower_dentry, symname, mode);
}
unlock_dir(lower_dir_dentry);

@@ -884,9 +884,11 @@ static void unionfs_put_link(struct dentry *dentry, struct nameidata *nd,
* readonly, to allow copyup to work.
* (3) we do call security_inode_permission, and therefore security inside
* SELinux, etc. are performed.
+ *
+ * @inode: the lower inode we're checking permission on
*/
-static int inode_permission(struct super_block *sb, struct inode *inode, int mask,
- struct nameidata *nd, int bindex)
+static int inode_permission(struct super_block *sb, struct inode *inode,
+ int mask, struct nameidata *nd, int bindex)
{
int retval, submask;

diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index 4faae44..8595750 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -275,14 +275,14 @@ int __parse_branch_mode(const char *name)
*/
int parse_branch_mode(const char *name)
{
- int perms = __parse_branch_mode(name);
+ int perms = __parse_branch_mode(name);

if (perms == 0)
perms = MAY_READ | MAY_WRITE;
return perms;
}

-/*
+/*
* parse the dirs= mount argument
*
* We don't need to lock the superblock private data's rwsem, as we get
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 5e9843b..755bc25 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -549,7 +549,7 @@ extern void __show_inode_counts(const struct inode *inode,

/* we leave useful hooks for these check functions throughout the code */
#define unionfs_check_inode(i) do { } while(0)
-#define unionfs_check_dentry(d) do { } while(0)
+#define unionfs_check_dentry(d) do { } while(0)
#define unionfs_check_file(f) do { } while(0)
#define show_branch_counts(sb) do { } while(0)
#define show_inode_times(i) do { } while(0)
--
1.5.2.2

2007-09-26 03:31:47

by Erez Zadok

Subject: [PATCH 23/25] Unionfs: add un/likely conditionals on unlink ops

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/unlink.c | 32 ++++++++++++++++----------------
1 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/fs/unionfs/unlink.c b/fs/unionfs/unlink.c
index 3924f7f..33d08d9 100644
--- a/fs/unionfs/unlink.c
+++ b/fs/unionfs/unlink.c
@@ -26,13 +26,13 @@ static int unionfs_unlink_whiteout(struct inode *dir, struct dentry *dentry)
int bindex;
int err = 0;

- if ((err = unionfs_partial_lookup(dentry)))
+ if (unlikely((err = unionfs_partial_lookup(dentry))))
goto out;

bindex = dbstart(dentry);

lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
- if (!lower_dentry)
+ if (unlikely(!lower_dentry))
goto out;

lower_dir_dentry = lock_parent(lower_dentry);
@@ -42,13 +42,13 @@ static int unionfs_unlink_whiteout(struct inode *dir, struct dentry *dentry)
if (!(err = is_robranch_super(dentry->d_sb, bindex)))
err = vfs_unlink(lower_dir_dentry->d_inode, lower_dentry);
/* if vfs_unlink succeeded, update our inode's times */
- if (!err)
+ if (likely(!err))
unionfs_copy_attr_times(dentry->d_inode);
dput(lower_dentry);
fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
unlock_dir(lower_dir_dentry);

- if (err && !IS_COPYUP_ERR(err))
+ if (unlikely(err && !IS_COPYUP_ERR(err)))
goto out;

if (err) {
@@ -62,11 +62,11 @@ static int unionfs_unlink_whiteout(struct inode *dir, struct dentry *dentry)
err = create_whiteout(dentry, dbstart(dentry));

out:
- if (!err)
+ if (likely(!err))
dentry->d_inode->i_nlink--;

/* We don't want to leave negative leftover dentries for revalidate. */
- if (!err && (dbopaque(dentry) != -1))
+ if (likely(!err && (dbopaque(dentry) != -1)))
update_bstart(dentry);

return err;
@@ -79,7 +79,7 @@ int unionfs_unlink(struct inode *dir, struct dentry *dentry)
unionfs_read_lock(dentry->d_sb);
unionfs_lock_dentry(dentry);

- if (!__unionfs_d_revalidate_chain(dentry, NULL, false)) {
+ if (unlikely(!__unionfs_d_revalidate_chain(dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
@@ -87,7 +87,7 @@ int unionfs_unlink(struct inode *dir, struct dentry *dentry)

err = unionfs_unlink_whiteout(dir, dentry);
/* call d_drop so the system "forgets" about us */
- if (!err) {
+ if (likely(!err)) {
if (!S_ISDIR(dentry->d_inode->i_mode))
unionfs_postcopyup_release(dentry);
d_drop(dentry);
@@ -99,7 +99,7 @@ int unionfs_unlink(struct inode *dir, struct dentry *dentry)
}

out:
- if (!err) {
+ if (likely(!err)) {
unionfs_check_dentry(dentry);
unionfs_check_inode(dir);
}
@@ -117,7 +117,7 @@ static int unionfs_rmdir_first(struct inode *dir, struct dentry *dentry,

/* Here we need to remove whiteout entries. */
err = delete_whiteouts(dentry, dbstart(dentry), namelist);
- if (err)
+ if (unlikely(err))
goto out;

lower_dentry = unionfs_lower_dentry(dentry);
@@ -135,7 +135,7 @@ static int unionfs_rmdir_first(struct inode *dir, struct dentry *dentry,
dentry->d_inode->i_nlink = unionfs_get_nlinks(dentry->d_inode);

out:
- if (lower_dir_dentry)
+ if (likely(lower_dir_dentry))
unlock_dir(lower_dir_dentry);
return err;
}
@@ -148,7 +148,7 @@ int unionfs_rmdir(struct inode *dir, struct dentry *dentry)
unionfs_read_lock(dentry->d_sb);
unionfs_lock_dentry(dentry);

- if (!__unionfs_d_revalidate_chain(dentry, NULL, false)) {
+ if (unlikely(!__unionfs_d_revalidate_chain(dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
@@ -156,7 +156,7 @@ int unionfs_rmdir(struct inode *dir, struct dentry *dentry)

/* check if this unionfs directory is empty or not */
err = check_empty(dentry, &namelist);
- if (err)
+ if (unlikely(err))
goto out;

err = unionfs_rmdir_first(dir, dentry, namelist);
@@ -170,7 +170,7 @@ int unionfs_rmdir(struct inode *dir, struct dentry *dentry)
goto out;

/* exit if the error returned was NOT -EROFS */
- if (!IS_COPYUP_ERR(err))
+ if (unlikely(!IS_COPYUP_ERR(err)))
goto out;

new_err = create_whiteout(dentry, dbstart(dentry) - 1);
@@ -180,10 +180,10 @@ int unionfs_rmdir(struct inode *dir, struct dentry *dentry)

out:
/* call d_drop so the system "forgets" about us */
- if (!err)
+ if (likely(!err))
d_drop(dentry);

- if (namelist)
+ if (likely(namelist))
free_rdstate(namelist);

unionfs_unlock_dentry(dentry);
--
1.5.2.2

2007-09-26 03:32:14

by Erez Zadok

Subject: [PATCH 15/25] Unionfs: add un/likely conditionals on fileops

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/file.c | 38 +++++++++++++++++++-------------------
1 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/fs/unionfs/file.c b/fs/unionfs/file.c
index d8eaaa5..06ca1fa 100644
--- a/fs/unionfs/file.c
+++ b/fs/unionfs/file.c
@@ -24,13 +24,13 @@ static ssize_t unionfs_read(struct file *file, char __user *buf,
int err;

unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, false)))
+ if (unlikely((err = unionfs_file_revalidate(file, false))))
goto out;
unionfs_check_file(file);

err = do_sync_read(file, buf, count, ppos);

- if (err >= 0)
+ if (likely(err >= 0))
touch_atime(unionfs_lower_mnt(file->f_path.dentry),
unionfs_lower_dentry(file->f_path.dentry));

@@ -47,16 +47,16 @@ static ssize_t unionfs_aio_read(struct kiocb *iocb, const struct iovec *iov,
struct file *file = iocb->ki_filp;

unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, false)))
+ if (unlikely((err = unionfs_file_revalidate(file, false))))
goto out;
unionfs_check_file(file);

err = generic_file_aio_read(iocb, iov, nr_segs, pos);

- if (err == -EIOCBQUEUED)
+ if (unlikely(err == -EIOCBQUEUED))
err = wait_on_sync_kiocb(iocb);

- if (err >= 0)
+ if (likely(err >= 0))
touch_atime(unionfs_lower_mnt(file->f_path.dentry),
unionfs_lower_dentry(file->f_path.dentry));

@@ -72,13 +72,13 @@ static ssize_t unionfs_write(struct file *file, const char __user *buf,
int err = 0;

unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, true)))
+ if (unlikely((err = unionfs_file_revalidate(file, true))))
goto out;
unionfs_check_file(file);

err = do_sync_write(file, buf, count, ppos);
/* update our inode times upon a successful lower write */
- if (err >= 0) {
+ if (likely(err >= 0)) {
unionfs_copy_attr_times(file->f_path.dentry->d_inode);
unionfs_check_file(file);
}
@@ -104,7 +104,7 @@ static int unionfs_mmap(struct file *file, struct vm_area_struct *vma)

/* This might be deferred to mmap's writepage */
willwrite = ((vma->vm_flags | VM_SHARED | VM_WRITE) == vma->vm_flags);
- if ((err = unionfs_file_revalidate(file, willwrite)))
+ if (unlikely((err = unionfs_file_revalidate(file, willwrite))))
goto out;
unionfs_check_file(file);

@@ -119,19 +119,19 @@ static int unionfs_mmap(struct file *file, struct vm_area_struct *vma)
* generic_file_readonly_mmap returns in that case).
*/
lower_file = unionfs_lower_file(file);
- if (willwrite && !lower_file->f_mapping->a_ops->writepage) {
+ if (unlikely(willwrite && !lower_file->f_mapping->a_ops->writepage)) {
err = -EINVAL;
printk("unionfs: branch %d file system does not support "
"writeable mmap\n", fbstart(file));
} else {
err = generic_file_mmap(file, vma);
- if (err)
+ if (unlikely(err))
printk("unionfs: generic_file_mmap failed %d\n", err);
}

out:
unionfs_read_unlock(file->f_path.dentry->d_sb);
- if (!err) {
+ if (likely(!err)) {
/* copyup could cause parent dir times to change */
unionfs_copy_attr_times(file->f_path.dentry->d_parent->d_inode);
unionfs_check_file(file);
@@ -149,7 +149,7 @@ int unionfs_fsync(struct file *file, struct dentry *dentry, int datasync)
int err = -EINVAL;

unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, true)))
+ if (unlikely((err = unionfs_file_revalidate(file, true))))
goto out;
unionfs_check_file(file);

@@ -159,14 +159,14 @@ int unionfs_fsync(struct file *file, struct dentry *dentry, int datasync)
goto out;

inode = dentry->d_inode;
- if (!inode) {
+ if (unlikely(!inode)) {
printk(KERN_ERR
"unionfs: null lower inode in unionfs_fsync\n");
goto out;
}
for (bindex = bstart; bindex <= bend; bindex++) {
lower_inode = unionfs_lower_inode_idx(inode, bindex);
- if (!lower_inode || !lower_inode->i_fop->fsync)
+ if (unlikely(!lower_inode || !lower_inode->i_fop->fsync))
continue;
lower_file = unionfs_lower_file_idx(file, bindex);
lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
@@ -175,7 +175,7 @@ int unionfs_fsync(struct file *file, struct dentry *dentry, int datasync)
lower_dentry,
datasync);
mutex_unlock(&lower_inode->i_mutex);
- if (err)
+ if (unlikely(err))
goto out;
}

@@ -196,7 +196,7 @@ int unionfs_fasync(int fd, struct file *file, int flag)
int err = 0;

unionfs_read_lock(file->f_path.dentry->d_sb);
- if ((err = unionfs_file_revalidate(file, true)))
+ if (unlikely((err = unionfs_file_revalidate(file, true))))
goto out;
unionfs_check_file(file);

@@ -207,20 +207,20 @@ int unionfs_fasync(int fd, struct file *file, int flag)

dentry = file->f_path.dentry;
inode = dentry->d_inode;
- if (!inode) {
+ if (unlikely(!inode)) {
printk(KERN_ERR
"unionfs: null lower inode in unionfs_fasync\n");
goto out;
}
for (bindex = bstart; bindex <= bend; bindex++) {
lower_inode = unionfs_lower_inode_idx(inode, bindex);
- if (!lower_inode || !lower_inode->i_fop->fasync)
+ if (unlikely(!lower_inode || !lower_inode->i_fop->fasync))
continue;
lower_file = unionfs_lower_file_idx(file, bindex);
mutex_lock(&lower_inode->i_mutex);
err = lower_inode->i_fop->fasync(fd, lower_file, flag);
mutex_unlock(&lower_inode->i_mutex);
- if (err)
+ if (unlikely(err))
goto out;
}

--
1.5.2.2

2007-09-26 03:32:36

by Erez Zadok

Subject: [PATCH 17/25] Unionfs: add un/likely conditionals on lookup ops

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/lookup.c | 44 ++++++++++++++++++++++----------------------
1 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/fs/unionfs/lookup.c b/fs/unionfs/lookup.c
index 2109714..92b5e0a 100644
--- a/fs/unionfs/lookup.c
+++ b/fs/unionfs/lookup.c
@@ -59,7 +59,7 @@ static noinline int is_opaque_dir(struct dentry *dentry, int bindex)

mutex_unlock(&lower_inode->i_mutex);

- if (IS_ERR(wh_lower_dentry)) {
+ if (unlikely(IS_ERR(wh_lower_dentry))) {
err = PTR_ERR(wh_lower_dentry);
goto out;
}
@@ -119,12 +119,12 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,
case INTERPOSE_PARTIAL:
break;
case INTERPOSE_LOOKUP:
- if ((err = new_dentry_private_data(dentry)))
+ if (unlikely((err = new_dentry_private_data(dentry))))
goto out;
break;
default:
/* default: can only be INTERPOSE_REVAL/REVAL_NEG */
- if ((err = realloc_dentry_private_data(dentry)))
+ if (unlikely((err = realloc_dentry_private_data(dentry))))
goto out;
break;
}
@@ -147,7 +147,7 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,
namelen = dentry->d_name.len;

/* No dentries should get created for possible whiteout names. */
- if (!is_validname(name)) {
+ if (unlikely(!is_validname(name))) {
err = -EPERM;
goto out_free;
}
@@ -179,7 +179,7 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,
unionfs_lower_dentry_idx(parent_dentry, bindex);

/* if the parent lower dentry does not exist skip this */
- if (!(lower_dir_dentry && lower_dir_dentry->d_inode))
+ if (unlikely(!(lower_dir_dentry && lower_dir_dentry->d_inode)))
continue;

/* also skip it if the parent isn't a directory. */
@@ -189,7 +189,7 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,
/* Reuse the whiteout name because its value doesn't change. */
if (!whname) {
whname = alloc_whname(name, namelen);
- if (IS_ERR(whname)) {
+ if (unlikely(IS_ERR(whname))) {
err = PTR_ERR(whname);
goto out_free;
}
@@ -198,7 +198,7 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,
/* check if whiteout exists in this branch: lookup .wh.foo */
wh_lower_dentry = lookup_one_len(whname, lower_dir_dentry,
namelen + UNIONFS_WHLEN);
- if (IS_ERR(wh_lower_dentry)) {
+ if (unlikely(IS_ERR(wh_lower_dentry))) {
dput(first_lower_dentry);
unionfs_mntput(first_dentry, first_dentry_offset);
err = PTR_ERR(wh_lower_dentry);
@@ -207,7 +207,7 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,

if (wh_lower_dentry->d_inode) {
/* We found a whiteout so lets give up. */
- if (S_ISREG(wh_lower_dentry->d_inode->i_mode)) {
+ if (likely(S_ISREG(wh_lower_dentry->d_inode->i_mode))) {
set_dbend(dentry, bindex);
set_dbopaque(dentry, bindex);
dput(wh_lower_dentry);
@@ -228,7 +228,7 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,

/* Now do regular lookup; lookup foo */
lower_dentry = lookup_one_len(name, lower_dir_dentry, namelen);
- if (IS_ERR(lower_dentry)) {
+ if (unlikely(IS_ERR(lower_dentry))) {
dput(first_lower_dentry);
unionfs_mntput(first_dentry, first_dentry_offset);
err = PTR_ERR(lower_dentry);
@@ -321,7 +321,7 @@ out_negative:
first_lower_dentry = lookup_one_len(name, lower_dir_dentry,
namelen);
first_dentry_offset = bindex;
- if (IS_ERR(first_lower_dentry)) {
+ if (unlikely(IS_ERR(first_lower_dentry))) {
err = PTR_ERR(first_lower_dentry);
goto out;
}
@@ -381,12 +381,12 @@ out_positive:
* dentry.
*/
d_interposed = unionfs_interpose(dentry, dentry->d_sb, lookupmode);
- if (IS_ERR(d_interposed))
+ if (unlikely(IS_ERR(d_interposed)))
err = PTR_ERR(d_interposed);
else if (d_interposed)
dentry = d_interposed;

- if (err)
+ if (unlikely(err))
goto out_drop;

goto out;
@@ -452,7 +452,7 @@ int unionfs_partial_lookup(struct dentry *dentry)
err = 0;
goto out;
}
- if (IS_ERR(tmp)) {
+ if (unlikely(IS_ERR(tmp))) {
err = PTR_ERR(tmp);
goto out;
}
@@ -476,13 +476,13 @@ int unionfs_init_dentry_cache(void)

void unionfs_destroy_dentry_cache(void)
{
- if (unionfs_dentry_cachep)
+ if (likely(unionfs_dentry_cachep))
kmem_cache_destroy(unionfs_dentry_cachep);
}

void free_dentry_private_data(struct dentry *dentry)
{
- if (!dentry || !dentry->d_fsdata)
+ if (unlikely(!dentry || !dentry->d_fsdata))
return;
kmem_cache_free(unionfs_dentry_cachep, dentry->d_fsdata);
dentry->d_fsdata = NULL;
@@ -498,7 +498,7 @@ static inline int __realloc_dentry_private_data(struct dentry *dentry)

size = sizeof(struct path) * sbmax(dentry->d_sb);
p = krealloc(info->lower_paths, size, GFP_ATOMIC);
- if (!p)
+ if (unlikely(!p))
return -ENOMEM;

info->lower_paths = p;
@@ -518,7 +518,7 @@ static inline int __realloc_dentry_private_data(struct dentry *dentry)
/* UNIONFS_D(dentry)->lock must be locked */
static int realloc_dentry_private_data(struct dentry *dentry)
{
- if (!__realloc_dentry_private_data(dentry))
+ if (likely(!__realloc_dentry_private_data(dentry)))
return 0;

kfree(UNIONFS_D(dentry)->lower_paths);
@@ -534,7 +534,7 @@ int new_dentry_private_data(struct dentry *dentry)
BUG_ON(info);

info = kmem_cache_alloc(unionfs_dentry_cachep, GFP_ATOMIC);
- if (!info)
+ if (unlikely(!info))
return -ENOMEM;

mutex_init(&info->lock);
@@ -544,7 +544,7 @@ int new_dentry_private_data(struct dentry *dentry)

dentry->d_fsdata = info;

- if (!__realloc_dentry_private_data(dentry))
+ if (likely(!__realloc_dentry_private_data(dentry)))
return 0;

mutex_unlock(&info->lock);
@@ -602,7 +602,7 @@ int init_lower_nd(struct nameidata *nd, unsigned int flags)
#endif /* ALLOC_LOWER_ND_FILE */

memset(nd, 0, sizeof(struct nameidata));
- if (!flags)
+ if (unlikely(!flags))
return err;

switch (flags) {
@@ -614,7 +614,7 @@ int init_lower_nd(struct nameidata *nd, unsigned int flags)
nd->intent.open.flags |= (FMODE_READ | FMODE_WRITE);
#ifdef ALLOC_LOWER_ND_FILE
file = kzalloc(sizeof(struct file), GFP_KERNEL);
- if (!file) {
+ if (unlikely(!file)) {
err = -ENOMEM;
break; /* exit switch statement and thus return */
}
@@ -641,7 +641,7 @@ void release_lower_nd(struct nameidata *nd, int err)
{
if (!nd->intent.open.file)
return;
- else if (!err)
+ else if (likely(!err))
release_open_intent(nd);
#ifdef ALLOC_LOWER_ND_FILE
kfree(nd->intent.open.file);
--
1.5.2.2

2007-09-26 03:32:54

by Erez Zadok

Subject: [PATCH 13/25] Unionfs: add un/likely conditionals on dir ops

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/dirfops.c | 22 +++++++++++-----------
fs/unionfs/dirhelper.c | 30 +++++++++++++++---------------
2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/fs/unionfs/dirfops.c b/fs/unionfs/dirfops.c
index c923e58..fa2df88 100644
--- a/fs/unionfs/dirfops.c
+++ b/fs/unionfs/dirfops.c
@@ -63,7 +63,7 @@ static int unionfs_filldir(void *dirent, const char *name, int namelen,
off_t pos = rdstate2offset(buf->rdstate);
u64 unionfs_ino = ino;

- if (!err) {
+ if (likely(!err)) {
err = buf->filldir(buf->dirent, name, namelen, pos,
unionfs_ino, d_type);
buf->rdstate->offset++;
@@ -74,7 +74,7 @@ static int unionfs_filldir(void *dirent, const char *name, int namelen,
* If we did fill it, stuff it in our hash, otherwise return an
* error.
*/
- if (err) {
+ if (unlikely(err)) {
buf->filldir_error = err;
goto out;
}
@@ -99,7 +99,7 @@ static int unionfs_readdir(struct file *file, void *dirent, filldir_t filldir)

unionfs_read_lock(file->f_path.dentry->d_sb);

- if ((err = unionfs_file_revalidate(file, false)))
+ if (unlikely((err = unionfs_file_revalidate(file, false))))
goto out;

inode = file->f_path.dentry->d_inode;
@@ -110,7 +110,7 @@ static int unionfs_readdir(struct file *file, void *dirent, filldir_t filldir)
goto out;
} else if (file->f_pos > 0) {
uds = find_rdstate(inode, file->f_pos);
- if (!uds) {
+ if (unlikely(!uds)) {
err = -ESTALE;
goto out;
}
@@ -124,7 +124,7 @@ static int unionfs_readdir(struct file *file, void *dirent, filldir_t filldir)

while (uds->bindex <= bend) {
lower_file = unionfs_lower_file_idx(file, uds->bindex);
- if (!lower_file) {
+ if (unlikely(!lower_file)) {
uds->bindex++;
uds->dirpos = 0;
continue;
@@ -141,7 +141,7 @@ static int unionfs_readdir(struct file *file, void *dirent, filldir_t filldir)

/* Read starting from where we last left off. */
offset = vfs_llseek(lower_file, uds->dirpos, SEEK_SET);
- if (offset < 0) {
+ if (unlikely(offset < 0)) {
err = offset;
goto out;
}
@@ -149,7 +149,7 @@ static int unionfs_readdir(struct file *file, void *dirent, filldir_t filldir)

/* Save the position for when we continue. */
offset = vfs_llseek(lower_file, 0, SEEK_CUR);
- if (offset < 0) {
+ if (unlikely(offset < 0)) {
err = offset;
goto out;
}
@@ -158,10 +158,10 @@ static int unionfs_readdir(struct file *file, void *dirent, filldir_t filldir)
/* Copy the atime. */
fsstack_copy_attr_atime(inode, lower_file->f_path.dentry->d_inode);

- if (err < 0)
+ if (unlikely(err < 0))
goto out;

- if (buf.filldir_error)
+ if (unlikely(buf.filldir_error))
break;

if (!buf.entries_written) {
@@ -201,7 +201,7 @@ static loff_t unionfs_dir_llseek(struct file *file, loff_t offset, int origin)

unionfs_read_lock(file->f_path.dentry->d_sb);

- if ((err = unionfs_file_revalidate(file, false)))
+ if (unlikely((err = unionfs_file_revalidate(file, false))))
goto out;

rdstate = UNIONFS_F(file)->rdstate;
@@ -241,7 +241,7 @@ static loff_t unionfs_dir_llseek(struct file *file, loff_t offset, int origin)
} else {
rdstate = find_rdstate(file->f_path.dentry->d_inode,
offset);
- if (rdstate) {
+ if (likely(rdstate)) {
UNIONFS_F(file)->rdstate = rdstate;
err = rdstate->offset;
} else
diff --git a/fs/unionfs/dirhelper.c b/fs/unionfs/dirhelper.c
index a72f711..d481ba4 100644
--- a/fs/unionfs/dirhelper.c
+++ b/fs/unionfs/dirhelper.c
@@ -43,7 +43,7 @@ int do_delete_whiteouts(struct dentry *dentry, int bindex,

err = -ENOMEM;
name = __getname();
- if (!name)
+ if (unlikely(!name))
goto out;
strcpy(name, UNIONFS_WHPFX);
p = name + UNIONFS_WHLEN;
@@ -65,14 +65,14 @@ int do_delete_whiteouts(struct dentry *dentry, int bindex,
lookup_one_len(name, lower_dir_dentry,
cursor->namelen +
UNIONFS_WHLEN);
- if (IS_ERR(lower_dentry)) {
+ if (unlikely(IS_ERR(lower_dentry))) {
err = PTR_ERR(lower_dentry);
break;
}
- if (lower_dentry->d_inode)
+ if (likely(lower_dentry->d_inode))
err = vfs_unlink(lower_dir, lower_dentry);
dput(lower_dentry);
- if (err)
+ if (unlikely(err))
break;
}
}
@@ -102,7 +102,7 @@ int delete_whiteouts(struct dentry *dentry, int bindex,
BUG_ON(bindex < dbstart(dentry));
BUG_ON(bindex > dbend(dentry));
err = is_robranch_super(sb, bindex);
- if (err)
+ if (unlikely(err))
goto out;

lower_dir_dentry = unionfs_lower_dentry_idx(dentry, bindex);
@@ -160,7 +160,7 @@ static int readdir_util_callback(void *dirent, const char *name, int namelen,

found = find_filldir_node(buf->rdstate, name, namelen);
/* If it was found in the table there was a previous whiteout. */
- if (found)
+ if (likely(found))
goto out;

/*
@@ -168,7 +168,7 @@ static int readdir_util_callback(void *dirent, const char *name, int namelen,
* empty.
*/
err = -ENOTEMPTY;
- if ((buf->mode == RD_CHECK_EMPTY) && !whiteout)
+ if (unlikely((buf->mode == RD_CHECK_EMPTY) && !whiteout))
goto out;

err = add_filldir_node(buf->rdstate, name, namelen,
@@ -194,7 +194,7 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist)

BUG_ON(!S_ISDIR(dentry->d_inode->i_mode));

- if ((err = unionfs_partial_lookup(dentry)))
+ if (unlikely((err = unionfs_partial_lookup(dentry))))
goto out;

bstart = dbstart(dentry);
@@ -204,14 +204,14 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist)
bend = bopaque;

buf = kmalloc(sizeof(struct unionfs_rdutil_callback), GFP_KERNEL);
- if (!buf) {
+ if (unlikely(!buf)) {
err = -ENOMEM;
goto out;
}
buf->err = 0;
buf->mode = RD_CHECK_EMPTY;
buf->rdstate = alloc_rdstate(dentry->d_inode, bstart);
- if (!buf->rdstate) {
+ if (unlikely(!buf->rdstate)) {
err = -ENOMEM;
goto out;
}
@@ -233,7 +233,7 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist)
dentry_open(lower_dentry,
unionfs_lower_mnt_idx(dentry, bindex),
O_RDONLY);
- if (IS_ERR(lower_file)) {
+ if (unlikely(IS_ERR(lower_file))) {
err = PTR_ERR(lower_file);
dput(lower_dentry);
branchput(sb, bindex);
@@ -245,7 +245,7 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist)
buf->rdstate->bindex = bindex;
err = vfs_readdir(lower_file,
readdir_util_callback, buf);
- if (buf->err)
+ if (unlikely(buf->err))
err = buf->err;
} while ((err >= 0) && buf->filldir_called);

@@ -253,15 +253,15 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist)
fput(lower_file);
branchput(sb, bindex);

- if (err < 0)
+ if (unlikely(err < 0))
goto out;
}

out:
- if (buf) {
+ if (likely(buf)) {
if (namelist && !err)
*namelist = buf->rdstate;
- else if (buf->rdstate)
+ else if (likely(buf->rdstate))
free_rdstate(buf->rdstate);
kfree(buf);
}
--
1.5.2.2

2007-09-26 03:33:25

by Erez Zadok

Subject: [PATCH 12/25] Unionfs: add un/likely conditionals on dentry ops

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/dentry.c | 68 ++++++++++++++++++++++++++------------------------
1 files changed, 35 insertions(+), 33 deletions(-)

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 52bcb18..3f3a18d 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -45,7 +45,7 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry,
verify_locked(dentry);

/* if the dentry is unhashed, do NOT revalidate */
- if (d_deleted(dentry)) {
+ if (unlikely(d_deleted(dentry))) {
dprintk(KERN_DEBUG "unionfs: unhashed dentry being "
"revalidated: %*s\n",
dentry->d_name.len, dentry->d_name.name);
@@ -53,7 +53,7 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry,
}

BUG_ON(dbstart(dentry) == -1);
- if (dentry->d_inode)
+ if (likely(dentry->d_inode))
positive = 1;
dgen = atomic_read(&UNIONFS_D(dentry)->generation);
sbgen = atomic_read(&UNIONFS_SB(dentry->d_sb)->generation);
@@ -62,7 +62,7 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry,
* revalidation to be done, because this file does not exist within
* the namespace, and Unionfs operates on the namespace, not data.
*/
- if (sbgen != dgen) {
+ if (unlikely(sbgen != dgen)) {
struct dentry *result;
int pdgen;

@@ -76,7 +76,7 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry,
/* Free the pointers for our inodes and this dentry. */
bstart = dbstart(dentry);
bend = dbend(dentry);
- if (bstart >= 0) {
+ if (likely(bstart >= 0)) {
struct dentry *lower_dentry;
for (bindex = bstart; bindex <= bend; bindex++) {
lower_dentry =
@@ -89,7 +89,7 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry,
set_dbend(dentry, -1);

interpose_flag = INTERPOSE_REVAL_NEG;
- if (positive) {
+ if (likely(positive)) {
interpose_flag = INTERPOSE_REVAL;
/*
* During BRM, the VFS could already hold a lock on
@@ -97,14 +97,14 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry,
* (deadlock), but if you lock it in this function,
* then release it here too.
*/
- if (!mutex_is_locked(&dentry->d_inode->i_mutex)) {
+ if (unlikely(!mutex_is_locked(&dentry->d_inode->i_mutex))) {
mutex_lock(&dentry->d_inode->i_mutex);
locked = 1;
}

bstart = ibstart(dentry->d_inode);
bend = ibend(dentry->d_inode);
- if (bstart >= 0) {
+ if (likely(bstart >= 0)) {
struct inode *lower_inode;
for (bindex = bstart; bindex <= bend;
bindex++) {
@@ -119,14 +119,14 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry,
UNIONFS_I(dentry->d_inode)->lower_inodes = NULL;
ibstart(dentry->d_inode) = -1;
ibend(dentry->d_inode) = -1;
- if (locked)
+ if (unlikely(locked))
mutex_unlock(&dentry->d_inode->i_mutex);
}

result = unionfs_lookup_backend(dentry, &lowernd,
interpose_flag);
- if (result) {
- if (IS_ERR(result)) {
+ if (likely(result)) {
+ if (unlikely(IS_ERR(result))) {
valid = false;
goto out;
}
@@ -138,7 +138,7 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry,
dentry = result;
}

- if (positive && UNIONFS_I(dentry->d_inode)->stale) {
+ if (unlikely(positive && UNIONFS_I(dentry->d_inode)->stale)) {
make_bad_inode(dentry->d_inode);
d_drop(dentry);
valid = false;
@@ -153,8 +153,8 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry,
BUG_ON(bstart == -1);
for (bindex = bstart; bindex <= bend; bindex++) {
lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
- if (!lower_dentry || !lower_dentry->d_op
- || !lower_dentry->d_op->d_revalidate)
+ if (unlikely(!lower_dentry || !lower_dentry->d_op
+ || !lower_dentry->d_op->d_revalidate))
continue;
/*
* Don't pass nameidata to lower file system, because we
@@ -164,14 +164,15 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry,
* invariants). We will open lower files as and when needed
* later on.
*/
- if (!lower_dentry->d_op->d_revalidate(lower_dentry, NULL))
+ if (unlikely(!lower_dentry->d_op->d_revalidate(lower_dentry,
+ NULL)))
valid = false;
}

- if (!dentry->d_inode)
+ if (unlikely(!dentry->d_inode))
valid = false;

- if (valid) {
+ if (likely(valid)) {
/*
* If we get here, and we copy the meta-data from the lower
* inode to our inode, then it is vital that we have already
@@ -200,32 +201,32 @@ bool is_newer_lower(const struct dentry *dentry)
struct inode *lower_inode;

/* ignore if we're called on semi-initialized dentries/inodes */
- if (!dentry || !UNIONFS_D(dentry))
+ if (likely(!dentry || !UNIONFS_D(dentry)))
return false;
inode = dentry->d_inode;
- if (!inode || !UNIONFS_I(inode) ||
- ibstart(inode) < 0 || ibend(inode) < 0)
+ if (unlikely(!inode || !UNIONFS_I(inode) ||
+ ibstart(inode) < 0 || ibend(inode) < 0))
return false;

for (bindex = ibstart(inode); bindex <= ibend(inode); bindex++) {
lower_inode = unionfs_lower_inode_idx(inode, bindex);
- if (!lower_inode)
+ if (unlikely(!lower_inode))
continue;
/*
* We may want to apply other tests to determine if the
* lower inode's data has changed, but checking for changed
* ctime and mtime on the lower inode should be enough.
*/
- if (timespec_compare(&inode->i_mtime,
- &lower_inode->i_mtime) < 0) {
+ if (unlikely(timespec_compare(&inode->i_mtime,
+ &lower_inode->i_mtime) < 0)) {
printk("unionfs: new lower inode mtime "
"(bindex=%d, name=%s)\n", bindex,
dentry->d_name.name);
show_dinode_times(dentry);
return true; /* mtime changed! */
}
- if (timespec_compare(&inode->i_ctime,
- &lower_inode->i_ctime) < 0) {
+ if (unlikely(timespec_compare(&inode->i_ctime,
+ &lower_inode->i_ctime) < 0)) {
printk("unionfs: new lower inode ctime "
"(bindex=%d, name=%s)\n", bindex,
dentry->d_name.name);
@@ -293,7 +294,7 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd,
dtmp = dentry->d_parent;
dgen = atomic_read(&UNIONFS_D(dtmp)->generation);
/* XXX: should we check if is_newer_lower all the way up? */
- if (is_newer_lower(dtmp)) {
+ if (unlikely(is_newer_lower(dtmp))) {
/*
* Special case: the root dentry's generation number must
* always be valid, but its lower inode times don't have to
@@ -327,7 +328,7 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd,
* and short lived, so locality will be better.
*/
chain = kzalloc(chain_len * sizeof(struct dentry *), GFP_KERNEL);
- if (!chain) {
+ if (unlikely(!chain)) {
printk("unionfs: no more memory in %s\n", __FUNCTION__);
goto out;
}
@@ -364,7 +365,7 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd,
}
unionfs_unlock_dentry(chain[i]);

- if (!valid)
+ if (unlikely(!valid))
goto out_free;
}

@@ -373,7 +374,7 @@ out_this:
/* finally, lock this dentry and revalidate it */
verify_locked(dentry);
dgen = atomic_read(&UNIONFS_D(dentry)->generation);
- if (is_newer_lower(dentry)) {
+ if (unlikely(is_newer_lower(dentry))) {
/* root dentry special case as aforementioned */
if (IS_ROOT(dentry))
unionfs_copy_attr_times(dentry->d_inode);
@@ -399,7 +400,7 @@ out_this:
* which __unionfs_d_revalidate_one has incremented. Note: the "if"
* test below does not depend on whether chain_len was 0 or greater.
*/
- if (valid && sbgen != dgen)
+ if (unlikely(valid && sbgen != dgen))
for (bindex = dbstart(dentry);
bindex <= dbend(dentry);
bindex++)
@@ -425,7 +426,7 @@ static int unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
unionfs_lock_dentry(dentry);
err = __unionfs_d_revalidate_chain(dentry, nd, false);
unionfs_unlock_dentry(dentry);
- if (err > 0) { /* true==1: dentry is valid */
+ if (likely(err > 0)) { /* true==1: dentry is valid */
unionfs_check_dentry(dentry);
unionfs_check_nd(nd);
}
@@ -447,11 +448,11 @@ static void unionfs_d_release(struct dentry *dentry)

unionfs_check_dentry(dentry);
/* this could be a negative dentry, so check first */
- if (!UNIONFS_D(dentry)) {
+ if (unlikely(!UNIONFS_D(dentry))) {
printk(KERN_DEBUG "unionfs: dentry without private data: %.*s\n",
dentry->d_name.len, dentry->d_name.name);
goto out;
- } else if (dbstart(dentry) < 0) {
+ } else if (unlikely(dbstart(dentry) < 0)) {
/* this is due to a failed lookup */
printk(KERN_DEBUG "unionfs: dentry without lower "
"dentries: %.*s\n",
@@ -466,7 +467,8 @@ static void unionfs_d_release(struct dentry *dentry)
dput(unionfs_lower_dentry_idx(dentry, bindex));
unionfs_set_lower_dentry_idx(dentry, bindex, NULL);
/* NULL lower mnt is ok if this is a negative dentry */
- if (!dentry->d_inode && !unionfs_lower_mnt_idx(dentry,bindex))
+ if (unlikely(!dentry->d_inode &&
+ !unionfs_lower_mnt_idx(dentry,bindex)))
continue;
unionfs_mntput(dentry, bindex);
unionfs_set_lower_mnt_idx(dentry, bindex, NULL);
--
1.5.2.2

2007-09-26 03:33:46

by Erez Zadok

Subject: [PATCH 20/25] Unionfs: add un/likely conditionals on rename ops

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/rename.c | 96 +++++++++++++++++++++++++-------------------------
1 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/fs/unionfs/rename.c b/fs/unionfs/rename.c
index 7b8fe39..92c4515 100644
--- a/fs/unionfs/rename.c
+++ b/fs/unionfs/rename.c
@@ -39,7 +39,7 @@ static int __unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
create_parents(new_dentry->d_parent->d_inode,
new_dentry, new_dentry->d_name.name,
bindex);
- if (IS_ERR(lower_new_dentry)) {
+ if (unlikely(IS_ERR(lower_new_dentry))) {
printk(KERN_DEBUG "unionfs: error creating directory "
"tree for rename, bindex = %d, err = %ld\n",
bindex, PTR_ERR(lower_new_dentry));
@@ -50,7 +50,7 @@ static int __unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,

wh_name = alloc_whname(new_dentry->d_name.name,
new_dentry->d_name.len);
- if (IS_ERR(wh_name)) {
+ if (unlikely(IS_ERR(wh_name))) {
err = PTR_ERR(wh_name);
goto out;
}
@@ -58,14 +58,14 @@ static int __unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
lower_wh_dentry = lookup_one_len(wh_name, lower_new_dentry->d_parent,
new_dentry->d_name.len +
UNIONFS_WHLEN);
- if (IS_ERR(lower_wh_dentry)) {
+ if (unlikely(IS_ERR(lower_wh_dentry))) {
err = PTR_ERR(lower_wh_dentry);
goto out;
}

if (lower_wh_dentry->d_inode) {
/* get rid of the whiteout that is existing */
- if (lower_new_dentry->d_inode) {
+ if (unlikely(lower_new_dentry->d_inode)) {
printk(KERN_WARNING "unionfs: both a whiteout and a "
"dentry exist when doing a rename!\n");
err = -EIO;
@@ -81,7 +81,7 @@ static int __unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,

dput(lower_wh_dentry);
unlock_dir(lower_wh_dir_dentry);
- if (err)
+ if (unlikely(err))
goto out;
} else
dput(lower_wh_dentry);
@@ -93,7 +93,7 @@ static int __unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
lock_rename(lower_old_dir_dentry, lower_new_dir_dentry);

err = is_robranch_super(old_dentry->d_sb, bindex);
- if (err)
+ if (unlikely(err))
goto out_unlock;

/*
@@ -105,14 +105,14 @@ static int __unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
whname = alloc_whname(old_dentry->d_name.name,
old_dentry->d_name.len);
err = PTR_ERR(whname);
- if (IS_ERR(whname))
+ if (unlikely(IS_ERR(whname)))
goto out_unlock;
*wh_old = lookup_one_len(whname, lower_old_dir_dentry,
old_dentry->d_name.len +
UNIONFS_WHLEN);
kfree(whname);
err = PTR_ERR(*wh_old);
- if (IS_ERR(*wh_old)) {
+ if (unlikely(IS_ERR(*wh_old))) {
*wh_old = NULL;
goto out_unlock;
}
@@ -129,7 +129,7 @@ out_unlock:
dput(lower_old_dentry);

out:
- if (!err) {
+ if (likely(!err)) {
/* Fixup the new_dentry. */
if (bindex < dbstart(new_dentry))
set_dbstart(new_dentry, bindex);
@@ -174,8 +174,8 @@ static int do_unionfs_rename(struct inode *old_dir,
/* Rename source to destination. */
err = __unionfs_rename(old_dir, old_dentry, new_dir, new_dentry,
old_bstart, &wh_old);
- if (err) {
- if (!IS_COPYUP_ERR(err))
+ if (unlikely(err)) {
+ if (unlikely(!IS_COPYUP_ERR(err)))
goto out;
do_copyup = old_bstart - 1;
} else
@@ -190,7 +190,7 @@ static int do_unionfs_rename(struct inode *old_dir,
struct dentry *unlink_dir_dentry;

unlink_dentry = unionfs_lower_dentry_idx(new_dentry, bindex);
- if (!unlink_dentry)
+ if (unlikely(!unlink_dentry))
continue;

unlink_dir_dentry = lock_parent(unlink_dentry);
@@ -205,15 +205,15 @@ static int do_unionfs_rename(struct inode *old_dir,
unionfs_get_nlinks(new_dentry->d_parent->d_inode);

unlock_dir(unlink_dir_dentry);
- if (!err) {
+ if (likely(!err)) {
if (bindex != new_bstart) {
dput(unlink_dentry);
unionfs_set_lower_dentry_idx(new_dentry,
bindex, NULL);
}
- } else if (IS_COPYUP_ERR(err)) {
+ } else if (unlikely(IS_COPYUP_ERR(err))) {
do_copyup = bindex - 1;
- } else if (revert) {
+ } else if (unlikely(revert)) {
dput(wh_old);
goto revert;
}
@@ -231,7 +231,7 @@ static int do_unionfs_rename(struct inode *old_dir,
old_dentry->d_name.len,
NULL, old_dentry->d_inode->i_size);
/* if copyup failed, try next branch to the left */
- if (err)
+ if (unlikely(err))
continue;
dput(wh_old);
bwh_old = bindex;
@@ -245,7 +245,7 @@ static int do_unionfs_rename(struct inode *old_dir,
/* make it opaque */
if (S_ISDIR(old_dentry->d_inode->i_mode)) {
err = make_dir_opaque(old_dentry, dbstart(old_dentry));
- if (err)
+ if (unlikely(err))
goto revert;
}

@@ -254,10 +254,10 @@ static int do_unionfs_rename(struct inode *old_dir,
* (1) There is more than one underlying instance of source.
* (2) We did a copy_up
*/
- if ((old_bstart != old_bend) || (do_copyup != -1)) {
+ if (unlikely((old_bstart != old_bend) || (do_copyup != -1))) {
struct dentry *lower_parent;
struct nameidata nd;
- if (!wh_old || wh_old->d_inode || bwh_old < 0) {
+ if (unlikely(!wh_old || wh_old->d_inode || bwh_old < 0)) {
printk(KERN_ERR "unionfs: rename error "
"(wh_old=%p/%p bwh_old=%d)\n", wh_old,
(wh_old ? wh_old->d_inode : NULL), bwh_old);
@@ -265,13 +265,13 @@ static int do_unionfs_rename(struct inode *old_dir,
goto out;
}
err = init_lower_nd(&nd, LOOKUP_CREATE);
- if (err < 0)
+ if (unlikely(err < 0))
goto out;
lower_parent = lock_parent(wh_old);
local_err = vfs_create(lower_parent->d_inode, wh_old, S_IRUGO,
&nd);
unlock_dir(lower_parent);
- if (!local_err)
+ if (likely(!local_err))
set_dbopaque(old_dentry, bwh_old);
else {
/*
@@ -292,30 +292,30 @@ out:
revert:
/* Do revert here. */
local_err = unionfs_refresh_lower_dentry(new_dentry, old_bstart);
- if (local_err) {
+ if (unlikely(local_err)) {
printk(KERN_WARNING "unionfs: revert failed in rename: "
"the new refresh failed.\n");
eio = -EIO;
}

local_err = unionfs_refresh_lower_dentry(old_dentry, old_bstart);
- if (local_err) {
+ if (unlikely(local_err)) {
printk(KERN_WARNING "unionfs: revert failed in rename: "
"the old refresh failed.\n");
eio = -EIO;
goto revert_out;
}

- if (!unionfs_lower_dentry_idx(new_dentry, bindex) ||
- !unionfs_lower_dentry_idx(new_dentry, bindex)->d_inode) {
+ if (unlikely(!unionfs_lower_dentry_idx(new_dentry, bindex) ||
+ !unionfs_lower_dentry_idx(new_dentry, bindex)->d_inode)) {
printk(KERN_WARNING "unionfs: revert failed in rename: "
"the object disappeared from under us!\n");
eio = -EIO;
goto revert_out;
}

- if (unionfs_lower_dentry_idx(old_dentry, bindex) &&
- unionfs_lower_dentry_idx(old_dentry, bindex)->d_inode) {
+ if (unlikely(unionfs_lower_dentry_idx(old_dentry, bindex) &&
+ unionfs_lower_dentry_idx(old_dentry, bindex)->d_inode)) {
printk(KERN_WARNING "unionfs: revert failed in rename: "
"the object was created underneath us!\n");
eio = -EIO;
@@ -326,16 +326,16 @@ revert:
old_dir, old_dentry, old_bstart, NULL);

/* If we can't fix it, then we cop-out with -EIO. */
- if (local_err) {
+ if (unlikely(local_err)) {
printk(KERN_WARNING "unionfs: revert failed in rename!\n");
eio = -EIO;
}

local_err = unionfs_refresh_lower_dentry(new_dentry, bindex);
- if (local_err)
+ if (unlikely(local_err))
eio = -EIO;
local_err = unionfs_refresh_lower_dentry(old_dentry, bindex);
- if (local_err)
+ if (unlikely(local_err))
eio = -EIO;

revert_out:
@@ -351,7 +351,7 @@ static struct dentry *lookup_whiteout(struct dentry *dentry)
struct dentry *parent, *lower_parent, *wh_dentry;

whname = alloc_whname(dentry->d_name.name, dentry->d_name.len);
- if (IS_ERR(whname))
+ if (unlikely(IS_ERR(whname)))
return (void *)whname;

parent = dget_parent(dentry);
@@ -361,11 +361,11 @@ static struct dentry *lookup_whiteout(struct dentry *dentry)
wh_dentry = ERR_PTR(-ENOENT);
for (bindex = bstart; bindex <= bend; bindex++) {
lower_parent = unionfs_lower_dentry_idx(parent, bindex);
- if (!lower_parent)
+ if (unlikely(!lower_parent))
continue;
wh_dentry = lookup_one_len(whname, lower_parent,
dentry->d_name.len + UNIONFS_WHLEN);
- if (IS_ERR(wh_dentry))
+ if (unlikely(IS_ERR(wh_dentry)))
continue;
if (wh_dentry->d_inode)
break;
@@ -389,10 +389,10 @@ static int may_rename_dir(struct dentry *dentry)
int err, bstart;

err = check_empty(dentry, NULL);
- if (err == -ENOTEMPTY) {
- if (is_robranch(dentry))
+ if (unlikely(err == -ENOTEMPTY)) {
+ if (unlikely(is_robranch(dentry)))
return -EXDEV;
- } else if (err)
+ } else if (likely(err))
return err;

bstart = dbstart(dentry);
@@ -402,7 +402,7 @@ static int may_rename_dir(struct dentry *dentry)
set_dbstart(dentry, bstart + 1);
err = check_empty(dentry, NULL);
set_dbstart(dentry, bstart);
- if (err == -ENOTEMPTY)
+ if (unlikely(err == -ENOTEMPTY))
err = -EXDEV;
return err;
}
@@ -416,12 +416,12 @@ int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
unionfs_read_lock(old_dentry->d_sb);
unionfs_double_lock_dentry(old_dentry, new_dentry);

- if (!__unionfs_d_revalidate_chain(old_dentry, NULL, false)) {
+ if (unlikely(!__unionfs_d_revalidate_chain(old_dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
- if (!d_deleted(new_dentry) && new_dentry->d_inode &&
- !__unionfs_d_revalidate_chain(new_dentry, NULL, false)) {
+ if (unlikely(!d_deleted(new_dentry) && new_dentry->d_inode &&
+ !__unionfs_d_revalidate_chain(new_dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
@@ -431,11 +431,11 @@ int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
else
err = may_rename_dir(old_dentry);

- if (err)
+ if (unlikely(err))
goto out;

err = unionfs_partial_lookup(new_dentry);
- if (err)
+ if (unlikely(err))
goto out;

/*
@@ -443,11 +443,11 @@ int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
* simply override it even if the whited-out dir is not empty.
*/
wh_dentry = lookup_whiteout(new_dentry);
- if (!IS_ERR(wh_dentry))
+ if (likely(!IS_ERR(wh_dentry)))
dput(wh_dentry);
else if (new_dentry->d_inode) {
- if (S_ISDIR(old_dentry->d_inode->i_mode) !=
- S_ISDIR(new_dentry->d_inode->i_mode)) {
+ if (unlikely(S_ISDIR(old_dentry->d_inode->i_mode) !=
+ S_ISDIR(new_dentry->d_inode->i_mode))) {
err = S_ISDIR(old_dentry->d_inode->i_mode) ?
-ENOTDIR : -EISDIR;
goto out;
@@ -457,7 +457,7 @@ int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
struct unionfs_dir_state *namelist;
/* check if this unionfs directory is empty or not */
err = check_empty(new_dentry, &namelist);
- if (err)
+ if (unlikely(err))
goto out;

if (!is_robranch(new_dentry))
@@ -467,13 +467,13 @@ int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,

free_rdstate(namelist);

- if (err)
+ if (unlikely(err))
goto out;
}
}
err = do_unionfs_rename(old_dir, old_dentry, new_dir, new_dentry);
out:
- if (err)
+ if (unlikely(err))
/* clear the new_dentry stuff created */
d_drop(new_dentry);
else {
--
1.5.2.2

2007-09-26 03:34:13

by Erez Zadok

Subject: [PATCH 09/25] Unionfs: add un/likely conditionals on common fileops

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/commonfops.c | 94 +++++++++++++++++++++++-----------------------
1 files changed, 47 insertions(+), 47 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index e69ccf6..db8f064 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -64,7 +64,7 @@ retry:

tmp_dentry = lookup_one_len(name, lower_dentry->d_parent,
nlen);
- if (IS_ERR(tmp_dentry)) {
+ if (unlikely(IS_ERR(tmp_dentry))) {
err = PTR_ERR(tmp_dentry);
goto out;
}
@@ -73,8 +73,8 @@ retry:

err = copyup_named_file(dentry->d_parent->d_inode, file, name, bstart,
bindex, file->f_path.dentry->d_inode->i_size);
- if (err) {
- if (err == -EEXIST)
+ if (unlikely(err)) {
+ if (unlikely(err == -EEXIST))
goto retry;
goto out;
}
@@ -91,7 +91,7 @@ retry:
unlock_dir(lower_dir_dentry);

out:
- if (!err)
+ if (likely(!err))
unionfs_check_dentry(dentry);
return err;
}
@@ -126,7 +126,7 @@ static void cleanup_file(struct file *file)
*/
old_bid = UNIONFS_F(file)->saved_branch_ids[bindex];
i = branch_id_to_idx(sb, old_bid);
- if (i < 0) {
+ if (unlikely(i < 0)) {
printk(KERN_ERR "unionfs: no superblock for "
"file %p\n", file);
continue;
@@ -179,7 +179,7 @@ static int open_all_files(struct file *file)
dentry_open(lower_dentry,
unionfs_lower_mnt_idx(dentry, bindex),
file->f_flags);
- if (IS_ERR(lower_file)) {
+ if (unlikely(IS_ERR(lower_file))) {
err = PTR_ERR(lower_file);
goto out;
} else
@@ -208,7 +208,7 @@ static int open_highest_file(struct file *file, bool willwrite)
for (bindex = bstart - 1; bindex >= 0; bindex--) {
err = copyup_file(parent_inode, file, bstart, bindex,
inode_size);
- if (!err)
+ if (likely(!err))
break;
}
atomic_set(&UNIONFS_F(file)->generation,
@@ -222,7 +222,7 @@ static int open_highest_file(struct file *file, bool willwrite)
lower_file = dentry_open(lower_dentry,
unionfs_lower_mnt_idx(dentry, bstart),
file->f_flags);
- if (IS_ERR(lower_file)) {
+ if (unlikely(IS_ERR(lower_file))) {
err = PTR_ERR(lower_file);
goto out;
}
@@ -252,17 +252,17 @@ static int do_delayed_copyup(struct file *file)
unionfs_check_file(file);
unionfs_check_dentry(dentry);
for (bindex = bstart - 1; bindex >= 0; bindex--) {
- if (!d_deleted(dentry))
+ if (likely(!d_deleted(dentry)))
err = copyup_file(parent_inode, file, bstart,
bindex, inode_size);
else
err = copyup_deleted_file(file, dentry, bstart,
bindex);

- if (!err)
+ if (likely(!err))
break;
}
- if (err || (bstart <= fbstart(file)))
+ if (unlikely(err || (bstart <= fbstart(file))))
goto out;
bend = fbend(file);
for (bindex = bstart; bindex <= bend; bindex++) {
@@ -317,8 +317,8 @@ int unionfs_file_revalidate(struct file *file, bool willwrite)
* First revalidate the dentry inside struct file,
* but not unhashed dentries.
*/
- if (!d_deleted(dentry) &&
- !__unionfs_d_revalidate_chain(dentry, NULL, willwrite)) {
+ if (unlikely(!d_deleted(dentry) &&
+ !__unionfs_d_revalidate_chain(dentry, NULL, willwrite))) {
err = -ESTALE;
goto out_nofree;
}
@@ -335,8 +335,8 @@ int unionfs_file_revalidate(struct file *file, bool willwrite)
* someone has copied up this file from underneath us, we also need
* to refresh things.
*/
- if (!d_deleted(dentry) &&
- (sbgen > fgen || dbstart(dentry) != fbstart(file))) {
+ if (unlikely(!d_deleted(dentry) &&
+ (sbgen > fgen || dbstart(dentry) != fbstart(file)))) {
/* save orig branch ID */
int orig_brid = UNIONFS_F(file)->saved_branch_ids[fbstart(file)];

@@ -349,13 +349,13 @@ int unionfs_file_revalidate(struct file *file, bool willwrite)

size = sizeof(struct file *) * sbmax(sb);
UNIONFS_F(file)->lower_files = kzalloc(size, GFP_KERNEL);
- if (!UNIONFS_F(file)->lower_files) {
+ if (unlikely(!UNIONFS_F(file)->lower_files)) {
err = -ENOMEM;
goto out;
}
size = sizeof(int) * sbmax(sb);
UNIONFS_F(file)->saved_branch_ids = kzalloc(size, GFP_KERNEL);
- if (!UNIONFS_F(file)->saved_branch_ids) {
+ if (unlikely(!UNIONFS_F(file)->saved_branch_ids)) {
err = -ENOMEM;
goto out;
}
@@ -363,17 +363,17 @@ int unionfs_file_revalidate(struct file *file, bool willwrite)
if (S_ISDIR(dentry->d_inode->i_mode)) {
/* We need to open all the files. */
err = open_all_files(file);
- if (err)
+ if (unlikely(err))
goto out;
} else {
int new_brid;
/* We only open the highest priority branch. */
err = open_highest_file(file, willwrite);
- if (err)
+ if (unlikely(err))
goto out;
new_brid = UNIONFS_F(file)->
saved_branch_ids[fbstart(file)];
- if (new_brid != orig_brid && sbgen > fgen) {
+ if (unlikely(new_brid != orig_brid && sbgen > fgen)) {
/*
* If we re-opened the file on a different
* branch than the original one, and this
@@ -400,12 +400,12 @@ int unionfs_file_revalidate(struct file *file, bool willwrite)
}

out:
- if (err) {
+ if (unlikely(err)) {
kfree(UNIONFS_F(file)->lower_files);
kfree(UNIONFS_F(file)->saved_branch_ids);
}
out_nofree:
- if (!err)
+ if (unlikely(!err))
unionfs_check_file(file);
unionfs_unlock_dentry(dentry);
return err;
@@ -424,7 +424,7 @@ static int __open_dir(struct inode *inode, struct file *file)
for (bindex = bstart; bindex <= bend; bindex++) {
lower_dentry =
unionfs_lower_dentry_idx(file->f_path.dentry, bindex);
- if (!lower_dentry)
+ if (unlikely(!lower_dentry))
continue;

dget(lower_dentry);
@@ -433,7 +433,7 @@ static int __open_dir(struct inode *inode, struct file *file)
unionfs_lower_mnt_idx(file->f_path.dentry,
bindex),
file->f_flags);
- if (IS_ERR(lower_file))
+ if (unlikely(IS_ERR(lower_file)))
return PTR_ERR(lower_file);

unionfs_set_lower_file_idx(file, bindex, lower_file);
@@ -480,7 +480,7 @@ static int __open_file(struct inode *inode, struct file *file)
err = copyup_file(
file->f_path.dentry->d_parent->d_inode,
file, bstart, bindex, size);
- if (!err)
+ if (likely(!err))
break;
}
return err;
@@ -499,7 +499,7 @@ static int __open_file(struct inode *inode, struct file *file)
dentry_open(lower_dentry,
unionfs_lower_mnt_idx(file->f_path.dentry, bstart),
lower_flags);
- if (IS_ERR(lower_file))
+ if (unlikely(IS_ERR(lower_file)))
return PTR_ERR(lower_file);

unionfs_set_lower_file(file, lower_file);
@@ -520,7 +520,7 @@ int unionfs_open(struct inode *inode, struct file *file)

file->private_data =
kzalloc(sizeof(struct unionfs_file_info), GFP_KERNEL);
- if (!UNIONFS_F(file)) {
+ if (unlikely(!UNIONFS_F(file))) {
err = -ENOMEM;
goto out_nofree;
}
@@ -531,13 +531,13 @@ int unionfs_open(struct inode *inode, struct file *file)

size = sizeof(struct file *) * sbmax(inode->i_sb);
UNIONFS_F(file)->lower_files = kzalloc(size, GFP_KERNEL);
- if (!UNIONFS_F(file)->lower_files) {
+ if (unlikely(!UNIONFS_F(file)->lower_files)) {
err = -ENOMEM;
goto out;
}
size = sizeof(int) * sbmax(inode->i_sb);
UNIONFS_F(file)->saved_branch_ids = kzalloc(size, GFP_KERNEL);
- if (!UNIONFS_F(file)->saved_branch_ids) {
+ if (unlikely(!UNIONFS_F(file)->saved_branch_ids)) {
err = -ENOMEM;
goto out;
}
@@ -561,11 +561,11 @@ int unionfs_open(struct inode *inode, struct file *file)
err = __open_file(inode, file); /* open a file */

/* freeing the allocated resources, and fput the opened files */
- if (err) {
+ if (unlikely(err)) {
atomic_dec(&UNIONFS_I(dentry->d_inode)->totalopens);
for (bindex = bstart; bindex <= bend; bindex++) {
lower_file = unionfs_lower_file_idx(file, bindex);
- if (!lower_file)
+ if (unlikely(!lower_file))
continue;

branchput(file->f_path.dentry->d_sb, bindex);
@@ -577,7 +577,7 @@ int unionfs_open(struct inode *inode, struct file *file)
unionfs_unlock_dentry(dentry);

out:
- if (err) {
+ if (unlikely(err)) {
kfree(UNIONFS_F(file)->lower_files);
kfree(UNIONFS_F(file)->saved_branch_ids);
kfree(UNIONFS_F(file));
@@ -585,7 +585,7 @@ out:
out_nofree:
unionfs_read_unlock(inode->i_sb);
unionfs_check_inode(inode);
- if (!err) {
+ if (likely(!err)) {
unionfs_check_file(file);
unionfs_check_dentry(file->f_path.dentry->d_parent);
}
@@ -612,7 +612,7 @@ int unionfs_file_release(struct inode *inode, struct file *file)
* This is important for open-but-unlinked files, as well as mmap
* support.
*/
- if ((err = unionfs_file_revalidate(file, true)))
+ if (unlikely((err = unionfs_file_revalidate(file, true))))
goto out;
unionfs_check_file(file);
fileinfo = UNIONFS_F(file);
@@ -627,7 +627,7 @@ int unionfs_file_release(struct inode *inode, struct file *file)
for (bindex = bstart; bindex <= bend; bindex++) {
lower_file = unionfs_lower_file_idx(file, bindex);

- if (lower_file) {
+ if (likely(lower_file)) {
fput(lower_file);
branchput(sb, bindex);
}
@@ -635,7 +635,7 @@ int unionfs_file_release(struct inode *inode, struct file *file)
kfree(fileinfo->lower_files);
kfree(fileinfo->saved_branch_ids);

- if (fileinfo->rdstate) {
+ if (unlikely(fileinfo->rdstate)) {
fileinfo->rdstate->access = jiffies;
printk(KERN_DEBUG "unionfs: saving rdstate with cookie "
"%u [%d.%lld]\n",
@@ -666,15 +666,15 @@ static long do_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
lower_file = unionfs_lower_file(file);

err = security_file_ioctl(lower_file, cmd, arg);
- if (err)
+ if (unlikely(err))
goto out;

err = -ENOTTY;
- if (!lower_file || !lower_file->f_op)
+ if (unlikely(!lower_file || !lower_file->f_op))
goto out;
if (lower_file->f_op->unlocked_ioctl) {
err = lower_file->f_op->unlocked_ioctl(lower_file, cmd, arg);
- } else if (lower_file->f_op->ioctl) {
+ } else if (likely(lower_file->f_op->ioctl)) {
lock_kernel();
err = lower_file->f_op->ioctl(lower_file->f_path.dentry->d_inode,
lower_file, cmd, arg);
@@ -705,7 +705,7 @@ static int unionfs_ioctl_queryfile(struct file *file, unsigned int cmd,
unionfs_lock_dentry(dentry);
orig_bstart = dbstart(dentry);
orig_bend = dbend(dentry);
- if ((err = unionfs_partial_lookup(dentry)))
+ if (unlikely((err = unionfs_partial_lookup(dentry))))
goto out;
bstart = dbstart(dentry);
bend = dbend(dentry);
@@ -714,9 +714,9 @@ static int unionfs_ioctl_queryfile(struct file *file, unsigned int cmd,

for (bindex = bstart; bindex <= bend; bindex++) {
lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
- if (!lower_dentry)
+ if (unlikely(!lower_dentry))
continue;
- if (lower_dentry->d_inode)
+ if (likely(lower_dentry->d_inode))
FD_SET(bindex, &branchlist);
/* purge any lower objects after partial_lookup */
if (bindex < orig_bstart || bindex > orig_bend) {
@@ -726,7 +726,7 @@ static int unionfs_ioctl_queryfile(struct file *file, unsigned int cmd,
unionfs_set_lower_inode_idx(dentry->d_inode, bindex,
NULL);
mnt = unionfs_lower_mnt_idx(dentry, bindex);
- if (!mnt)
+ if (unlikely(!mnt))
continue;
unionfs_mntput(dentry, bindex);
unionfs_set_lower_mnt_idx(dentry, bindex, NULL);
@@ -739,7 +739,7 @@ static int unionfs_ioctl_queryfile(struct file *file, unsigned int cmd,
ibend(dentry->d_inode) = orig_bend;

err = copy_to_user((void __user *)arg, &branchlist, sizeof(fd_set));
- if (err)
+ if (unlikely(err))
err = -EFAULT;

out:
@@ -753,7 +753,7 @@ long unionfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)

unionfs_read_lock(file->f_path.dentry->d_sb);

- if ((err = unionfs_file_revalidate(file, true)))
+ if (unlikely((err = unionfs_file_revalidate(file, true))))
goto out;

/* check if asked for local commands */
@@ -791,7 +791,7 @@ int unionfs_flush(struct file *file, fl_owner_t id)

unionfs_read_lock(dentry->d_sb);

- if ((err = unionfs_file_revalidate(file, true)))
+ if (unlikely((err = unionfs_file_revalidate(file, true))))
goto out;
unionfs_check_file(file);

@@ -808,7 +808,7 @@ int unionfs_flush(struct file *file, fl_owner_t id)
if (lower_file && lower_file->f_op &&
lower_file->f_op->flush) {
err = lower_file->f_op->flush(lower_file, id);
- if (err)
+ if (unlikely(err))
goto out_lock;

/* if there are no more refs to the dentry, dput it */
--
1.5.2.2

2007-09-26 03:34:33

by Erez Zadok

Subject: [PATCH 11/25] Unionfs: add un/likely conditionals on debug ops

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/debug.c | 108 +++++++++++++++++++++++++++------------------------
1 files changed, 57 insertions(+), 51 deletions(-)

diff --git a/fs/unionfs/debug.c b/fs/unionfs/debug.c
index 9546a41..09b52ce 100644
--- a/fs/unionfs/debug.c
+++ b/fs/unionfs/debug.c
@@ -56,19 +56,19 @@ void __unionfs_check_inode(const struct inode *inode,
sb = inode->i_sb;
istart = ibstart(inode);
iend = ibend(inode);
- if (istart > iend) {
+ if (unlikely(istart > iend)) {
PRINT_CALLER(fname, fxn, line);
printk(" Ci0: inode=%p istart/end=%d:%d\n",
inode, istart, iend);
}
- if ((istart == -1 && iend != -1) ||
- (istart != -1 && iend == -1)) {
+ if (unlikely((istart == -1 && iend != -1) ||
+ (istart != -1 && iend == -1))) {
PRINT_CALLER(fname, fxn, line);
printk(" Ci1: inode=%p istart/end=%d:%d\n",
inode, istart, iend);
}
if (!S_ISDIR(inode->i_mode)) {
- if (iend != istart) {
+ if (unlikely(iend != istart)) {
PRINT_CALLER(fname, fxn, line);
printk(" Ci2: inode=%p istart=%d iend=%d\n",
inode, istart, iend);
@@ -76,24 +76,24 @@ void __unionfs_check_inode(const struct inode *inode,
}

for (bindex = sbstart(sb); bindex < sbmax(sb); bindex++) {
- if (!UNIONFS_I(inode)) {
+ if (unlikely(!UNIONFS_I(inode))) {
PRINT_CALLER(fname, fxn, line);
printk(" Ci3: no inode_info %p\n", inode);
return;
}
- if (!UNIONFS_I(inode)->lower_inodes) {
+ if (unlikely(!UNIONFS_I(inode)->lower_inodes)) {
PRINT_CALLER(fname, fxn, line);
printk(" Ci4: no lower_inodes %p\n", inode);
return;
}
lower_inode = unionfs_lower_inode_idx(inode, bindex);
if (lower_inode) {
- if (bindex < istart || bindex > iend) {
+ if (unlikely(bindex < istart || bindex > iend)) {
PRINT_CALLER(fname, fxn, line);
printk(" Ci5: inode/linode=%p:%p bindex=%d "
"istart/end=%d:%d\n", inode,
lower_inode, bindex, istart, iend);
- } else if (lower_inode == POISONED_PTR) {
+ } else if (unlikely(lower_inode == POISONED_PTR)) {
/* freed inode! */
PRINT_CALLER(fname, fxn, line);
printk(" Ci6: inode/linode=%p:%p bindex=%d "
@@ -107,8 +107,9 @@ void __unionfs_check_inode(const struct inode *inode,
* b/t start/end, but NOT if at the
* start/end range.
*/
- if (!(S_ISDIR(inode->i_mode) &&
- bindex > istart && bindex < iend)) {
+ if (unlikely(!(S_ISDIR(inode->i_mode) &&
+ bindex > istart &&
+ bindex < iend))) {
PRINT_CALLER(fname, fxn, line);
printk(" Ci7: inode/linode=%p:%p "
"bindex=%d istart/end=%d:%d\n",
@@ -138,8 +139,8 @@ void __unionfs_check_dentry(const struct dentry *dentry,
dend = dbend(dentry);
BUG_ON(dstart > dend);

- if ((dstart == -1 && dend != -1) ||
- (dstart != -1 && dend == -1)) {
+ if (unlikely((dstart == -1 && dend != -1) ||
+ (dstart != -1 && dend == -1))) {
PRINT_CALLER(fname, fxn, line);
printk(" CD0: dentry=%p dstart/end=%d:%d\n",
dentry, dstart, dend);
@@ -151,7 +152,7 @@ void __unionfs_check_dentry(const struct dentry *dentry,
for (bindex = sbstart(sb); bindex < sbmax(sb); bindex++) {
lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
if (lower_dentry) {
- if (bindex < dstart || bindex > dend) {
+ if (unlikely(bindex < dstart || bindex > dend)) {
PRINT_CALLER(fname, fxn, line);
printk(" CD1: dentry/lower=%p:%p(%p) "
"bindex=%d dstart/end=%d:%d\n",
@@ -169,10 +170,10 @@ void __unionfs_check_dentry(const struct dentry *dentry,
* however, if this is a NULL dentry or a
* deleted dentry.
*/
- if (!d_deleted((struct dentry *) dentry) &&
- inode &&
- !(inode && S_ISDIR(inode->i_mode) &&
- bindex > dstart && bindex < dend)) {
+ if (unlikely(!d_deleted((struct dentry *) dentry) &&
+ inode &&
+ !(inode && S_ISDIR(inode->i_mode) &&
+ bindex > dstart && bindex < dend))) {
PRINT_CALLER(fname, fxn, line);
printk(" CD2: dentry/lower=%p:%p(%p) "
"bindex=%d dstart/end=%d:%d\n",
@@ -190,7 +191,7 @@ void __unionfs_check_dentry(const struct dentry *dentry,
for (bindex = sbstart(sb); bindex < sbmax(sb); bindex++) {
lower_mnt = unionfs_lower_mnt_idx(dentry, bindex);
if (lower_mnt) {
- if (bindex < dstart || bindex > dend) {
+ if (unlikely(bindex < dstart || bindex > dend)) {
PRINT_CALLER(fname, fxn, line);
printk(" CM0: dentry/lmnt=%p:%p bindex=%d "
"dstart/end=%d:%d\n", dentry,
@@ -204,9 +205,9 @@ void __unionfs_check_dentry(const struct dentry *dentry,
* start/end range. Ignore this rule,
* however, if this is a NULL dentry.
*/
- if (inode &&
- !(inode && S_ISDIR(inode->i_mode) &&
- bindex > dstart && bindex < dend)) {
+ if (unlikely(inode &&
+ !(inode && S_ISDIR(inode->i_mode) &&
+ bindex > dstart && bindex < dend))) {
PRINT_CALLER(fname, fxn, line);
printk(" CM1: dentry/lmnt=%p:%p "
"bindex=%d dstart/end=%d:%d\n",
@@ -223,30 +224,30 @@ void __unionfs_check_dentry(const struct dentry *dentry,
istart = ibstart(inode);
iend = ibend(inode);
BUG_ON(istart > iend);
- if ((istart == -1 && iend != -1) ||
- (istart != -1 && iend == -1)) {
+ if (unlikely((istart == -1 && iend != -1) ||
+ (istart != -1 && iend == -1))) {
PRINT_CALLER(fname, fxn, line);
printk(" CI0: dentry/inode=%p:%p istart/end=%d:%d\n",
dentry, inode, istart, iend);
}
- if (istart != dstart) {
+ if (unlikely(istart != dstart)) {
PRINT_CALLER(fname, fxn, line);
printk(" CI1: dentry/inode=%p:%p istart=%d dstart=%d\n",
dentry, inode, istart, dstart);
}
- if (iend != dend) {
+ if (unlikely(iend != dend)) {
PRINT_CALLER(fname, fxn, line);
printk(" CI2: dentry/inode=%p:%p iend=%d dend=%d\n",
dentry, inode, iend, dend);
}

if (!S_ISDIR(inode->i_mode)) {
- if (dend != dstart) {
+ if (unlikely(dend != dstart)) {
PRINT_CALLER(fname, fxn, line);
printk(" CI3: dentry/inode=%p:%p dstart=%d dend=%d\n",
dentry, inode, dstart, dend);
}
- if (iend != istart) {
+ if (unlikely(iend != istart)) {
PRINT_CALLER(fname, fxn, line);
printk(" CI4: dentry/inode=%p:%p istart=%d iend=%d\n",
dentry, inode, istart, iend);
@@ -256,12 +257,12 @@ void __unionfs_check_dentry(const struct dentry *dentry,
for (bindex = sbstart(sb); bindex < sbmax(sb); bindex++) {
lower_inode = unionfs_lower_inode_idx(inode, bindex);
if (lower_inode) {
- if (bindex < istart || bindex > iend) {
+ if (unlikely(bindex < istart || bindex > iend)) {
PRINT_CALLER(fname, fxn, line);
printk(" CI5: dentry/linode=%p:%p bindex=%d "
"istart/end=%d:%d\n", dentry,
lower_inode, bindex, istart, iend);
- } else if (lower_inode == POISONED_PTR) {
+ } else if (unlikely(lower_inode == POISONED_PTR)) {
/* freed inode! */
PRINT_CALLER(fname, fxn, line);
printk(" CI6: dentry/linode=%p:%p bindex=%d "
@@ -275,8 +276,9 @@ void __unionfs_check_dentry(const struct dentry *dentry,
* b/t start/end, but NOT if at the
* start/end range.
*/
- if (!(S_ISDIR(inode->i_mode) &&
- bindex > istart && bindex < iend)) {
+ if (unlikely(!(S_ISDIR(inode->i_mode) &&
+ bindex > istart &&
+ bindex < iend))) {
PRINT_CALLER(fname, fxn, line);
printk(" CI7: dentry/linode=%p:%p "
"bindex=%d istart/end=%d:%d\n",
@@ -298,8 +300,10 @@ void __unionfs_check_dentry(const struct dentry *dentry,
lower_dentry = unionfs_lower_dentry_idx(dentry,
bindex);
lower_mnt = unionfs_lower_mnt_idx(dentry, bindex);
- if (!((lower_inode && lower_dentry && lower_mnt) ||
- (!lower_inode && !lower_dentry && !lower_mnt))) {
+ if (unlikely(!((lower_inode && lower_dentry &&
+ lower_mnt) ||
+ (!lower_inode &&
+ !lower_dentry && !lower_mnt)))) {
PRINT_CALLER(fname, fxn, line);
printk(" Cx: lmnt/ldentry/linode=%p:%p:%p "
"bindex=%d dstart/end=%d:%d\n",
@@ -308,11 +312,11 @@ void __unionfs_check_dentry(const struct dentry *dentry,
}
}
/* check if lower inode is newer than upper one (it shouldn't) */
- if (is_newer_lower(dentry)) {
+ if (unlikely(is_newer_lower(dentry))) {
PRINT_CALLER(fname, fxn, line);
for (bindex=ibstart(inode); bindex <= ibend(inode); bindex++) {
lower_inode = unionfs_lower_inode_idx(inode, bindex);
- if (!lower_inode)
+ if (unlikely(!lower_inode))
continue;
printk(" CI8: bindex=%d mtime/lmtime=%lu.%lu/%lu.%lu "
"ctime/lctime=%lu.%lu/%lu.%lu\n",
@@ -350,30 +354,30 @@ void __unionfs_check_file(const struct file *file,
fend = fbend(file);
BUG_ON(fstart > fend);

- if ((fstart == -1 && fend != -1) ||
- (fstart != -1 && fend == -1)) {
+ if (unlikely((fstart == -1 && fend != -1) ||
+ (fstart != -1 && fend == -1))) {
PRINT_CALLER(fname, fxn, line);
printk(" CF0: file/dentry=%p:%p fstart/end=%d:%d\n",
file, dentry, fstart, fend);
}
- if (fstart != dstart) {
+ if (unlikely(fstart != dstart)) {
PRINT_CALLER(fname, fxn, line);
printk(" CF1: file/dentry=%p:%p fstart=%d dstart=%d\n",
file, dentry, fstart, dstart);
}
- if (fend != dend) {
+ if (unlikely(fend != dend)) {
PRINT_CALLER(fname, fxn, line);
printk(" CF2: file/dentry=%p:%p fend=%d dend=%d\n",
file, dentry, fend, dend);
}
inode = dentry->d_inode;
if (!S_ISDIR(inode->i_mode)) {
- if (fend != fstart) {
+ if (unlikely(fend != fstart)) {
PRINT_CALLER(fname, fxn, line);
printk(" CF3: file/inode=%p:%p fstart=%d fend=%d\n",
file, inode, fstart, fend);
}
- if (dend != dstart) {
+ if (unlikely(dend != dstart)) {
PRINT_CALLER(fname, fxn, line);
printk(" CF4: file/dentry=%p:%p dstart=%d dend=%d\n",
file, dentry, dstart, dend);
@@ -387,7 +391,7 @@ void __unionfs_check_file(const struct file *file,
for (bindex = sbstart(sb); bindex < sbmax(sb); bindex++) {
lower_file = unionfs_lower_file_idx(file, bindex);
if (lower_file) {
- if (bindex < fstart || bindex > fend) {
+ if (unlikely(bindex < fstart || bindex > fend)) {
PRINT_CALLER(fname, fxn, line);
printk(" CF5: file/lower=%p:%p bindex=%d "
"fstart/end=%d:%d\n",
@@ -400,8 +404,9 @@ void __unionfs_check_file(const struct file *file,
* b/t start/end, but NOT if at the
* start/end range.
*/
- if (!(S_ISDIR(inode->i_mode) &&
- bindex > fstart && bindex < fend)) {
+ if (unlikely(!(S_ISDIR(inode->i_mode) &&
+ bindex > fstart &&
+ bindex < fend))) {
PRINT_CALLER(fname, fxn, line);
printk(" CF6: file/lower=%p:%p "
"bindex=%d fstart/end=%d:%d\n",
@@ -421,12 +426,13 @@ void __unionfs_check_nd(const struct nameidata *nd,
struct file *file;
int printed_caller = 0;

- if (!nd)
+ if (unlikely(!nd))
return;
if (nd->flags & LOOKUP_OPEN) {
file = nd->intent.open.file;
- if (file->f_path.dentry &&
- strcmp(file->f_dentry->d_sb->s_type->name, "unionfs")) {
+ if (unlikely(file->f_path.dentry &&
+ strcmp(file->f_dentry->d_sb->s_type->name,
+ "unionfs"))) {
PRINT_CALLER(fname, fxn, line);
printk(" CND1: lower_file of type %s\n",
file->f_path.dentry->d_sb->s_type->name);
@@ -444,7 +450,7 @@ void __show_branch_counts(const struct super_block *sb,

printk("BC:");
for (i=0; i<sbmax(sb); i++) {
- if (sb->s_root)
+ if (likely(sb->s_root))
mnt = UNIONFS_D(sb->s_root)->lower_paths[i].mnt;
else
mnt = NULL;
@@ -461,7 +467,7 @@ void __show_inode_times(const struct inode *inode,

for (bindex=ibstart(inode); bindex <= ibend(inode); bindex++) {
lower_inode = unionfs_lower_inode_idx(inode, bindex);
- if (!lower_inode)
+ if (unlikely(!lower_inode))
continue;
printk("IT(%lu:%d): ", inode->i_ino, bindex);
printk("%s:%s:%d ",file,fxn,line);
@@ -507,14 +513,14 @@ void __show_inode_counts(const struct inode *inode,
struct inode *lower_inode;
int bindex;

- if (!inode) {
+ if (unlikely(!inode)) {
printk("SiC: Null inode\n");
return;
}
for (bindex=sbstart(inode->i_sb); bindex <= sbend(inode->i_sb);
bindex++) {
lower_inode = unionfs_lower_inode_idx(inode, bindex);
- if (!lower_inode)
+ if (unlikely(!lower_inode))
continue;
printk("SIC(%lu:%d:%d): ", inode->i_ino, bindex,
atomic_read(&(inode)->i_count));
--
1.5.2.2

2007-09-26 03:34:51

by Erez Zadok

Subject: [PATCH 10/25] Unionfs: add un/likely conditionals on copyup ops

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/copyup.c | 102 +++++++++++++++++++++++++-------------------------
1 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index 23ac4c8..e3c5f15 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -36,14 +36,14 @@ static int copyup_xattrs(struct dentry *old_lower_dentry,

/* query the actual size of the xattr list */
list_size = vfs_listxattr(old_lower_dentry, NULL, 0);
- if (list_size <= 0) {
+ if (unlikely(list_size <= 0)) {
err = list_size;
goto out;
}

/* allocate space for the actual list */
name_list = unionfs_xattr_alloc(list_size + 1, XATTR_LIST_MAX);
- if (!name_list || IS_ERR(name_list)) {
+ if (unlikely(!name_list || IS_ERR(name_list))) {
err = PTR_ERR(name_list);
goto out;
}
@@ -52,14 +52,14 @@ static int copyup_xattrs(struct dentry *old_lower_dentry,

/* now get the actual xattr list of the source file */
list_size = vfs_listxattr(old_lower_dentry, name_list, list_size);
- if (list_size <= 0) {
+ if (unlikely(list_size <= 0)) {
err = list_size;
goto out;
}

/* allocate space to hold each xattr's value */
attr_value = unionfs_xattr_alloc(XATTR_SIZE_MAX, XATTR_SIZE_MAX);
- if (!attr_value || IS_ERR(attr_value)) {
+ if (unlikely(!attr_value || IS_ERR(attr_value))) {
err = PTR_ERR(name_list);
goto out;
}
@@ -73,11 +73,11 @@ static int copyup_xattrs(struct dentry *old_lower_dentry,
size = vfs_getxattr(old_lower_dentry, name_list,
attr_value, XATTR_SIZE_MAX);
mutex_unlock(&old_lower_dentry->d_inode->i_mutex);
- if (size < 0) {
+ if (unlikely(size < 0)) {
err = size;
goto out;
}
- if (size > XATTR_SIZE_MAX) {
+ if (unlikely(size > XATTR_SIZE_MAX)) {
err = -E2BIG;
goto out;
}
@@ -91,13 +91,13 @@ static int copyup_xattrs(struct dentry *old_lower_dentry,
* temporarily get FOWNER privileges.
* XXX: move entire copyup code to SIOQ.
*/
- if (err == -EPERM && !capable(CAP_FOWNER)) {
+ if (unlikely(err == -EPERM && !capable(CAP_FOWNER))) {
cap_raise(current->cap_effective, CAP_FOWNER);
err = vfs_setxattr(new_lower_dentry, name_list,
attr_value, size, 0);
cap_lower(current->cap_effective, CAP_FOWNER);
}
- if (err < 0)
+ if (unlikely(err < 0))
goto out;
name_list += strlen(name_list) + 1;
}
@@ -105,7 +105,7 @@ out:
unionfs_xattr_kfree(name_list_buf);
unionfs_xattr_kfree(attr_value);
/* Ignore if xattr isn't supported */
- if (err == -ENOTSUPP || err == -EOPNOTSUPP)
+ if (unlikely(err == -ENOTSUPP || err == -EOPNOTSUPP))
err = 0;
return err;
}
@@ -136,15 +136,15 @@ static int copyup_permissions(struct super_block *sb,
ATTR_ATIME_SET | ATTR_MTIME_SET | ATTR_FORCE |
ATTR_GID | ATTR_UID;
err = notify_change(new_lower_dentry, &newattrs);
- if (err)
+ if (unlikely(err))
goto out;

/* now try to change the mode and ignore EOPNOTSUPP on symlinks */
newattrs.ia_mode = i->i_mode;
newattrs.ia_valid = ATTR_MODE | ATTR_FORCE;
err = notify_change(new_lower_dentry, &newattrs);
- if (err == -EOPNOTSUPP &&
- S_ISLNK(new_lower_dentry->d_inode->i_mode)) {
+ if (unlikely(err == -EOPNOTSUPP &&
+ S_ISLNK(new_lower_dentry->d_inode->i_mode))) {
printk(KERN_WARNING
"unionfs: changing \"%s\" symlink mode unsupported\n",
new_lower_dentry->d_name.name);
@@ -178,7 +178,7 @@ static int __copyup_ndentry(struct dentry *old_lower_dentry,

run_sioq(__unionfs_mkdir, &args);
err = args.err;
- } else if (S_ISLNK(old_mode)) {
+ } else if (unlikely(S_ISLNK(old_mode))) {
args.symlink.parent = new_lower_parent_dentry->d_inode;
args.symlink.dentry = new_lower_dentry;
args.symlink.symbuf = symbuf;
@@ -186,8 +186,8 @@ static int __copyup_ndentry(struct dentry *old_lower_dentry,

run_sioq(__unionfs_symlink, &args);
err = args.err;
- } else if (S_ISBLK(old_mode) || S_ISCHR(old_mode) ||
- S_ISFIFO(old_mode) || S_ISSOCK(old_mode)) {
+ } else if (unlikely(S_ISBLK(old_mode) || S_ISCHR(old_mode) ||
+ S_ISFIFO(old_mode) || S_ISSOCK(old_mode))) {
args.mknod.parent = new_lower_parent_dentry->d_inode;
args.mknod.dentry = new_lower_dentry;
args.mknod.mode = old_mode;
@@ -198,7 +198,7 @@ static int __copyup_ndentry(struct dentry *old_lower_dentry,
} else if (S_ISREG(old_mode)) {
struct nameidata nd;
err = init_lower_nd(&nd, LOOKUP_CREATE);
- if (err < 0)
+ if (unlikely(err < 0))
goto out;
args.create.nd = &nd;
args.create.parent = new_lower_parent_dentry->d_inode;
@@ -240,12 +240,12 @@ static int __copyup_reg_data(struct dentry *dentry,
input_file = dentry_open(old_lower_dentry,
unionfs_lower_mnt_idx(dentry, old_bindex),
O_RDONLY | O_LARGEFILE);
- if (IS_ERR(input_file)) {
+ if (unlikely(IS_ERR(input_file))) {
dput(old_lower_dentry);
err = PTR_ERR(input_file);
goto out;
}
- if (!input_file->f_op || !input_file->f_op->read) {
+ if (unlikely(!input_file->f_op || !input_file->f_op->read)) {
err = -EINVAL;
goto out_close_in;
}
@@ -256,18 +256,18 @@ static int __copyup_reg_data(struct dentry *dentry,
branchget(sb, new_bindex);
output_file = dentry_open(new_lower_dentry, output_mnt,
O_RDWR | O_LARGEFILE);
- if (IS_ERR(output_file)) {
+ if (unlikely(IS_ERR(output_file))) {
err = PTR_ERR(output_file);
goto out_close_in2;
}
- if (!output_file->f_op || !output_file->f_op->write) {
+ if (unlikely(!output_file->f_op || !output_file->f_op->write)) {
err = -EINVAL;
goto out_close_out;
}

/* allocating a buffer */
buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
- if (!buf) {
+ if (unlikely(!buf)) {
err = -ENOMEM;
goto out_close_out;
}
@@ -302,7 +302,7 @@ static int __copyup_reg_data(struct dentry *dentry,
(char __user *)buf,
read_bytes,
&output_file->f_pos);
- if ((write_bytes < 0) || (write_bytes < read_bytes)) {
+ if (unlikely((write_bytes < 0) || (write_bytes < read_bytes))) {
err = write_bytes;
break;
}
@@ -312,11 +312,11 @@ static int __copyup_reg_data(struct dentry *dentry,

kfree(buf);

- if (!err)
+ if (likely(!err))
err = output_file->f_op->fsync(output_file,
new_lower_dentry, 0);

- if (err)
+ if (unlikely(err))
goto out_close_out;

if (copyup_file) {
@@ -399,7 +399,7 @@ int copyup_dentry(struct inode *dir, struct dentry *dentry, int bstart,

/* Create the directory structure above this dentry. */
new_lower_dentry = create_parents(dir, dentry, name, new_bindex);
- if (IS_ERR(new_lower_dentry)) {
+ if (unlikely(IS_ERR(new_lower_dentry))) {
err = PTR_ERR(new_lower_dentry);
goto out;
}
@@ -409,10 +409,10 @@ int copyup_dentry(struct inode *dir, struct dentry *dentry, int bstart,
dget(old_lower_dentry);

/* For symlinks, we must read the link before we lock the directory. */
- if (S_ISLNK(old_lower_dentry->d_inode->i_mode)) {
+ if (unlikely(S_ISLNK(old_lower_dentry->d_inode->i_mode))) {

symbuf = kmalloc(PATH_MAX, GFP_KERNEL);
- if (!symbuf) {
+ if (unlikely(!symbuf)) {
__clear(dentry, old_lower_dentry,
old_bstart, old_bend,
new_lower_dentry, new_bindex);
@@ -427,7 +427,7 @@ int copyup_dentry(struct inode *dir, struct dentry *dentry, int bstart,
(char __user *)symbuf,
PATH_MAX);
set_fs(oldfs);
- if (err < 0) {
+ if (unlikely(err < 0)) {
__clear(dentry, old_lower_dentry,
old_bstart, old_bend,
new_lower_dentry, new_bindex);
@@ -443,7 +443,7 @@ int copyup_dentry(struct inode *dir, struct dentry *dentry, int bstart,
err = __copyup_ndentry(old_lower_dentry, new_lower_dentry,
new_lower_parent_dentry, symbuf);

- if (err) {
+ if (unlikely(err)) {
__clear(dentry, old_lower_dentry,
old_bstart, old_bend,
new_lower_dentry, new_bindex);
@@ -455,22 +455,22 @@ int copyup_dentry(struct inode *dir, struct dentry *dentry, int bstart,
err = __copyup_reg_data(dentry, new_lower_dentry, new_bindex,
old_lower_dentry, old_bindex,
copyup_file, len);
- if (err)
+ if (unlikely(err))
goto out_unlink;

/* Set permissions. */
- if ((err = copyup_permissions(sb, old_lower_dentry,
- new_lower_dentry)))
+ if (unlikely((err = copyup_permissions(sb, old_lower_dentry,
+ new_lower_dentry))))
goto out_unlink;

#ifdef CONFIG_UNION_FS_XATTR
/* Selinux uses extended attributes for permissions. */
- if ((err = copyup_xattrs(old_lower_dentry, new_lower_dentry)))
+ if (unlikely((err = copyup_xattrs(old_lower_dentry, new_lower_dentry))))
goto out_unlink;
#endif /* CONFIG_UNION_FS_XATTR */

/* do not allow files getting deleted to be re-interposed */
- if (!d_deleted(dentry))
+ if (likely(!d_deleted(dentry)))
unionfs_reinterpose(dentry);

goto out_unlock;
@@ -513,11 +513,11 @@ out_free:
dput(old_lower_dentry);
kfree(symbuf);

- if (err)
+ if (unlikely(err))
goto out;
if (!S_ISDIR(dentry->d_inode->i_mode)) {
unionfs_postcopyup_release(dentry);
- if (!unionfs_lower_inode(dentry->d_inode)) {
+ if (unlikely(!unionfs_lower_inode(dentry->d_inode))) {
/*
* If we got here, then we copied up to an
* unlinked-open file, whose name is .unionfsXXXXX.
@@ -551,7 +551,7 @@ int copyup_named_file(struct inode *dir, struct file *file, char *name,

err = copyup_dentry(dir, file->f_path.dentry, bstart, new_bindex,
name, strlen(name), &output_file, len);
- if (!err) {
+ if (likely(!err)) {
fbstart(file) = new_bindex;
unionfs_set_lower_file_idx(file, new_bindex, output_file);
}
@@ -573,7 +573,7 @@ int copyup_file(struct inode *dir, struct file *file, int bstart,
err = copyup_dentry(dir, dentry, bstart, new_bindex,
dentry->d_name.name, dentry->d_name.len,
&output_file, len);
- if (!err) {
+ if (likely(!err)) {
fbstart(file) = new_bindex;
unionfs_set_lower_file_idx(file, new_bindex, output_file);
}
@@ -600,7 +600,7 @@ static void __cleanup_dentry(struct dentry *dentry, int bindex,
* dentries except bindex
*/
for (i = loop_start; i <= loop_end; i++) {
- if (!unionfs_lower_dentry_idx(dentry, i))
+ if (unlikely(!unionfs_lower_dentry_idx(dentry, i)))
continue;

if (i == bindex) {
@@ -623,9 +623,9 @@ static void __cleanup_dentry(struct dentry *dentry, int bindex,
}
}

- if (new_bstart < 0)
+ if (unlikely(new_bstart < 0))
new_bstart = bindex;
- if (new_bend < 0)
+ if (unlikely(new_bend < 0))
new_bend = bindex;
set_dbstart(dentry, new_bstart);
set_dbend(dentry, new_bend);
@@ -679,7 +679,7 @@ struct dentry *create_parents(struct inode *dir, struct dentry *dentry,

verify_locked(dentry);

- if ((err = is_robranch_super(dir->i_sb, bindex))) {
+ if (unlikely((err = is_robranch_super(dir->i_sb, bindex)))) {
lower_dentry = ERR_PTR(err);
goto out;
}
@@ -692,7 +692,7 @@ struct dentry *create_parents(struct inode *dir, struct dentry *dentry,
/* There is no sense allocating any less than the minimum. */
nr_dentry = 1;
path = kmalloc(nr_dentry * sizeof(struct dentry *), GFP_KERNEL);
- if (!path)
+ if (unlikely(!path))
goto out;

/* assume the negative dentry of unionfs as the parent dentry */
@@ -719,13 +719,13 @@ struct dentry *create_parents(struct inode *dir, struct dentry *dentry,
unionfs_lower_dentry_idx(parent_dentry, bindex);

/* grow path table */
- if (count == nr_dentry) {
+ if (unlikely(count == nr_dentry)) {
void *p;

nr_dentry *= 2;
p = krealloc(path, nr_dentry * sizeof(struct dentry *),
GFP_KERNEL);
- if (!p) {
+ if (unlikely(!p)) {
lower_dentry = ERR_PTR(-ENOMEM);
goto out;
}
@@ -757,7 +757,7 @@ begin:
/* lookup child in the underlying file system */
lower_dentry = lookup_one_len(childname, lower_parent_dentry,
childnamelen);
- if (IS_ERR(lower_dentry))
+ if (unlikely(IS_ERR(lower_dentry)))
goto out;
} else {
/*
@@ -766,7 +766,7 @@ begin:
*/
lower_dentry = lookup_one_len(name, lower_parent_dentry,
strlen(name));
- if (IS_ERR(lower_dentry))
+ if (unlikely(IS_ERR(lower_dentry)))
goto out;

/* Replace the current dentry (if any) with the new one */
@@ -797,11 +797,11 @@ begin:
run_sioq(__unionfs_mkdir, &args);
err = args.err;

- if (!err)
+ if (likely(!err))
err = copyup_permissions(dir->i_sb, child_dentry,
lower_dentry);
unlock_dir(lower_parent_dentry);
- if (err) {
+ if (unlikely(err)) {
struct inode *inode = lower_dentry->d_inode;
/*
* If we get here, it means that we created a new
@@ -836,7 +836,7 @@ begin:
goto begin;
out:
/* cleanup any leftover locks from the do/while loop above */
- if (IS_ERR(lower_dentry))
+ if (unlikely(IS_ERR(lower_dentry)))
while (count)
unionfs_unlock_dentry(path[count--]);
kfree(path);
@@ -852,7 +852,7 @@ void unionfs_postcopyup_setmnt(struct dentry *dentry)
struct dentry *parent, *hasone;
int bindex = dbstart(dentry);

- if (unionfs_lower_mnt_idx(dentry, bindex))
+ if (unlikely(unionfs_lower_mnt_idx(dentry, bindex)))
return;
hasone = dentry->d_parent;
/* this loop should stop at root dentry */
--
1.5.2.2

2007-09-26 03:35:26

by Erez Zadok

Subject: [PATCH 16/25] Unionfs: add un/likely conditionals on inode ops

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/inode.c | 160 ++++++++++++++++++++++++++--------------------------
1 files changed, 80 insertions(+), 80 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 7ee4760..7ae4a25 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -35,7 +35,7 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
unionfs_lock_dentry(dentry->d_parent);
valid = __unionfs_d_revalidate_chain(dentry->d_parent, nd, false);
unionfs_unlock_dentry(dentry->d_parent);
- if (!valid) {
+ if (unlikely(!valid)) {
err = -ESTALE; /* same as what real_lookup does */
goto out;
}
@@ -60,26 +60,26 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
* We _always_ create on branch 0
*/
lower_dentry = unionfs_lower_dentry_idx(dentry, 0);
- if (lower_dentry) {
+ if (likely(lower_dentry)) {
/*
* check if whiteout exists in this branch, i.e. lookup .wh.foo
* first.
*/
name = alloc_whname(dentry->d_name.name, dentry->d_name.len);
- if (IS_ERR(name)) {
+ if (unlikely(IS_ERR(name))) {
err = PTR_ERR(name);
goto out;
}

wh_dentry = lookup_one_len(name, lower_dentry->d_parent,
dentry->d_name.len + UNIONFS_WHLEN);
- if (IS_ERR(wh_dentry)) {
+ if (unlikely(IS_ERR(wh_dentry))) {
err = PTR_ERR(wh_dentry);
wh_dentry = NULL;
goto out;
}

- if (wh_dentry->d_inode) {
+ if (unlikely(wh_dentry->d_inode)) {
/*
* .wh.foo has been found, so let's unlink it
*/
@@ -89,7 +89,7 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
err = vfs_unlink(lower_dir_dentry->d_inode, wh_dentry);
unlock_dir(lower_dir_dentry);

- if (err) {
+ if (unlikely(err)) {
printk("unionfs_create: could not unlink "
"whiteout, err = %d\n", err);
goto out;
@@ -102,28 +102,28 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
*/
lower_dentry = create_parents(parent, dentry,
dentry->d_name.name, 0);
- if (IS_ERR(lower_dentry)) {
+ if (unlikely(IS_ERR(lower_dentry))) {
err = PTR_ERR(lower_dentry);
goto out;
}
}

lower_parent_dentry = lock_parent(lower_dentry);
- if (IS_ERR(lower_parent_dentry)) {
+ if (unlikely(IS_ERR(lower_parent_dentry))) {
err = PTR_ERR(lower_parent_dentry);
goto out;
}

err = init_lower_nd(&lower_nd, LOOKUP_CREATE);
- if (err < 0)
+ if (unlikely(err < 0))
goto out;
err = vfs_create(lower_parent_dentry->d_inode, lower_dentry, mode,
&lower_nd);
release_lower_nd(&lower_nd, err);

- if (!err) {
+ if (likely(!err)) {
err = PTR_ERR(unionfs_interpose(dentry, parent->i_sb, 0));
- if (!err) {
+ if (likely(!err)) {
unionfs_copy_attr_times(parent);
fsstack_copy_inode_size(parent,
lower_parent_dentry->d_inode);
@@ -138,13 +138,13 @@ out:
dput(wh_dentry);
kfree(name);

- if (!err)
+ if (likely(!err))
unionfs_postcopyup_setmnt(dentry);
unionfs_unlock_dentry(dentry);
unionfs_read_unlock(dentry->d_sb);

unionfs_check_inode(parent);
- if (!err) {
+ if (likely(!err)) {
unionfs_check_dentry(dentry->d_parent);
unionfs_check_nd(nd);
}
@@ -183,7 +183,7 @@ static struct dentry *unionfs_lookup(struct inode *parent,
nd->dentry = path_save.dentry;
nd->mnt = path_save.mnt;
}
- if (!IS_ERR(ret)) {
+ if (likely(!IS_ERR(ret))) {
if (ret)
dentry = ret;
/* parent times may have changed */
@@ -213,12 +213,12 @@ static int unionfs_link(struct dentry *old_dentry, struct inode *dir,
unionfs_read_lock(old_dentry->d_sb);
unionfs_double_lock_dentry(new_dentry, old_dentry);

- if (!__unionfs_d_revalidate_chain(old_dentry, NULL, false)) {
+ if (unlikely(!__unionfs_d_revalidate_chain(old_dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
- if (new_dentry->d_inode &&
- !__unionfs_d_revalidate_chain(new_dentry, NULL, false)) {
+ if (unlikely(new_dentry->d_inode &&
+ !__unionfs_d_revalidate_chain(new_dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
@@ -230,7 +230,7 @@ static int unionfs_link(struct dentry *old_dentry, struct inode *dir,
* .wh.foo first. If present, delete it
*/
name = alloc_whname(new_dentry->d_name.name, new_dentry->d_name.len);
- if (IS_ERR(name)) {
+ if (unlikely(IS_ERR(name))) {
err = PTR_ERR(name);
goto out;
}
@@ -238,7 +238,7 @@ static int unionfs_link(struct dentry *old_dentry, struct inode *dir,
whiteout_dentry = lookup_one_len(name, lower_new_dentry->d_parent,
new_dentry->d_name.len +
UNIONFS_WHLEN);
- if (IS_ERR(whiteout_dentry)) {
+ if (unlikely(IS_ERR(whiteout_dentry))) {
err = PTR_ERR(whiteout_dentry);
goto out;
}
@@ -250,7 +250,7 @@ static int unionfs_link(struct dentry *old_dentry, struct inode *dir,
/* found a .wh.foo entry, unlink it and then call vfs_link() */
lower_dir_dentry = lock_parent(whiteout_dentry);
err = is_robranch_super(new_dentry->d_sb, dbstart(new_dentry));
- if (!err)
+ if (likely(!err))
err = vfs_unlink(lower_dir_dentry->d_inode,
whiteout_dentry);

@@ -259,7 +259,7 @@ static int unionfs_link(struct dentry *old_dentry, struct inode *dir,
unlock_dir(lower_dir_dentry);
lower_dir_dentry = NULL;
dput(whiteout_dentry);
- if (err)
+ if (unlikely(err))
goto out;
}

@@ -268,9 +268,9 @@ static int unionfs_link(struct dentry *old_dentry, struct inode *dir,
new_dentry->d_name.name,
dbstart(old_dentry));
err = PTR_ERR(lower_new_dentry);
- if (IS_COPYUP_ERR(err))
+ if (unlikely(IS_COPYUP_ERR(err)))
goto docopyup;
- if (!lower_new_dentry || IS_ERR(lower_new_dentry))
+ if (likely(!lower_new_dentry || IS_ERR(lower_new_dentry)))
goto out;
}
lower_new_dentry = unionfs_lower_dentry(new_dentry);
@@ -284,7 +284,7 @@ static int unionfs_link(struct dentry *old_dentry, struct inode *dir,
unlock_dir(lower_dir_dentry);

docopyup:
- if (IS_COPYUP_ERR(err)) {
+ if (unlikely(IS_COPYUP_ERR(err))) {
int old_bstart = dbstart(old_dentry);
int bindex;

@@ -294,7 +294,7 @@ docopyup:
bindex, old_dentry->d_name.name,
old_dentry->d_name.len, NULL,
old_dentry->d_inode->i_size);
- if (!err) {
+ if (likely(!err)) {
lower_new_dentry =
create_parents(dir, new_dentry,
new_dentry->d_name.name,
@@ -315,7 +315,7 @@ docopyup:
}

check_link:
- if (err || !lower_new_dentry->d_inode)
+ if (unlikely(err || !lower_new_dentry->d_inode))
goto out;

/* Its a hard link, so use the same inode */
@@ -334,7 +334,7 @@ out:
d_drop(new_dentry);

kfree(name);
- if (!err)
+ if (likely(!err))
unionfs_postcopyup_setmnt(new_dentry);

unionfs_unlock_dentry(new_dentry);
@@ -362,8 +362,8 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry,
unionfs_read_lock(dentry->d_sb);
unionfs_lock_dentry(dentry);

- if (dentry->d_inode &&
- !__unionfs_d_revalidate_chain(dentry, NULL, false)) {
+ if (unlikely(dentry->d_inode &&
+ !__unionfs_d_revalidate_chain(dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
@@ -378,7 +378,7 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry,
* first. If present, delete it
*/
name = alloc_whname(dentry->d_name.name, dentry->d_name.len);
- if (IS_ERR(name)) {
+ if (unlikely(IS_ERR(name))) {
err = PTR_ERR(name);
goto out;
}
@@ -386,7 +386,7 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry,
whiteout_dentry =
lookup_one_len(name, lower_dentry->d_parent,
dentry->d_name.len + UNIONFS_WHLEN);
- if (IS_ERR(whiteout_dentry)) {
+ if (unlikely(IS_ERR(whiteout_dentry))) {
err = PTR_ERR(whiteout_dentry);
goto out;
}
@@ -412,9 +412,9 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry,

unlock_dir(lower_dir_dentry);

- if (err) {
+ if (unlikely(err)) {
/* exit if the error returned was NOT -EROFS */
- if (!IS_COPYUP_ERR(err))
+ if (unlikely(!IS_COPYUP_ERR(err)))
goto out;
/*
* should now try to create symlink in the another
@@ -442,8 +442,8 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry,
lower_dentry = create_parents(dir, dentry,
dentry->d_name.name,
bindex);
- if (!lower_dentry || IS_ERR(lower_dentry)) {
- if (IS_ERR(lower_dentry))
+ if (unlikely(!lower_dentry || IS_ERR(lower_dentry))) {
+ if (unlikely(IS_ERR(lower_dentry)))
err = PTR_ERR(lower_dentry);

printk(KERN_DEBUG "unionfs: lower dentry "
@@ -462,12 +462,12 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry,
}
unlock_dir(lower_dir_dentry);

- if (err || !lower_dentry->d_inode) {
+ if (unlikely(err || !lower_dentry->d_inode)) {
/*
* break out of for loop if error returned was NOT
* -EROFS.
*/
- if (!IS_COPYUP_ERR(err))
+ if (unlikely(!IS_COPYUP_ERR(err)))
break;
} else {
/*
@@ -476,7 +476,7 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry,
*/
err = PTR_ERR(unionfs_interpose(dentry,
dir->i_sb, 0));
- if (!err) {
+ if (likely(!err)) {
fsstack_copy_attr_times(dir,
lower_dir_dentry->
d_inode);
@@ -498,7 +498,7 @@ out:
d_drop(dentry);

kfree(name);
- if (!err)
+ if (likely(!err))
unionfs_postcopyup_setmnt(dentry);
unionfs_unlock_dentry(dentry);

@@ -522,8 +522,8 @@ static int unionfs_mkdir(struct inode *parent, struct dentry *dentry, int mode)
unionfs_read_lock(dentry->d_sb);
unionfs_lock_dentry(dentry);

- if (dentry->d_inode &&
- !__unionfs_d_revalidate_chain(dentry, NULL, false)) {
+ if (unlikely(dentry->d_inode &&
+ !__unionfs_d_revalidate_chain(dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
@@ -537,14 +537,14 @@ static int unionfs_mkdir(struct inode *parent, struct dentry *dentry, int mode)
* first.
*/
name = alloc_whname(dentry->d_name.name, dentry->d_name.len);
- if (IS_ERR(name)) {
+ if (unlikely(IS_ERR(name))) {
err = PTR_ERR(name);
goto out;
}

whiteout_dentry = lookup_one_len(name, lower_dentry->d_parent,
dentry->d_name.len + UNIONFS_WHLEN);
- if (IS_ERR(whiteout_dentry)) {
+ if (unlikely(IS_ERR(whiteout_dentry))) {
err = PTR_ERR(whiteout_dentry);
goto out;
}
@@ -566,9 +566,9 @@ static int unionfs_mkdir(struct inode *parent, struct dentry *dentry, int mode)

unlock_dir(lower_parent_dentry);

- if (err) {
+ if (unlikely(err)) {
/* exit if the error returned was NOT -EROFS */
- if (!IS_COPYUP_ERR(err))
+ if (unlikely(!IS_COPYUP_ERR(err)))
goto out;
bstart--;
} else
@@ -587,7 +587,7 @@ static int unionfs_mkdir(struct inode *parent, struct dentry *dentry, int mode)
lower_dentry = create_parents(parent, dentry,
dentry->d_name.name,
bindex);
- if (!lower_dentry || IS_ERR(lower_dentry)) {
+ if (unlikely(!lower_dentry || IS_ERR(lower_dentry))) {
printk(KERN_DEBUG "unionfs: lower dentry "
" NULL for bindex = %d\n", bindex);
continue;
@@ -596,7 +596,7 @@ static int unionfs_mkdir(struct inode *parent, struct dentry *dentry, int mode)

lower_parent_dentry = lock_parent(lower_dentry);

- if (IS_ERR(lower_parent_dentry)) {
+ if (unlikely(IS_ERR(lower_parent_dentry))) {
err = PTR_ERR(lower_parent_dentry);
goto out;
}
@@ -607,7 +607,7 @@ static int unionfs_mkdir(struct inode *parent, struct dentry *dentry, int mode)
unlock_dir(lower_parent_dentry);

/* did the mkdir succeed? */
- if (err)
+ if (unlikely(err))
break;

for (i = bindex + 1; i < bend; i++) {
@@ -623,7 +623,7 @@ static int unionfs_mkdir(struct inode *parent, struct dentry *dentry, int mode)
* err.
*/
err = PTR_ERR(unionfs_interpose(dentry, parent->i_sb, 0));
- if (!err) {
+ if (likely(!err)) {
unionfs_copy_attr_times(parent);
fsstack_copy_inode_size(parent,
lower_parent_dentry->d_inode);
@@ -633,7 +633,7 @@ static int unionfs_mkdir(struct inode *parent, struct dentry *dentry, int mode)
}

err = make_dir_opaque(dentry, dbstart(dentry));
- if (err) {
+ if (unlikely(err)) {
printk(KERN_ERR "unionfs: mkdir: error creating "
".wh.__dir_opaque: %d\n", err);
goto out;
@@ -649,7 +649,7 @@ out:

kfree(name);

- if (!err)
+ if (likely(!err))
unionfs_copy_attr_times(dentry->d_inode);
unionfs_unlock_dentry(dentry);
unionfs_check_inode(parent);
@@ -672,8 +672,8 @@ static int unionfs_mknod(struct inode *dir, struct dentry *dentry, int mode,
unionfs_read_lock(dentry->d_sb);
unionfs_lock_dentry(dentry);

- if (dentry->d_inode &&
- !__unionfs_d_revalidate_chain(dentry, NULL, false)) {
+ if (unlikely(dentry->d_inode &&
+ !__unionfs_d_revalidate_chain(dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
@@ -687,14 +687,14 @@ static int unionfs_mknod(struct inode *dir, struct dentry *dentry, int mode,
* first.
*/
name = alloc_whname(dentry->d_name.name, dentry->d_name.len);
- if (IS_ERR(name)) {
+ if (unlikely(IS_ERR(name))) {
err = PTR_ERR(name);
goto out;
}

whiteout_dentry = lookup_one_len(name, lower_dentry->d_parent,
dentry->d_name.len + UNIONFS_WHLEN);
- if (IS_ERR(whiteout_dentry)) {
+ if (unlikely(IS_ERR(whiteout_dentry))) {
err = PTR_ERR(whiteout_dentry);
goto out;
}
@@ -714,8 +714,8 @@ static int unionfs_mknod(struct inode *dir, struct dentry *dentry, int mode,

unlock_dir(lower_parent_dentry);

- if (err) {
- if (!IS_COPYUP_ERR(err))
+ if (unlikely(err)) {
+ if (unlikely(!IS_COPYUP_ERR(err)))
goto out;
bstart--;
} else
@@ -731,7 +731,7 @@ static int unionfs_mknod(struct inode *dir, struct dentry *dentry, int mode,
lower_dentry = create_parents(dir, dentry,
dentry->d_name.name,
bindex);
- if (IS_ERR(lower_dentry)) {
+ if (unlikely(IS_ERR(lower_dentry))) {
printk(KERN_DEBUG "unionfs: failed to create "
"parents on %d, err = %ld\n",
bindex, PTR_ERR(lower_dentry));
@@ -740,7 +740,7 @@ static int unionfs_mknod(struct inode *dir, struct dentry *dentry, int mode,
}

lower_parent_dentry = lock_parent(lower_dentry);
- if (IS_ERR(lower_parent_dentry)) {
+ if (unlikely(IS_ERR(lower_parent_dentry))) {
err = PTR_ERR(lower_parent_dentry);
goto out;
}
@@ -748,7 +748,7 @@ static int unionfs_mknod(struct inode *dir, struct dentry *dentry, int mode,
err = vfs_mknod(lower_parent_dentry->d_inode,
lower_dentry, mode, dev);

- if (err) {
+ if (unlikely(err)) {
unlock_dir(lower_parent_dentry);
break;
}
@@ -758,7 +758,7 @@ static int unionfs_mknod(struct inode *dir, struct dentry *dentry, int mode,
* err.
*/
err = PTR_ERR(unionfs_interpose(dentry, dir->i_sb, 0));
- if (!err) {
+ if (likely(!err)) {
fsstack_copy_attr_times(dir,
lower_parent_dentry->d_inode);
fsstack_copy_inode_size(dir,
@@ -777,7 +777,7 @@ out:

kfree(name);

- if (!err)
+ if (likely(!err))
unionfs_postcopyup_setmnt(dentry);
unionfs_unlock_dentry(dentry);

@@ -797,22 +797,22 @@ static int unionfs_readlink(struct dentry *dentry, char __user *buf,
unionfs_read_lock(dentry->d_sb);
unionfs_lock_dentry(dentry);

- if (!__unionfs_d_revalidate_chain(dentry, NULL, false)) {
+ if (unlikely(!__unionfs_d_revalidate_chain(dentry, NULL, false))) {
err = -ESTALE;
goto out;
}

lower_dentry = unionfs_lower_dentry(dentry);

- if (!lower_dentry->d_inode->i_op ||
- !lower_dentry->d_inode->i_op->readlink) {
+ if (unlikely(!lower_dentry->d_inode->i_op ||
+ !lower_dentry->d_inode->i_op->readlink)) {
err = -EINVAL;
goto out;
}

err = lower_dentry->d_inode->i_op->readlink(lower_dentry,
buf, bufsiz);
- if (err > 0)
+ if (likely(err > 0))
fsstack_copy_attr_atime(dentry->d_inode,
lower_dentry->d_inode);

@@ -844,7 +844,7 @@ static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd)

/* This is freed by the put_link method assuming a successful call. */
buf = kmalloc(len, GFP_KERNEL);
- if (!buf) {
+ if (unlikely(!buf)) {
err = -ENOMEM;
goto out;
}
@@ -854,7 +854,7 @@ static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd)
set_fs(KERNEL_DS);
err = dentry->d_inode->i_op->readlink(dentry, (char __user *)buf, len);
set_fs(old_fs);
- if (err < 0) {
+ if (unlikely(err < 0)) {
kfree(buf);
buf = NULL;
goto out;
@@ -877,7 +877,7 @@ static void unionfs_put_link(struct dentry *dentry, struct nameidata *nd,
unionfs_read_lock(dentry->d_sb);

unionfs_lock_dentry(dentry);
- if (!__unionfs_d_revalidate_chain(dentry, nd, false))
+ if (unlikely(!__unionfs_d_revalidate_chain(dentry, nd, false)))
printk("unionfs: put_link failed to revalidate dentry\n");
unionfs_unlock_dentry(dentry);

@@ -913,7 +913,7 @@ static int inode_permission(struct super_block *sb, struct inode *inode,
/*
* Nobody gets write access to an immutable file.
*/
- if (IS_IMMUTABLE(inode))
+ if (unlikely(IS_IMMUTABLE(inode)))
return -EACCES;
/*
* For all other branches than the first one, we ignore
@@ -959,7 +959,7 @@ static int unionfs_permission(struct inode *inode, int mask,

bstart = ibstart(inode);
bend = ibend(inode);
- if (bstart < 0 || bend < 0) {
+ if (unlikely(bstart < 0 || bend < 0)) {
/*
* With branch-management, we can get a stale inode here.
* If so, we return ESTALE back to link_path_walk, which
@@ -973,7 +973,7 @@ static int unionfs_permission(struct inode *inode, int mask,

for (bindex = bstart; bindex <= bend; bindex++) {
lower_inode = unionfs_lower_inode_idx(inode, bindex);
- if (!lower_inode)
+ if (unlikely(!lower_inode))
continue;

/*
@@ -981,7 +981,7 @@ static int unionfs_permission(struct inode *inode, int mask,
* we don't have to check for files, if we are checking for
* directories.
*/
- if (!is_file && !S_ISDIR(lower_inode->i_mode))
+ if (unlikely(!is_file && !S_ISDIR(lower_inode->i_mode)))
continue;

/*
@@ -995,14 +995,14 @@ static int unionfs_permission(struct inode *inode, int mask,
* The permissions are an intersection of the overall directory
* permissions, so we fail if one fails.
*/
- if (err)
+ if (unlikely(err))
goto out;

/* only the leftmost file matters. */
if (is_file || write_mask) {
if (is_file && write_mask) {
err = get_write_access(lower_inode);
- if (!err)
+ if (unlikely(!err))
put_write_access(lower_inode);
}
break;
@@ -1030,7 +1030,7 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia)
unionfs_read_lock(dentry->d_sb);
unionfs_lock_dentry(dentry);

- if (!__unionfs_d_revalidate_chain(dentry, NULL, false)) {
+ if (unlikely(!__unionfs_d_revalidate_chain(dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
@@ -1042,7 +1042,7 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia)
for (bindex = bstart; (bindex <= bend) || (bindex == bstart);
bindex++) {
lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
- if (!lower_dentry)
+ if (unlikely(!lower_dentry))
continue;
BUG_ON(lower_dentry->d_inode == NULL);

@@ -1062,7 +1062,7 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia)
dentry->d_name.len,
NULL, size);

- if (!err) {
+ if (unlikely(!err)) {
copyup = 1;
lower_dentry =
unionfs_lower_dentry(dentry);
@@ -1078,7 +1078,7 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia)

}
err = notify_change(lower_dentry, ia);
- if (err)
+ if (unlikely(err))
goto out;
break;
}
@@ -1087,7 +1087,7 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia)
if (ia->ia_valid & ATTR_SIZE) {
if (ia->ia_size != i_size_read(inode)) {
err = vmtruncate(inode, ia->ia_size);
- if (err)
+ if (unlikely(err))
printk("unionfs_setattr: vmtruncate failed\n");
}
}
--
1.5.2.2

2007-09-26 03:35:46

by Erez Zadok

Subject: [PATCH 18/25] Unionfs: add un/likely conditionals on super ops

Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/main.c | 98 ++++++++++++++++++++++++++-------------------------
fs/unionfs/super.c | 90 ++++++++++++++++++++++++------------------------
2 files changed, 95 insertions(+), 93 deletions(-)

diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index 8595750..82cb35a 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -32,13 +32,13 @@ static void unionfs_fill_inode(struct dentry *dentry,

for (bindex = bstart; bindex <= bend; bindex++) {
lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
- if (!lower_dentry) {
+ if (unlikely(!lower_dentry)) {
unionfs_set_lower_inode_idx(inode, bindex, NULL);
continue;
}

/* Initialize the lower inode to the new lower inode. */
- if (!lower_dentry->d_inode)
+ if (unlikely(!lower_dentry->d_inode))
continue;

unionfs_set_lower_inode_idx(inode, bindex,
@@ -52,7 +52,7 @@ static void unionfs_fill_inode(struct dentry *dentry,
lower_inode = unionfs_lower_inode(inode);

/* Use different set of inode ops for symlinks & directories */
- if (S_ISLNK(lower_inode->i_mode))
+ if (unlikely(S_ISLNK(lower_inode->i_mode)))
inode->i_op = &unionfs_symlink_iops;
else if (S_ISDIR(lower_inode->i_mode))
inode->i_op = &unionfs_dir_iops;
@@ -62,8 +62,10 @@ static void unionfs_fill_inode(struct dentry *dentry,
inode->i_fop = &unionfs_dir_fops;

/* properly initialize special inodes */
- if (S_ISBLK(lower_inode->i_mode) || S_ISCHR(lower_inode->i_mode) ||
- S_ISFIFO(lower_inode->i_mode) || S_ISSOCK(lower_inode->i_mode))
+ if (unlikely(S_ISBLK(lower_inode->i_mode) ||
+ S_ISCHR(lower_inode->i_mode) ||
+ S_ISFIFO(lower_inode->i_mode) ||
+ S_ISSOCK(lower_inode->i_mode)))
init_special_inode(inode, lower_inode->i_mode,
lower_inode->i_rdev);

@@ -122,14 +124,14 @@ struct dentry *unionfs_interpose(struct dentry *dentry, struct super_block *sb,

UNIONFS_I(inode)->lower_inodes =
kcalloc(sbmax(sb), sizeof(struct inode *), GFP_KERNEL);
- if (!UNIONFS_I(inode)->lower_inodes) {
+ if (unlikely(!UNIONFS_I(inode)->lower_inodes)) {
err = -ENOMEM;
goto out;
}
} else {
/* get unique inode number for unionfs */
inode = iget(sb, iunique(sb, UNIONFS_ROOT_INO));
- if (!inode) {
+ if (unlikely(!inode)) {
err = -EACCES;
goto out;
}
@@ -149,7 +151,7 @@ skip:
break;
case INTERPOSE_LOOKUP:
spliced = d_splice_alias(inode, dentry);
- if (IS_ERR(spliced))
+ if (unlikely(IS_ERR(spliced)))
err = PTR_ERR(spliced);
else if (spliced && spliced != dentry) {
/*
@@ -181,7 +183,7 @@ skip:
goto out;

out_spliced:
- if (!err)
+ if (likely(!err))
return spliced;
out:
return ERR_PTR(err);
@@ -203,12 +205,12 @@ void unionfs_reinterpose(struct dentry *dentry)
bend = dbend(dentry);
for (bindex = bstart; bindex <= bend; bindex++) {
lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
- if (!lower_dentry)
+ if (unlikely(!lower_dentry))
continue;

- if (!lower_dentry->d_inode)
+ if (unlikely(!lower_dentry->d_inode))
continue;
- if (unionfs_lower_inode_idx(inode, bindex))
+ if (unlikely(unionfs_lower_inode_idx(inode, bindex)))
continue;
unionfs_set_lower_inode_idx(inode, bindex,
igrab(lower_dentry->d_inode));
@@ -227,11 +229,11 @@ void unionfs_reinterpose(struct dentry *dentry)
int check_branch(struct nameidata *nd)
{
/* XXX: remove in ODF code -- stacking unions allowed there */
- if (!strcmp(nd->dentry->d_sb->s_type->name, "unionfs"))
+ if (unlikely(!strcmp(nd->dentry->d_sb->s_type->name, "unionfs")))
return -EINVAL;
- if (!nd->dentry->d_inode)
+ if (unlikely(!nd->dentry->d_inode))
return -ENOENT;
- if (!S_ISDIR(nd->dentry->d_inode->i_mode))
+ if (unlikely(!S_ISDIR(nd->dentry->d_inode->i_mode)))
return -ENOTDIR;
return 0;
}
@@ -245,7 +247,7 @@ static int is_branch_overlap(struct dentry *dent1, struct dentry *dent2)
while ((dent != dent2) && (dent->d_parent != dent))
dent = dent->d_parent;

- if (dent == dent2)
+ if (unlikely(dent == dent2))
return 1;

dent = dent2;
@@ -260,7 +262,7 @@ static int is_branch_overlap(struct dentry *dent1, struct dentry *dent2)
*/
int __parse_branch_mode(const char *name)
{
- if (!name)
+ if (unlikely(!name))
return 0;
if (!strcmp(name, "ro"))
return MAY_READ;
@@ -302,7 +304,7 @@ static int parse_dirs_option(struct super_block *sb, struct unionfs_dentry_info
struct dentry *dent1;
struct dentry *dent2;

- if (options[0] == '\0') {
+ if (unlikely(options[0] == '\0')) {
printk(KERN_WARNING "unionfs: no branches specified\n");
err = -EINVAL;
goto out;
@@ -319,14 +321,14 @@ static int parse_dirs_option(struct super_block *sb, struct unionfs_dentry_info
/* allocate space for underlying pointers to lower dentry */
UNIONFS_SB(sb)->data =
kcalloc(branches, sizeof(struct unionfs_data), GFP_KERNEL);
- if (!UNIONFS_SB(sb)->data) {
+ if (unlikely(!UNIONFS_SB(sb)->data)) {
err = -ENOMEM;
goto out;
}

lower_root_info->lower_paths =
kcalloc(branches, sizeof(struct path), GFP_KERNEL);
- if (!lower_root_info->lower_paths) {
+ if (unlikely(!lower_root_info->lower_paths)) {
err = -ENOMEM;
goto out;
}
@@ -339,7 +341,7 @@ static int parse_dirs_option(struct super_block *sb, struct unionfs_dentry_info

if (!name)
continue;
- if (!*name) { /* bad use of ':' (extra colons) */
+ if (unlikely(!*name)) { /* bad use of ':' (extra colons)) */
err = -EINVAL;
goto out;
}
@@ -351,20 +353,20 @@ static int parse_dirs_option(struct super_block *sb, struct unionfs_dentry_info
*mode++ = '\0';

perms = parse_branch_mode(mode);
- if (!bindex && !(perms & MAY_WRITE)) {
+ if (unlikely(!bindex && !(perms & MAY_WRITE))) {
err = -EINVAL;
goto out;
}

err = path_lookup(name, LOOKUP_FOLLOW, &nd);
- if (err) {
+ if (unlikely(err)) {
printk(KERN_WARNING "unionfs: error accessing "
"lower directory '%s' (error %d)\n",
name, err);
goto out;
}

- if ((err = check_branch(&nd))) {
+ if (unlikely((err = check_branch(&nd)))) {
printk(KERN_WARNING "unionfs: lower directory "
"'%s' is not a valid branch\n", name);
path_release(&nd);
@@ -384,7 +386,7 @@ static int parse_dirs_option(struct super_block *sb, struct unionfs_dentry_info
bindex++;
}

- if (branches == 0) {
+ if (unlikely(branches == 0)) {
printk(KERN_WARNING "unionfs: no branches specified\n");
err = -EINVAL;
goto out;
@@ -411,7 +413,7 @@ static int parse_dirs_option(struct super_block *sb, struct unionfs_dentry_info
dent1 = lower_root_info->lower_paths[i].dentry;
for (j = i + 1; j < branches; j++) {
dent2 = lower_root_info->lower_paths[j].dentry;
- if (is_branch_overlap(dent1, dent2)) {
+ if (unlikely(is_branch_overlap(dent1, dent2))) {
printk(KERN_WARNING "unionfs: branches %d and "
"%d overlap\n", i, j);
err = -EINVAL;
@@ -421,7 +423,7 @@ static int parse_dirs_option(struct super_block *sb, struct unionfs_dentry_info
}

out:
- if (err) {
+ if (unlikely(err)) {
for (i = 0; i < branches; i++)
if (lower_root_info->lower_paths[i].dentry) {
dput(lower_root_info->lower_paths[i].dentry);
@@ -462,7 +464,7 @@ static struct unionfs_dentry_info *unionfs_parse_options(
err = -ENOMEM;
lower_root_info =
kzalloc(sizeof(struct unionfs_dentry_info), GFP_KERNEL);
- if (!lower_root_info)
+ if (unlikely(!lower_root_info))
goto out_error;
lower_root_info->bstart = -1;
lower_root_info->bend = -1;
@@ -473,7 +475,7 @@ static struct unionfs_dentry_info *unionfs_parse_options(
char *endptr;
int intval;

- if (!optname || !*optname)
+ if (unlikely(!optname || !*optname))
continue;

optarg = strchr(optname, '=');
@@ -484,28 +486,28 @@ static struct unionfs_dentry_info *unionfs_parse_options(
* All of our options take an argument now. Insert ones that
* don't, above this check.
*/
- if (!optarg) {
+ if (unlikely(!optarg)) {
printk("unionfs: %s requires an argument.\n", optname);
err = -EINVAL;
goto out_error;
}

if (!strcmp("dirs", optname)) {
- if (++dirsfound > 1) {
+ if (unlikely(++dirsfound > 1)) {
printk(KERN_WARNING
"unionfs: multiple dirs specified\n");
err = -EINVAL;
goto out_error;
}
err = parse_dirs_option(sb, lower_root_info, optarg);
- if (err)
+ if (unlikely(err))
goto out_error;
continue;
}

/* All of these options require an integer argument. */
intval = simple_strtoul(optarg, &endptr, 0);
- if (*endptr) {
+ if (unlikely(*endptr)) {
printk(KERN_WARNING
"unionfs: invalid %s option '%s'\n",
optname, optarg);
@@ -518,7 +520,7 @@ static struct unionfs_dentry_info *unionfs_parse_options(
"unionfs: unrecognized option '%s'\n", optname);
goto out_error;
}
- if (dirsfound != 1) {
+ if (unlikely(dirsfound != 1)) {
printk(KERN_WARNING "unionfs: dirs option required\n");
err = -EINVAL;
goto out_error;
@@ -563,11 +565,11 @@ static struct dentry *unionfs_d_alloc_root(struct super_block *sb)
{
struct dentry *ret = NULL;

- if (sb) {
+ if (likely(sb)) {
static const struct qstr name = {.name = "/",.len = 1 };

ret = d_alloc(NULL, &name);
- if (ret) {
+ if (likely(ret)) {
ret->d_op = &unionfs_dops;
ret->d_sb = sb;
ret->d_parent = ret;
@@ -587,7 +589,7 @@ static int unionfs_read_super(struct super_block *sb, void *raw_data,
struct unionfs_dentry_info *lower_root_info = NULL;
int bindex, bstart, bend;

- if (!raw_data) {
+ if (unlikely(!raw_data)) {
printk(KERN_WARNING
"unionfs: read_super: missing data argument\n");
err = -EINVAL;
@@ -596,7 +598,7 @@ static int unionfs_read_super(struct super_block *sb, void *raw_data,

/* Allocate superblock private data */
sb->s_fs_info = kzalloc(sizeof(struct unionfs_sb_info), GFP_KERNEL);
- if (!UNIONFS_SB(sb)) {
+ if (unlikely(!UNIONFS_SB(sb))) {
printk(KERN_WARNING "unionfs: read_super: out of memory\n");
err = -ENOMEM;
goto out;
@@ -608,7 +610,7 @@ static int unionfs_read_super(struct super_block *sb, void *raw_data,
UNIONFS_SB(sb)->high_branch_id = -1; /* -1 == invalid branch ID */

lower_root_info = unionfs_parse_options(sb, raw_data);
- if (IS_ERR(lower_root_info)) {
+ if (unlikely(IS_ERR(lower_root_info))) {
printk(KERN_WARNING
"unionfs: read_super: error while parsing options "
"(err = %ld)\n", PTR_ERR(lower_root_info));
@@ -616,7 +618,7 @@ static int unionfs_read_super(struct super_block *sb, void *raw_data,
lower_root_info = NULL;
goto out_free;
}
- if (lower_root_info->bstart == -1) {
+ if (unlikely(lower_root_info->bstart == -1)) {
err = -ENOENT;
goto out_free;
}
@@ -637,14 +639,14 @@ static int unionfs_read_super(struct super_block *sb, void *raw_data,

/* See comment next to the definition of unionfs_d_alloc_root */
sb->s_root = unionfs_d_alloc_root(sb);
- if (!sb->s_root) {
+ if (unlikely(!sb->s_root)) {
err = -ENOMEM;
goto out_dput;
}

/* link the upper and lower dentries */
sb->s_root->d_fsdata = NULL;
- if ((err = new_dentry_private_data(sb->s_root)))
+ if (unlikely((err = new_dentry_private_data(sb->s_root))))
goto out_freedpd;

/* Set the lower dentries for s_root */
@@ -670,7 +672,7 @@ static int unionfs_read_super(struct super_block *sb, void *raw_data,
*/
err = PTR_ERR(unionfs_interpose(sb->s_root, sb, 0));
unionfs_unlock_dentry(sb->s_root);
- if (!err)
+ if (likely(!err))
goto out;
/* else fall through */

@@ -734,17 +736,17 @@ static int __init init_unionfs_fs(void)

printk("Registering unionfs " UNIONFS_VERSION "\n");

- if ((err = unionfs_init_filldir_cache()))
+ if (unlikely((err = unionfs_init_filldir_cache())))
goto out;
- if ((err = unionfs_init_inode_cache()))
+ if (unlikely((err = unionfs_init_inode_cache())))
goto out;
- if ((err = unionfs_init_dentry_cache()))
+ if (unlikely((err = unionfs_init_dentry_cache())))
goto out;
- if ((err = init_sioq()))
+ if (unlikely((err = init_sioq())))
goto out;
err = register_filesystem(&unionfs_fs_type);
out:
- if (err) {
+ if (unlikely(err)) {
stop_sioq();
unionfs_destroy_filldir_cache();
unionfs_destroy_inode_cache();
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 4e0fe7c..b07bcb7 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -44,7 +44,7 @@ static void unionfs_read_inode(struct inode *inode)

size = sbmax(inode->i_sb) * sizeof(struct inode *);
info->lower_inodes = kzalloc(size, GFP_KERNEL);
- if (!info->lower_inodes) {
+ if (unlikely(!info->lower_inodes)) {
printk(KERN_ERR "unionfs: no kernel memory when allocating "
"lower-pointer array!\n");
BUG();
@@ -90,7 +90,7 @@ static void unionfs_put_super(struct super_block *sb)
int leaks = 0;

spd = UNIONFS_SB(sb);
- if (!spd)
+ if (unlikely(!spd))
return;

bstart = sbstart(sb);
@@ -98,7 +98,7 @@ static void unionfs_put_super(struct super_block *sb)

/* Make sure we have no leaks of branchget/branchput. */
for (bindex = bstart; bindex <= bend; bindex++)
- if (branch_count(sb, bindex) != 0) {
+ if (unlikely(branch_count(sb, bindex) != 0)) {
printk("unionfs: branch %d has %d references left!\n",
bindex, branch_count(sb, bindex));
leaks = 1;
@@ -126,7 +126,7 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf)
unionfs_read_lock(sb);
unionfs_lock_dentry(dentry);

- if (!__unionfs_d_revalidate_chain(dentry, NULL, false)) {
+ if (unlikely(!__unionfs_d_revalidate_chain(dentry, NULL, false))) {
err = -ESTALE;
goto out;
}
@@ -170,17 +170,17 @@ static noinline int do_remount_mode_option(char *optarg, int cur_branches,
struct nameidata nd;

/* by now, optarg contains the branch name */
- if (!*optarg) {
+ if (unlikely(!*optarg)) {
printk("unionfs: no branch specified for mode change.\n");
goto out;
}
- if (!modename) {
+ if (unlikely(!modename)) {
printk("unionfs: branch \"%s\" requires a mode.\n", optarg);
goto out;
}
*modename++ = '\0';
perms = __parse_branch_mode(modename);
- if (perms == 0) {
+ if (unlikely(perms == 0)) {
printk("unionfs: invalid mode \"%s\" for \"%s\".\n",
modename, optarg);
goto out;
@@ -193,7 +193,7 @@ static noinline int do_remount_mode_option(char *optarg, int cur_branches,
* uniqueness.
*/
err = path_lookup(optarg, LOOKUP_FOLLOW, &nd);
- if (err) {
+ if (unlikely(err)) {
printk(KERN_WARNING "unionfs: error accessing "
"lower directory \"%s\" (error %d)\n",
optarg, err);
@@ -204,7 +204,7 @@ static noinline int do_remount_mode_option(char *optarg, int cur_branches,
nd.dentry == new_lower_paths[idx].dentry)
break;
path_release(&nd); /* no longer needed */
- if (idx == cur_branches) {
+ if (unlikely(idx == cur_branches)) {
err = -ENOENT; /* err may have been reset above */
printk(KERN_WARNING "unionfs: branch \"%s\" "
"not found\n", optarg);
@@ -236,7 +236,7 @@ static noinline int do_remount_del_option(char *optarg, int cur_branches,
* uniqueness.
*/
err = path_lookup(optarg, LOOKUP_FOLLOW, &nd);
- if (err) {
+ if (unlikely(err)) {
printk(KERN_WARNING "unionfs: error accessing "
"lower directory \"%s\" (error %d)\n",
optarg, err);
@@ -247,14 +247,14 @@ static noinline int do_remount_del_option(char *optarg, int cur_branches,
nd.dentry == new_lower_paths[idx].dentry)
break;
path_release(&nd); /* no longer needed */
- if (idx == cur_branches) {
+ if (unlikely(idx == cur_branches)) {
printk(KERN_WARNING "unionfs: branch \"%s\" "
"not found\n", optarg);
err = -ENOENT;
goto out;
}
/* check if there are any open files on the branch to be deleted */
- if (atomic_read(&new_data[idx].open_files) > 0) {
+ if (unlikely(atomic_read(&new_data[idx].open_files) > 0)) {
err = -EBUSY;
goto out;
}
@@ -320,7 +320,7 @@ static noinline int do_remount_add_option(char *optarg, int cur_branches,
* uniqueness.
*/
err = path_lookup(optarg, LOOKUP_FOLLOW, &nd);
- if (err) {
+ if (unlikely(err)) {
printk(KERN_WARNING "unionfs: error accessing "
"lower directory \"%s\" (error %d)\n",
optarg, err);
@@ -331,7 +331,7 @@ static noinline int do_remount_add_option(char *optarg, int cur_branches,
nd.dentry == new_lower_paths[idx].dentry)
break;
path_release(&nd); /* no longer needed */
- if (idx == cur_branches) {
+ if (unlikely(idx == cur_branches)) {
printk(KERN_WARNING "unionfs: branch \"%s\" "
"not found\n", optarg);
err = -ENOENT;
@@ -350,13 +350,13 @@ found_insertion_point:
*modename++ = '\0';
perms = parse_branch_mode(modename);

- if (!new_branch || !*new_branch) {
+ if (unlikely(!new_branch || !*new_branch)) {
printk(KERN_WARNING "unionfs: null new branch\n");
err = -EINVAL;
goto out;
}
err = path_lookup(new_branch, LOOKUP_FOLLOW, &nd);
- if (err) {
+ if (unlikely(err)) {
printk(KERN_WARNING "unionfs: error accessing "
"lower directory \"%s\" (error %d)\n",
new_branch, err);
@@ -369,7 +369,7 @@ found_insertion_point:
* because this code base doesn't support stacking unionfs: the ODF
* code base supports that correctly.
*/
- if ((err = check_branch(&nd))) {
+ if (unlikely((err = check_branch(&nd)))) {
printk(KERN_WARNING "unionfs: lower directory "
"\"%s\" is not a valid branch\n", optarg);
path_release(&nd);
@@ -453,7 +453,7 @@ static int unionfs_remount_fs(struct super_block *sb, int *flags,
* need to check if any other flags may have been passed (none are
* allowed/supported as of now).
*/
- if ((*flags & ~(MS_RDONLY | MS_SILENT)) != 0) {
+ if (unlikely((*flags & ~(MS_RDONLY | MS_SILENT)) != 0)) {
printk(KERN_WARNING
"unionfs: remount flags 0x%x unsupported\n", *flags);
err = -EINVAL;
@@ -465,7 +465,7 @@ static int unionfs_remount_fs(struct super_block *sb, int *flags,
* the union to a "ro" or "rw" and the VFS took care of it. So
* nothing to do and we're done.
*/
- if (!options || options[0] == '\0')
+ if (unlikely(!options || options[0] == '\0'))
goto out_error;

/*
@@ -474,7 +474,7 @@ static int unionfs_remount_fs(struct super_block *sb, int *flags,
* strsep modifies the string and we need it later.
*/
optionstmp = tmp_to_free = kstrdup(options, GFP_KERNEL);
- if (!optionstmp) {
+ if (unlikely(!optionstmp)) {
err = -ENOMEM;
goto out_free;
}
@@ -484,7 +484,7 @@ static int unionfs_remount_fs(struct super_block *sb, int *flags,
while ((optname = strsep(&optionstmp, ",")) != NULL) {
char *optarg;

- if (!optname || !*optname)
+ if (unlikely(!optname || !*optname))
continue;

optarg = strchr(optname, '=');
@@ -498,7 +498,7 @@ static int unionfs_remount_fs(struct super_block *sb, int *flags,
}
kfree(tmp_to_free);
/* after all changes, will we have at least one branch left? */
- if ((new_branches + add_branches - del_branches) < 1) {
+ if (unlikely((new_branches + add_branches - del_branches) < 1)) {
printk(KERN_WARNING
"unionfs: no branches left after remount\n");
err = -EINVAL;
@@ -521,14 +521,14 @@ static int unionfs_remount_fs(struct super_block *sb, int *flags,
/* allocate space for new pointers to lower dentry */
tmp_data = kcalloc(max_branches,
sizeof(struct unionfs_data), GFP_KERNEL);
- if (!tmp_data) {
+ if (unlikely(!tmp_data)) {
err = -ENOMEM;
goto out_free;
}
/* allocate space for new pointers to lower paths */
tmp_lower_paths = kcalloc(max_branches,
sizeof(struct path), GFP_KERNEL);
- if (!tmp_lower_paths) {
+ if (unlikely(!tmp_lower_paths)) {
err = -ENOMEM;
goto out_free;
}
@@ -556,7 +556,7 @@ static int unionfs_remount_fs(struct super_block *sb, int *flags,
while ((optname = strsep(&options, ",")) != NULL) {
char *optarg;

- if (!optname || !*optname)
+ if (unlikely(!optname || !*optname))
continue;
/*
* At this stage optname holds a comma-delimited option, but
@@ -579,7 +579,7 @@ static int unionfs_remount_fs(struct super_block *sb, int *flags,
* that don't above this check.) So at this stage optname
* contains the CMD part and optarg contains the ARG part.
*/
- if (!optarg || !*optarg) {
+ if (unlikely(!optarg || !*optarg)) {
printk("unionfs: all remount options require "
"an argument (%s).\n", optname);
err = -EINVAL;
@@ -591,10 +591,10 @@ static int unionfs_remount_fs(struct super_block *sb, int *flags,
tmp_data,
tmp_lower_paths,
&new_high_branch_id);
- if (err)
+ if (unlikely(err))
goto out_release;
new_branches++;
- if (new_branches > UNIONFS_MAX_BRANCHES) {
+ if (unlikely(new_branches > UNIONFS_MAX_BRANCHES)) {
printk("unionfs: command exceeds "
"%d branches\n", UNIONFS_MAX_BRANCHES);
err = -E2BIG;
@@ -606,7 +606,7 @@ static int unionfs_remount_fs(struct super_block *sb, int *flags,
err = do_remount_del_option(optarg, new_branches,
tmp_data,
tmp_lower_paths);
- if (err)
+ if (unlikely(err))
goto out_release;
new_branches--;
continue;
@@ -615,7 +615,7 @@ static int unionfs_remount_fs(struct super_block *sb, int *flags,
err = do_remount_mode_option(optarg, new_branches,
tmp_data,
tmp_lower_paths);
- if (err)
+ if (unlikely(err))
goto out_release;
continue;
}
@@ -629,7 +629,7 @@ static int unionfs_remount_fs(struct super_block *sb, int *flags,
* actually process the ro/rw remount options, we have to
* return 0 from this function.
*/
- if (!strcmp("dirs", optname)) {
+ if (unlikely(!strcmp("dirs", optname))) {
printk(KERN_WARNING
"unionfs: remount ignoring option \"%s\".\n",
optname);
@@ -652,7 +652,7 @@ out_no_change:
* have to be re-read.
*******************************************************************/

- if (!(tmp_data[0].branchperms & MAY_WRITE)) {
+ if (unlikely(!(tmp_data[0].branchperms & MAY_WRITE))) {
printk("unionfs: leftmost branch cannot be read-only "
"(use \"remount,ro\" to create a read-only union)\n");
err = -EINVAL;
@@ -662,7 +662,7 @@ out_no_change:
/* (re)allocate space for new pointers to lower dentry */
size = new_branches * sizeof(struct unionfs_data);
new_data = krealloc(tmp_data, size, GFP_KERNEL);
- if (!new_data) {
+ if (unlikely(!new_data)) {
err = -ENOMEM;
goto out_release;
}
@@ -670,7 +670,7 @@ out_no_change:
/* allocate space for new pointers to lower paths */
size = new_branches * sizeof(struct path);
new_lower_paths = krealloc(tmp_lower_paths, size, GFP_KERNEL);
- if (!new_lower_paths) {
+ if (unlikely(!new_lower_paths)) {
err = -ENOMEM;
goto out_release;
}
@@ -678,7 +678,7 @@ out_no_change:
/* allocate space for new pointers to lower inodes */
new_lower_inodes = kcalloc(new_branches,
sizeof(struct inode *), GFP_KERNEL);
- if (!new_lower_inodes) {
+ if (unlikely(!new_lower_inodes)) {
err = -ENOMEM;
goto out_release;
}
@@ -765,7 +765,7 @@ out_no_change:
i = atomic_inc_return(&UNIONFS_SB(sb)->generation);
atomic_set(&UNIONFS_D(sb->s_root)->generation, i);
atomic_set(&UNIONFS_I(sb->s_root->d_inode)->generation, i);
- if (!(*flags & MS_SILENT))
+ if (likely(!(*flags & MS_SILENT)))
printk("unionfs: new generation number %d\n", i);
/* finally, update the root dentry's times */
unionfs_copy_attr_times(sb->s_root->d_inode);
@@ -781,7 +781,7 @@ out_no_change:
*/
out_release:
/* no need to cleanup/release anything in tmp_data */
- if (tmp_lower_paths)
+ if (likely(tmp_lower_paths))
for (i=0; i<new_branches; i++)
pathput(&tmp_lower_paths[i]);
out_free:
@@ -823,10 +823,10 @@ static void unionfs_clear_inode(struct inode *inode)
*/
bstart = ibstart(inode);
bend = ibend(inode);
- if (bstart >= 0) {
+ if (likely(bstart >= 0)) {
for (bindex = bstart; bindex <= bend; bindex++) {
lower_inode = unionfs_lower_inode_idx(inode, bindex);
- if (!lower_inode)
+ if (unlikely(!lower_inode))
continue;
iput(lower_inode);
}
@@ -841,7 +841,7 @@ static struct inode *unionfs_alloc_inode(struct super_block *sb)
struct unionfs_inode_info *i;

i = kmem_cache_alloc(unionfs_inode_cachep, GFP_KERNEL);
- if (!i)
+ if (unlikely(!i))
return NULL;

/* memset everything up to the inode to 0 */
@@ -872,7 +872,7 @@ int unionfs_init_inode_cache(void)
kmem_cache_create("unionfs_inode_cache",
sizeof(struct unionfs_inode_info), 0,
SLAB_RECLAIM_ACCOUNT, init_once);
- if (!unionfs_inode_cachep)
+ if (unlikely(!unionfs_inode_cachep))
err = -ENOMEM;
return err;
}
@@ -880,7 +880,7 @@ int unionfs_init_inode_cache(void)
/* unionfs inode cache destructor */
void unionfs_destroy_inode_cache(void)
{
- if (unionfs_inode_cachep)
+ if (likely(unionfs_inode_cachep))
kmem_cache_destroy(unionfs_inode_cachep);
}

@@ -920,7 +920,7 @@ static void unionfs_umount_begin(struct vfsmount *mnt, int flags)
struct vfsmount *lower_mnt;
int bindex, bstart, bend;

- if (!(flags & MNT_FORCE))
+ if (likely(!(flags & MNT_FORCE)))
/*
* we are not being MNT_FORCE'd, therefore we should emulate
* old behavior
@@ -959,7 +959,7 @@ static int unionfs_show_options(struct seq_file *m, struct vfsmount *mnt)
unionfs_lock_dentry(sb->s_root);

tmp_page = (char*) __get_free_page(GFP_KERNEL);
- if (!tmp_page) {
+ if (unlikely(!tmp_page)) {
ret = -ENOMEM;
goto out;
}
@@ -972,7 +972,7 @@ static int unionfs_show_options(struct seq_file *m, struct vfsmount *mnt)
path = d_path(unionfs_lower_dentry_idx(sb->s_root, bindex),
unionfs_lower_mnt_idx(sb->s_root, bindex),
tmp_page, PAGE_SIZE);
- if (IS_ERR(path)) {
+ if (unlikely(IS_ERR(path))) {
ret = PTR_ERR(path);
goto out;
}
--
1.5.2.2

2007-09-26 03:36:08

by Erez Zadok

Subject: [PATCH 02/25] Unionfs: Remove unused #defines

From: Josef 'Jeff' Sipek <[email protected]>

Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/union.h | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 1cb2e1d..140b8ae 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -437,10 +437,6 @@ static inline int is_robranch(const struct dentry *dentry)
#define UNIONFS_DIR_OPAQUE_NAME "__dir_opaque"
#define UNIONFS_DIR_OPAQUE UNIONFS_WHPFX UNIONFS_DIR_OPAQUE_NAME

-#ifndef DEFAULT_POLLMASK
-#define DEFAULT_POLLMASK (POLLIN | POLLOUT | POLLRDNORM | POLLWRNORM)
-#endif /* not DEFAULT_POLLMASK */
-
/*
* EXTERNALS:
*/
--
1.5.2.2

2007-09-26 03:36:28

by Erez Zadok

Subject: [PATCH 04/25] Unionfs: cache-coherency fixes

From: Olivier Blin <[email protected]>

Do not update mtime if there is no upper branch for the inode. This
prevents from calling unionfs_lower_inode_idx() with a negative index, which
triggers a bug.

Signed-off-by: Olivier Blin <[email protected]>
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/fanout.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/unionfs/fanout.h b/fs/unionfs/fanout.h
index afeb9f6..51aa0de 100644
--- a/fs/unionfs/fanout.h
+++ b/fs/unionfs/fanout.h
@@ -308,7 +308,7 @@ static inline void unionfs_copy_attr_times(struct inode *upper)
int bindex;
struct inode *lower;

- if (!upper)
+ if (!upper || ibstart(upper) < 0)
return;
for (bindex=ibstart(upper); bindex <= ibend(upper); bindex++) {
lower = unionfs_lower_inode_idx(upper, bindex);
--
1.5.2.2

2007-09-26 03:36:46

by Erez Zadok

Subject: [PATCH 01/25] Unionfs: Simplify unionfs_get_nlinks

From: Josef 'Jeff' Sipek <[email protected]>

Since we set the right value for d_type in readdir, there's really no point
in having to calculate the number of directory links. Some on-disk
filesystems don't even store the number of links for directories.

Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/subr.c | 41 +++++++----------------------------------
1 files changed, 7 insertions(+), 34 deletions(-)

diff --git a/fs/unionfs/subr.c b/fs/unionfs/subr.c
index b7e7904..6b93b64 100644
--- a/fs/unionfs/subr.c
+++ b/fs/unionfs/subr.c
@@ -188,16 +188,10 @@ out:
}

/*
- * returns the sum of the n_link values of all the underlying inodes of the
- * passed inode
+ * returns the right n_link value based on the inode type
*/
int unionfs_get_nlinks(const struct inode *inode)
{
- int sum_nlinks = 0;
- int dirs = 0;
- int bindex;
- struct inode *lower_inode;
-
/* don't bother to do all the work since we're unlinked */
if (inode->i_nlink == 0)
return 0;
@@ -205,33 +199,12 @@ int unionfs_get_nlinks(const struct inode *inode)
if (!S_ISDIR(inode->i_mode))
return unionfs_lower_inode(inode)->i_nlink;

- for (bindex = ibstart(inode); bindex <= ibend(inode); bindex++) {
- lower_inode = unionfs_lower_inode_idx(inode, bindex);
-
- /* ignore files */
- if (!lower_inode || !S_ISDIR(lower_inode->i_mode))
- continue;
-
- BUG_ON(lower_inode->i_nlink < 0);
-
- /* A deleted directory. */
- if (lower_inode->i_nlink == 0)
- continue;
- dirs++;
-
- /*
- * A broken directory...
- *
- * Some filesystems don't properly set the number of links
- * on empty directories
- */
- if (lower_inode->i_nlink == 1)
- sum_nlinks += 2;
- else
- sum_nlinks += (lower_inode->i_nlink - 2);
- }
-
- return (!dirs ? 0 : sum_nlinks + 2);
+ /*
+ * For directories, we return 1. The only place that could cares
+ * about links is readdir, and there's d_type there so even that
+ * doesn't matter.
+ */
+ return 1;
}

/* construct whiteout filename */
--
1.5.2.2

2007-09-26 03:37:04

by Erez Zadok

Subject: [PATCH 25/25] Unionfs: use poison.h for safe poison pointers

This also fixes a compile warning on 64-bit systems.

Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
Signed-off-by: Erez Zadok <[email protected]>
---
fs/unionfs/debug.c | 16 ++++++----------
fs/unionfs/union.h | 1 +
2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/fs/unionfs/debug.c b/fs/unionfs/debug.c
index 09b52ce..b103eb9 100644
--- a/fs/unionfs/debug.c
+++ b/fs/unionfs/debug.c
@@ -25,14 +25,6 @@
} \
} while (0)

-#if BITS_PER_LONG == 32
-#define POISONED_PTR ((void*) 0x5a5a5a5a)
-#elif BITS_PER_LONG == 64
-#define POISONED_PTR ((void*) 0x5a5a5a5a5a5a5a5a)
-#else
-#error Unknown BITS_PER_LONG value
-#endif /* BITS_PER_LONG != known */
-
/*
* __unionfs_check_{inode,dentry,file} perform exhaustive sanity checking on
* the fan-out of various Unionfs objects. We check that no lower objects
@@ -50,6 +42,7 @@ void __unionfs_check_inode(const struct inode *inode,
struct inode *lower_inode;
struct super_block *sb;
int printed_caller = 0;
+ void *poison_ptr;

/* for inodes now */
BUG_ON(!inode);
@@ -88,12 +81,13 @@ void __unionfs_check_inode(const struct inode *inode,
}
lower_inode = unionfs_lower_inode_idx(inode, bindex);
if (lower_inode) {
+ memset(&poison_ptr, POISON_INUSE, sizeof(void *));
if (unlikely(bindex < istart || bindex > iend)) {
PRINT_CALLER(fname, fxn, line);
printk(" Ci5: inode/linode=%p:%p bindex=%d "
"istart/end=%d:%d\n", inode,
lower_inode, bindex, istart, iend);
- } else if (unlikely(lower_inode == POISONED_PTR)) {
+ } else if (unlikely(lower_inode == poison_ptr)) {
/* freed inode! */
PRINT_CALLER(fname, fxn, line);
printk(" Ci6: inode/linode=%p:%p bindex=%d "
@@ -131,6 +125,7 @@ void __unionfs_check_dentry(const struct dentry *dentry,
struct super_block *sb;
struct vfsmount *lower_mnt;
int printed_caller = 0;
+ void *poison_ptr;

BUG_ON(!dentry);
sb = dentry->d_sb;
@@ -257,12 +252,13 @@ void __unionfs_check_dentry(const struct dentry *dentry,
for (bindex = sbstart(sb); bindex < sbmax(sb); bindex++) {
lower_inode = unionfs_lower_inode_idx(inode, bindex);
if (lower_inode) {
+ memset(&poison_ptr, POISON_INUSE, sizeof(void *));
if (unlikely(bindex < istart || bindex > iend)) {
PRINT_CALLER(fname, fxn, line);
printk(" CI5: dentry/linode=%p:%p bindex=%d "
"istart/end=%d:%d\n", dentry,
lower_inode, bindex, istart, iend);
- } else if (unlikely(lower_inode == POISONED_PTR)) {
+ } else if (unlikely(lower_inode == poison_ptr)) {
/* freed inode! */
PRINT_CALLER(fname, fxn, line);
printk(" CI6: dentry/linode=%p:%p bindex=%d "
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 8df44a9..510267f 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -43,6 +43,7 @@
#include <linux/fs_stack.h>
#include <linux/magic.h>
#include <linux/log2.h>
+#include <linux/poison.h>

#include <asm/mman.h>
#include <asm/system.h>
--
1.5.2.2

2007-09-26 04:33:18

by Kok, Auke

Subject: Re: [PATCH 10/25] Unionfs: add un/likely conditionals on copyup ops

Erez Zadok wrote:
> Signed-off-by: Erez Zadok <[email protected]>
> ---
> fs/unionfs/copyup.c | 102 +++++++++++++++++++++++++-------------------------
> 1 files changed, 51 insertions(+), 51 deletions(-)
>
> diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
> index 23ac4c8..e3c5f15 100644
> --- a/fs/unionfs/copyup.c
> +++ b/fs/unionfs/copyup.c
> @@ -36,14 +36,14 @@ static int copyup_xattrs(struct dentry *old_lower_dentry,
>
> /* query the actual size of the xattr list */
> list_size = vfs_listxattr(old_lower_dentry, NULL, 0);
> - if (list_size <= 0) {
> + if (unlikely(list_size <= 0)) {


I've been told several times that adding these is almost always bogus - either it
messes up the CPU branch prediction or the compiler/CPU just does a lot better at
finding the right way without these hints.

Adding them as a blanket seems rather strange. Have you got any numbers that this
really improves performance?

Auke

2007-09-26 08:31:37

by Christoph Hellwig

Subject: Re: [PATCH 05/25] Unionfs: cast page->index loff_t before shifting

On Tue, Sep 25, 2007 at 11:09:44PM -0400, Erez Zadok wrote:
> Fixes bugs in number promotion/demotion computation, as per
> <http://lkml.org/lkml/2007/9/20/17>

It's better to use te page_offset helper as that avoids any confusion
on where to cast.

2007-09-26 08:36:38

by Jan Engelhardt

Subject: Re: [PATCH 03/25] Unionfs: display informational messages only if debug is on

On Sep 25 2007 23:09, Erez Zadok wrote:
>--- a/fs/unionfs/commonfops.c
>+++ b/fs/unionfs/commonfops.c
>@@ -394,8 +394,8 @@ int unionfs_file_revalidate(struct file *file, bool willwrite)
> if (willwrite && IS_WRITE_FLAG(file->f_flags) &&
> !IS_WRITE_FLAG(unionfs_lower_file(file)->f_flags) &&
> is_robranch(dentry)) {
>- printk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n",
>- dentry->d_name.name);
>+ dprintk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n",
>+ dentry->d_name.name);

Don't we have pr_debug() for that?

2007-09-26 13:41:23

by Erez Zadok

Subject: Re: [PATCH 10/25] Unionfs: add un/likely conditionals on copyup ops

In message <[email protected]>, "Kok, Auke" writes:
> Erez Zadok wrote:
> > Signed-off-by: Erez Zadok <[email protected]>
> > ---
> > fs/unionfs/copyup.c | 102 +++++++++++++++++++++++++-------------------------
> > 1 files changed, 51 insertions(+), 51 deletions(-)
> >
> > diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
> > index 23ac4c8..e3c5f15 100644
> > --- a/fs/unionfs/copyup.c
> > +++ b/fs/unionfs/copyup.c
> > @@ -36,14 +36,14 @@ static int copyup_xattrs(struct dentry *old_lower_dentry,
> >
> > /* query the actual size of the xattr list */
> > list_size = vfs_listxattr(old_lower_dentry, NULL, 0);
> > - if (list_size <= 0) {
> > + if (unlikely(list_size <= 0)) {
>
>
> I've been told several times that adding these is almost always bogus - either it
> messes up the CPU branch prediction or the compiler/CPU just does a lot better at
> finding the right way without these hints.
>
> Adding them as a blanket seems rather strange. Have you got any numbers that this
> really improves performance?
>
> Auke

Auke, that's a good question, but I found it hard to find any info about it.
There's no discussion on it in Documentation/, and very little I could find
elsewhere. I did see one url explaining what un/likely does precisely, but
no guidelines. My understanding is that it can improve performance, as long
as it's used carefully (otherwise it may hurt performance).

Background: we used un/likely in Unionfs only sparingly until now. We
looked at what other filesystems and kernel components do, and it seems that
it varies a lot: some subsystems use it religiously wherever they can, and
some use it just a little here and there. We looked at what other
filesystems do in particular and tried to follow a similar model under what
cases other filesystems use un/likely.

Recently we've done a full audit of the entire code, and added un/likely
where we felt that the chance of succeeding is 95% or better (e.g., error
conditions that should rarely happen, and such). So while it looks like
we've added many of those in this series of patches, I can assure you we
didn't just wrap every conditional in an un/likely just for the sake of
using it. :-) There are plenty of conditionals (over 250) left untouched b/c
it didn't make sense to force the branch prediction on them one way or
another.

So my questions are, for everyone, what's the policy on using un/likely?
Any common conventions/wisdom? I think we need something added to
Documentation/.

Also, Auke, if indeed compilers are [sic] likely to do better than
programmers adding un/likely wrappers, then why do we still support that in
the kernel? (Working for a company tat produces high-quality compilers, you
may know the answer better.)

Personally I'm not too fond of what those wrappers do the code: they make it
a bit harder to read the code (yet another extra set of parentheses); and
they cause the code to be indented further to the right, which you sometimes
have to split to multiple lines to avoid going over 80 chars.

Cheers,
Erez.

2007-09-26 13:45:18

by Erez Zadok

Subject: Re: [PATCH 05/25] Unionfs: cast page->index loff_t before shifting

In message <[email protected]>, Christoph Hellwig writes:
> On Tue, Sep 25, 2007 at 11:09:44PM -0400, Erez Zadok wrote:
> > Fixes bugs in number promotion/demotion computation, as per
> > <http://lkml.org/lkml/2007/9/20/17>
>
> It's better to use te page_offset helper as that avoids any confusion
> on where to cast.

Excellent. Will do.

Erez.

2007-09-26 14:01:40

by Erez Zadok

Subject: Re: [PATCH 03/25] Unionfs: display informational messages only if debug is on

In message <[email protected]>, Jan Engelhardt writes:
>
> On Sep 25 2007 23:09, Erez Zadok wrote:
> >--- a/fs/unionfs/commonfops.c
> >+++ b/fs/unionfs/commonfops.c
> >@@ -394,8 +394,8 @@ int unionfs_file_revalidate(struct file *file, bool willwrite)
> > if (willwrite && IS_WRITE_FLAG(file->f_flags) &&
> > !IS_WRITE_FLAG(unionfs_lower_file(file)->f_flags) &&
> > is_robranch(dentry)) {
> >- printk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n",
> >- dentry->d_name.name);
> >+ dprintk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n",
> >+ dentry->d_name.name);
>
> Don't we have pr_debug() for that?

Jan, what's the policy on people writing their own debugging printk systems.
I've looked at what other file systems do, and found out that it varies a
lot: some use a simple mapping wrapper from printk->dprintk, while others
define complex ways to have debugging levels/bitmaps where users can
selectively decide (say, by a mount option), what debugging output they'd
like to see or not. Here's a small sampling of what some file systems use:

9p/fid.c: P9_DPRINTK(P9_DEBUG_VFS, "fid %d dentry %s\n",
autofs/inode.c: DPRINTK(("autofs: shutting down\n"));
lockd/svcproc.c: dprintk("lockd: GRANTED_RES called\n");
ncpfs/dir.c: DDPRINTK("ncp_lookup_validate: result=%d\n", val);
nfs/client.c: dprintk("<-- nfs_free_client()\n");
nfsd/export.c: dprintk("found domain %s\n", buf);
partitions/efi.c: Dprintk("GPT my_lba incorrect: %lld != %lld\n",

Not much standardization there :-) but "dprintk" does appear to be the more
common use.

I wanted something simple to allow me to not printk something that's just
for informational/debugging purposes, but w/o having to #ifdef the printk in
question, or define a complex debugging-level printk system.

Now, looking at pr_debug (in linux/kernel.h), and indeed some filesystems
(e.g., affs and configfs) use it. But pr_debug is only active when #define
DEBUG is on (not CONFIG_DEBUG). I didn't see a config option that enable
DEBUG: is there one?

>From other file systems, it appears that the more common convention is for
filesystem XYZ to offer a config option CONFIG_DEBUG_XYZ, which makes its
debugging more self-contained and keeps the config option closer to where
the f/s itself is enabled.

Thanks,
Erez.

2007-09-26 15:24:22

by Jan Engelhardt

Subject: Re: [PATCH 03/25] Unionfs: display informational messages only if debug is on

On Sep 26 2007 10:01, Erez Zadok wrote:
>>
>> On Sep 25 2007 23:09, Erez Zadok wrote:
>> >--- a/fs/unionfs/commonfops.c
>> >+++ b/fs/unionfs/commonfops.c
>> >@@ -394,8 +394,8 @@ int unionfs_file_revalidate(struct file *file, bool willwrite)
>> > if (willwrite && IS_WRITE_FLAG(file->f_flags) &&
>> > !IS_WRITE_FLAG(unionfs_lower_file(file)->f_flags) &&
>> > is_robranch(dentry)) {
>> >- printk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n",
>> >- dentry->d_name.name);
>> >+ dprintk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n",
>> >+ dentry->d_name.name);
>>
>> Don't we have pr_debug() for that?
>
>Jan, what's the policy on people writing their own debugging printk systems.

Generally, the rule is "don't (re)invent another debug system"

>I've looked at what other file systems do, and found out that it varies a lot:

Oh that's either old code or code that has not been reviewed properly.
Fact is that I have been made aware of pr_debug() "often enough"
on netfilter-devel, and it looks like a good idea.

>I wanted something simple to allow me to not printk something that's just
>for informational/debugging purposes, but w/o having to #ifdef the printk in
>question, or define a complex debugging-level printk system.

Surprise, pr_debug() is just that all nicely wrapped up.
Want debug? Do it like this.

#ifdef CONFIG_UNIONFS_DEBUG
# define DEBUG 1
#endif

and pr_debug() works magic.

>Now, looking at pr_debug (in linux/kernel.h), and indeed some filesystems
>(e.g., affs and configfs) use it. But pr_debug is only active when #define
>DEBUG is on (not CONFIG_DEBUG). I didn't see a config option that enable
>DEBUG: is there one?

No I do not think so. But when you grep for dprintk, you should also
grep for DEBUG, to be fair :p

2007-09-26 15:24:37

by Kyle Moffett

Subject: Re: [PATCH 10/25] Unionfs: add un/likely conditionals on copyup ops

On Sep 26, 2007, at 09:40:20, Erez Zadok wrote:
> In message <[email protected]>, "Kok, Auke" writes:
>> I've been told several times that adding these is almost always
>> bogus - either it messes up the CPU branch prediction or the
>> compiler/CPU just does a lot better at finding the right way
>> without these hints.
>>
>> Adding them as a blanket seems rather strange. Have you got any
>> numbers that this really improves performance?
>
> Auke, that's a good question, but I found it hard to find any info
> about it. There's no discussion on it in Documentation/, and very
> little I could find elsewhere. I did see one url explaining what
> un/likely does precisely, but no guidelines. My understanding is
> that it can improve performance, as long as it's used carefully
> (otherwise it may hurt performance).

Hmm, even still I agree with Auke, you probably use it too much.


> Recently we've done a full audit of the entire code, and added un/
> likely where we felt that the chance of succeeding is 95% or better
> (e.g., error conditions that should rarely happen, and such).

Actually due to the performance penalty on some systems I think you
only want to use it if the chance of succeeding is 99% or better, as
the benefit if predicted is a cycle or two and the harm if
mispredicted can be more than 50 cycles, depending on the CPU. You
should also remember than in filesystems many "failures" are
triggered by things like the ld.so library searches, where it
literally calls access() 20 different times on various possible paths
for library files, failing the first 19. It does this once for each
necessary library.

Typically you only want to add unlikely() or likely() for about 2
reasons:
(A) It's a hot path and the unlikely case is just going to burn a
bunch of CPU anyways
(B) It really is extremely unlikely that it fails (Think physical
hardware failure)

Anything else is just bogus.

Cheers,
Kyle Moffett

2007-09-26 15:28:57

by Erez Zadok

Subject: Re: [PATCH 03/25] Unionfs: display informational messages only if debug is on

In message <[email protected]>, Jan Engelhardt writes:
[...]
> Surprise, pr_debug() is just that all nicely wrapped up.
> Want debug? Do it like this.
>
> #ifdef CONFIG_UNIONFS_DEBUG
> # define DEBUG 1
> #endif
>
> and pr_debug() works magic.
[...]

Sounds good. I'll do that.

Thanks,
Erez.

2007-09-26 15:44:22

by Erez Zadok

Subject: Re: [PATCH 10/25] Unionfs: add un/likely conditionals on copyup ops

In message <[email protected]>, Kyle Moffett writes:
> On Sep 26, 2007, at 09:40:20, Erez Zadok wrote:
[...]
> > Recently we've done a full audit of the entire code, and added un/
> > likely where we felt that the chance of succeeding is 95% or better
> > (e.g., error conditions that should rarely happen, and such).
>
> Actually due to the performance penalty on some systems I think you
> only want to use it if the chance of succeeding is 99% or better, as
> the benefit if predicted is a cycle or two and the harm if
> mispredicted can be more than 50 cycles, depending on the CPU.

*That's* the information I was looking for, Kyle: what's the estimated
probability I should be using as my guideline. I used 95% (20/1 ratio), and
you're telling me I should use 99% (100/1 ratio). The difference between
the number of cycles saved/added is very compelling. Given that I certainly
agree with you that I'm using un/likely too much. I'll re-evaluate and
update my patch series then.

Thanks,
Erez.

2007-09-26 16:48:17

by Jan Engelhardt

Subject: Re: [PATCH 10/25] Unionfs: add un/likely conditionals on copyup ops

On Sep 26 2007 11:43, Erez Zadok wrote:
>
>*That's* the information I was looking for, Kyle: what's the estimated
>probability I should be using as my guideline. I used 95% (20/1 ratio), and

;-)

19:1 <=> 95:5 <=> 95% <=> ratio=0.95 != 20.0 (=20/1)

>you're telling me I should use 99% (100/1 ratio). The difference between

99:1 <=> 99% <=> ratio=0.99 != 100.0 (=100/1)

>the number of cycles saved/added is very compelling. Given that I certainly
>agree with you that I'm using un/likely too much. I'll re-evaluate and
>update my patch series then.

2007-09-26 16:52:03

by Erez Zadok

Subject: Re: [PATCH 10/25] Unionfs: add un/likely conditionals on copyup ops

In message <[email protected]>, Jan Engelhardt writes:
>
> On Sep 26 2007 11:43, Erez Zadok wrote:
> >
> >*That's* the information I was looking for, Kyle: what's the estimated
> >probability I should be using as my guideline. I used 95% (20/1 ratio), and
>
> ;-)
>
> 19:1 <=> 95:5 <=> 95% <=> ratio=0.95 != 20.0 (=20/1)
>
> >you're telling me I should use 99% (100/1 ratio). The difference between
>
> 99:1 <=> 99% <=> ratio=0.99 != 100.0 (=100/1)
>
> >the number of cycles saved/added is very compelling. Given that I certainly
> >agree with you that I'm using un/likely too much. I'll re-evaluate and
> >update my patch series then.

Yeah, close enough. :-)

The important issue is that I'm probably using about five times too many
un/likely wrappers.

Erez.

2007-09-26 18:34:39

by Adrian Bunk

Subject: Re: [PATCH 10/25] Unionfs: add un/likely conditionals on copyup ops

On Wed, Sep 26, 2007 at 09:40:20AM -0400, Erez Zadok wrote:
>...
> Also, Auke, if indeed compilers are [sic] likely to do better than
> programmers adding un/likely wrappers, then why do we still support that in
> the kernel? (Working for a company tat produces high-quality compilers, you
> may know the answer better.)
>
> Personally I'm not too fond of what those wrappers do the code: they make it
> a bit harder to read the code (yet another extra set of parentheses); and
> they cause the code to be indented further to the right, which you sometimes
> have to split to multiple lines to avoid going over 80 chars.

There are some places in generic code where it makes sense, e.g.:
#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while(0)

If you run into a BUG() it's anyway game over.

And there are some rare hotpaths in the kernel where it might make
sense, and many other places where the likely/unlikely usage that might
be present doesn't make sense.

Unless you know you need it you simply shouldn't use likely/unlikely.

> Cheers,
> Erez.

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2007-09-26 21:34:44

by Roel Kluin

Subject: Re: [PATCH 11/25] Unionfs: add un/likely conditionals on debug ops

Erez Zadok wrote:
> Signed-off-by: Erez Zadok <[email protected]>
> ---
> fs/unionfs/debug.c | 108 +++++++++++++++++++++++++++------------------------
> 1 files changed, 57 insertions(+), 51 deletions(-)
>
> diff --git a/fs/unionfs/debug.c b/fs/unionfs/debug.c
> index 9546a41..09b52ce 100644
> --- a/fs/unionfs/debug.c
> +++ b/fs/unionfs/debug.c
> @@ -56,19 +56,19 @@ void __unionfs_check_inode(const struct inode *inode,
> sb = inode->i_sb;
> istart = ibstart(inode);
> iend = ibend(inode);
> - if (istart > iend) {
> + if (unlikely(istart > iend)) {
> PRINT_CALLER(fname, fxn, line);
> printk(" Ci0: inode=%p istart/end=%d:%d\n",
> inode, istart, iend);
> }
> - if ((istart == -1 && iend != -1) ||
> - (istart != -1 && iend == -1)) {
> + if (unlikely((istart == -1 && iend != -1) ||
> + (istart != -1 && iend == -1))) {

you could also do
if (unlikely((istart == -1 || iend == -1) && istart != iend)) {

[...]

> @@ -138,8 +139,8 @@ void __unionfs_check_dentry(const struct dentry *dentry,
> dend = dbend(dentry);
> BUG_ON(dstart > dend);
>
> - if ((dstart == -1 && dend != -1) ||
> - (dstart != -1 && dend == -1)) {
> + if (unlikely((dstart == -1 && dend != -1) ||
> + (dstart != -1 && dend == -1))) {

[...]

the same here

> @@ -223,30 +224,30 @@ void __unionfs_check_dentry(const struct dentry *dentry,
> istart = ibstart(inode);
> iend = ibend(inode);
> BUG_ON(istart > iend);
> - if ((istart == -1 && iend != -1) ||
> - (istart != -1 && iend == -1)) {
> + if (unlikely((istart == -1 && iend != -1) ||
> + (istart != -1 && iend == -1))) {

and here

[...]

> @@ -350,30 +354,30 @@ void __unionfs_check_file(const struct file *file,
> fend = fbend(file);
> BUG_ON(fstart > fend);
>
> - if ((fstart == -1 && fend != -1) ||
> - (fstart != -1 && fend == -1)) {
> + if (unlikely((fstart == -1 && fend != -1) ||
> + (fstart != -1 && fend == -1))) {

and here

[...]

2007-09-26 21:40:53

by Roel Kluin

Subject: Re: [PATCH 13/25] Unionfs: add un/likely conditionals on dir ops

Erez Zadok wrote:

> @@ -194,7 +194,7 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist)
>
> BUG_ON(!S_ISDIR(dentry->d_inode->i_mode));
>
> - if ((err = unionfs_partial_lookup(dentry)))
> + if (unlikely((err = unionfs_partial_lookup(dentry))))
> goto out;
>
> bstart = dbstart(dentry);

Is it bad to leave this assignment within the unlikely()?

2007-09-27 14:28:32

by Erez Zadok

Subject: Re: [PATCH 13/25] Unionfs: add un/likely conditionals on dir ops

In message <[email protected]>, roel writes:
> Erez Zadok wrote:
>
> > @@ -194,7 +194,7 @@ int check_empty(struct dentry *dentry, struct unionfs_dir_state **namelist)
> >
> > BUG_ON(!S_ISDIR(dentry->d_inode->i_mode));
> >
> > - if ((err = unionfs_partial_lookup(dentry)))
> > + if (unlikely((err = unionfs_partial_lookup(dentry))))
> > goto out;
> >
> > bstart = dbstart(dentry);
>
> Is it bad to leave this assignment within the unlikely()?

I don't know. Anyone? Will un/likely "break" in the above form?

Actually, it looks like assignments within conditionals are just prohibited
(even if you use double parentheses to shut gcc up :-). It's not mentioned
in the CodingStyle, but it is in scripts/checkpatch.pl. So I'll have to
take out all those assignments out of the conditionals anyway. Good. It
makes code more readable (and besides, most modern compilers will optimize
it just the same).

Cheers,
Erez.