linux-ext4 - Re: [PATCH 3/4] ext4: Add batched discard support for ext4

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: <20101025185010.GH16981@thunk.org>
Date:	Mon, 25 Oct 2010 14:50:10 -0400
From:	Ted Ts'o <tytso@....edu>
To:	Lukas Czerner <lczerner@...hat.com>
Cc:	linux-ext4@...r.kernel.org, rwheeler@...hat.com,
	sandeen@...hat.com, adilger@...ger.ca,
	Dmitry Monakhov <dmonakhov@...nvz.org>
Subject: Re: [PATCH 3/4] ext4: Add batched discard support for ext4

On Mon, Sep 27, 2010 at 04:09:59PM +0200, Lukas Czerner wrote:
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 93eb6c2..80a5139 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
      ....
> +	struct ext4_buddy e4b;
> +	ext4_fsblk_t first_group, last_group;

This should be ext4_group_t, shouldn't it?  

> +	/* Determine first and last group to examine based on start and len */
> +	first_group = (start - le32_to_cpu(es->s_first_data_block)) /
> +		      EXT4_BLOCKS_PER_GROUP(sb);
> +	last_group = (start + len - le32_to_cpu(es->s_first_data_block)) /
> +		     EXT4_BLOCKS_PER_GROUP(sb);

I've tried compiling this for 32-bit x86, and this blows up because
you can't divide long long's in the kernel.  (This is what do_div is
for, and it's why ext4_get_group_no_and_offset() exists.)

> +	first_block = (start - le32_to_cpu(es->s_first_data_block)) %
> +			EXT4_BLOCKS_PER_GROUP(sb);
> +	last_block = EXT4_BLOCKS_PER_GROUP(sb);

This means that the ext4 FITRIM ioctl will trim to the end of the
blockgroup, and not just to the last block specified by the user.  Is
this intentional?

Also, it looks like there's nothing to check for the last blockgroup,
where the last block might be less than a grpblk_t offset of
EXT4_BLOCKS_PER_GROUP()?

						- Ted
--
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