Re: [f2fs-dev] [PATCH v2] f2fs: add unlikely() macro for compiler more aggressively
From d373d7084e4d132fa596fd5def1b9a9843e1410f Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim jaegeuk@samsung.com
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 chao2...@samsung.com
Cc: Andi Kleen a...@firstfloor.org
Reviewed-by: Chao Yu chao2...@samsung.com
Signed-off-by: Jaegeuk Kim jaegeuk@samsung.com
---
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) {
Re: [f2fs-dev] [PATCH 3/3 V3] f2fs: introduce f2fs_cache_node_page() to add page into node_inode cache
-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 jaegeuk@samsung.com
Signed-off-by: Chao Yu chao2...@samsung.com
---
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;
}
@@ -1640,6 +1676,7 @@ int restore_node_summary(struct f2fs_sb_info *sbi,
list_del(page-lru);
unlock_page(page);
__free_pages(page, 0);
+ blkaddr++;
}
}
return err;
--
Jaegeuk Kim
Samsung
Re: [f2fs-dev] [PATCH 00/18] Consolidate Posix ACL implementation
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=84349831iu=/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 12/18] ocfs2: use generic posix ACL infrastructure
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=84349831iu=/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-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, ACL_TYPE_ACCESS);
- if (IS_ERR(acl) || !acl)
- return PTR_ERR(acl);
-
- error = __posix_acl_chmod(acl, GFP_KERNEL, mode);
-
Re: [f2fs-dev] [Cluster-devel] [PATCH 16/18] gfs2: use generic posix ACL infrastructure
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=84349831iu=/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
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 inode number */
-
Re: [f2fs-dev] [Cluster-devel] [PATCH 16/18] gfs2: use generic posix ACL infrastructure
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 h...@lst.de
---
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=84349831iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
