On Wed, 2024-02-28 at 10:23 +1100, NeilBrown wrote:
> when AOP_WRITEPAGE_ACTIVATE is returned (as NFS does when it detects
> congestion) it is important that the folio is redirtied.
> nfs_writepage_locked() doesn't do this, so files can become corrupted as
> writes can be lost.
>
> Note that this is not needed in v6.8 as AOP_WRITEPAGE_ACTIVATE cannot be
> returned. It is needed for kernels v5.18..v6.7. Prior to 6.3 the patch
> is different as it needs to mention "page", not "folio".
>
Neil, I have a question about the above statement. In Linus's tree as of
this morning (v6.8-rc7-ish), it does this in nfs_writepages_locked:
if (wbc->sync_mode == WB_SYNC_NONE &&
NFS_SERVER(inode)->write_congested)
return AOP_WRITEPAGE_ACTIVATE;
The only caller of nfs_writepages_locked, and I don't see where it
redirties the page. Why don't we need this in v6.8?
> Reported-and-tested-by: Jacek Tomaka <Jacek.Tomaka@xxxxxxxxx>
> Fixes: 6df25e58532b ("nfs: remove reliance on bdi congestion")
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
> ---
> fs/nfs/write.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
> index b664caea8b4e..9e345d3c305a 100644
> --- a/fs/nfs/write.c
> +++ b/fs/nfs/write.c
> @@ -668,8 +668,10 @@ static int nfs_writepage_locked(struct folio *folio,
> int err;
>
> if (wbc->sync_mode == WB_SYNC_NONE &&
> - NFS_SERVER(inode)->write_congested)
> + NFS_SERVER(inode)->write_congested) {
> + folio_redirty_for_writepage(wbc, folio);
> return AOP_WRITEPAGE_ACTIVATE;
> + }
>
> nfs_inc_stats(inode, NFSIOS_VFSWRITEPAGE);
> nfs_pageio_init_write(&pgio, inode, 0, false,
--
Jeff Layton <jlayton@xxxxxxxxxx>
