linux-ext4 - [PATCH] ext4: restart ext4_ext_remove_space() after transaction restart V2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87pr0ilw3n.fsf_-_@openvz.org>
Date:	Wed, 26 May 2010 15:51:24 +0400
From:	Dmitry Monakhov <dmonakhov@...nvz.org>
To:	tytso@....edu
Cc:	linux-ext4@...r.kernel.org, jack@...e.cz,
	aneesh.kumar@...ux.vnet.ibm.com
Subject: [PATCH] ext4: restart ext4_ext_remove_space() after transaction restart V2

Dmitry Monakhov <dmonakhov@...nvz.org> writes:

> tytso@....edu writes:
>
>> On Tue, May 25, 2010 at 06:28:29PM +0400, Dmitry Monakhov wrote:
>>> tytso@....edu writes:
>>> 
>>> > On Thu, Apr 22, 2010 at 08:31:11AM +0400, Dmitry Monakhov wrote:
>>> >> @@ -2480,6 +2480,11 @@ static int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start)
>>> >>  out:
>>> >>  	ext4_ext_drop_refs(path);
>>> >>  	kfree(path);
>>> >> +	if (err == EAGAIN) {
>>> >
>>> > Surely this should be "err == -EAGAIN", no?  I'm curious how this
>>> > patch worked for with this typo....
>>> As usually it fix one thing, and broke another :(.
>>> So in case of alloc/truncate restart truncate will be aborted,
>>> so i_size != i_disk_size which must be caught by fsck (my test run
>>> it every time) but this never happens which is very strange.
> Ohh i ment to say blocks beyond i_disk_size due to aborted truncate.
>> What test case are you using?  And does it require a system crash to
>> show up, or are you seeing an fsck problem after the test completes
>> and you unmount the file system?
> crash is not required.
> I use proposed xfsqa tests from the bug, may be i've changed some 
> numbers, but core idea stays the same.
> mount /dev/sdb1 /mnt
> fsstress ..... &
> sleep 300; killall -9 fsstress
> umount /mnt
> fsck -f /dev/sdb1
> After you have spotted the mistypo i've add explicit fault injection 
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -98,9 +98,15 @@ static int ext4_ext_truncate_extend_restart(handle_t
>>> > *handle,                                            int needed)
>  {
>         int err;
> +       static int fault = 0;
>
>         if (!ext4_handle_valid(handle))
>                 return 0;
> +       if (inode->i_size % 1234 == 0 && fault++ % 2) {
> +               printk("EXT4 TRUNC fault inject inode:%ld\n",inode->i_ino);
> +               dump_stack();
> +               return -EAGAIN;
> +       }
>
> And i've got complain from fsck about incorrect i_size which should be
> increased due to block beyond i_disk_size as expected.
> And when i've fixed the mistypo i've had different complain due to
> bitmap  difference.
This is more than just a bad luck, seems what my brain wasn't enabled
yesterday and at the time i wrote the patch.
I've added 'again' label but forgot to reinitialize "i" variable to zero
again :( . Sorry for wasting you time for this sort of foolishness.
Now it is pass all my tests:
1) fsstress -p100
2) fsstress -p100 with fault injection from journal_restart.
See correct version attached. 

View attachment "0001-ext4-restart-ext4_ext_remove_space-after-transaction.patch" of type "text/plain" (3228 bytes)