On Tue, Oct 5, 2021 at 8:52 AM David Howells <dhowells@xxxxxxxxxx> wrote:
>
> Dave Wysochanski <dwysocha@xxxxxxxxxx> wrote:
>
> >
> > Handle failed return values of fscache_fallback_read_page() in
> > switch statement.
>
> After some discussion on IRC, the attached is probably better. Returning 1
> might result in 1 being returned through ->readpage(), which could be a
> problem.
>
> David
> ---
> diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
> index 5b0e78742444..68e266a37675 100644
> --- a/fs/nfs/fscache.c
> +++ b/fs/nfs/fscache.c
> @@ -346,33 +346,18 @@ int __nfs_readpage_from_fscache(struct inode *inode, struct page *page)
>
> ret = fscache_fallback_read_page(nfs_i_fscache(inode), page);
> if (ret < 0) {
> - dfprintk(FSCACHE, "NFS: readpage_from_fscache: "
> - "fscache_fallback_read_page failed ret = %d\n", ret);
> - return ret;
> - }
> -
> - switch (ret) {
> - case 0: /* Read completed synchronously */
> - dfprintk(FSCACHE,
> - "NFS: readpage_from_fscache: read successful\n");
> - nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_READ_OK);
> - SetPageUptodate(page);
> - return 0;
> -
> - case -ENOBUFS: /* inode not in cache */
> - case -ENODATA: /* page not in cache */
> nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_READ_FAIL);
> dfprintk(FSCACHE,
> - "NFS: readpage_from_fscache %d\n", ret);
> - SetPageChecked(page);
> - return 1;
> -
> - default:
> - dfprintk(FSCACHE, "NFS: readpage_from_fscache %d\n", ret);
> - nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_READ_FAIL);
> + "NFS: readpage_from_fscache failed %d\n", ret);
> SetPageChecked(page);
> + return ret;
> }
> - return ret;
> +
> + /* Read completed synchronously */
> + dfprintk(FSCACHE, "NFS: readpage_from_fscache: read successful\n");
> + nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_READ_OK);
> + SetPageUptodate(page);
> + return 0;
> }
>
> /*
>
Yes this looks good.
Acked-by: Dave Wysochanski <dwysocha@xxxxxxxxxx>
--
Linux-cachefs mailing list
Linux-cachefs@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/linux-cachefs
