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

[Erez Zadok]

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.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[roel]

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()?
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Erez Zadok]

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,