linux-ext4 - Re: [PATCH v5 0/7] ext4: Add multi-fsblock atomic write support with bigalloc

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: <877c2cx69z.fsf@gmail.com>
Date: Mon, 19 May 2025 15:37:52 +0530
From: Ritesh Harjani (IBM) <ritesh.list@...il.com>
To: Theodore Ts'o <tytso@....edu>
Cc: Jan Kara <jack@...e.cz>, John Garry <john.g.garry@...cle.com>, djwong@...nel.org, Ojaswin Mujoo <ojaswin@...ux.ibm.com>, linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org
Subject: Re: [PATCH v5 0/7] ext4: Add multi-fsblock atomic write support with bigalloc

"Ritesh Harjani (IBM)" <ritesh.list@...il.com> writes:

> This adds multi-fsblock atomic write support to ext4 using bigalloc. The major
> chunk of the design changes are kept in Patch-4 & 5.
>
> v4 -> v5:
> =========
> 1. Addressed review comments and added Acked-by from Darrick.
> 2. Changed a minor WARN_ON(1) at one place to WARN_ON_ONCE(1) in patch-5 in
>    ext4_iomap_begin(). Ideally we may never hit it.
> 3. Added force commit related info in the Documentation section where
>    mixed mapping details are mentioned.
> [v4]: https://lore.kernel.org/linux-ext4/cover.1747289779.git.ritesh.list@gmail.com/
> <...>
> Ritesh Harjani (IBM) (7):
>   ext4: Document an edge case for overwrites
>   ext4: Check if inode uses extents in ext4_inode_can_atomic_write()
>   ext4: Make ext4_meta_trans_blocks() non-static for later use
>   ext4: Add support for EXT4_GET_BLOCKS_QUERY_LEAF_BLOCKS
>   ext4: Add multi-fsblock atomic write support with bigalloc
>   ext4: Enable support for ext4 multi-fsblock atomic write using bigalloc
>   ext4: Add atomic block write documentation
>
>  .../filesystems/ext4/atomic_writes.rst        | 225 +++++++++++++
>  Documentation/filesystems/ext4/overview.rst   |   1 +
>  fs/ext4/ext4.h                                |  26 +-
>  fs/ext4/extents.c                             |  99 ++++++
>  fs/ext4/file.c                                |   7 +-
>  fs/ext4/inode.c                               | 315 ++++++++++++++++--
>  fs/ext4/super.c                               |   7 +-
>  7 files changed, 655 insertions(+), 25 deletions(-)
>  create mode 100644 Documentation/filesystems/ext4/atomic_writes.rst

Hi Ted, 

I was working on the rebase over the weekend as I figured that there
will be merge conflicts with Zhang's series. But later I saw that you
have already rebased [1] and merged atomic write series in dev branch. 

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git/log/?h=dev

So, thanks for taking care of that. After looking at Zhang's series, I
figured, we may need EXT4_EX_CACHE flag too in
ext4_convert_unwritten_extents_atomic() i.e.

	int flags = EXT4_GET_BLOCKS_IO_CONVERT_EXT | EXT4_EX_NOCACHE;

This is since, in ext4_map_blocks(), we have a condition check which can
emit a WARN_ON like this:
	/*
	 * Callers from the context of data submission are the only exceptions
	 * for regular files that do not hold the i_rwsem or invalidate_lock.
	 * However, caching unrelated ranges is not permitted.
	 */
	if (flags & EXT4_GET_BLOCKS_IO_SUBMIT)
		WARN_ON_ONCE(!(flags & EXT4_EX_NOCACHE));
	else
		ext4_check_map_extents_env(inode);


Other than adding the no cache flag, couple of other minor
simplifications can be done too, I guess for e.g. simplifying the
query_flags logic in ext4_map_query_blocks() function. 

So I am thinking maybe I will provide the above fix and few other minor
simplfications which we could do on top of ext4's dev branch (after we
rebased atomic write changes on top of Zhang's series). Please let me
know if that is ok?

Or do we want to fix the original atomic write patch and do a force push
to dev branch again?

Please let me know whichever way works best?

Thanks again!
-ritesh