linux-ext4 - Re: [PATCH v3]Ext4: journal credits reservation fixes for DIO, fallocate and delalloc writepages

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: <1217613795.12413.15.camel@mingming-laptop>
Date:	Fri, 01 Aug 2008 11:03:15 -0700
From:	Mingming Cao <cmm@...ibm.com>
To:	Theodore Tso <tytso@....edu>
Cc:	Frédéric Bohé <frederic.bohe@...l.net>,
	Shehjar Tikoo <shehjart@....unsw.edu.au>,
	linux-ext4@...r.kernel.org,
	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
	Andreas Dilger <adilger@....com>
Subject: Re: [PATCH v3]Ext4: journal credits reservation fixes for DIO,
	fallocate and delalloc writepages


Ted, thanks for going through this,

在 2008-08-01五的 01:49 -0400，Theodore Tso写道：

>     ext4: Fix lack of credits BUG() when deleting a badly fragmented inode
>     
>     The extents codepath for ext4_truncate() requests journal transaction
>     credits in very small chunks, requesting only what is needed.  This
>     means there may not be enough credits left on the transaction handle
>     after ext4_truncate() returns and then when ext4_delete_inode() tries
>     finish up its work, it may not have enough transaction credits,
>     causing a BUG() oops in the jbd2 core.

But, ext4_delete_inode()'s transaction and ext4_truncate()'s transaction
are different,  ext4_truncate() transaction is nested inside
ext4_delete_inode.

Inside ext4_delete_inode, the transaction is to log the changes in
ext4_orhan_del() and  ext4_free_inode(). If I get right,
ext4_orhan_del() need credit to modify superblock and inode block,
ext4_free_inode() needs credit for modifying quota,xattr and inode
bitmap, group descriptor, superblock, and inode block. 

Currently ext4_delete_inode() uses blocks_for_truncate(inode) to
calculate credits, by the time ext4_delete_inode() is called, i_blocks
seems to 0 (I need to double check this). So the credits is just
EXT4_DATA_TRANS_BLOCKS, which take care of the credits for inode,
superblock, xattr and quota, but missing is inode bitmap...I will double
check before I post a patch...



>     
>     Signed-off-by: "Theodore Ts'o" <tytso@....edu>
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index c7fb647..6d27e78 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -215,6 +215,18 @@ void ext4_delete_inode (struct inode * inode)
>  	inode->i_size = 0;
>  	if (inode->i_blocks)
>  		ext4_truncate(inode);
> +
> +	/*
> +	 * ext4_ext_truncate() doesn't reserve any slop when it
> +	 * restarts journal transactions; therefore there may not be
> +	 * enough credits left in the handle to remove the inode from
> +	 * the orphan list and set the dtime field.
> +	 */
> +	if (ext4_ext_journal_restart(handle, 3)) {
> +		ext4_journal_stop(handle);
> +		goto no_delete;
> +	}
> +
>  	/*
>  	 * Kill off the orphan record which ext4_truncate created.
>  	 * AKPM: I think this can be inside the above `if'.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html