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] [thread-next>] [day] [month] [year] [list]
Message-ID: <cec9eab6-8e3b-47af-94c1-56fa1e449e82@oracle.com>
Date: Sun, 20 Oct 2024 12:21:10 +0100
From: John Garry <john.g.garry@...cle.com>
To: "Ritesh Harjani (IBM)" <ritesh.list@...il.com>, axboe@...nel.dk,
        brauner@...nel.org, djwong@...nel.org, viro@...iv.linux.org.uk,
        jack@...e.cz, dchinner@...hat.com, hch@....de, cem@...nel.org
Cc: linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org, hare@...e.de,
        martin.petersen@...cle.com, catherine.hoang@...cle.com,
        mcgrof@...nel.org, ojaswin@...ux.ibm.com
Subject: Re: [PATCH v10 5/8] fs: iomap: Atomic write support

On 20/10/2024 09:21, Ritesh Harjani (IBM) wrote:
>>   -293,7 +295,8 @@ 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;
>> -	loff_t length = iomap_length(iter);
>> +	const loff_t length = iomap_length(iter);
>> +	bool atomic = iter->flags & IOMAP_ATOMIC;
>>   	loff_t pos = iter->pos;
>>   	blk_opf_t bio_opf;
>>   	struct bio *bio;
>> @@ -303,6 +306,9 @@ static loff_t iomap_dio_bio_iter(const struct iomap_iter *iter,
>>   	size_t copied = 0;
>>   	size_t orig_count;
>>   
>> +	if (atomic && length != fs_block_size)
>> +		return -EINVAL;
> We anyway mandate iov_iter_count() write should be same as sb_blocksize
> in xfs_file_write_iter() for atomic writes.
> This comparison here is not required. I believe we do plan to lift this
> restriction maybe when we are going to add forcealign support right?

Yes, we would lift this restriction if and when forcealign is added. Or 
when bigalloc is leveraged for ext4 atomic writes.

But I think that today it is proper to add this check, as we are saying 
that iomap DIO path does not support anything else than fs_block_size.

For forcealign, we were introducing support for atomic writes spanning 
mixed unwritten and written extents in [0]. We don't have that support 
here, so it is prudent to say that we just support fs_block_size.

[0] 
https://lore.kernel.org/linux-xfs/20240607143919.2622319-4-john.g.garry@oracle.com/

> 
> And similarly this needs to be lifted when ext4 adds support for atomic
> write even with bigalloc. I hope we can do so when we add such support, right?

Right

> 
> (I guess, that is also the reason we haven't mentioned this restriction
> in description of "IOMAP_ATOMIC" in Documentation.)

The documentation does not mention the size, but that is not intentional.

Thanks,
John



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ