2008-02-12 16:45:09

by Aneesh Kumar K.V

[permalink] [raw]
Subject: [PATCH] ext4: Set directory link count to zero if we fail to create the directory.

With ext4 we fail directory creation if we fail to allocate initial
block for the directory. With EXT4_FEATURE_RO_COMPAT_DIR_NLINK,
if we fail to create the directory, the inode link count was wrongly
set to 1. This cause e2fsck throws the below warning

Inode 1015 is a zero-length directory. Clear? yes
Inode 1120 is a zero-length directory. Clear? yes
Inode 1121 is a zero-length directory. Clear? yes

Fix the same by dropping the inode link count using drop_nlink

Signed-off-by: Aneesh Kumar K.V <[email protected]>
---
fs/ext4/namei.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index a9347fb..fd3b031 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1805,7 +1805,7 @@ retry:
inode->i_size = EXT4_I(inode)->i_disksize = inode->i_sb->s_blocksize;
dir_block = ext4_bread (handle, inode, 0, 1, &err);
if (!dir_block) {
- ext4_dec_count(handle, inode); /* is this nlink == 0? */
+ drop_nlink(inode);
ext4_mark_inode_dirty(handle, inode);
iput (inode);
goto out_stop;
--
1.5.4.1.97.g40aab-dirty


2008-02-16 00:08:27

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: Set directory link count to zero if we fail to create the directory.

I've replaced this patch with the following (see attached). The
changes to the patch from Aneesh's original one are:

1) Replace the patch description with one that describes what is going
on at a higher level. (Always write with a deep sympathy for the
reader, who will be reading the description as part of "git log".
So mention that the problem was in ext4_mkdir, and the
circumstances in which it would occur.)

2) The error handling code was duplicated later in the function
(although in a slightly more efficient fashion; since we *know* the
newly created inode has an inode->i_nlink of 1, it's easier just to
set i_nlink to zero)

3) Replace the inode->i_nlink = 0 with the higher level equivalent
inline function, clear_nlink(inode).

- Ted

ext4: Don't leave behind a half-created inode if ext4_mkdir() fails

From: "Aneesh Kumar K.V" <[email protected]>

If ext4_mkdir() fails to allocate the initial block for the directory,
don't leave behind a half-created directory inode with the link count
left at one. This was caused by an inappropriate call to ext4_dec_count().

Signed-off-by: Aneesh Kumar K.V <[email protected]>
Signed-off-by: Mingming Cao <[email protected]>
Signed-off-by: "Theodore Ts'o" <[email protected]>

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index a9347fb..7970e81 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1804,12 +1804,8 @@ retry:
inode->i_fop = &ext4_dir_operations;
inode->i_size = EXT4_I(inode)->i_disksize = inode->i_sb->s_blocksize;
dir_block = ext4_bread (handle, inode, 0, 1, &err);
- if (!dir_block) {
- ext4_dec_count(handle, inode); /* is this nlink == 0? */
- ext4_mark_inode_dirty(handle, inode);
- iput (inode);
- goto out_stop;
- }
+ if (!dir_block)
+ goto out_clear_inode;
BUFFER_TRACE(dir_block, "get_write_access");
ext4_journal_get_write_access(handle, dir_block);
de = (struct ext4_dir_entry_2 *) dir_block->b_data;
@@ -1832,7 +1828,8 @@ retry:
ext4_mark_inode_dirty(handle, inode);
err = ext4_add_entry (handle, dentry, inode);
if (err) {
- inode->i_nlink = 0;
+ out_clear_inode:
+ clear_nlink(inode);
ext4_mark_inode_dirty(handle, inode);
iput (inode);
goto out_stop;