linux-mips
[Top] [All Lists]

Re: [PATCH v2] SUNRPC: Fix TCP receive code on archs with flush_dcache_p

To: "geert@linux-m68k.org" <geert@linux-m68k.org>
Subject: Re: [PATCH v2] SUNRPC: Fix TCP receive code on archs with flush_dcache_page()
From: Trond Myklebust <trondmy@hammerspace.com>
Date: Thu, 3 Jan 2019 14:07:51 +0000
Accept-language: en-US, en-GB
Authentication-results: spf=none (sender IP is ) smtp.mailfrom=trondmy@hammerspace.com;
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>, "linux-mips@linux-mips.org" <linux-mips@linux-mips.org>
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hammerspace.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=uKX8yH2tfS3CC5ggY0G+kMCKWGLLYtMm0fS9LxkrMOc=; b=DTNJQc01CjRab7R/LQTyEqUCj+hYnMFadQsQYY5GVVcQ2Wy6iLjV5nIdVL7ZJ2HkkOUG2q3UtN2rhwqLS4jPjlLDMcj1HBgCi+8w2HMhXVXjN07au1/hwhtR90e6IXgnsLVwAKV9TcWMnwD4+ump+XaArdvpYUR+aJeamEAOyNI=
In-reply-to: <CAMuHMdXpHLjJBMQWDaWKYQFfY4_3yVi9NdBYnWr6Xfzttrb5iA@mail.gmail.com>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-id: linux-mips <linux-mips.eddie.linux-mips.org>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-software: Ecartis version 1.0.0
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <20190103061324.16607-1-trond.myklebust@hammerspace.com> <CAMuHMdXpHLjJBMQWDaWKYQFfY4_3yVi9NdBYnWr6Xfzttrb5iA@mail.gmail.com>
Sender: linux-mips-bounce@linux-mips.org
Spamdiagnosticmetadata: NSPM
Spamdiagnosticoutput: 1:99
Thread-index: AQHUoyua7l5ZKHM4hESBaVcRAYnWxaWdVHGAgABAooA=
Thread-topic: [PATCH v2] SUNRPC: Fix TCP receive code on archs with flush_dcache_page()
On Thu, 2019-01-03 at 11:16 +0100, Geert Uytterhoeven wrote:
> Hi Trond,
> 
> On Thu, Jan 3, 2019 at 7:14 AM Trond Myklebust <trondmy@gmail.com>
> wrote:
> > After receiving data into the page cache, we need to call
> > flush_dcache_page()
> > for the architectures that define it.
> > 
> > Fixes: 277e4ab7d530b ("SUNRPC: Simplify TCP receive code by
> > switching...")
> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> > Cc: stable@vger.kernel.org # v4.20
> 
> Thanks for your patch!
> 
> > --- a/net/sunrpc/xprtsock.c
> > +++ b/net/sunrpc/xprtsock.c
> > @@ -48,6 +48,7 @@
> >  #include <net/udp.h>
> >  #include <net/tcp.h>
> >  #include <linux/bvec.h>
> > +#include <linux/highmem.h>
> >  #include <linux/uio.h>
> > 
> >  #include <trace/events/sunrpc.h>
> > @@ -380,6 +381,27 @@ xs_read_discard(struct socket *sock, struct
> > msghdr *msg, int flags,
> >         return sock_recvmsg(sock, msg, flags);
> >  }
> > 
> > +#if ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE
> > +static void
> > +xs_flush_bvec(const struct bio_vec *bvec, size_t count, size_t
> > seek)
> > +{
> > +       struct bvec_iter bi, __start = {
> 
> As for_each_bvec() assigns __start to bi, and you don't need __start
> afterwards, both variables can be merged into a single one.
> But perhaps that would make too many assumptions about the
> implementation of for_each_bvec()?

No, that's a good suggestion. I've sent out a (hopefully) final v3 with
that change.

> 
> > +               .bi_size = count,
> > +       };
> > +       struct bio_vec bv;
> > +
> > +       bvec_iter_advance(bvec, &__start, seek & PAGE_MASK);
> > +
> > +       for_each_bvec(bv, bvec, bi, __start)
> > +               flush_dcache_page(bv.bv_page);
> > +}
> > +#else
> > +static inline void
> > +xs_flush_bvec(const struct bio_vec *bvec, size_t count, size_t
> > seek)
> > +{
> > +}
> > +#endif
> > +
> >  static ssize_t
> >  xs_read_xdr_buf(struct socket *sock, struct msghdr *msg, int
> > flags,
> >                 struct xdr_buf *buf, size_t count, size_t seek,
> > size_t *read)
> > @@ -413,6 +435,7 @@ xs_read_xdr_buf(struct socket *sock, struct
> > msghdr *msg, int flags,
> >                                 seek + buf->page_base);
> >                 if (ret <= 0)
> >                         goto sock_err;
> > +               xs_flush_bvec(buf->bvec, ret, seek + buf-
> > >page_base);
> >                 offset += ret - buf->page_base;
> >                 if (offset == count || msg->msg_flags &
> > (MSG_EOR|MSG_TRUNC))
> >                         goto out;
> 
> I don't understand the code well enough to see why the call to
> xs_flush_bvec() is needed in this branch only, but it does fix TCP
> NFS on RBTX4927, so
> Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>

Thanks!
  Trond

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com


<Prev in Thread] Current Thread [Next in Thread>