| Message ID | 20200918125052.2493006-1-zhe.he@windriver.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SUNRPC: Flush dcache only when receiving more seeking | expand |
> On Sep 18, 2020, at 8:50 AM, zhe.he@windriver.com wrote: > > From: He Zhe <zhe.he@windriver.com> > > 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. > > 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) > > 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(-) > > 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 -= seek; > } > len = 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); > > /* If we read a full record, then assume there may be more > -- > 2.17.1 > -- Chuck Lever
On 9/19/20 12:23 AM, Chuck Lever wrote: > >> On Sep 18, 2020, at 8:50 AM, zhe.he@windriver.com wrote: >> >> From: He Zhe <zhe.he@windriver.com> >> >> 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. >> >> 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) >> >> 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(-) >> >> 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 -= seek; >> } >> len = 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? I was trying to meet the not warning condition in bvec_iter_advance to make the flushing meaningful. svc_flush_bvec bvec_iter_advance WARN_ONCE(bytes > iter->bi_size,... Here are my steps: mkdir /root/mount_point/ mount /dev/sda1 /root/mount_point/ systemctl restart nfs-server exportfs mount -vvv -t nfs 127.0.0.1:/root/mount_point/ /mnt cp /bin/bash ./bash.tmp > > 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). Thanks, I didn't notice the different types. What about this? if (len > 0 && (size_t)len > (seek & PAGE_MASK)) Zhe > > >> svc_flush_bvec(bvec, len, seek); >> >> /* If we read a full record, then assume there may be more >> -- >> 2.17.1 >> > -- > Chuck Lever > > >
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 -= seek; } len = sock_recvmsg(svsk->sk_sock, &msg, MSG_DONTWAIT); - if (len > 0) + if (len > (seek & PAGE_MASK)) svc_flush_bvec(bvec, len, seek); /* If we read a full record, then assume there may be more