From: "Duane Griffin" <[email protected]>
If the orphan node list includes valid, untruncatable nodes with nlink > 0
the ext3_orphan_cleanup loop which attempts to delete them will not do so,
causing it to loop forever. Fix by checking for such nodes in the
ext3_orphan_get function.
This patch fixes the second case (image hdb.20000009.softlockup.gz)
reported in http://bugzilla.kernel.org/show_bug.cgi?id=10882.
[[email protected]: coding-style fixes]
[[email protected]: printk warning fix]
Signed-off-by: Duane Griffin <[email protected]>
Cc: <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
fs/ext3/ialloc.c | 9 +++++++++
fs/ext3/inode.c | 20 ++++++++++++++------
include/linux/ext3_fs.h | 1 +
3 files changed, 24 insertions(+), 6 deletions(-)
diff -puN fs/ext3/ialloc.c~ext3-handle-corrupted-orphan-list-at-mount fs/ext3/ialloc.c
--- a/fs/ext3/ialloc.c~ext3-handle-corrupted-orphan-list-at-mount
+++ a/fs/ext3/ialloc.c
@@ -669,6 +669,14 @@ struct inode *ext3_orphan_get(struct sup
if (IS_ERR(inode))
goto iget_failed;
+ /*
+ * If the orphans has i_nlinks > 0 then it should be able to be
+ * truncated, otherwise it won't be removed from the orphan list
+ * during processing and an infinite loop will result.
+ */
+ if (inode->i_nlink && !ext3_can_truncate(inode))
+ goto bad_orphan;
+
if (NEXT_ORPHAN(inode) > max_ino)
goto bad_orphan;
brelse(bitmap_bh);
@@ -690,6 +698,7 @@ bad_orphan:
printk(KERN_NOTICE "NEXT_ORPHAN(inode)=%u\n",
NEXT_ORPHAN(inode));
printk(KERN_NOTICE "max_ino=%lu\n", max_ino);
+ printk(KERN_NOTICE "i_nlink=%u\n", inode->i_nlink);
/* Avoid freeing blocks if we got a bad deleted inode */
if (inode->i_nlink == 0)
inode->i_blocks = 0;
diff -puN fs/ext3/inode.c~ext3-handle-corrupted-orphan-list-at-mount fs/ext3/inode.c
--- a/fs/ext3/inode.c~ext3-handle-corrupted-orphan-list-at-mount
+++ a/fs/ext3/inode.c
@@ -2253,6 +2253,19 @@ static void ext3_free_branches(handle_t
}
}
+int ext3_can_truncate(struct inode *inode)
+{
+ if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
+ return 0;
+ if (S_ISREG(inode->i_mode))
+ return 1;
+ if (S_ISDIR(inode->i_mode))
+ return 1;
+ if (S_ISLNK(inode->i_mode))
+ return !ext3_inode_is_fast_symlink(inode);
+ return 0;
+}
+
/*
* ext3_truncate()
*
@@ -2297,12 +2310,7 @@ void ext3_truncate(struct inode *inode)
unsigned blocksize = inode->i_sb->s_blocksize;
struct page *page;
- if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
- S_ISLNK(inode->i_mode)))
- return;
- if (ext3_inode_is_fast_symlink(inode))
- return;
- if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
+ if (!ext3_can_truncate(inode))
return;
/*
diff -puN include/linux/ext3_fs.h~ext3-handle-corrupted-orphan-list-at-mount include/linux/ext3_fs.h
--- a/include/linux/ext3_fs.h~ext3-handle-corrupted-orphan-list-at-mount
+++ a/include/linux/ext3_fs.h
@@ -832,6 +832,7 @@ extern void ext3_discard_reservation (st
extern void ext3_dirty_inode(struct inode *);
extern int ext3_change_inode_journal_flag(struct inode *, int);
extern int ext3_get_inode_loc(struct inode *, struct ext3_iloc *);
+extern int ext3_can_truncate(struct inode *inode);
extern void ext3_truncate (struct inode *);
extern void ext3_set_inode_flags(struct inode *);
extern void ext3_get_inode_flags(struct ext3_inode_info *);
_
On Fri, Jul 25, 2008 at 3:02 AM, <[email protected]> wrote:
> From: "Duane Griffin" <[email protected]>
>
> If the orphan node list includes valid, untruncatable nodes with nlink > 0
> the ext3_orphan_cleanup loop which attempts to delete them will not do so,
> causing it to loop forever. Fix by checking for such nodes in the
> ext3_orphan_get function.
>
> This patch fixes the second case (image hdb.20000009.softlockup.gz)
> reported in http://bugzilla.kernel.org/show_bug.cgi?id=10882.
>
> [[email protected]: coding-style fixes]
> [[email protected]: printk warning fix]
> Signed-off-by: Duane Griffin <[email protected]>
> Cc: <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> diff -puN fs/ext3/inode.c~ext3-handle-corrupted-orphan-list-at-mount fs/ext3/inode.c
> --- a/fs/ext3/inode.c~ext3-handle-corrupted-orphan-list-at-mount
> +++ a/fs/ext3/inode.c
> @@ -2253,6 +2253,19 @@ static void ext3_free_branches(handle_t
> }
> }
>
> +int ext3_can_truncate(struct inode *inode)
> +{
> + if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
> + return 0;
> + if (S_ISREG(inode->i_mode))
> + return 1;
> + if (S_ISDIR(inode->i_mode))
> + return 1;
> + if (S_ISLNK(inode->i_mode))
> + return !ext3_inode_is_fast_symlink(inode);
> + return 0;
> +}
> +
> /*
> * ext3_truncate()
> *
> @@ -2297,12 +2310,7 @@ void ext3_truncate(struct inode *inode)
> unsigned blocksize = inode->i_sb->s_blocksize;
> struct page *page;
>
> - if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
> - S_ISLNK(inode->i_mode)))
> - return;
> - if (ext3_inode_is_fast_symlink(inode))
> - return;
> - if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
> + if (!ext3_can_truncate(inode))
> return;
I may be missing something here but doesn't the above change the logic
that was used in ext3_truncate for the S_ISDIR and S_ISLNK cases?
Before ext3_truncate would be short-circuited if S_ISDIR or S_ISLNK,
now it won't... is that intended?
On Fri, Jul 25, 2008 at 2:33 PM, Mike Snitzer <[email protected]> wrote:
> On Fri, Jul 25, 2008 at 3:02 AM, <[email protected]> wrote:
>> From: "Duane Griffin" <[email protected]>
>>
>> If the orphan node list includes valid, untruncatable nodes with nlink > 0
>> the ext3_orphan_cleanup loop which attempts to delete them will not do so,
>> causing it to loop forever. Fix by checking for such nodes in the
>> ext3_orphan_get function.
>>
>> This patch fixes the second case (image hdb.20000009.softlockup.gz)
>> reported in http://bugzilla.kernel.org/show_bug.cgi?id=10882.
>>
>> [[email protected]: coding-style fixes]
>> [[email protected]: printk warning fix]
>> Signed-off-by: Duane Griffin <[email protected]>
>> Cc: <[email protected]>
>> Signed-off-by: Andrew Morton <[email protected]>
>
>> diff -puN fs/ext3/inode.c~ext3-handle-corrupted-orphan-list-at-mount fs/ext3/inode.c
>> --- a/fs/ext3/inode.c~ext3-handle-corrupted-orphan-list-at-mount
>> +++ a/fs/ext3/inode.c
>> @@ -2253,6 +2253,19 @@ static void ext3_free_branches(handle_t
>> }
>> }
>>
>> +int ext3_can_truncate(struct inode *inode)
>> +{
>> + if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
>> + return 0;
>> + if (S_ISREG(inode->i_mode))
>> + return 1;
>> + if (S_ISDIR(inode->i_mode))
>> + return 1;
>> + if (S_ISLNK(inode->i_mode))
>> + return !ext3_inode_is_fast_symlink(inode);
>> + return 0;
>> +}
>> +
>> /*
>> * ext3_truncate()
>> *
>> @@ -2297,12 +2310,7 @@ void ext3_truncate(struct inode *inode)
>> unsigned blocksize = inode->i_sb->s_blocksize;
>> struct page *page;
>>
>> - if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
>> - S_ISLNK(inode->i_mode)))
>> - return;
>> - if (ext3_inode_is_fast_symlink(inode))
>> - return;
>> - if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
>> + if (!ext3_can_truncate(inode))
>> return;
>
> I may be missing something here but doesn't the above change the logic
> that was used in ext3_truncate for the S_ISDIR and S_ISLNK cases?
>
> Before ext3_truncate would be short-circuited if S_ISDIR or S_ISLNK,
> now it won't... is that intended?
Gah, I mistakenly read it as only being !S_ISDIR (I missed the fact
that it was !S_ISREG, !S_ISDIR, or !S_ISLNK). And after Duane's change
that logic is maintained... and more readable!
Sorry for the noise.
On Fri, Jul 25, 2008 at 2:41 PM, Mike Snitzer <[email protected]> wrote:
> On Fri, Jul 25, 2008 at 2:33 PM, Mike Snitzer <[email protected]> wrote:
>> On Fri, Jul 25, 2008 at 3:02 AM, <[email protected]> wrote:
>>> From: "Duane Griffin" <[email protected]>
>>>
>>> If the orphan node list includes valid, untruncatable nodes with nlink > 0
>>> the ext3_orphan_cleanup loop which attempts to delete them will not do so,
>>> causing it to loop forever. Fix by checking for such nodes in the
>>> ext3_orphan_get function.
>>>
>>> This patch fixes the second case (image hdb.20000009.softlockup.gz)
>>> reported in http://bugzilla.kernel.org/show_bug.cgi?id=10882.
>>>
>>> [[email protected]: coding-style fixes]
>>> [[email protected]: printk warning fix]
>>> Signed-off-by: Duane Griffin <[email protected]>
>>> Cc: <[email protected]>
>>> Signed-off-by: Andrew Morton <[email protected]>
>>
>>> diff -puN fs/ext3/inode.c~ext3-handle-corrupted-orphan-list-at-mount fs/ext3/inode.c
>>> --- a/fs/ext3/inode.c~ext3-handle-corrupted-orphan-list-at-mount
>>> +++ a/fs/ext3/inode.c
>>> @@ -2253,6 +2253,19 @@ static void ext3_free_branches(handle_t
>>> }
>>> }
>>>
>>> +int ext3_can_truncate(struct inode *inode)
>>> +{
>>> + if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
>>> + return 0;
>>> + if (S_ISREG(inode->i_mode))
>>> + return 1;
>>> + if (S_ISDIR(inode->i_mode))
>>> + return 1;
>>> + if (S_ISLNK(inode->i_mode))
>>> + return !ext3_inode_is_fast_symlink(inode);
>>> + return 0;
>>> +}
>>> +
>>> /*
>>> * ext3_truncate()
>>> *
>>> @@ -2297,12 +2310,7 @@ void ext3_truncate(struct inode *inode)
>>> unsigned blocksize = inode->i_sb->s_blocksize;
>>> struct page *page;
>>>
>>> - if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
>>> - S_ISLNK(inode->i_mode)))
>>> - return;
>>> - if (ext3_inode_is_fast_symlink(inode))
>>> - return;
>>> - if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
>>> + if (!ext3_can_truncate(inode))
>>> return;
>>
>> I may be missing something here but doesn't the above change the logic
>> that was used in ext3_truncate for the S_ISDIR and S_ISLNK cases?
>>
>> Before ext3_truncate would be short-circuited if S_ISDIR or S_ISLNK,
>> now it won't... is that intended?
>
> Gah, I mistakenly read it as only being !S_ISDIR
heh.. I read it as only being !S_ISREG .. again sorry.