Re: [f2fs-dev] [PATCH 2/2 v2] f2fs: fix the location of tracepoint

2013-12-10 Thread Jaegeuk Kim
Change log from v1:
 o fix a mistake

>From 59df095b83e601b1b317ce97a5f767a8c3303902 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim 
Date: Wed, 11 Dec 2013 14:29:39 +0900
Subject: [PATCH] f2fs: fix the location of tracepoint
Cc: linux-fsde...@vger.kernel.org, linux-ker...@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net

We need to get a trace before submit_bio, since its bi_sector is
remapped during
the submit_bio.

Signed-off-by: Jaegeuk Kim 
---
 fs/f2fs/data.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index ebc9177..15956fa 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -104,11 +104,12 @@ static void __submit_merged_bio(struct
f2fs_bio_info *io)
rw = fio->rw | fio->rw_flag;
 
if (is_read_io(rw)) {
-   submit_bio(rw, io->bio);
trace_f2fs_submit_read_bio(io->sbi->sb, rw, fio->type, io->bio);
+   submit_bio(rw, io->bio);
io->bio = NULL;
return;
}
+   trace_f2fs_submit_write_bio(io->sbi->sb, rw, fio->type, io->bio);
 
/*
 * META_FLUSH is only from the checkpoint procedure, and we should
wait
@@ -122,7 +123,6 @@ static void __submit_merged_bio(struct f2fs_bio_info
*io)
} else {
submit_bio(rw, io->bio);
}
-   trace_f2fs_submit_write_bio(io->sbi->sb, rw, fio->type, io->bio);
io->bio = NULL;
 }
 
-- 
1.8.4.474.g128a96c


-- 
Jaegeuk Kim
Samsung


--
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH 2/2] f2fs: fix the location of tracepoint

2013-12-10 Thread Jaegeuk Kim
Oops,
Please ignore the second patch.
Sorry for the noise.

2013-12-11 (수), 15:05 +0900, Jaegeuk Kim:
> We need to get a trace before submit_bio, since its bi_sector is remapped 
> during
> the submit_bio.
> 
> Signed-off-by: Jaegeuk Kim 
> ---
>  fs/f2fs/data.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index ebc9177..969df55 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -103,9 +103,10 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
>  
>   rw = fio->rw | fio->rw_flag;
>  
> + trace_f2fs_submit_write_bio(io->sbi->sb, rw, fio->type, io->bio);
> +
>   if (is_read_io(rw)) {
>   submit_bio(rw, io->bio);
> - trace_f2fs_submit_read_bio(io->sbi->sb, rw, fio->type, io->bio);
>   io->bio = NULL;
>   return;
>   }
> @@ -122,7 +123,6 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
>   } else {
>   submit_bio(rw, io->bio);
>   }
> - trace_f2fs_submit_write_bio(io->sbi->sb, rw, fio->type, io->bio);
>   io->bio = NULL;
>  }
>  

-- 
Jaegeuk Kim
Samsung



--
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH 2/2] f2fs: fix the location of tracepoint

2013-12-10 Thread Jaegeuk Kim
We need to get a trace before submit_bio, since its bi_sector is remapped during
the submit_bio.

Signed-off-by: Jaegeuk Kim 
---
 fs/f2fs/data.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index ebc9177..969df55 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -103,9 +103,10 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
 
rw = fio->rw | fio->rw_flag;
 
+   trace_f2fs_submit_write_bio(io->sbi->sb, rw, fio->type, io->bio);
+
if (is_read_io(rw)) {
submit_bio(rw, io->bio);
-   trace_f2fs_submit_read_bio(io->sbi->sb, rw, fio->type, io->bio);
io->bio = NULL;
return;
}
@@ -122,7 +123,6 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
} else {
submit_bio(rw, io->bio);
}
-   trace_f2fs_submit_write_bio(io->sbi->sb, rw, fio->type, io->bio);
io->bio = NULL;
 }
 
-- 
1.8.4.474.g128a96c


--
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH 1/2] f2fs: refactor bio->rw handling

2013-12-10 Thread Jaegeuk Kim
This patch introduces f2fs_io_info to mitigate the complex parameter list.

struct f2fs_io_info {
enum page_type type;/* contains DATA/NODE/META/META_FLUSH */
int rw; /* contains R/RS/W/WS */
int rw_flag;/* contains REQ_META/REQ_PRIO */
}

1. f2fs_write_data_pages
 - DATA
 - WRITE_SYNC is set when wbc->WB_SYNC_ALL.

2. sync_node_pages
 - NODE
 - WRITE_SYNC all the time

3. sync_meta_pages
 - META
 - WRITE_SYNC all the time
 - REQ_META | REQ_PRIO all the time

 ** f2fs_submit_merged_bio() handles META_FLUSH.

4. ra_nat_pages, ra_sit_pages, ra_sum_pages
 - META
 - READ_SYNC

Cc: Fan Li 
Cc: Changman Lee 
Signed-off-by: Jaegeuk Kim 
---
 fs/f2fs/checkpoint.c | 11 ---
 fs/f2fs/data.c   | 85 ++--
 fs/f2fs/f2fs.h   | 22 +-
 fs/f2fs/gc.c | 10 ---
 fs/f2fs/node.c   | 22 ++
 fs/f2fs/segment.c| 70 ++-
 fs/f2fs/super.c  |  7 -
 7 files changed, 132 insertions(+), 95 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index cf505eb..f8c0749 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -164,8 +164,7 @@ long sync_meta_pages(struct f2fs_sb_info *sbi, enum 
page_type type,
}
 
if (nwritten)
-   f2fs_submit_merged_bio(sbi, type, nr_to_write == LONG_MAX,
-   WRITE);
+   f2fs_submit_merged_bio(sbi, type, WRITE);
 
return nwritten;
 }
@@ -598,7 +597,7 @@ retry:
 * We should submit bio, since it exists several
 * wribacking dentry pages in the freeing inode.
 */
-   f2fs_submit_merged_bio(sbi, DATA, true, WRITE);
+   f2fs_submit_merged_bio(sbi, DATA, WRITE);
}
goto retry;
 }
@@ -804,9 +803,9 @@ void write_checkpoint(struct f2fs_sb_info *sbi, bool 
is_umount)
 
trace_f2fs_write_checkpoint(sbi->sb, is_umount, "finish block_ops");
 
-   f2fs_submit_merged_bio(sbi, DATA, true, WRITE);
-   f2fs_submit_merged_bio(sbi, NODE, true, WRITE);
-   f2fs_submit_merged_bio(sbi, META, true, WRITE);
+   f2fs_submit_merged_bio(sbi, DATA, WRITE);
+   f2fs_submit_merged_bio(sbi, NODE, WRITE);
+   f2fs_submit_merged_bio(sbi, META, WRITE);
 
/*
 * update checkpoint pack index
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index fb5e5c2..ebc9177 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -93,37 +93,28 @@ static void f2fs_write_end_io(struct bio *bio, int err)
bio_put(bio);
 }
 
-static void __submit_merged_bio(struct f2fs_sb_info *sbi,
-   struct f2fs_bio_info *io,
-   enum page_type type, bool sync, int rw)
+static void __submit_merged_bio(struct f2fs_bio_info *io)
 {
-   enum page_type btype = PAGE_TYPE_OF_BIO(type);
+   struct f2fs_io_info *fio = &io->fio;
+   int rw;
 
if (!io->bio)
return;
 
-   if (btype == META)
-   rw |= REQ_META;
+   rw = fio->rw | fio->rw_flag;
 
if (is_read_io(rw)) {
-   if (sync)
-   rw |= READ_SYNC;
submit_bio(rw, io->bio);
-   trace_f2fs_submit_read_bio(sbi->sb, rw, type, io->bio);
+   trace_f2fs_submit_read_bio(io->sbi->sb, rw, fio->type, io->bio);
io->bio = NULL;
return;
}
 
-   if (sync)
-   rw |= WRITE_SYNC;
-   if (type >= META_FLUSH)
-   rw |= WRITE_FLUSH_FUA;
-
/*
 * META_FLUSH is only from the checkpoint procedure, and we should wait
 * this metadata bio for FS consistency.
 */
-   if (type == META_FLUSH) {
+   if (fio->type == META_FLUSH) {
DECLARE_COMPLETION_ONSTACK(wait);
io->bio->bi_private = &wait;
submit_bio(rw, io->bio);
@@ -131,12 +122,12 @@ static void __submit_merged_bio(struct f2fs_sb_info *sbi,
} else {
submit_bio(rw, io->bio);
}
-   trace_f2fs_submit_write_bio(sbi->sb, rw, btype, io->bio);
+   trace_f2fs_submit_write_bio(io->sbi->sb, rw, fio->type, io->bio);
io->bio = NULL;
 }
 
 void f2fs_submit_merged_bio(struct f2fs_sb_info *sbi,
-   enum page_type type, bool sync, int rw)
+   enum page_type type, int rw)
 {
enum page_type btype = PAGE_TYPE_OF_BIO(type);
struct f2fs_bio_info *io;
@@ -144,7 +135,13 @@ void f2fs_submit_merged_bio(struct f2fs_sb_info *sbi,
io = is_read_io(rw) ? &sbi->read_io : &sbi->write_io[btype];
 
mutex_lock(&io->io_mutex);
-   __submit_merged_bio(sbi, io, type, sync, rw);
+
+   /* change META to META_FLUSH in the checkpoint procedure */
+   if (type >= META_FLUSH) {
+

[f2fs-dev] [PATCH] f2fs: missing kmem_cache_destroy for discard_entry

2013-12-10 Thread Changman Lee
insmod f2fs.ko is failed after insmod and rmmod firstly.

$ sudo insmod fs/f2fs/f2fs.ko
insmod: error inserting 'fs/f2fs/f2fs.ko': -1 Cannot allocate memory

-- dmesg --
kmem_cache_sanity_check (free_nid): Cache name already exists.

Signed-off-by: Changman Lee 
---
 fs/f2fs/super.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index dd55074..b35f371 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1135,6 +1135,7 @@ static void __exit exit_f2fs_fs(void)
unregister_filesystem(&f2fs_fs_type);
destroy_checkpoint_caches();
destroy_gc_caches();
+   destroy_segment_manager_caches();
destroy_node_manager_caches();
destroy_inodecache();
kset_unregister(f2fs_kset);
-- 
1.7.9.5


--
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [SPAM] luxury goods - The order in 2 clicks. + 15% discount! -enjoy!

2013-12-10 Thread Jill
offer luxury-simple and elegant. - http://tiny.cc/pdtk7w
<>--
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH 09/18] f2fs: use generic posix ACL infrastructure

2013-12-10 Thread Christoph Hellwig
On Fri, Dec 06, 2013 at 10:37:34AM +0900, Jaegeuk Kim wrote:
> f2fs caches a new mode bit for a while to make the consistency between
> xattr's acl mode and the inode mode.

Can you explain what exactly you're trying to do there?  I've been
trying to unwrap what's going on and can't really see the point:

 - i_acl_mode and FI_ACL_MODE get set in __setattr_copy, but right
   after that call, still under i_mutex and before marking the inode
   dirty f2fs_acl_chmod makes use of it, and it gets cleared right
   after. Is there any race that could happen with a locked inode
   not marked dirty yet on f2fs?  We could pass a mode argument
   to posix_acl_create, but I'd prefer to avoid that if we can.
 - on the set_acl side it gets set in __f2fs_set_acl, and then
   i_mode is update in __f2fs_setxattr which could easily done with
   a stack variable.

RFC patch below:


diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
index 4f52fe0f..6647545 100644
--- a/fs/f2fs/acl.c
+++ b/fs/f2fs/acl.c
@@ -17,9 +17,6 @@
 #include "xattr.h"
 #include "acl.h"
 
-#define get_inode_mode(i)  ((is_inode_flag_set(F2FS_I(i), FI_ACL_MODE)) ? \
-   (F2FS_I(i)->i_acl_mode) : ((i)->i_mode))
-
 static inline size_t f2fs_acl_size(int count)
 {
if (count <= 4) {
@@ -209,11 +206,11 @@ static int __f2fs_set_acl(struct inode *inode, int type,
struct posix_acl *acl, struct page *ipage)
 {
struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
-   struct f2fs_inode_info *fi = F2FS_I(inode);
int name_index;
void *value = NULL;
size_t size = 0;
int error;
+   umode_t mode = 0;
 
if (!test_opt(sbi, POSIX_ACL))
return 0;
@@ -224,10 +221,10 @@ static int __f2fs_set_acl(struct inode *inode, int type,
case ACL_TYPE_ACCESS:
name_index = F2FS_XATTR_INDEX_POSIX_ACL_ACCESS;
if (acl) {
-   error = posix_acl_equiv_mode(acl, &inode->i_mode);
+   mode = inode->i_mode;
+   error = posix_acl_equiv_mode(acl, &mode);
if (error < 0)
return error;
-   set_acl_inode(fi, inode->i_mode);
if (error == 0)
acl = NULL;
}
@@ -245,19 +242,15 @@ static int __f2fs_set_acl(struct inode *inode, int type,
 
if (acl) {
value = f2fs_acl_to_disk(acl, &size);
-   if (IS_ERR(value)) {
-   cond_clear_inode_flag(fi, FI_ACL_MODE);
+   if (IS_ERR(value))
return (int)PTR_ERR(value);
-   }
}
 
-   error = f2fs_setxattr(inode, name_index, "", value, size, ipage);
+   error = f2fs_setxattr(inode, name_index, "", value, size, ipage, mode);
 
kfree(value);
if (!error)
set_cached_acl(inode, type, acl);
-
-   cond_clear_inode_flag(fi, FI_ACL_MODE);
return error;
 }
 
@@ -289,28 +282,3 @@ int f2fs_init_acl(struct inode *inode, struct inode *dir, 
struct page *ipage)
 
return error;
 }
-
-int f2fs_acl_chmod(struct inode *inode)
-{
-   struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
-   struct posix_acl *acl;
-   int error;
-   umode_t mode = get_inode_mode(inode);
-
-   if (!test_opt(sbi, POSIX_ACL))
-   return 0;
-   if (S_ISLNK(mode))
-   return -EOPNOTSUPP;
-
-   acl = f2fs_get_acl(inode, ACL_TYPE_ACCESS);
-   if (IS_ERR(acl) || !acl)
-   return PTR_ERR(acl);
-
-   error = __posix_acl_chmod(&acl, GFP_KERNEL, mode);
-   if (error)
-   return error;
-
-   error = __f2fs_set_acl(inode, ACL_TYPE_ACCESS, acl, NULL);
-   posix_acl_release(acl);
-   return error;
-}
diff --git a/fs/f2fs/acl.h b/fs/f2fs/acl.h
index 2af31fe..e086465 100644
--- a/fs/f2fs/acl.h
+++ b/fs/f2fs/acl.h
@@ -38,18 +38,12 @@ struct f2fs_acl_header {
 
 extern struct posix_acl *f2fs_get_acl(struct inode *, int);
 extern int f2fs_set_acl(struct inode *inode, struct posix_acl *acl, int type);
-extern int f2fs_acl_chmod(struct inode *);
 extern int f2fs_init_acl(struct inode *, struct inode *, struct page *);
 #else
 #define f2fs_check_acl NULL
 #define f2fs_get_acl   NULL
 #define f2fs_set_acl   NULL
 
-static inline int f2fs_acl_chmod(struct inode *inode)
-{
-   return 0;
-}
-
 static inline int f2fs_init_acl(struct inode *inode, struct inode *dir,
struct page *page)
 {
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 89dc750..1e774e6 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -181,7 +181,6 @@ struct f2fs_inode_info {
unsigned char i_advise; /* use to give file attribute hints */
unsigned int i_current_depth;   /* use only in directory structure */
unsigned int i_pino;/* parent ino

Re: [f2fs-dev] [Cluster-devel] [PATCH 16/18] gfs2: use generic posix ACL infrastructure

2013-12-10 Thread Steven Whitehouse
Hi,

On Sun, 2013-12-01 at 03:59 -0800, Christoph Hellwig wrote:
> plain text document attachment
> (0016-gfs2-use-generic-posix-ACL-infrastructure.patch)
> This contains some major refactoring for the create path so that
> inodes are created with the right mode to start with instead of
> fixing it up later.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  fs/gfs2/acl.c   |  229 
> +++
>  fs/gfs2/acl.h   |4 +-
>  fs/gfs2/inode.c |   33 ++--
>  fs/gfs2/xattr.c |4 +-
>  4 files changed, 61 insertions(+), 209 deletions(-)
> 
Looks very good. I'd really like to be able to do something similar with
the security xattrs, in terms of the refactoring that at inode creation
to give the xattrs ahead of the inode allocation itself. That way it
should be possible to allocate the xattr blocks at the same time as the
inode, rather than as an after thought.

Some more comments below

> diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
> index e82e4ac..e6c7a2c 100644
> --- a/fs/gfs2/acl.c
> +++ b/fs/gfs2/acl.c
[snip]
> -
> -static int gfs2_xattr_system_set(struct dentry *dentry, const char *name,
> -  const void *value, size_t size, int flags,
> -  int xtype)
> -{
> - struct inode *inode = dentry->d_inode;
> - struct gfs2_sbd *sdp = GFS2_SB(inode);
> - struct posix_acl *acl = NULL;
> - int error = 0, type;
> -
> - if (!sdp->sd_args.ar_posix_acl)
> - return -EOPNOTSUPP;
> -
> - type = gfs2_acl_type(name);
> - if (type < 0)
> - return type;
> - if (flags & XATTR_CREATE)
> - return -EINVAL;
> - if (type == ACL_TYPE_DEFAULT && !S_ISDIR(inode->i_mode))
> - return value ? -EACCES : 0;
> - if (!uid_eq(current_fsuid(), inode->i_uid) && !capable(CAP_FOWNER))
> - return -EPERM;
> - if (S_ISLNK(inode->i_mode))
> - return -EOPNOTSUPP;
> -
> - if (!value)
> - goto set_acl;
>  
> - acl = posix_acl_from_xattr(&init_user_ns, value, size);
> - if (!acl) {
> - /*
> -  * acl_set_file(3) may request that we set default ACLs with
> -  * zero length -- defend (gracefully) against that here.
> -  */
> - goto out;
> - }
> - if (IS_ERR(acl)) {
> - error = PTR_ERR(acl);
> - goto out;
> - }
> -
> - error = posix_acl_valid(acl);
> - if (error)
> - goto out_release;
> -
> - error = -EINVAL;
>   if (acl->a_count > GFS2_ACL_MAX_ENTRIES)
> - goto out_release;
> + return -EINVAL;
>  
>   if (type == ACL_TYPE_ACCESS) {
>   umode_t mode = inode->i_mode;
> +
>   error = posix_acl_equiv_mode(acl, &mode);
> + if (error < 0)
>  
Andy Price has pointed out a missing "return error;" here

> - if (error <= 0) {
> - posix_acl_release(acl);
> + if (error == 0)
>   acl = NULL;
>  
> - if (error < 0)
> - return error;
> - }
> -

Also, there seems to be a white space error in the xfs patch around line
170 in fs/xfs/xfs_iops.c where there is an added "if (default_acl)" with
a space before the tab,

Steve.



--
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH 00/18] Consolidate Posix ACL implementation

2013-12-10 Thread Christoph Hellwig
On Thu, Dec 05, 2013 at 06:57:14PM +0100, Andreas Gruenbacher wrote:
> I see that get_acl and set_acl are being defined in some but not all symlink 
> inode operations (for example, btrfs them while ext4 does not), and that 
> posix_acl_xattr_set() doesn't check if set_acl is defined.  Symlinks cannot 
> have ACLs, so set_acl should either never be defined for symlinks (and a NULL 
> check is then needed in posix_acl_xattr_set()), or it is defined in all inode 
> operations, and S_ISNLNK() check is needed in posix_acl_xattr_set().  That 
> latter check should probably be added in any case to be on the safe side.

Yes, we should add the check.  We also in general should not have
set_acl/get_acl on links and I'll look over it.

> Patch 6 also declares posix_acl_prepare() but this function is never 
> introduced; this must be a leftover from a previous version.

Indeed.

Thanks for the review!


--
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [Cluster-devel] [PATCH 16/18] gfs2: use generic posix ACL infrastructure

2013-12-10 Thread Christoph Hellwig
On Wed, Dec 04, 2013 at 12:12:37PM +, Steven Whitehouse wrote:
> > error = posix_acl_equiv_mode(acl, &mode);
> > +   if (error < 0)
> >  
> Andy Price has pointed out a missing "return error;" here
> 
> > -   if (error <= 0) {
> > -   posix_acl_release(acl);
> > +   if (error == 0)
> > acl = NULL;
> >  
> > -   if (error < 0)
> > -   return error;
> > -   }
> > -
> 
> Also, there seems to be a white space error in the xfs patch around line
> 170 in fs/xfs/xfs_iops.c where there is an added "if (default_acl)" with
> a space before the tab,

I'll take care of these for the next version.

--
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH 14/18] xfs: use generic posix ACL infrastructure

2013-12-10 Thread Dave Chinner
On Sun, Dec 01, 2013 at 03:59:17AM -0800, Christoph Hellwig wrote:
> Also create inodes with the proper mode instead of fixing it up later.
> 
> Signed-off-by: Christoph Hellwig 

Nice cleanup work, Christoph.

Reviewed-by: Dave Chinner 

-- 
Dave Chinner
da...@fromorbit.com

--
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH 09/18] f2fs: use generic posix ACL infrastructure

2013-12-10 Thread Jaegeuk Kim
2013-12-01 (일), 03:59 -0800, Christoph Hellwig:

> f2fs has some weird mode bit handling, so still using the old
> chmod code for now.

f2fs caches a new mode bit for a while to make the consistency between
xattr's acl mode and the inode mode.
Anyway, it's a very good job.
Thanks,

You can add:
Reviewed-by: Jaegeuk Kim 

> 
> Signed-off-by: Christoph Hellwig 
> ---
>  fs/f2fs/acl.c   |  140 
> +--
>  fs/f2fs/acl.h   |1 +
>  fs/f2fs/file.c  |1 +
>  fs/f2fs/namei.c |2 +
>  fs/f2fs/xattr.c |9 ++--
>  fs/f2fs/xattr.h |2 -
>  6 files changed, 30 insertions(+), 125 deletions(-)
> 
> diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
> index 45e8430..4f52fe0f 100644
> --- a/fs/f2fs/acl.c
> +++ b/fs/f2fs/acl.c
> @@ -205,7 +205,7 @@ struct posix_acl *f2fs_get_acl(struct inode *inode, int 
> type)
>   return acl;
>  }
>  
> -static int f2fs_set_acl(struct inode *inode, int type,
> +static int __f2fs_set_acl(struct inode *inode, int type,
>   struct posix_acl *acl, struct page *ipage)
>  {
>   struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> @@ -261,37 +261,32 @@ static int f2fs_set_acl(struct inode *inode, int type,
>   return error;
>  }
>  
> +int f2fs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
> +{
> + return __f2fs_set_acl(inode, type, acl, NULL);
> +}
> +
>  int f2fs_init_acl(struct inode *inode, struct inode *dir, struct page *ipage)
>  {
> - struct f2fs_sb_info *sbi = F2FS_SB(dir->i_sb);
> - struct posix_acl *acl = NULL;
> + struct posix_acl *default_acl, *acl;
>   int error = 0;
>  
> - if (!S_ISLNK(inode->i_mode)) {
> - if (test_opt(sbi, POSIX_ACL)) {
> - acl = f2fs_get_acl(dir, ACL_TYPE_DEFAULT);
> - if (IS_ERR(acl))
> - return PTR_ERR(acl);
> - }
> - if (!acl)
> - inode->i_mode &= ~current_umask();
> - }
> -
> - if (!test_opt(sbi, POSIX_ACL) || !acl)
> - goto cleanup;
> + error = posix_acl_create(dir, &inode->i_mode, &default_acl, &acl);
> + if (error)
> + return error;
>  
> - if (S_ISDIR(inode->i_mode)) {
> - error = f2fs_set_acl(inode, ACL_TYPE_DEFAULT, acl, ipage);
> + if (default_acl) {
> + error = __f2fs_set_acl(inode, ACL_TYPE_DEFAULT, default_acl,
> +ipage);
> + posix_acl_release(default_acl);
> + }
> + if (acl) {
>   if (error)
> - goto cleanup;
> + error = __f2fs_set_acl(inode, ACL_TYPE_ACCESS, acl,
> +ipage);
> + posix_acl_release(acl);
>   }
> - error = __posix_acl_create(&acl, GFP_KERNEL, &inode->i_mode);
> - if (error < 0)
> - return error;
> - if (error > 0)
> - error = f2fs_set_acl(inode, ACL_TYPE_ACCESS, acl, ipage);
> -cleanup:
> - posix_acl_release(acl);
> +
>   return error;
>  }
>  
> @@ -315,100 +310,7 @@ int f2fs_acl_chmod(struct inode *inode)
>   if (error)
>   return error;
>  
> - error = f2fs_set_acl(inode, ACL_TYPE_ACCESS, acl, NULL);
> - posix_acl_release(acl);
> - return error;
> -}
> -
> -static size_t f2fs_xattr_list_acl(struct dentry *dentry, char *list,
> - size_t list_size, const char *name, size_t name_len, int type)
> -{
> - struct f2fs_sb_info *sbi = F2FS_SB(dentry->d_sb);
> - const char *xname = POSIX_ACL_XATTR_DEFAULT;
> - size_t size;
> -
> - if (!test_opt(sbi, POSIX_ACL))
> - return 0;
> -
> - if (type == ACL_TYPE_ACCESS)
> - xname = POSIX_ACL_XATTR_ACCESS;
> -
> - size = strlen(xname) + 1;
> - if (list && size <= list_size)
> - memcpy(list, xname, size);
> - return size;
> -}
> -
> -static int f2fs_xattr_get_acl(struct dentry *dentry, const char *name,
> - void *buffer, size_t size, int type)
> -{
> - struct f2fs_sb_info *sbi = F2FS_SB(dentry->d_sb);
> - struct posix_acl *acl;
> - int error;
> -
> - if (strcmp(name, "") != 0)
> - return -EINVAL;
> - if (!test_opt(sbi, POSIX_ACL))
> - return -EOPNOTSUPP;
> -
> - acl = f2fs_get_acl(dentry->d_inode, type);
> - if (IS_ERR(acl))
> - return PTR_ERR(acl);
> - if (!acl)
> - return -ENODATA;
> - error = posix_acl_to_xattr(&init_user_ns, acl, buffer, size);
> - posix_acl_release(acl);
> -
> - return error;
> -}
> -
> -static int f2fs_xattr_set_acl(struct dentry *dentry, const char *name,
> - const void *value, size_t size, int flags, int type)
> -{
> - struct f2fs_sb_info *sbi = F2FS_SB(dentry->d_sb);
> - struct inode *inode = dentry->d_inode;
> - struct posix_acl *acl = NULL;
> - int error;
> -
> - if (strcmp(name, "") != 0)
> - ret

Re: [f2fs-dev] [PATCH 12/18] ocfs2: use generic posix ACL infrastructure

2013-12-10 Thread Christoph Hellwig
On Tue, Dec 03, 2013 at 12:00:07AM +0100, Jan Kara wrote:
>   Hum, this changes the cluster locking. Previously ocfs2_acl_get() used
> from ocfs2_acl_chmod() grabbed cluster wide inode lock. Now getting of ACL
> isn't protected by the inode lock. That being said the cluster locking
> around setattr looks fishy anyway - if two processes on different
> nodes are changing attributes of the same file, changing ACLs post fact
> after dropping inode lock could cause interesting effects. Also I'm
> wondering how inode_change_ok() can ever be safe without holding inode
> lock... Until we grab that other node is free to change e.g. owner of the
> inode thus leading even to security implications. But maybe I'm missing
> something. Mark, Joel?

Hmm, indeed.  How does ocfs2_iop_get_acl get away without that lock?

Btw, ocfs2 changes will need careful testing as I couldn't find any easy
way to run xfstests on ocfs2 out of the box.


--
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH 09/18] f2fs: use generic posix ACL infrastructure

2013-12-10 Thread Jaegeuk Kim
2013-12-08 (일), 01:14 -0800, Christoph Hellwig:
> On Fri, Dec 06, 2013 at 10:37:34AM +0900, Jaegeuk Kim wrote:
> > f2fs caches a new mode bit for a while to make the consistency between
> > xattr's acl mode and the inode mode.
> 
> Can you explain what exactly you're trying to do there?  I've been
> trying to unwrap what's going on and can't really see the point:
> 
>  - i_acl_mode and FI_ACL_MODE get set in __setattr_copy, but right
>after that call, still under i_mutex and before marking the inode
>dirty f2fs_acl_chmod makes use of it, and it gets cleared right
>after. Is there any race that could happen with a locked inode
>not marked dirty yet on f2fs?

As you guess, there is no race problem, but the problem is on acl
consistency between xattr->mode and inode->mode.

Previously, f2fs_setattr triggers:
  new_mode inode->mode xattr->mode iblock->mode
f2fs_setattr x->x   y y   
[update_inode]  x---  [ y ]  ---> x
[checkpoint]x   y x
__f2fs_setxattr x ->x x

In this flow, f2fs is able to break the consistency between xattr->mode
and iblock->mode after checkpoint followed by sudden-power-off.

So, fi->mode was introduced to address the problem.
The new f2fs_setattr triggers:
  new_mode inode->mode fi->mode xattr->mode iblock->mode
f2fs_setattr x---  [y]  --->   x  y  y
[update_inode]  y  x  y  y
[checkpoint]y  x  y  y
__f2fs_setxattr x<-x->x ->   x

Finally, __f2fs_setxattr synchronizes inode->mode, xattr->mode, and
iblock->mode all together.

The root question is "is it possible to call update_inode in the
i_mutex-covered region like f2fs_setattr?".
The update_inode of f2fs is called from a bunch of places so currently
I'm not sure it can be impossible.

Thanks,

> We could pass a mode argument
>to posix_acl_create, but I'd prefer to avoid that if we can.
>  - on the set_acl side it gets set in __f2fs_set_acl, and then
>i_mode is update in __f2fs_setxattr which could easily done with
>a stack variable.
> 
> RFC patch below:
> 
> 
> diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
> index 4f52fe0f..6647545 100644
> --- a/fs/f2fs/acl.c
> +++ b/fs/f2fs/acl.c
> @@ -17,9 +17,6 @@
>  #include "xattr.h"
>  #include "acl.h"
>  
> -#define get_inode_mode(i)((is_inode_flag_set(F2FS_I(i), FI_ACL_MODE)) ? \
> - (F2FS_I(i)->i_acl_mode) : ((i)->i_mode))
> -
>  static inline size_t f2fs_acl_size(int count)
>  {
>   if (count <= 4) {
> @@ -209,11 +206,11 @@ static int __f2fs_set_acl(struct inode *inode, int type,
>   struct posix_acl *acl, struct page *ipage)
>  {
>   struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> - struct f2fs_inode_info *fi = F2FS_I(inode);
>   int name_index;
>   void *value = NULL;
>   size_t size = 0;
>   int error;
> + umode_t mode = 0;
>  
>   if (!test_opt(sbi, POSIX_ACL))
>   return 0;
> @@ -224,10 +221,10 @@ static int __f2fs_set_acl(struct inode *inode, int type,
>   case ACL_TYPE_ACCESS:
>   name_index = F2FS_XATTR_INDEX_POSIX_ACL_ACCESS;
>   if (acl) {
> - error = posix_acl_equiv_mode(acl, &inode->i_mode);
> + mode = inode->i_mode;
> + error = posix_acl_equiv_mode(acl, &mode);
>   if (error < 0)
>   return error;
> - set_acl_inode(fi, inode->i_mode);
>   if (error == 0)
>   acl = NULL;
>   }
> @@ -245,19 +242,15 @@ static int __f2fs_set_acl(struct inode *inode, int type,
>  
>   if (acl) {
>   value = f2fs_acl_to_disk(acl, &size);
> - if (IS_ERR(value)) {
> - cond_clear_inode_flag(fi, FI_ACL_MODE);
> + if (IS_ERR(value))
>   return (int)PTR_ERR(value);
> - }
>   }
>  
> - error = f2fs_setxattr(inode, name_index, "", value, size, ipage);
> + error = f2fs_setxattr(inode, name_index, "", value, size, ipage, mode);
>  
>   kfree(value);
>   if (!error)
>   set_cached_acl(inode, type, acl);
> -
> - cond_clear_inode_flag(fi, FI_ACL_MODE);
>   return error;
>  }
>  
> @@ -289,28 +282,3 @@ int f2fs_init_acl(struct inode *inode, struct inode 
> *dir, struct page *ipage)
>  
>   return error;
>  }
> -
> -int f2fs_acl_chmod(struct inode *inode)
> -{
> - struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> - struct posix_acl *acl;
> - int error;
> - umode_t mode = get_inode_mode(inode);
> -
> - if (!test_opt(sbi, POSIX_ACL))
> - return 0;
> - if (S_ISLNK(mode))
> - return -EOPNOTSUPP;
> -
> - acl = f2fs_get_acl(inode, A

Re: [f2fs-dev] [PATCH 00/18] Consolidate Posix ACL implementation

2013-12-10 Thread Andreas Gruenbacher
Christoph,

nice work, and a pretty diffstat.

I see that get_acl and set_acl are being defined in some but not all symlink 
inode operations (for example, btrfs them while ext4 does not), and that 
posix_acl_xattr_set() doesn't check if set_acl is defined.  Symlinks cannot 
have ACLs, so set_acl should either never be defined for symlinks (and a NULL 
check is then needed in posix_acl_xattr_set()), or it is defined in all inode 
operations, and S_ISNLNK() check is needed in posix_acl_xattr_set().  That 
latter check should probably be added in any case to be on the safe side.

  Test case:

  setfattr -h -n system.posix_acl_access \
   -v 0sAgEABgD/AgAGABMEAAAEAAYA/xAABgD/IAAEAP8= \
   symlink

Patch 6 also declares posix_acl_prepare() but this function is never 
introduced; this must be a leftover from a previous version.

Thanks,
Andreas

--
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH 3/3 V3] f2fs: introduce f2fs_cache_node_page() to add page into node_inode cache

2013-12-10 Thread Chao Yu
> -Original Message-
> From: Chao Yu [mailto:chao2...@samsung.com]
> Sent: Tuesday, December 10, 2013 9:52 AM
> To: jaegeuk@samsung.com
> Cc: linux-fsde...@vger.kernel.org; linux-ker...@vger.kernel.org; 
> linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 3/3 V3] f2fs: introduce f2fs_cache_node_page() 
> to add page into node_inode cache
> 
> Hi,
> 
> > -Original Message-
> > From: Jaegeuk Kim [mailto:jaegeuk@samsung.com]
> > Sent: Monday, December 09, 2013 4:01 PM
> > To: Chao Yu
> > Cc: linux-fsde...@vger.kernel.org; linux-ker...@vger.kernel.org; 
> > linux-f2fs-devel@lists.sourceforge.net
> > Subject: Re: [f2fs-dev] [PATCH 3/3 V3] f2fs: introduce 
> > f2fs_cache_node_page() to add page into node_inode cache
> >
> > Hi,
> >
> > Sorry for the noise.
> > Once I tested this patch, I recognized that I made a mistake:
> >
> > f2fs_fill_super()
> >  1. build_segment_manager()
> > -> restore_node_summary()
> >-> NULL pointer exception, due to no NAT cache.
> >  2. build_node_manager()
> >
> > So, we should not call get_node_info() prior to build_node_manager().
> > And, it's not easy to change the order of build_*_manager simply.
> >
> > Let's consider in more details.
> 
> Before we call restore_node_summary(), we have read HOT_DATA
> Summary. It means already we got the NAT cache in curseg.
> Although we could not call get_node_info() directly, we still could use
> NAT cache in HOT_DATA curseg.
> 
> So how about using this cache to verify validity of page like this?
> 
> f2fs_cache_node_page()
>   if(!looup_nat_curseg_cache())
>   return;

Code of verification is missed here.
if (curseg cache blk_addr != current blkaddr)
return;

>   grab_cache_page();
>   memcpy();
>   SetPageUptodate();
>   f2fs_put_page();
> 
> > Thanks,
> >
> > 2013-12-09 (월), 13:35 +0800, Chao Yu:
> > > This patch introduces f2fs_cache_node_page(), in this function, page 
> > > which is
> > > readed ahead will be copy to node_inode's mapping cache.
> > > It will avoid rereading these node pages.
> > >
> > > change log:
> > >  o check validity of page by searching NAT suggested by Jaegeuk Kim.
> > >  o add 'unlikely' for compiler optimization suggested by Jaegeuk Kim.
> > >
> > > Suggested-by: Jaegeuk Kim 
> > > Signed-off-by: Chao Yu 
> > > ---
> > >  fs/f2fs/node.c |   39 ++-
> > >  1 file changed, 38 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> > > index 099f06f..4c6da98 100644
> > > --- a/fs/f2fs/node.c
> > > +++ b/fs/f2fs/node.c
> > > @@ -1600,13 +1600,46 @@ static int ra_sum_pages(struct f2fs_sb_info *sbi, 
> > > struct list_head *pages,
> > >   return 0;
> > >  }
> > >
> > > +/*
> > > + * f2fs_cache_node_page() check validaty of input page by searching NAT.
> > > + * Then, it will copy updated data of vaild page to node_inode cache.
> > > + */
> > > +void f2fs_cache_node_page(struct f2fs_sb_info *sbi, struct page *page,
> > > + nid_t nid, block_t blkaddr)
> > > +{
> > > + struct address_space *mapping = sbi->node_inode->i_mapping;
> > > + struct page *npage;
> > > + struct node_info ni;
> > > +
> > > + get_node_info(sbi, nid, &ni);
> > > +
> > > + if (ni.blk_addr != blkaddr)
> > > + return;
> > > +
> > > + npage = grab_cache_page(mapping, nid);
> > > + if (unlikely(!npage))
> > > + return;
> > > +
> > > + if (PageUptodate(npage)) {
> > > + f2fs_put_page(npage, 1);
> > > + return;
> > > + }
> > > +
> > > + memcpy(page_address(npage), page_address(page), PAGE_CACHE_SIZE);
> > > +
> > > + SetPageUptodate(npage);
> > > + f2fs_put_page(npage, 1);
> > > +
> > > + return;
> > > +}
> > > +
> > >  int restore_node_summary(struct f2fs_sb_info *sbi,
> > >   unsigned int segno, struct f2fs_summary_block *sum)
> > >  {
> > >   struct f2fs_node *rn;
> > >   struct f2fs_summary *sum_entry;
> > >   struct page *page, *tmp;
> > > - block_t addr;
> > > + block_t addr, blkaddr;
> > >   int bio_blocks = MAX_BIO_BLOCKS(max_hw_blocks(sbi));
> > >   int i, last_offset, nrpages, err = 0;
> > >   LIST_HEAD(page_list);
> > > @@ -1624,6 +1657,7 @@ int restore_node_summary(struct f2fs_sb_info *sbi,
> > >   if (err)
> > >   return err;
> > >
> > > + blkaddr = addr;
> > >   list_for_each_entry_safe(page, tmp, &page_list, lru) {
> > >
> > >   lock_page(page);
> > > @@ -1633,6 +1667,8 @@ int restore_node_summary(struct f2fs_sb_info *sbi,
> > >   sum_entry->version = 0;
> > >   sum_entry->ofs_in_node = 0;
> > >   sum_entry++;
> > > + f2fs_cache_node_page(sbi, page,
> > > + le32_to_cpu(rn->footer.nid), blkaddr);
> > >   } else {
> > >   err = -EIO;
> > >   }
> > > @@ -1

Re: [f2fs-dev] [PATCH v2] f2fs: add unlikely() macro for compiler more aggressively

2013-12-10 Thread Jaegeuk Kim
>From d373d7084e4d132fa596fd5def1b9a9843e1410f Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim 
Date: Fri, 6 Dec 2013 15:00:58 +0900
Subject: [PATCH] f2fs: add unlikely() macro for compiler more
aggressively

This patch adds unlikely() macro into the most of codes.
The basic rule is to add that when:
- checking unusual errors,
- checking page mappings,
- and the other unlikely conditions.

Change log from v1:
 - Don't add unlikely for the NULL test and error test: advised by Andi
Kleen.

Cc: Chao Yu 
Cc: Andi Kleen 
Reviewed-by: Chao Yu 
Signed-off-by: Jaegeuk Kim 
---
 fs/f2fs/checkpoint.c | 10 +-
 fs/f2fs/data.c   | 29 ++---
 fs/f2fs/file.c   |  9 -
 fs/f2fs/gc.c |  3 +--
 fs/f2fs/node.c   | 28 ++--
 fs/f2fs/recovery.c   |  2 +-
 fs/f2fs/super.c  | 14 +++---
 fs/f2fs/xattr.c  |  2 +-
 8 files changed, 47 insertions(+), 50 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 6b21066..cf505eb 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -66,7 +66,7 @@ repeat:
goto repeat;
 
lock_page(page);
-   if (page->mapping != mapping) {
+   if (unlikely(page->mapping != mapping)) {
f2fs_put_page(page, 1);
goto repeat;
}
@@ -473,7 +473,7 @@ static int __add_dirty_inode(struct inode *inode,
struct dir_inode_entry *new)
list_for_each(this, head) {
struct dir_inode_entry *entry;
entry = list_entry(this, struct dir_inode_entry, list);
-   if (entry->inode == inode)
+   if (unlikely(entry->inode == inode))
return -EEXIST;
}
list_add_tail(&new->list, head);
@@ -783,7 +783,7 @@ static void do_checkpoint(struct f2fs_sb_info *sbi,
bool is_umount)
/* Here, we only have one bio having CP pack */
sync_meta_pages(sbi, META_FLUSH, LONG_MAX);
 
-   if (!is_set_ckpt_flags(ckpt, CP_ERROR_FLAG)) {
+   if (unlikely(!is_set_ckpt_flags(ckpt, CP_ERROR_FLAG))) {
clear_prefree_segments(sbi);
F2FS_RESET_SB_DIRT(sbi);
}
@@ -840,11 +840,11 @@ int __init create_checkpoint_caches(void)
 {
orphan_entry_slab = f2fs_kmem_cache_create("f2fs_orphan_entry",
sizeof(struct orphan_inode_entry), NULL);
-   if (unlikely(!orphan_entry_slab))
+   if (!orphan_entry_slab)
return -ENOMEM;
inode_entry_slab = f2fs_kmem_cache_create("f2fs_dirty_dir_entry",
sizeof(struct dir_inode_entry), NULL);
-   if (unlikely(!inode_entry_slab)) {
+   if (!inode_entry_slab) {
kmem_cache_destroy(orphan_entry_slab);
return -ENOMEM;
}
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 2ce5a9e..5607393 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -49,11 +49,11 @@ static void f2fs_read_end_io(struct bio *bio, int
err)
if (--bvec >= bio->bi_io_vec)
prefetchw(&bvec->bv_page->flags);
 
-   if (uptodate) {
-   SetPageUptodate(page);
-   } else {
+   if (unlikely(!uptodate)) {
ClearPageUptodate(page);
SetPageError(page);
+   } else {
+   SetPageUptodate(page);
}
unlock_page(page);
} while (bvec >= bio->bi_io_vec);
@@ -73,7 +73,7 @@ static void f2fs_write_end_io(struct bio *bio, int
err)
if (--bvec >= bio->bi_io_vec)
prefetchw(&bvec->bv_page->flags);
 
-   if (!uptodate) {
+   if (unlikely(!uptodate)) {
SetPageError(page);
set_bit(AS_EIO, &page->mapping->flags);
set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
@@ -249,7 +249,7 @@ int reserve_new_block(struct dnode_of_data *dn)
 {
struct f2fs_sb_info *sbi = F2FS_SB(dn->inode->i_sb);
 
-   if (is_inode_flag_set(F2FS_I(dn->inode), FI_NO_ALLOC))
+   if (unlikely(is_inode_flag_set(F2FS_I(dn->inode), FI_NO_ALLOC)))
return -EPERM;
if (unlikely(!inc_valid_block_count(sbi, dn->inode, 1)))
return -ENOSPC;
@@ -424,7 +424,7 @@ struct page *find_data_page(struct inode *inode,
pgoff_t index, bool sync)
return ERR_PTR(-ENOENT);
 
/* By fallocate(), there is no cached page, but with NEW_ADDR */
-   if (dn.data_blkaddr == NEW_ADDR)
+   if (unlikely(dn.data_blkaddr == NEW_ADDR))
return ERR_PTR(-EINVAL);
 
page = grab_cache_page_write_begin(mapping, index, AOP_FLAG_NOFS);
@@ -443,7 +443,7 @@ struct page *find_data_page(struct inode *inode,
pgoff_t index, bool sync)
 
if (sync) {
wait_on_page_locked(page);
-   if (!PageUptodate(page)) {
+   if (unlikely(