2013-08-12 07:58:56

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 1/2] f2fs: check the free space first in new_node_page

Let's check the free space in prior to the main process of allocating a new node
page.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/node.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index bb8fbda..858a333 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -833,29 +833,29 @@ struct page *new_node_page(struct dnode_of_data *dn,
if (!page)
return ERR_PTR(-ENOMEM);

- get_node_info(sbi, dn->nid, &old_ni);
+ if (!inc_valid_node_count(sbi, dn->inode, 1)) {
+ err = -ENOSPC;
+ goto fail;
+ }

- SetPageUptodate(page);
- fill_node_footer(page, dn->nid, dn->inode->i_ino, ofs, true);
+ get_node_info(sbi, dn->nid, &old_ni);

/* Reinitialize old_ni with new node page */
BUG_ON(old_ni.blk_addr != NULL_ADDR);
new_ni = old_ni;
new_ni.ino = dn->inode->i_ino;
-
- if (!inc_valid_node_count(sbi, dn->inode, 1)) {
- err = -ENOSPC;
- goto fail;
- }
set_node_addr(sbi, &new_ni, NEW_ADDR);
+
+ fill_node_footer(page, dn->nid, dn->inode->i_ino, ofs, true);
set_cold_node(dn->inode, page);
+ SetPageUptodate(page);
+ set_page_dirty(page);

dn->node_page = page;
if (ipage)
update_inode(dn->inode, ipage);
else
sync_inode_page(dn);
- set_page_dirty(page);
if (ofs == 0)
inc_valid_inode_count(sbi);

--
1.8.3.1.437.g0dbd812


2013-08-12 07:59:15

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 2/2] f2fs: should cover i_xattr_nid with its xattr node page lock

Previously, f2fs_setxattr assigns i_xattr_nid in the inode page inconsistently.

The scenario is:

= Thread 1 = = Thread 2 = = fi->i_xattr_nid = = on-disk nid =

f2fs_setxattr 0 0
new_node_page X 0
sync_inode_page X X
checkpoint X X -.
grab_cache_page X X |
--> allocate a new xattr node block or -ENOSPC <----------------'

At this moment, the checkpoint stores inconsistent data where the inode has
i_xattr_nid but actual xattr node block is not allocated yet.

So, we should assign the real i_xattr_nid only after its xattr node block is
allocated.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/node.c | 3 +++
fs/f2fs/xattr.c | 10 +++++-----
2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 858a333..1c21344 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -851,6 +851,9 @@ struct page *new_node_page(struct dnode_of_data *dn,
SetPageUptodate(page);
set_page_dirty(page);

+ if (ofs == XATTR_NODE_OFFSET)
+ F2FS_I(dn->inode)->i_xattr_nid = dn->nid;
+
dn->node_page = page;
if (ipage)
update_inode(dn->inode, ipage);
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index fb16f71..3bc307c 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -378,23 +378,23 @@ int f2fs_setxattr(struct inode *inode, int name_index, const char *name,
if (!fi->i_xattr_nid) {
/* Allocate new attribute block */
struct dnode_of_data dn;
+ nid_t new_nid;

- if (!alloc_nid(sbi, &fi->i_xattr_nid)) {
+ if (!alloc_nid(sbi, &new_nid)) {
error = -ENOSPC;
goto exit;
}
- set_new_dnode(&dn, inode, NULL, NULL, fi->i_xattr_nid);
+ set_new_dnode(&dn, inode, NULL, NULL, new_nid);
mark_inode_dirty(inode);

page = new_node_page(&dn, XATTR_NODE_OFFSET, ipage);
if (IS_ERR(page)) {
- alloc_nid_failed(sbi, fi->i_xattr_nid);
- fi->i_xattr_nid = 0;
+ alloc_nid_failed(sbi, new_nid);
error = PTR_ERR(page);
goto exit;
}

- alloc_nid_done(sbi, fi->i_xattr_nid);
+ alloc_nid_done(sbi, new_nid);
base_addr = page_address(page);
header = XATTR_HDR(base_addr);
header->h_magic = cpu_to_le32(F2FS_XATTR_MAGIC);
--
1.8.3.1.437.g0dbd812