All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chuck Lever <chuck.lever@oracle.com>
To: Bruce Fields <bfields@fieldses.org>
Cc: Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v1 19/42] SUNRPC: Fix xdr_get_next_encode_buffer() page boundary handling
Date: Wed, 3 Mar 2021 18:30:40 +0000	[thread overview]
Message-ID: <0908A838-D518-4F81-B6EA-8C088D5538E9@oracle.com> (raw)
In-Reply-To: <FB4626C1-C2C1-4927-971B-8937420F963A@oracle.com>



> On Mar 3, 2021, at 1:27 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
> 
> 
> 
>> On Mar 3, 2021, at 1:25 PM, Bruce Fields <bfields@fieldses.org> wrote:
>> 
>> On Wed, Mar 03, 2021 at 06:19:33PM +0000, Chuck Lever wrote:
>>> 
>>> 
>>>> On Mar 3, 2021, at 11:52 AM, Bruce Fields <bfields@fieldses.org> wrote:
>>>> 
>>>> On Wed, Mar 03, 2021 at 03:43:28PM +0000, Chuck Lever wrote:
>>>>> Why would that not be OK? the next call to xdr_get_next_encode_buffer()
>>>>> should do the right thing and bounce the new encoded data from the
>>>>> next page into this one again.
>>>>> 
>>>>> So far I have not encountered any problems. Would such a problem show
>>>>> up with some frequency under normal use, or would it be especially
>>>>> subtle?
>>>> 
>>>> I mainly just want to make sure we've got a coherent idea what this code
>>>> is doing....
>>> 
>>> Agreed, that's a good thing.
>> 
>> I'm also a little vague on what exactly the problem is you're running
>> into.  (Probably because I haven't really looked at the v3 readdir
>> encoding.)
>> 
>> Is it approaching the end of a page, or is it running out of buflen?
>> How exactly does it fail?
> 
> I don't recall exactly, it was a late last summer when I wrote all these.
> 
> Approaching the end of a page, IIRC, the unpatched code would leave
> a gap in the directory entry stream.

Well, when I converted the entry encoders to use xdr_stream, it would
have a problem around the end of a page. The existing encoders are
open-coded to deal with this case.


>> --b.
>> 
>>> 
>>> 
>>>> For testing: large replies that aren't just read data are readdir and
>>>> getacl.  So reading large directories with lots of variably-named files
>>>> might be good. Also pynfs could easily send a single compound with lots
>>>> of variable-sized reads, that might be interesting.
>>> 
>>> I've run the full git regression suite over NFSv3 many many times with
>>> this patch applied. That includes unpacking via tar, a full build from
>>> scratch, and then running thousands of individual tests.
>>> 
>>> So that doesn't exercise a particular corner case, but it does reflect
>>> a broad variety of directory operations.
>>> 
>>> 
>>>> Constructing a compound that will result in xdr_reserve_space calls that
>>>> exactly hit the various corner cases may be hard.  But maybe if we just
>>>> send a bunch of compounds that vary over some range we can still
>>>> guarantee hitting those cases.
>>>> 
>>>> --b.
>>> 
>>> --
>>> Chuck Lever
> 
> --
> Chuck Lever

--
Chuck Lever




  reply	other threads:[~2021-03-04  0:29 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-01 15:15 [PATCH v1 00/42] NFSv2/3 XDR encoder overhaul Chuck Lever
2021-03-01 15:15 ` [PATCH v1 01/42] NFSD: Extract the svcxdr_init_encode() helper Chuck Lever
2021-03-02 16:47   ` J. Bruce Fields
2021-03-02 16:57     ` Chuck Lever
2021-03-01 15:15 ` [PATCH v1 02/42] NFSD: Update the GETATTR3res encoder to use struct xdr_stream Chuck Lever
2021-03-01 15:15 ` [PATCH v1 03/42] NFSD: Update the NFSv3 ACCESS3res " Chuck Lever
2021-03-01 15:15 ` [PATCH v1 04/42] NFSD: Update the NFSv3 LOOKUP3res " Chuck Lever
2021-03-01 15:15 ` [PATCH v1 05/42] NFSD: Update the NFSv3 wccstat result " Chuck Lever
2021-03-01 15:15 ` [PATCH v1 06/42] NFSD: Update the NFSv3 READLINK3res " Chuck Lever
2021-03-01 15:16 ` [PATCH v1 07/42] NFSD: Update the NFSv3 READ3res encode " Chuck Lever
2021-03-01 15:16 ` [PATCH v1 08/42] NFSD: Update the NFSv3 WRITE3res encoder " Chuck Lever
2021-03-01 15:16 ` [PATCH v1 09/42] NFSD: Update the NFSv3 CREATE family of encoders " Chuck Lever
2021-03-01 15:16 ` [PATCH v1 10/42] NFSD: Update the NFSv3 RENAMEv3res encoder " Chuck Lever
2021-03-01 15:16 ` [PATCH v1 11/42] NFSD: Update the NFSv3 LINK3res " Chuck Lever
2021-03-01 15:16 ` [PATCH v1 12/42] NFSD: Update the NFSv3 FSSTAT3res " Chuck Lever
2021-03-02 20:45   ` J. Bruce Fields
2021-03-02 20:57     ` Chuck Lever
2021-03-01 15:16 ` [PATCH v1 13/42] NFSD: Update the NFSv3 FSINFO3res " Chuck Lever
2021-03-01 15:16 ` [PATCH v1 14/42] NFSD: Update the NFSv3 PATHCONF3res " Chuck Lever
2021-03-03  7:13   ` Christoph Hellwig
2021-03-03 15:34     ` Chuck Lever
2021-03-01 15:16 ` [PATCH v1 15/42] NFSD: Update the NFSv3 COMMIT3res " Chuck Lever
2021-03-01 15:16 ` [PATCH v1 16/42] NFSD: Add a helper that encodes NFSv3 directory offset cookies Chuck Lever
2021-03-01 15:17 ` [PATCH v1 17/42] NFSD: Count bytes instead of pages in the NFSv3 READDIR encoder Chuck Lever
2021-03-01 15:17 ` [PATCH v1 18/42] NFSD: Update the NFSv3 READDIR3res encoder to use struct xdr_stream Chuck Lever
2021-03-01 15:17 ` [PATCH v1 19/42] SUNRPC: Fix xdr_get_next_encode_buffer() page boundary handling Chuck Lever
2021-03-02 22:11   ` J. Bruce Fields
2021-03-03 15:43     ` Chuck Lever
2021-03-03 16:52       ` Bruce Fields
2021-03-03 18:19         ` Chuck Lever
2021-03-03 18:25           ` Bruce Fields
2021-03-03 18:27             ` Chuck Lever
2021-03-03 18:30               ` Chuck Lever [this message]
2021-03-03 18:38                 ` Bruce Fields
2021-03-03 18:42                   ` Chuck Lever
2021-03-01 15:17 ` [PATCH v1 20/42] NFSD: Update NFSv3 READDIR entry encoders to use struct xdr_stream Chuck Lever
2021-03-01 15:17 ` [PATCH v1 21/42] NFSD: Remove unused NFSv3 directory entry encoders Chuck Lever
2021-03-01 15:17 ` [PATCH v1 22/42] NFSD: Reduce svc_rqst::rq_pages churn during READDIR operations Chuck Lever
2021-03-01 15:17 ` [PATCH v1 23/42] NFSD: Update the NFSv2 stat encoder to use struct xdr_stream Chuck Lever
2021-03-01 15:17 ` [PATCH v1 24/42] NFSD: Update the NFSv2 attrstat " Chuck Lever
2021-03-01 15:17 ` [PATCH v1 25/42] NFSD: Update the NFSv2 diropres " Chuck Lever
2021-03-01 15:17 ` [PATCH v1 26/42] NFSD: Update the NFSv2 READLINK result " Chuck Lever
2021-03-01 15:18 ` [PATCH v1 27/42] NFSD: Update the NFSv2 READ " Chuck Lever
2021-03-01 15:18 ` [PATCH v1 28/42] NFSD: Update the NFSv2 STATFS " Chuck Lever
2021-03-01 15:18 ` [PATCH v1 29/42] NFSD: Add a helper that encodes NFSv3 directory offset cookies Chuck Lever
2021-03-01 15:18 ` [PATCH v1 30/42] NFSD: Count bytes instead of pages in the NFSv2 READDIR encoder Chuck Lever
2021-03-01 15:18 ` [PATCH v1 31/42] NFSD: Update the NFSv2 READDIR result encoder to use struct xdr_stream Chuck Lever
2021-03-01 15:18 ` [PATCH v1 32/42] NFSD: Update the NFSv2 READDIR entry " Chuck Lever
2021-03-01 15:18 ` [PATCH v1 33/42] NFSD: Remove unused NFSv2 directory entry encoders Chuck Lever
2021-03-01 15:18 ` [PATCH v1 34/42] NFSD: Add an xdr_stream-based encoder for NFSv2/3 ACLs Chuck Lever
2021-03-01 15:18 ` [PATCH v1 35/42] NFSD: Update the NFSv2 GETACL result encoder to use struct xdr_stream Chuck Lever
2021-03-01 15:18 ` [PATCH v1 36/42] NFSD: Update the NFSv2 SETACL " Chuck Lever
2021-03-01 15:19 ` [PATCH v1 37/42] NFSD: Update the NFSv2 ACL GETATTR " Chuck Lever
2021-03-01 15:19 ` [PATCH v1 38/42] NFSD: Update the NFSv2 ACL ACCESS " Chuck Lever
2021-03-01 15:19 ` [PATCH v1 39/42] NFSD: Clean up after updating NFSv2 ACL encoders Chuck Lever
2021-03-01 15:19 ` [PATCH v1 40/42] NFSD: Update the NFSv3 GETACL result encoder to use struct xdr_stream Chuck Lever
2021-03-01 15:19 ` [PATCH v1 41/42] NFSD: Update the NFSv3 SETACL " Chuck Lever
2021-03-01 15:19 ` [PATCH v1 42/42] NFSD: Clean up after updating NFSv3 ACL encoders Chuck Lever

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0908A838-D518-4F81-B6EA-8C088D5538E9@oracle.com \
    --to=chuck.lever@oracle.com \
    --cc=bfields@fieldses.org \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.