Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.1\))
Subject: Re: [PATCH] SUNRPC: Flush dcache only when receiving more seeking
From:   Chuck Lever <chuck.lever@oracle.com>
In-Reply-To: <20200918125052.2493006-1-zhe.he@windriver.com>
Date:   Fri, 18 Sep 2020 12:23:40 -0400
Cc:     Bruce Fields <bfields@fieldses.org>,
        Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna.schumaker@netapp.com>,
        "David S . Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "open list:NETWORKING DRIVERS" <netdev@vger.kernel.org>,
        linux-kernel@vger.kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <6C34098D-7227-4461-A345-825729E0BDEB@oracle.com>
References: <20200918125052.2493006-1-zhe.he@windriver.com>
To:     zhe.he@windriver.com
Precedence: bulk


> On Sep 18, 2020, at 8:50 AM, zhe.he@windriver.com wrote:
>=20
> From: He Zhe <zhe.he@windriver.com>
>=20
> commit ca07eda33e01 ("SUNRPC: Refactor svc_recvfrom()") introduces
> svc_flush_bvec to after sock_recvmsg, but sometimes we receive less =
than we
> seek, which triggers the following warning.
>=20
> WARNING: CPU: 0 PID: 18266 at include/linux/bvec.h:101 =
bvec_iter_advance+0x44/0xa8
> Attempted to advance past end of bvec iter
> Modules linked in: sch_fq_codel openvswitch nsh nf_conncount nf_nat
> nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4
> CPU: 1 PID: 18266 Comm: nfsd Not tainted 5.9.0-rc5 #1
> Hardware name: Xilinx Zynq Platform
> [<80112ec0>] (unwind_backtrace) from [<8010c3a8>] =
(show_stack+0x18/0x1c)
> [<8010c3a8>] (show_stack) from [<80755214>] (dump_stack+0x9c/0xd0)
> [<80755214>] (dump_stack) from [<80125e64>] (__warn+0xdc/0xf4)
> [<80125e64>] (__warn) from [<80126244>] (warn_slowpath_fmt+0x84/0xac)
> [<80126244>] (warn_slowpath_fmt) from [<80c88514>] =
(bvec_iter_advance+0x44/0xa8)
> [<80c88514>] (bvec_iter_advance) from [<80c88940>] =
(svc_tcp_read_msg+0x10c/0x1bc)
> [<80c88940>] (svc_tcp_read_msg) from [<80c895d4>] =
(svc_tcp_recvfrom+0x98/0x63c)
> [<80c895d4>] (svc_tcp_recvfrom) from [<80c97bf4>] =
(svc_handle_xprt+0x48c/0x4f8)
> [<80c97bf4>] (svc_handle_xprt) from [<80c98038>] (svc_recv+0x94/0x1e0)
> [<80c98038>] (svc_recv) from [<804747cc>] (nfsd+0xf0/0x168)
> [<804747cc>] (nfsd) from [<80148a0c>] (kthread+0x144/0x154)
> [<80148a0c>] (kthread) from [<80100114>] (ret_from_fork+0x14/0x20)
>=20
> Fixes: ca07eda33e01 ("SUNRPC: Refactor svc_recvfrom()")
> Cc: <stable@vger.kernel.org> # 5.8+
> Signed-off-by: He Zhe <zhe.he@windriver.com>
> ---
> net/sunrpc/svcsock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>=20
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index d5805fa1d066..ea3bc9635448 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -277,7 +277,7 @@ static ssize_t svc_tcp_read_msg(struct svc_rqst =
*rqstp, size_t buflen,
> 		buflen -=3D seek;
> 	}
> 	len =3D sock_recvmsg(svsk->sk_sock, &msg, MSG_DONTWAIT);
> -	if (len > 0)
> +	if (len > (seek & PAGE_MASK))

I don't understand how this addresses the WARNING. Can you provide
an example set of inputs that trigger the issue?

Also this change introduces a mixed-sign comparison, so NACK on
this particular patch unless it can be demonstrated that the
implicit type conversion here is benign (I don't think it is,
but I haven't thought through it).


> 		svc_flush_bvec(bvec, len, seek);
>=20
> 	/* If we read a full record, then assume there may be more
> --=20
> 2.17.1
>=20

--
Chuck Lever