On Tue, Dec 01, 2020 at 06:03:31PM -0500, Olga Kornievskaia wrote:
> On Tue, Dec 1, 2020 at 5:15 PM Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote:
> >
> > On Tue, 2020-12-01 at 21:31 +0000, Frank van der Linden wrote:
> > > XDRBUF_SPARSE_PAGES can cause problems for the RDMA transport,
> > > and it's easy enough to allocate enough pages for the request
> > > up front, so do that.
> > >
> > > Also, since we've allocated the pages anyway, use the full
> > > page aligned length for the receive buffer. This will allow
> > > caching of valid replies that are too large for the caller,
> > > but that still fit in the allocated pages.
> > >
> > > Signed-off-by: Frank van der Linden <fllinden@xxxxxxxxxx>
> > > ---
> > > fs/nfs/nfs42proc.c | 39 ++++++++++++++++++++++++++++++---------
> > > fs/nfs/nfs42xdr.c | 22 +++++++++++++++++-----
> > > 2 files changed, 47 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> > > index 2b2211d1234e..bfe15ac77bd9 100644
> > > --- a/fs/nfs/nfs42proc.c
> > > +++ b/fs/nfs/nfs42proc.c
> > > @@ -1176,11 +1176,9 @@ static ssize_t _nfs42_proc_getxattr(struct
> > > inode *inode, const char *name,
> > > void *buf, size_t buflen)
> > > {
> > > struct nfs_server *server = NFS_SERVER(inode);
> > > - struct page *pages[NFS4XATTR_MAXPAGES] = {};
> > > + struct page **pages;
> > > struct nfs42_getxattrargs arg = {
> > > .fh = NFS_FH(inode),
> > > - .xattr_pages = pages,
> > > - .xattr_len = buflen,
> > > .xattr_name = name,
> > > };
> > > struct nfs42_getxattrres res;
> > > @@ -1189,12 +1187,31 @@ static ssize_t _nfs42_proc_getxattr(struct
> >
> > Why can't we set up the page array in nfs42_proc_getxattr()? This means
> > that if we get a retryable error from the server, then we perform
> > multiple allocations of the same buffer.
>
> I wonder if the same then is needed for the LISTXATTR as we allocated
> pages in _nfs42_proc_listxattr() and instead should do it in
> nfs42_proc_listxattr()....
There are a few more places of which this is true, yep. getactl is the
other one.
In any case, fixed for the getxattr case in v2.
- Frank
