Skip to content

Commit

Permalink
NFS: add atomic_open for NFSv3 to handle O_TRUNC correctly.
Browse files Browse the repository at this point in the history
With two clients, each with NFSv3 mounts of the same directory, the sequence:

   client1            client2
  ls -l afile
                      echo hello there > afile
  echo HELLO > afile
  cat afile

will show
   HELLO
   there

because the O_TRUNC requested in the final 'echo' doesn't take effect.
This is because the "Negative dentry, just create a file" section in
lookup_open() assumes that the file *does* get created since the dentry
was negative, so it sets FMODE_CREATED, and this causes do_open() to
clear O_TRUNC and so the file doesn't get truncated.

Even mounting with -o lookupcache=none does not help as
nfs_neg_need_reval() always returns false if LOOKUP_CREATE is set.

This patch fixes the problem by providing an atomic_open inode operation
for NFSv3 (and v2).  The code is largely the code from the branch in
lookup_open() when atomic_open is not provided.  The significant change
is that the O_TRUNC flag is passed a new nfs_do_create() which add
'trunc' handling to nfs_create().

With this change we also optimise away an unnecessary LOOKUP before the
file is created.

Signed-off-by: NeilBrown <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
  • Loading branch information
neilbrown authored and Trond Myklebust committed May 20, 2024
1 parent 464b424 commit 7c6c524
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 3 deletions.
54 changes: 51 additions & 3 deletions fs/nfs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ static int nfs_readdir(struct file *, struct dir_context *);
static int nfs_fsync_dir(struct file *, loff_t, loff_t, int);
static loff_t nfs_llseek_dir(struct file *, loff_t, int);
static void nfs_readdir_clear_array(struct folio *);
static int nfs_do_create(struct inode *dir, struct dentry *dentry,
umode_t mode, int open_flags);

const struct file_operations nfs_dir_operations = {
.llseek = nfs_llseek_dir,
Expand Down Expand Up @@ -2243,6 +2245,41 @@ static int nfs4_lookup_revalidate(struct dentry *dentry, unsigned int flags)

#endif /* CONFIG_NFSV4 */

int nfs_atomic_open_v23(struct inode *dir, struct dentry *dentry,
struct file *file, unsigned int open_flags,
umode_t mode)
{

/* Same as look+open from lookup_open(), but with different O_TRUNC
* handling.
*/
int error = 0;

if (open_flags & O_CREAT) {
file->f_mode |= FMODE_CREATED;
error = nfs_do_create(dir, dentry, mode, open_flags);
if (error)
return error;
return finish_open(file, dentry, NULL);
} else if (d_in_lookup(dentry)) {
/* The only flags nfs_lookup considers are
* LOOKUP_EXCL and LOOKUP_RENAME_TARGET, and
* we want those to be zero so the lookup isn't skipped.
*/
struct dentry *res = nfs_lookup(dir, dentry, 0);

d_lookup_done(dentry);
if (unlikely(res)) {
if (IS_ERR(res))
return PTR_ERR(res);
return finish_no_open(file, res);
}
}
return finish_no_open(file, NULL);

}
EXPORT_SYMBOL_GPL(nfs_atomic_open_v23);

struct dentry *
nfs_add_or_obtain(struct dentry *dentry, struct nfs_fh *fhandle,
struct nfs_fattr *fattr)
Expand Down Expand Up @@ -2303,18 +2340,23 @@ EXPORT_SYMBOL_GPL(nfs_instantiate);
* that the operation succeeded on the server, but an error in the
* reply path made it appear to have failed.
*/
int nfs_create(struct mnt_idmap *idmap, struct inode *dir,
struct dentry *dentry, umode_t mode, bool excl)
static int nfs_do_create(struct inode *dir, struct dentry *dentry,
umode_t mode, int open_flags)
{
struct iattr attr;
int open_flags = excl ? O_CREAT | O_EXCL : O_CREAT;
int error;

open_flags |= O_CREAT;

dfprintk(VFS, "NFS: create(%s/%lu), %pd\n",
dir->i_sb->s_id, dir->i_ino, dentry);

attr.ia_mode = mode;
attr.ia_valid = ATTR_MODE;
if (open_flags & O_TRUNC) {
attr.ia_size = 0;
attr.ia_valid |= ATTR_SIZE;
}

trace_nfs_create_enter(dir, dentry, open_flags);
error = NFS_PROTO(dir)->create(dir, dentry, &attr, open_flags);
Expand All @@ -2326,6 +2368,12 @@ int nfs_create(struct mnt_idmap *idmap, struct inode *dir,
d_drop(dentry);
return error;
}

int nfs_create(struct mnt_idmap *idmap, struct inode *dir,
struct dentry *dentry, umode_t mode, bool excl)
{
return nfs_do_create(dir, dentry, mode, excl ? O_EXCL : 0);
}
EXPORT_SYMBOL_GPL(nfs_create);

/*
Expand Down
1 change: 1 addition & 0 deletions fs/nfs/nfs3proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,7 @@ static int nfs3_have_delegation(struct inode *inode, fmode_t flags)

static const struct inode_operations nfs3_dir_inode_operations = {
.create = nfs_create,
.atomic_open = nfs_atomic_open_v23,
.lookup = nfs_lookup,
.link = nfs_link,
.unlink = nfs_unlink,
Expand Down
1 change: 1 addition & 0 deletions fs/nfs/proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,7 @@ static int nfs_have_delegation(struct inode *inode, fmode_t flags)
static const struct inode_operations nfs_dir_inode_operations = {
.create = nfs_create,
.lookup = nfs_lookup,
.atomic_open = nfs_atomic_open_v23,
.link = nfs_link,
.unlink = nfs_unlink,
.symlink = nfs_symlink,
Expand Down
3 changes: 3 additions & 0 deletions include/linux/nfs_fs.h
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,9 @@ extern int nfs_may_open(struct inode *inode, const struct cred *cred, int openfl
extern void nfs_access_zap_cache(struct inode *inode);
extern int nfs_access_get_cached(struct inode *inode, const struct cred *cred,
u32 *mask, bool may_block);
extern int nfs_atomic_open_v23(struct inode *dir, struct dentry *dentry,
struct file *file, unsigned int open_flags,
umode_t mode);

/*
* linux/fs/nfs/symlink.c
Expand Down

0 comments on commit 7c6c524

Please sign in to comment.