[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251029160101.GE3356773@frogsfrogsfrogs>
Date: Wed, 29 Oct 2025 09:01:01 -0700
From: "Darrick J. Wong" <djwong@...nel.org>
To: Christoph Hellwig <hch@....de>
Cc: Carlos Maiolino <cem@...nel.org>,
	Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	linux-kernel@...r.kernel.org, linux-xfs@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, linux-raid@...r.kernel.org,
	linux-block@...r.kernel.org
Subject: Re: [PATCH 2/4] fs: return writeback errors for IOCB_DONTCACHE in
 generic_write_sync
On Wed, Oct 29, 2025 at 08:15:03AM +0100, Christoph Hellwig wrote:
> Currently generic_write_sync only kicks of writeback for IOCB_DONTCACHE
> writes, but never looks at the writeback errors.  When using
> IOCB_DONTCACHE as a fallback for IOCB_DIRECT for devcies that require
                                                   devices
> stable writes, this breaks a few xfstests test cases that expect instant
> errors like removed devices to be directly propagated to the writer.
> While I don't know of real applications that would expect this, trying to
> keep the behavior as similar as possible sounds useful and can be
> trivially done by checking for and returning writeback errors in
> generic_write_sync.
Hum.  So we kick writeback but don't wait for any of it to start, and
immediately sample wberr.  Does that mean that in the "bdev died" case,
the newly initiated writeback will have failed so quickly that
file_check_and_advance_wb_err will see that?  Or are we only reflecting
past write failures back to userspace on the *second* write after the
device dies?
It would be helpful to know which fstests break, btw.
--D
> Signed-off-by: Christoph Hellwig <hch@....de>
> ---
>  include/linux/fs.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 09b47effc55e..34a843cf4c1c 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3047,9 +3047,13 @@ static inline ssize_t generic_write_sync(struct kiocb *iocb, ssize_t count)
>  			return ret;
>  	} else if (iocb->ki_flags & IOCB_DONTCACHE) {
>  		struct address_space *mapping = iocb->ki_filp->f_mapping;
> +		int err;
>  
>  		filemap_fdatawrite_range_kick(mapping, iocb->ki_pos - count,
>  					      iocb->ki_pos - 1);
> +		err = file_check_and_advance_wb_err(iocb->ki_filp);
> +		if (err)
> +			return err;
>  	}
>  
>  	return count;
> -- 
> 2.47.3
> 
> 
Powered by blists - more mailing lists
 
