Hi Ted,
The below test case fails x86
#ln -s sfdajsfsdsdfskjdgfkshdfkshdfkgdsjdsjdhsdfdafdsgakfasdgfsjksdsdsdfsd p
#sync
#rm p
root@elm3b165:~# /usr/local/e2fsprogs/sbin/e2fsck -f /dev/sda5
e2fsck 1.40.8 (13-Mar-2008)
Pass 1: Checking inodes, blocks, and sizes
Fast symlink 12 has EXTENT_FL set. Clear<y>?
ie because after deleting the file we have
debugfs: stat <12>
Inode: 12 Type: symlink Mode: 0777 Flags: 0x80000 Generation:
25346603
User: 0 Group: 0 Size: 0
File ACL: 0 Directory ACL: 0
Links: 0 Blockcount: 0
Fragment: Address: 0 Number: 0 Size: 0
ctime: 0x47e39ea3 -- Fri Mar 21 11:40:19 2008
atime: 0x47e39e9e -- Fri Mar 21 11:40:14 2008
mtime: 0x47e39e9a -- Fri Mar 21 11:40:10 2008
dtime: 0x47e39ea3 -- Fri Mar 21 11:40:19 2008
Size of extra inode fields: 28
Fast_link_dest:
shouldn't e2fsprogs also look at whether the inode is in use ?
-aneesh
On Fri, Mar 21, 2008 at 05:12:34PM +0530, Aneesh Kumar K.V wrote:
> Hi Ted,
>
> The below test case fails x86
> #ln -s sfdajsfsdsdfskjdgfkshdfkshdfkgdsjdsjdhsdfdafdsgakfasdgfsjksdsdsdfsd p
> #sync
> #rm p
> root@elm3b165:~# /usr/local/e2fsprogs/sbin/e2fsck -f /dev/sda5
> e2fsck 1.40.8 (13-Mar-2008)
> Pass 1: Checking inodes, blocks, and sizes
> Fast symlink 12 has EXTENT_FL set. Clear<y>?
>
> ie because after deleting the file we have
> debugfs: stat <12>
> Inode: 12 Type: symlink Mode: 0777 Flags: 0x80000 Generation:
> 25346603
> User: 0 Group: 0 Size: 0
> File ACL: 0 Directory ACL: 0
> Links: 0 Blockcount: 0
> Fragment: Address: 0 Number: 0 Size: 0
> ctime: 0x47e39ea3 -- Fri Mar 21 11:40:19 2008
> atime: 0x47e39e9e -- Fri Mar 21 11:40:14 2008
> mtime: 0x47e39e9a -- Fri Mar 21 11:40:10 2008
> dtime: 0x47e39ea3 -- Fri Mar 21 11:40:19 2008
> Size of extra inode fields: 28
> Fast_link_dest:
>
>
> shouldn't e2fsprogs also look at whether the inode is in use ?
Or something like this in kernel ?
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 50d700f..cc124b1 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -204,6 +204,14 @@ void ext4_delete_inode (struct inode * inode)
inode->i_size = 0;
if (inode->i_blocks)
ext4_truncate(inode);
+
+ /*
+ * In case of link clear the extent flag. Fast symlinks are not
+ * stored in extent format we use i_blocks count to determine
+ * whether it is fast link or not.
+ */
+ if (S_ISLNK(inode->i_mode))
+ EXT4_I(inode)->i_flags &= ~EXT4_EXTENTS_FL;
/*
* Kill off the orphan record which ext4_truncate created.
* AKPM: I think this can be inside the above `if'.
Thanks to Aneesh Kumar for pointing this out.
Signed-off-by: "Theodore Ts'o" <[email protected]>
---
e2fsck/pass1.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index 79e9f23..67a8370 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -713,14 +713,6 @@ void e2fsck_pass1(e2fsck_t ctx)
}
}
- if (extent_fs && (inode->i_flags & EXT4_EXTENTS_FL) &&
- LINUX_S_ISLNK(inode->i_mode) &&
- !ext2fs_inode_has_valid_blocks(inode) &&
- fix_problem(ctx, PR_1_FAST_SYMLINK_EXTENT_FL, &pctx)) {
- inode->i_flags &= ~EXT4_EXTENTS_FL;
- e2fsck_write_inode(ctx, ino, inode, "pass1");
- }
On Fri, 21 Mar 2008, Aneesh Kumar K.V wrote:
> The below test case fails x86
> #ln -s sfdajsfsdsdfskjdgfkshdfkshdfkgdsjdsjdhsdfdafdsgakfasdgfsjksdsdsdfsd p
> #sync
> #rm p
> root@elm3b165:~# /usr/local/e2fsprogs/sbin/e2fsck -f /dev/sda5
> e2fsck 1.40.8 (13-Mar-2008)
> Pass 1: Checking inodes, blocks, and sizes
> Fast symlink 12 has EXTENT_FL set. Clear<y>?
>
> ie because after deleting the file we have
> debugfs: stat <12>
> Inode: 12 Type: symlink Mode: 0777 Flags: 0x80000 Generation:
> 25346603
> User: 0 Group: 0 Size: 0
> File ACL: 0 Directory ACL: 0
> Links: 0 Blockcount: 0
> Fragment: Address: 0 Number: 0 Size: 0
> ctime: 0x47e39ea3 -- Fri Mar 21 11:40:19 2008
> atime: 0x47e39e9e -- Fri Mar 21 11:40:14 2008
> mtime: 0x47e39e9a -- Fri Mar 21 11:40:10 2008
> dtime: 0x47e39ea3 -- Fri Mar 21 11:40:19 2008
> Size of extra inode fields: 28
> Fast_link_dest:
Aneesh, can you elaborate a bit for me (a user, not a developer) what this
means? I too had these "Fast symlink 12 has EXTENT_FL set. Clear<y>?"
messages on my ext4 fs and I went on to "Clear" these things. Now I notice
some serious performance (or better: load) problems when doing stuff on
ext4 and I'm not sure if this could be related to the changes e2fsprogs
did.
Thanks,
Christian.
--
BOFH excuse #199:
the curls in your keyboard cord are losing electricity.
On Mon, Mar 24, 2008 at 01:25:33PM +0100, Christian Kujau wrote:
> Aneesh, can you elaborate a bit for me (a user, not a developer) what this
> means? I too had these "Fast symlink 12 has EXTENT_FL set. Clear<y>?"
> messages on my ext4 fs and I went on to "Clear" these things. Now I notice
> some serious performance (or better: load) problems when doing stuff on
> ext4 and I'm not sure if this could be related to the changes e2fsprogs
> did.
It should make no difference. The bug which Aneesh was pointing out
was that e2fsck was complaining about long symlinks in ext4 which got
deleted, and which thus got mistaken for fast symlinks with the
EXTENTS_FL flag set, even though said inodes were already deleted.
So this should not make any difference in terms of ext4 performance;
just noise in terms of needlessly annoying someone running e2fsck.
- Ted
On Mon, 24 Mar 2008, Theodore Tso wrote:
> So this should not make any difference in terms of ext4 performance;
> just noise in terms of needlessly annoying someone running e2fsck.
Ah, ok, then it's something else.
Thanks,
Christian.
--
BOFH excuse #379:
We've picked COBOL as the language of choice.
On Mar 24, 2008 17:07 +0530, Aneesh Kumar K.V wrote:
> @@ -204,6 +204,14 @@ void ext4_delete_inode (struct inode * inode)
> if (inode->i_blocks)
> ext4_truncate(inode);
> +
> + /*
> + * In case of link clear the extent flag. Fast symlinks are not
> + * stored in extent format we use i_blocks count to determine
> + * whether it is fast link or not.
> + */
> + if (S_ISLNK(inode->i_mode))
> + EXT4_I(inode)->i_flags &= ~EXT4_EXTENTS_FL;
This is a big hack I think. It shouldn't matter to e2fsck whether a
deleted long symlink is in extent format or not, and clearing the flag
like this would make it harder to undelete this symlink if needed. This
should be fixed in e2fsck and not the kernel.
Also, it is (IMHO) a hack that we use the i_blocks count to determine
if a symlink is fast or slow. That is what caused the breakage when
EAs on symlinks were added, and if we add extra blocks to symlinks for
some other reason (e.g. more than a single EA block, or other) it will
break again.
Instead I propose that we just use the i_size itself to determine if
there is a fast symlink, because there has never (AFAIK) been a kernel
that created slow symlinks for files < 60 bytes in length.
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.
On Mon, Mar 24, 2008 at 02:45:10PM -0600, Andreas Dilger wrote:
> Instead I propose that we just use the i_size itself to determine if
> there is a fast symlink, because there has never (AFAIK) been a kernel
> that created slow symlinks for files < 60 bytes in length.
I have a vague memory that at one point (along time ago, over ten
years ago) there were slow symlinks where the target was < 60 bytes.
And the kernel has always determined whether or not a symlink was fast
or slow by looking i_blocks. (See ext3_inode_is_fast_symlink() in
fs/ext3/inode.c).
In retrospect, the true clean way to do this would have been an
explicit i_flags bitfield. One thing we could do is make a change
into ext4 (and ext3) so that we silently set an EXT3_SLOW_LINK_FL and
EXT3_FAST_LINK_FL, and if neither is set, we fall back to a hueristic
involving i_blocks. This gives e2fsck one more bit of redundancy to
make sure it notices problems and to make sure it gets things right.
I'm not sure it's worth it, but eventually it would allow us to clean
things up.
- Ted
On Mar 24, 2008 17:26 -0400, Theodore Ts'o wrote:
> On Mon, Mar 24, 2008 at 02:45:10PM -0600, Andreas Dilger wrote:
> > Instead I propose that we just use the i_size itself to determine if
> > there is a fast symlink, because there has never (AFAIK) been a kernel
> > that created slow symlinks for files < 60 bytes in length.
>
> I have a vague memory that at one point (along time ago, over ten
> years ago) there were slow symlinks where the target was < 60 bytes.
Hmm, I don't recall this, but it seems possible. As far back as 2.2
kernels I checked this wasn't the case, I haven't looked back further.
> And the kernel has always determined whether or not a symlink was fast
> or slow by looking i_blocks. (See ext3_inode_is_fast_symlink() in
> fs/ext3/inode.c).
Sure, but that doesn't mean it is the best way...
> In retrospect, the true clean way to do this would have been an
> explicit i_flags bitfield. One thing we could do is make a change
> into ext4 (and ext3) so that we silently set an EXT3_SLOW_LINK_FL and
> EXT3_FAST_LINK_FL, and if neither is set, we fall back to a hueristic
> involving i_blocks. This gives e2fsck one more bit of redundancy to
> make sure it notices problems and to make sure it gets things right.
> I'm not sure it's worth it, but eventually it would allow us to clean
> things up.
Since it is impossible to have a fast symlink with > 60 bytes of data
it seems reasonable to only flag slow symlinks explicitly. The unusual,
but theoretically possible, case would be slow symlinks <= 60 bytes, so
we may as well flag all slow symlinks and assume fast symlinks for others.
I don't think there are a huge number of available flags left (12 or less)
so we can't use them without good reason.
Hmm, that brings up a question as I look at the used flags in 2.6.24 -
did the HUGE_FILE support make it into the ext4 upstream?
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.