Re: [PATCHv4 13/13] nvmet: report ns's vwc not present

[Christoph Hellwig <hch@lst.de>]

On Fri, Nov 08, 2024 at 09:15:47AM -0700, Keith Busch wrote:
> > > +	if ((req->ns->file && !req->ns->buffered_io) ||
> > 
> > I don't think this check is correct.  Direct I/O still requires cache
> > flushes for durability.
> 
> Oh, right. Would it be sufficient to check the file's block device for
> write caching?
> 
> 	if ((req->ns->file && !req->ns->buffered_io &&
> 		bdev_write_cache(file_inode(req->ns->file)->i_sb->s_bdev))

No, that's wrong for two reasons:

 1) sb->s_bdev is only really relevant for mounting and st_dev reported
    in stat.  It might not relate to where the file data is placed at
    all
 2) the presence of cache in the underlying block device only has a
    minimal relation to the need to fdatasync a file you wrote to.
    Except for the corner case of overwritting a fully allocated (and
    not just preallocated) extent on an in-place write file system there
    always is metadata for a write that needs to be synchronized.
    You could work around that with an O_DYNC write that does that
    sync per I/O, but performance would be awful (and nvmet donsn't
    support that anyway)