Re: [PATCH 2/2] btrfs: allocate dummy ordereded_sums objects for nocsum I/O on zoned file systems

[Christoph Hellwig <hch@lst.de>]

On Tue, Jun 06, 2023 at 07:11:43PM +0200, David Sterba wrote:
> > +	/*
> > +	 * If we are called for a nodatasum inode, this means we are on a zoned
> > +	 * file system.  In this case a ordered_csum structure needs to be
> > +	 * allocated to track the logical address actually written, but it does
> > +	 * notactually carry any checksums.
> > +	 */
> > +	if (btrfs_inode_is_nodatasum(inode))
> 
> So nodatasum in checksum bio implies zoned mode, this looks fragile. Why
> can't you check btrfs_is_zoned() instead? The comment is needed but I
> don't agree the condition should omit zoned mode itself.

We can't check that instead, as for zoned mode with checksums we should
not take this branch.  If we want to check btrfs_is_zoned() here it
would have to be an assert inside this branch to make things more clear.

Alternatively we could be a "bool skip_csum" argument, which is what
I did initially, but which felt more clumsy to me.

> >  
> >  	if (ordered->disk_bytenr != logical)
> >  		btrfs_rewrite_logical_zoned(ordered, logical);
> > +
> > +out:
> > +	if (btrfs_inode_is_nodatasum(BTRFS_I(ordered->inode))) {
> 
> The zoned mode here is implied by the calling function, open coding the
> condtions should not be a big deal, i.e. not needing the helper.

The zoned mode is implied.  But that's not what the helper checks for,
it checks for the per-inode or per-fs nodatasum bits.  But I can open
code it if you prefer that.