On Sat, 09 Nov 2024, Mike Snitzer wrote:
> Push the read_iter and write_iter availability checks down to
> nfs_do_local_read and nfs_do_local_write respectively.
>
> This eliminates a redundant nfs_to->nfsd_file_file() call.
Do it?
The patch removes 2 of these calls and add 2 of these calls. So it
isn't clear what is being eliminated.
Maybe it is a good think to do, but it isn't obvious to me why.
Thanks,
NeilBrown
>
> Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx>
> ---
> fs/nfs/localio.c | 32 +++++++++++++++++++-------------
> 1 file changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
> index a7eb83a604d0..a77ac7e8a05c 100644
> --- a/fs/nfs/localio.c
> +++ b/fs/nfs/localio.c
> @@ -273,7 +273,7 @@ nfs_local_iocb_free(struct nfs_local_kiocb *iocb)
>
> static struct nfs_local_kiocb *
> nfs_local_iocb_alloc(struct nfs_pgio_header *hdr,
> - struct nfsd_file *localio, gfp_t flags)
> + struct file *file, gfp_t flags)
> {
> struct nfs_local_kiocb *iocb;
>
> @@ -286,9 +286,8 @@ nfs_local_iocb_alloc(struct nfs_pgio_header *hdr,
> kfree(iocb);
> return NULL;
> }
> - init_sync_kiocb(&iocb->kiocb, nfs_to->nfsd_file_file(localio));
> + init_sync_kiocb(&iocb->kiocb, file);
> iocb->kiocb.ki_pos = hdr->args.offset;
> - iocb->localio = localio;
> iocb->hdr = hdr;
> iocb->kiocb.ki_flags &= ~IOCB_APPEND;
> return iocb;
> @@ -395,13 +394,19 @@ nfs_do_local_read(struct nfs_pgio_header *hdr,
> const struct rpc_call_ops *call_ops)
> {
> struct nfs_local_kiocb *iocb;
> + struct file *file = nfs_to->nfsd_file_file(localio);
> +
> + /* Don't support filesystems without read_iter */
> + if (!file->f_op->read_iter)
> + return -EAGAIN;
>
> dprintk("%s: vfs_read count=%u pos=%llu\n",
> __func__, hdr->args.count, hdr->args.offset);
>
> - iocb = nfs_local_iocb_alloc(hdr, localio, GFP_KERNEL);
> + iocb = nfs_local_iocb_alloc(hdr, file, GFP_KERNEL);
> if (iocb == NULL)
> return -ENOMEM;
> + iocb->localio = localio;
>
> nfs_local_pgio_init(hdr, call_ops);
> hdr->res.eof = false;
> @@ -564,14 +569,20 @@ nfs_do_local_write(struct nfs_pgio_header *hdr,
> const struct rpc_call_ops *call_ops)
> {
> struct nfs_local_kiocb *iocb;
> + struct file *file = nfs_to->nfsd_file_file(localio);
> +
> + /* Don't support filesystems without write_iter */
> + if (!file->f_op->write_iter)
> + return -EAGAIN;
>
> dprintk("%s: vfs_write count=%u pos=%llu %s\n",
> __func__, hdr->args.count, hdr->args.offset,
> (hdr->args.stable == NFS_UNSTABLE) ? "unstable" : "stable");
>
> - iocb = nfs_local_iocb_alloc(hdr, localio, GFP_NOIO);
> + iocb = nfs_local_iocb_alloc(hdr, file, GFP_NOIO);
> if (iocb == NULL)
> return -ENOMEM;
> + iocb->localio = localio;
>
> switch (hdr->args.stable) {
> default:
> @@ -597,16 +608,9 @@ int nfs_local_doio(struct nfs_client *clp, struct nfsd_file *localio,
> const struct rpc_call_ops *call_ops)
> {
> int status = 0;
> - struct file *filp = nfs_to->nfsd_file_file(localio);
>
> if (!hdr->args.count)
> return 0;
> - /* Don't support filesystems without read_iter/write_iter */
> - if (!filp->f_op->read_iter || !filp->f_op->write_iter) {
> - nfs_local_disable(clp);
> - status = -EAGAIN;
> - goto out;
> - }
>
> switch (hdr->rw_mode) {
> case FMODE_READ:
> @@ -620,8 +624,10 @@ int nfs_local_doio(struct nfs_client *clp, struct nfsd_file *localio,
> hdr->rw_mode);
> status = -EINVAL;
> }
> -out:
> +
> if (status != 0) {
> + if (status == -EAGAIN)
> + nfs_local_disable(clp);
> nfs_to_nfsd_file_put_local(localio);
> hdr->task.tk_status = status;
> nfs_local_hdr_release(hdr, call_ops);
> --
> 2.44.0
>
>
