| Message ID | 20201102180658.6218-4-trondmy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Readdir enhancements | expand |
On 2 Nov 2020, at 13:06, trondmy@kernel.org wrote: > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > Clean up handling of the case where there are no entries in the > readdir > reply. > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > --- > fs/nfs/dir.c | 39 ++++++++++++++++++--------------------- > 1 file changed, 18 insertions(+), 21 deletions(-) > > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c > index 604ebe015387..68acbde3f914 100644 > --- a/fs/nfs/dir.c > +++ b/fs/nfs/dir.c > @@ -601,16 +601,12 @@ int > nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct > nfs_entry *en > struct xdr_stream stream; > struct xdr_buf buf; > struct page *scratch; > - unsigned int count = 0; > int status; > > scratch = alloc_page(GFP_KERNEL); > if (scratch == NULL) > return -ENOMEM; > > - if (buflen == 0) > - goto out_nopages; > - > xdr_init_decode_pages(&stream, &buf, xdr_pages, buflen); > xdr_set_scratch_buffer(&stream, page_address(scratch), PAGE_SIZE); > > @@ -619,27 +615,27 @@ int > nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct > nfs_entry *en > entry->label->len = NFS4_MAXLABELLEN; > > status = xdr_decode(desc, entry, &stream); > - if (status != 0) { > - if (status == -EAGAIN) > - status = 0; > + if (status != 0) > break; > - } > - > - count++; > > if (desc->plus) > nfs_prime_dcache(file_dentry(desc->file), entry, > desc->dir_verifier); > > status = nfs_readdir_add_to_array(entry, page); > - if (status != 0) > - break; > - } while (!entry->eof); > + } while (!status && !entry->eof); > > -out_nopages: > - if (count == 0 || (status == -EBADCOOKIE && entry->eof != 0)) { > - nfs_readdir_page_set_eof(page); > + switch (status) { > + case -EBADCOOKIE: > + if (entry->eof) { > + nfs_readdir_page_set_eof(page); > + status = 0; > + } > + break; > + case -ENOSPC: If you return ENOSPC, then you don't need to use nfs_readdir_array_is_full(array) below.. > + case -EAGAIN: > status = 0; > + break; > } > > put_page(scratch); > @@ -714,14 +710,15 @@ int > nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page > *page, > > if (status < 0) > break; > + > pglen = status; > - status = nfs_readdir_page_filler(desc, &entry, pages, page, pglen); > - if (status < 0) { > - if (status == -ENOSPC) > - status = 0; > + if (pglen == 0) { > + nfs_readdir_page_set_eof(page); > break; > } > - } while (!nfs_readdir_array_is_full(array)); > + > + status = nfs_readdir_page_filler(desc, &entry, pages, page, pglen); > + } while (!status && !nfs_readdir_array_is_full(array)); ^^ here. I suppose nfs_readdir_array_is_full() is nice and clear.. I wonder if the compiler would optimize it away. Ben
On 3 Nov 2020, at 9:18, Benjamin Coddington wrote: > On 2 Nov 2020, at 13:06, trondmy@kernel.org wrote: > >> From: Trond Myklebust <trond.myklebust@hammerspace.com> >> >> Clean up handling of the case where there are no entries in the >> readdir >> reply. >> >> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> >> --- >> fs/nfs/dir.c | 39 ++++++++++++++++++--------------------- >> 1 file changed, 18 insertions(+), 21 deletions(-) >> >> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c >> index 604ebe015387..68acbde3f914 100644 >> --- a/fs/nfs/dir.c >> +++ b/fs/nfs/dir.c >> @@ -601,16 +601,12 @@ int >> nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct >> nfs_entry *en >> struct xdr_stream stream; >> struct xdr_buf buf; >> struct page *scratch; >> - unsigned int count = 0; >> int status; >> >> scratch = alloc_page(GFP_KERNEL); >> if (scratch == NULL) >> return -ENOMEM; >> >> - if (buflen == 0) >> - goto out_nopages; >> - >> xdr_init_decode_pages(&stream, &buf, xdr_pages, buflen); >> xdr_set_scratch_buffer(&stream, page_address(scratch), PAGE_SIZE); >> >> @@ -619,27 +615,27 @@ int >> nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct >> nfs_entry *en >> entry->label->len = NFS4_MAXLABELLEN; >> >> status = xdr_decode(desc, entry, &stream); >> - if (status != 0) { >> - if (status == -EAGAIN) >> - status = 0; >> + if (status != 0) >> break; >> - } >> - >> - count++; >> >> if (desc->plus) >> nfs_prime_dcache(file_dentry(desc->file), entry, >> desc->dir_verifier); >> >> status = nfs_readdir_add_to_array(entry, page); >> - if (status != 0) >> - break; >> - } while (!entry->eof); >> + } while (!status && !entry->eof); >> >> -out_nopages: >> - if (count == 0 || (status == -EBADCOOKIE && entry->eof != 0)) { >> - nfs_readdir_page_set_eof(page); >> + switch (status) { >> + case -EBADCOOKIE: >> + if (entry->eof) { >> + nfs_readdir_page_set_eof(page); >> + status = 0; >> + } >> + break; >> + case -ENOSPC: > > If you return ENOSPC, then you don't need to use > nfs_readdir_array_is_full(array) below.. > >> + case -EAGAIN: >> status = 0; >> + break; >> } >> >> put_page(scratch); >> @@ -714,14 +710,15 @@ int >> nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page >> *page, >> >> if (status < 0) >> break; >> + >> pglen = status; >> - status = nfs_readdir_page_filler(desc, &entry, pages, page, >> pglen); >> - if (status < 0) { >> - if (status == -ENOSPC) >> - status = 0; >> + if (pglen == 0) { >> + nfs_readdir_page_set_eof(page); >> break; >> } >> - } while (!nfs_readdir_array_is_full(array)); >> + >> + status = nfs_readdir_page_filler(desc, &entry, pages, page, >> pglen); >> + } while (!status && !nfs_readdir_array_is_full(array)); > > ^^ here. > > I suppose nfs_readdir_array_is_full() is nice and clear.. I wonder if > the > compiler would optimize it away. Ah, I see now -- the check handles the case of eof as well.. I suppose I shouldn't send a stream of consciousness to the list. Sorry about that. Ben
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 604ebe015387..68acbde3f914 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -601,16 +601,12 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct nfs_entry *en struct xdr_stream stream; struct xdr_buf buf; struct page *scratch; - unsigned int count = 0; int status; scratch = alloc_page(GFP_KERNEL); if (scratch == NULL) return -ENOMEM; - if (buflen == 0) - goto out_nopages; - xdr_init_decode_pages(&stream, &buf, xdr_pages, buflen); xdr_set_scratch_buffer(&stream, page_address(scratch), PAGE_SIZE); @@ -619,27 +615,27 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct nfs_entry *en entry->label->len = NFS4_MAXLABELLEN; status = xdr_decode(desc, entry, &stream); - if (status != 0) { - if (status == -EAGAIN) - status = 0; + if (status != 0) break; - } - - count++; if (desc->plus) nfs_prime_dcache(file_dentry(desc->file), entry, desc->dir_verifier); status = nfs_readdir_add_to_array(entry, page); - if (status != 0) - break; - } while (!entry->eof); + } while (!status && !entry->eof); -out_nopages: - if (count == 0 || (status == -EBADCOOKIE && entry->eof != 0)) { - nfs_readdir_page_set_eof(page); + switch (status) { + case -EBADCOOKIE: + if (entry->eof) { + nfs_readdir_page_set_eof(page); + status = 0; + } + break; + case -ENOSPC: + case -EAGAIN: status = 0; + break; } put_page(scratch); @@ -714,14 +710,15 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page, if (status < 0) break; + pglen = status; - status = nfs_readdir_page_filler(desc, &entry, pages, page, pglen); - if (status < 0) { - if (status == -ENOSPC) - status = 0; + if (pglen == 0) { + nfs_readdir_page_set_eof(page); break; } - } while (!nfs_readdir_array_is_full(array)); + + status = nfs_readdir_page_filler(desc, &entry, pages, page, pglen); + } while (!status && !nfs_readdir_array_is_full(array)); nfs_readdir_free_pages(pages, array_size); out_release_array: