linux-kernel - Re: [PATCH v2 07/14] fs: iomap: Sub-extent zeroing

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]

Date: Thu, 7 Mar 2024 11:51:01 +0000
From: John Garry <john.g.garry@...cle.com>
To: Dave Chinner <david@...morbit.com>
Cc: djwong@...nel.org, hch@....de, viro@...iv.linux.org.uk, brauner@...nel.org,
        jack@...e.cz, chandan.babu@...cle.com, axboe@...nel.dk,
        martin.petersen@...cle.com, linux-kernel@...r.kernel.org,
        linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        tytso@....edu, jbongio@...gle.com, ojaswin@...ux.ibm.com,
        ritesh.list@...il.com, linux-block@...r.kernel.org
Subject: Re: [PATCH v2 07/14] fs: iomap: Sub-extent zeroing

On 06/03/2024 21:14, Dave Chinner wrote:
>> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
>> index bcd3f8cf5ea4..733f83f839b6 100644
>> --- a/fs/iomap/direct-io.c
>> +++ b/fs/iomap/direct-io.c
>> @@ -277,7 +277,7 @@ static loff_t iomap_dio_bio_iter(const struct iomap_iter *iter,
>>   {
>>   	const struct iomap *iomap = &iter->iomap;
>>   	struct inode *inode = iter->inode;
>> -	unsigned int fs_block_size = i_blocksize(inode), pad;
>> +	unsigned int zeroing_size, pad;
>>   	loff_t length = iomap_length(iter);
>>   	loff_t pos = iter->pos;
>>   	blk_opf_t bio_opf;
>> @@ -288,6 +288,8 @@ static loff_t iomap_dio_bio_iter(const struct iomap_iter *iter,
>>   	size_t copied = 0;
>>   	size_t orig_count;
>>   
>> +	zeroing_size = i_blocksize(inode) << iomap->extent_shift;
> The iomap interfaces use units of bytes for offsets, sizes, ranges,
> etc. Using shifts to define a granularity value seems like a
> throwback to decades old XFS code and just a bit weird nowdays.  Can
> we just pass this as a byte count? i.e.:
> 
> 	zeroing_size = i_blocksize(inode);
> 	if (iomap->extent_size)
> 		zeroing_size = iomap->extent_size;

Fine

I was also thinking of something like i_extentsize() for vfs, which 
would save requiring adding iomap->extent_size, but decided to limit vfs 
changes.

Thanks,
John