Re: [PATCH 4/4] xfs: fallback to buffered I/O for direct I/O when stable writes are required

[Qu Wenruo]

在 2025/10/30 03:05, Christoph Hellwig 写道:
> [Adding Qu and the btrfs list]
>
> On Wed, Oct 29, 2025 at 08:53:06AM -0700, Darrick J. Wong wrote:
> > > +	if (mapping_stable_writes(iocb->ki_filp->f_mapping)) {
> > > +		xfs_info_once(mp,
> > > +			"falling back from direct to buffered I/O for write");
> > > +		return -ENOTBLK;
> > > +	}
> >
> > /me wonders if the other filesystems will have to implement this same
> > fallback and hence this should be a common helper ala
> > dio_warn_stale_pagecache?  But we'll get there when we get there.
>
> As far as I'm concerned they should.  Btrfs in fact has recently done
> that, as they are even more exposed due to the integrated checksumming.
>
> So yes, a common helper might make sense.  Especially if we want common
> configuration for opt-outs eventually.

Yep, a common helper will help, or even integrate the check into 
__iomap_dio_rw().

Although btrfs currently uses some btrfs specific flags to do the check, 
we're also setting stable writes flags for the address space, thus we 
can share the same check.

However I'm not sure if a warning will be that useful.

If the warning is only outputted once like here, it doesn't show the ino 
number to tell which file is affected.
If the warning is shown every time, it will flood the dmesg.

It will be much straightforward if there is some flag allowing us to 
return error directly if true zero-copy direct IO can not be executed.

Thanks,
Qu

> > >   	file->f_mode |= FMODE_NOWAIT | FMODE_CAN_ODIRECT;
> > > -	file->f_mode |= FMODE_DIO_PARALLEL_WRITE;
> > > -	if (xfs_get_atomic_write_min(XFS_I(inode)) > 0)
> > > -		file->f_mode |= FMODE_CAN_ATOMIC_WRITE;
> > > +	if (!mapping_stable_writes(file->f_mapping)) {
> > > +		file->f_mode |= FMODE_DIO_PARALLEL_WRITE;
> >
> > Hrm.  So parallel directio writes are disabled for writes to files on
> > stable_pages devices because we have to fall back to buffered writes.
> > Those serialize on i_rwsem so that's why we don't set
> > FMODE_DIO_PARALLEL_WRITE, correct?
>
> Yes.
>
> > There's not some more subtle reason
> > for not supporting it, right?
>
> Not that I know of anyway.