[RFC,0/8] Make NFSv4 OPEN(CREATE) less brittle

Message

Chuck Lever April 20, 2022, 6:28 p.m. UTC

Attempt to address occasional reports of test failures caused by
NFSv4 OPEN(CREATE) having failing internally after the target
file object has been created.

The basic approach is to re-organize the NFSv4 OPEN code path so
that common failure modes occur /before/ the call to vfs_create()
rather than afterwards. In addition, the file is opened and created
atomically so that another client can't race and de-permit the
file just after it was created but before the server has opened it.

So far I haven't found any regressions. However I have not been
able to reproduce the original failures.


---

Chuck Lever (8):
      NFSD: Clean up nfsd3_proc_create()
      NFSD: Avoid calling fh_drop_write() twice in do_nfsd_create()
      NFSD: Refactor nfsd_create_setattr()
      NFSD: Refactor NFSv3 CREATE
      NFSD: Refactor NFSv4 OPEN(CREATE)
      NFSD: Remove do_nfsd_create()
      NFSD: Clean up nfsd_open_verified()
      NFSD: Instantiate a struct file when creating a regular NFSv4 file


 fs/nfsd/filecache.c |  51 +++++++--
 fs/nfsd/filecache.h |   2 +
 fs/nfsd/nfs3proc.c  | 141 +++++++++++++++++++++----
 fs/nfsd/nfs4proc.c  | 197 +++++++++++++++++++++++++++++++++--
 fs/nfsd/nfs4state.c |  16 ++-
 fs/nfsd/vfs.c       | 245 ++++++++++----------------------------------
 fs/nfsd/vfs.h       |  14 +--
 fs/nfsd/xdr4.h      |   1 +
 fs/open.c           |  44 ++++++++
 include/linux/fs.h  |   2 +
 10 files changed, 471 insertions(+), 242 deletions(-)

--
Chuck Lever

Comments

J. Bruce Fields April 20, 2022, 7:24 p.m. UTC | #1

On Wed, Apr 20, 2022 at 02:28:34PM -0400, Chuck Lever wrote:
> Attempt to address occasional reports of test failures caused by
> NFSv4 OPEN(CREATE) having failing internally after the target
> file object has been created.
> 
> The basic approach is to re-organize the NFSv4 OPEN code path so
> that common failure modes occur /before/ the call to vfs_create()
> rather than afterwards. In addition, the file is opened and created
> atomically so that another client can't race and de-permit the
> file just after it was created but before the server has opened it.
> 
> So far I haven't found any regressions. However I have not been
> able to reproduce the original failures.

I'll admit I don't know how big an impact those races have in practice.
But this has bugged me for a long time--thanks for finally tackling it.

I haven't reviewed it in detail but the basic idea looks good.

--b.

> Chuck Lever (8):
>       NFSD: Clean up nfsd3_proc_create()
>       NFSD: Avoid calling fh_drop_write() twice in do_nfsd_create()
>       NFSD: Refactor nfsd_create_setattr()
>       NFSD: Refactor NFSv3 CREATE
>       NFSD: Refactor NFSv4 OPEN(CREATE)
>       NFSD: Remove do_nfsd_create()
>       NFSD: Clean up nfsd_open_verified()
>       NFSD: Instantiate a struct file when creating a regular NFSv4 file
> 
> 
>  fs/nfsd/filecache.c |  51 +++++++--
>  fs/nfsd/filecache.h |   2 +
>  fs/nfsd/nfs3proc.c  | 141 +++++++++++++++++++++----
>  fs/nfsd/nfs4proc.c  | 197 +++++++++++++++++++++++++++++++++--
>  fs/nfsd/nfs4state.c |  16 ++-
>  fs/nfsd/vfs.c       | 245 ++++++++++----------------------------------
>  fs/nfsd/vfs.h       |  14 +--
>  fs/nfsd/xdr4.h      |   1 +
>  fs/open.c           |  44 ++++++++
>  include/linux/fs.h  |   2 +
>  10 files changed, 471 insertions(+), 242 deletions(-)
> 
> --
> Chuck Lever