| Message ID | 159976711218.1334.14422329830210182280.stgit@klimt.1015granger.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] NFSD: synchronously unhash a file on NFSv4 CLOSE | expand |
On Thu, Sep 10, 2020 at 03:45:12PM -0400, Chuck Lever wrote: > I recently observed a significant slowdown in a long-running > test on NFSv4.0 mounts. > > An OPEN for write seems to block the NFS server from offering > delegations on that file for a few seconds. The problem is that > when that file is closed, the filecache retains an open-for-write > file descriptor until the laundrette runs. That keeps the inode's > i_writecount positive until the cached file is finally unhashed > and closed. > > Force the NFSv4 CLOSE logic to call filp_close() to eliminate > the underlying cached open-for-write file as soon as the client > closes the file. > > This minor change claws back about 80% of the performance > regression. That's really useful to know. But mainly this makes me think that nfsd4_check_conflicting_opens() is wrong. I'm trying to determine whether a given file has a non-nfsd writer by counting the number of write opens nfsd holds on a given file and subtracting that from i_writecount. But the way I'm counting write opens probably isn't correct. --b. > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > --- > fs/nfsd/nfs4state.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index 3ac40ba7efe1..0b3059b8b36c 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -613,10 +613,14 @@ static void __nfs4_file_put_access(struct nfs4_file *fp, int oflag) > if (atomic_read(&fp->fi_access[1 - oflag]) == 0) > swap(f2, fp->fi_fds[O_RDWR]); > spin_unlock(&fp->fi_lock); > - if (f1) > + if (f1) { > + nfsd_file_close_inode_sync(locks_inode(f1->nf_file)); > nfsd_file_put(f1); > - if (f2) > + } > + if (f2) { > + nfsd_file_close_inode_sync(locks_inode(f2->nf_file)); > nfsd_file_put(f2); > + } > } > } > >
> On Sep 14, 2020, at 4:51 PM, J. Bruce Fields <bfields@fieldses.org> wrote: > > On Thu, Sep 10, 2020 at 03:45:12PM -0400, Chuck Lever wrote: >> I recently observed a significant slowdown in a long-running >> test on NFSv4.0 mounts. >> >> An OPEN for write seems to block the NFS server from offering >> delegations on that file for a few seconds. The problem is that >> when that file is closed, the filecache retains an open-for-write >> file descriptor until the laundrette runs. That keeps the inode's >> i_writecount positive until the cached file is finally unhashed >> and closed. >> >> Force the NFSv4 CLOSE logic to call filp_close() to eliminate >> the underlying cached open-for-write file as soon as the client >> closes the file. >> >> This minor change claws back about 80% of the performance >> regression. > > That's really useful to know. But mainly this makes me think that > nfsd4_check_conflicting_opens() is wrong. > > I'm trying to determine whether a given file has a non-nfsd writer by > counting the number of write opens nfsd holds on a given file and > subtracting that from i_writecount. > > But the way I'm counting write opens probably isn't correct. I entertained that possibility, but I couldn't figure out a better way to count opens-for-write. > --b. > >> >> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> >> --- >> fs/nfsd/nfs4state.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >> index 3ac40ba7efe1..0b3059b8b36c 100644 >> --- a/fs/nfsd/nfs4state.c >> +++ b/fs/nfsd/nfs4state.c >> @@ -613,10 +613,14 @@ static void __nfs4_file_put_access(struct nfs4_file *fp, int oflag) >> if (atomic_read(&fp->fi_access[1 - oflag]) == 0) >> swap(f2, fp->fi_fds[O_RDWR]); >> spin_unlock(&fp->fi_lock); >> - if (f1) >> + if (f1) { >> + nfsd_file_close_inode_sync(locks_inode(f1->nf_file)); >> nfsd_file_put(f1); >> - if (f2) >> + } >> + if (f2) { >> + nfsd_file_close_inode_sync(locks_inode(f2->nf_file)); >> nfsd_file_put(f2); >> + } >> } >> } -- Chuck Lever
> On Sep 14, 2020, at 4:53 PM, Chuck Lever <chuck.lever@oracle.com> wrote: > > > >> On Sep 14, 2020, at 4:51 PM, J. Bruce Fields <bfields@fieldses.org> wrote: >> >> On Thu, Sep 10, 2020 at 03:45:12PM -0400, Chuck Lever wrote: >>> I recently observed a significant slowdown in a long-running >>> test on NFSv4.0 mounts. >>> >>> An OPEN for write seems to block the NFS server from offering >>> delegations on that file for a few seconds. The problem is that >>> when that file is closed, the filecache retains an open-for-write >>> file descriptor until the laundrette runs. That keeps the inode's >>> i_writecount positive until the cached file is finally unhashed >>> and closed. >>> >>> Force the NFSv4 CLOSE logic to call filp_close() to eliminate >>> the underlying cached open-for-write file as soon as the client >>> closes the file. >>> >>> This minor change claws back about 80% of the performance >>> regression. >> >> That's really useful to know. But mainly this makes me think that >> nfsd4_check_conflicting_opens() is wrong. >> >> I'm trying to determine whether a given file has a non-nfsd writer by >> counting the number of write opens nfsd holds on a given file and >> subtracting that from i_writecount. >> >> But the way I'm counting write opens probably isn't correct. > > I entertained that possibility, but I couldn't figure out > a better way to count opens-for-write. Also, the regression started long before nfsd4_check_conflicting_opens was added. >> --b. >> >>> >>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> >>> --- >>> fs/nfsd/nfs4state.c | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >>> index 3ac40ba7efe1..0b3059b8b36c 100644 >>> --- a/fs/nfsd/nfs4state.c >>> +++ b/fs/nfsd/nfs4state.c >>> @@ -613,10 +613,14 @@ static void __nfs4_file_put_access(struct nfs4_file *fp, int oflag) >>> if (atomic_read(&fp->fi_access[1 - oflag]) == 0) >>> swap(f2, fp->fi_fds[O_RDWR]); >>> spin_unlock(&fp->fi_lock); >>> - if (f1) >>> + if (f1) { >>> + nfsd_file_close_inode_sync(locks_inode(f1->nf_file)); >>> nfsd_file_put(f1); >>> - if (f2) >>> + } >>> + if (f2) { >>> + nfsd_file_close_inode_sync(locks_inode(f2->nf_file)); >>> nfsd_file_put(f2); >>> + } >>> } >>> } > > -- > Chuck Lever -- Chuck Lever
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 3ac40ba7efe1..0b3059b8b36c 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -613,10 +613,14 @@ static void __nfs4_file_put_access(struct nfs4_file *fp, int oflag) if (atomic_read(&fp->fi_access[1 - oflag]) == 0) swap(f2, fp->fi_fds[O_RDWR]); spin_unlock(&fp->fi_lock); - if (f1) + if (f1) { + nfsd_file_close_inode_sync(locks_inode(f1->nf_file)); nfsd_file_put(f1); - if (f2) + } + if (f2) { + nfsd_file_close_inode_sync(locks_inode(f2->nf_file)); nfsd_file_put(f2); + } } }
I recently observed a significant slowdown in a long-running test on NFSv4.0 mounts. An OPEN for write seems to block the NFS server from offering delegations on that file for a few seconds. The problem is that when that file is closed, the filecache retains an open-for-write file descriptor until the laundrette runs. That keeps the inode's i_writecount positive until the cached file is finally unhashed and closed. Force the NFSv4 CLOSE logic to call filp_close() to eliminate the underlying cached open-for-write file as soon as the client closes the file. This minor change claws back about 80% of the performance regression. Signed-off-by: Chuck Lever <chuck.lever@oracle.com> --- fs/nfsd/nfs4state.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)