linux-ext4 - Re: [PATCH 2/2] ext4: Get rid of extend_disksize parameter of ext4_get_blocks

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 for Android: free password hash cracker in your pocket

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

Date:	Mon, 25 May 2009 09:46:45 +0200
From:	Jan Kara <jack@...e.cz>
To:	Theodore Tso <tytso@....edu>
Cc:	linux-ext4@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 2/2] ext4: Get rid of extend_disksize parameter of
	ext4_get_blocks_handle()

  Hi,

On Fri 22-05-09 00:57:34, Theodore Tso wrote:
> On Wed, May 20, 2009 at 07:25:34PM +0200, Jan Kara wrote:
> > Get rid of extenddisksize parameter of ext4_get_blocks_handle(). This seems to
> > be a relict from some old days and setting disksize in this function does not
> > make much sence. Currently it was set only by ext4_getblk().  Since the
> 
> s/sence/sense/
  Thanks.

> > parameter has some effect only if create == 1, it is easy to check that the
> > three callers which end up calling ext4_getblk() with create == 1 (ext4_append,
> > ext4_quota_write, ext4_mkdir) do the right thing and set disksize themselves.
> 
> So this patch doesn't apply any more since I've done my own set of
> cleanups to the ext4 code base, so extend_disksize is no longer a
> separate parameter, but a bit flag (and ext4_get_blocks_wrap has been
> renamed to a more sensible ext4_get_blocks).
> 
> More to the point, this removess the code which sets the disksize --
> which I agree is funny that it is done in ext4_ext4_get_blocks() and
> ext4_ind_get_blocks() --- but I don't see anything in your patch which
> actually sets disksize in ext4_append(), ext4_quota_write(), or
> ext4_mkdir().  Do none of these actually need disksize to be set?  If
> so, the commit message should explain that.
  All these functions already set i_disksize themselves. E.g. ext4_append()
does
bh = ext4_bread(handle, inode, *block, 1, err);
if (bh) {
         inode->i_size += inode->i_sb->s_blocksize;
         EXT4_I(inode)->i_disksize = inode->i_size;
         *err = ext4_journal_get_write_access(handle, bh);
...
  BTW: I've tried to mention this in the changelog but obviously I've
failed doing it clearly enough ;).

> If not, perhaps it would be better if the update of the disksize field
> be done in ext4_getblk()?
  I'd keep ext4_getblk() and ext4_bread() just being lowlevel functions
mapping / allocating a block. The caller is responsible for updating i_size
and i_disksize if it needs to be changed.
  I'll rediff the ext4 patch against your tree and resend it.

								Honza
-- 
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
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