linux-kernel - Re: [PATCH 5/6] iomap: Lift blocksize restriction on atomic writes

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: <87v7xgmpwo.fsf@gmail.com>
Date: Fri, 25 Oct 2024 16:05:03 +0530
From: Ritesh Harjani (IBM) <ritesh.list@...il.com>
To: John Garry <john.g.garry@...cle.com>, linux-ext4@...r.kernel.org
Cc: Theodore Ts'o <tytso@....edu>, Jan Kara <jack@...e.cz>, "Darrick J . Wong" <djwong@...nel.org>, Christoph Hellwig <hch@...radead.org>, Ojaswin Mujoo <ojaswin@...ux.ibm.com>, Dave Chinner <david@...morbit.com>, linux-kernel@...r.kernel.org, linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 5/6] iomap: Lift blocksize restriction on atomic writes

John Garry <john.g.garry@...cle.com> writes:

> On 25/10/2024 10:31, Ritesh Harjani (IBM) wrote:
>>>>    
>>>> -	if (atomic && length != fs_block_size)
>>>> +	if (atomic && length != iter->len)
>>>>    		return -EINVAL;
>>> Here you expect just one iter for an atomic write always.
>> Here we are lifting the limitation of iomap to support entire iter->len
>> rather than just 1 fsblock.
>
> Sure
>
>> 
>>> In 6/6, you are saying that iomap does not allow an atomic write which
>>> covers unwritten and written extents, right?
>> No, it's not that. If FS does not provide a full mapping to iomap in
>> ->iomap_begin then the writes will get split. 
>
> but why would it provide multiple mapping?
>
>> For atomic writes this
>> should not happen, hence the check in iomap above to return -EINVAL if
>> mapped length does not match iter->len.
>> 
>>> But for writing a single fs block atomically, we don't mandate it to be
>>> in unwritten state. So there is a difference in behavior in writing a
>>> single FS block vs multiple FS blocks atomically.
>> Same as mentioned above. We can't have atomic writes to get split.
>> This patch is just lifting the restriction of iomap to allow more than
>> blocksize but the mapped length should still meet iter->len, as
>> otherwise the writes can get split.
>
> Sure, I get this. But I wonder why would we be getting multiple 
> mappings? Why cannot the FS always provide a single mapping?

FS can decide to split the mappings when it couldn't allocate a single
large mapping of the requested length. Could be due to - 
- already allocated extent followed by EOF, 
- already allocated extent followed by a hole
- already mapped extent followed by an extent of different type (e.g. written followed by unwritten or unwritten followed by written)
- delalloc (not delalloc since we invalidate respective page cache pages before doing DIO).
- fragmentation or ENOSPC - For ext4 bigalloc this will not happen since
we reserve the entire cluster. So we know there should be space. But I
am not sure how other filesystems might end up implementing this functionality.

Thanks!

-ritesh