Re: [RFC] ext4: Add support for ext4_map_blocks_atomic()

From: John Garry <john.g.garry@oracle.com>
To: "Ritesh Harjani (IBM)" <ritesh.list@gmail.com>,
	linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org
Cc: Jan Kara <jack@suse.cz>, Theodore Ts'o <tytso@mit.edu>,
	Ojaswin Mujoo <ojaswin@linux.ibm.com>,
	Matthew Wilcox <willy@infradead.org>,
	"Darrick J . Wong" <djwong@kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	linux-kernel@vger.kernel.org, Dave Chinner <david@fromorbit.com>
Subject: Re: [RFC] ext4: Add support for ext4_map_blocks_atomic()
Date: Mon, 18 Mar 2024 08:22:15 +0000	[thread overview]
Message-ID: <22a24d3c-9d0e-410b-a49c-b89d0c00ccd8@oracle.com> (raw)
In-Reply-To: <87r0gcn74h.fsf@doe.com>

On 14/03/2024 15:52, Ritesh Harjani (IBM) wrote:
>> and same as method 3 at
>> https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/cover.1709356594.git.ritesh.list@gmail.com/?__;!!ACWV5N9M2RV99hQ!Pb-HbBdm2OWUIGDFfG1OkemtRSy2LyHsc5s6WiyTtGHW4uGWV6sMkoVjmknmBydf_i6TF_CDqp7dR0Y-CGY8EIc$   
> Hi John,
> 
> No. So this particular patch to add ext4_map_blocks_atomic() method is
> only to support the usecase which you listed should work for a good user
> behaviour. This is because, with bigalloc we advertizes fsawu_min and
> fsawu_max as [blocksize, clustersize]
> i.e.
> 
> That means a user should be allowed to -
> 1. pwrite 0 4k /mnt/test/f1
> followed by
> 2. pwrite 0 16k /mnt/test/f1
> 
> 
> So earlier we were failing the second 16k write at an offset where there
> is already an existing extent smaller that 16k (that was because of the
> assumption that the most of the users won't do such a thing).
> 
> But for a more general usecase, it is not difficult to support the
> second 16k write in such a way for atomic writes with bigalloc,
> so this patch just adds that support to this series.

Is there some reason for which the generic iomap solution in 
https://lore.kernel.org/linux-xfs/20240304130428.13026-1-john.g.garry@oracle.com/ 
won't work? That is, you would just need to set iomap->extent_shift 
appropriately. I will note that we gate this feature on XFS based on 
forcealign enabled for the inode - I am not sure if you would want this 
always for bigalloc.

Thanks,
John