[CIFS] improve posix semantics of file create
authorSteve French <sfrench@us.ibm.com>
Fri, 20 Feb 2009 04:32:45 +0000 (04:32 +0000)
committerSteve French <sfrench@us.ibm.com>
Sat, 21 Feb 2009 03:37:09 +0000 (03:37 +0000)
Samba server added support for a new posix open/create/mkdir operation
a year or so ago, and we added support to cifs for mkdir to use it,
but had not added the corresponding code to file create.

The following patch helps improve the performance of the cifs create
path (to Samba and servers which support the cifs posix protocol
extensions).  Using Connectathon basic test1, with 2000 files, the
performance improved about 15%, and also helped reduce network traffic
(17% fewer SMBs sent over the wire) due to saving a network round trip
for the SetPathInfo on every file create.

It should also help the semantics (and probably the performance) of
write (e.g. when posix byte range locks are on the file) on file
handles opened with posix create, and adds support for a few flags
which would have to be ignored otherwise.

Signed-off-by: Steve French <sfrench@us.ibm.com>
fs/cifs/CHANGES
fs/cifs/dir.c

index 1cfa72e..72063f5 100644 (file)
@@ -8,7 +8,9 @@ top of the share.  Fix problem in 2.6.28 resolving DFS paths to
 Samba servers (worked to Windows).  Fix rmdir so that pending search
 (readdir) requests do not get invalid results which include the now
 removed directory.  Fix oops in cifs_dfs_ref.c when prefixpath is not reachable
-when using DFS.
+when using DFS.  Add better file create support to servers which support
+the CIFS POSIX protocol extensions (this adds support for new flags
+on create, and improves semantics for write of locked ranges).
 
 Version 1.55
 ------------
index 964aad0..89fb728 100644 (file)
@@ -3,7 +3,7 @@
  *
  *   vfs operations that deal with dentries
  *
- *   Copyright (C) International Business Machines  Corp., 2002,2008
+ *   Copyright (C) International Business Machines  Corp., 2002,2009
  *   Author(s): Steve French (sfrench@us.ibm.com)
  *
  *   This library is free software; you can redistribute it and/or modify
@@ -129,6 +129,78 @@ cifs_bp_rename_retry:
        return full_path;
 }
 
+static int cifs_posix_open(char *full_path, struct inode **pinode,
+                   struct super_block *sb, int mode, int oflags,
+                   int *poplock, __u16 *pnetfid, int xid)
+{
+       int rc;
+       __u32 oplock;
+       FILE_UNIX_BASIC_INFO *presp_data;
+       __u32 posix_flags = 0;
+       struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
+
+       cFYI(1, ("posix open %s", full_path));
+
+       presp_data = kzalloc(sizeof(FILE_UNIX_BASIC_INFO), GFP_KERNEL);
+       if (presp_data == NULL)
+               return -ENOMEM;
+
+/* So far cifs posix extensions can only map the following flags.
+   There are other valid fmode oflags such as FMODE_LSEEK, FMODE_PREAD, but
+   so far we do not seem to need them, and we can treat them as local only */
+       if ((oflags & (FMODE_READ | FMODE_WRITE)) ==
+               (FMODE_READ | FMODE_WRITE))
+               posix_flags = SMB_O_RDWR;
+       else if (oflags & FMODE_READ)
+               posix_flags = SMB_O_RDONLY;
+       else if (oflags & FMODE_WRITE)
+               posix_flags = SMB_O_WRONLY;
+       if (oflags & O_CREAT)
+               posix_flags |= SMB_O_CREAT;
+       if (oflags & O_EXCL)
+               posix_flags |= SMB_O_EXCL;
+       if (oflags & O_TRUNC)
+               posix_flags |= SMB_O_TRUNC;
+       if (oflags & O_APPEND)
+               posix_flags |= SMB_O_APPEND;
+       if (oflags & O_SYNC)
+               posix_flags |= SMB_O_SYNC;
+       if (oflags & O_DIRECTORY)
+               posix_flags |= SMB_O_DIRECTORY;
+       if (oflags & O_NOFOLLOW)
+               posix_flags |= SMB_O_NOFOLLOW;
+       if (oflags & O_DIRECT)
+               posix_flags |= SMB_O_DIRECT;
+
+
+       rc = CIFSPOSIXCreate(xid, cifs_sb->tcon, posix_flags, mode,
+                       pnetfid, presp_data, &oplock, full_path,
+                       cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
+                                       CIFS_MOUNT_MAP_SPECIAL_CHR);
+       if (rc)
+               goto posix_open_ret;
+
+       if (presp_data->Type == cpu_to_le32(-1))
+               goto posix_open_ret; /* open ok, caller does qpathinfo */
+
+       /* get new inode and set it up */
+       if (!pinode)
+               goto posix_open_ret; /* caller does not need info */
+
+       *pinode = cifs_new_inode(sb, &presp_data->UniqueId);
+
+       /* We do not need to close the file if new_inode fails since
+          the caller will retry qpathinfo as long as inode is null */
+       if (*pinode == NULL)
+               goto posix_open_ret;
+
+       posix_fill_in_inode(*pinode, presp_data, 1);
+
+posix_open_ret:
+       kfree(presp_data);
+       return rc;
+}
+
 static void setup_cifs_dentry(struct cifsTconInfo *tcon,
                              struct dentry *direntry,
                              struct inode *newinode)
@@ -150,7 +222,14 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
        int xid;
        int create_options = CREATE_NOT_DIR;
        int oplock = 0;
-       /* BB below access is too much for the mknod to request */
+       int oflags;
+       /*
+        * BB below access is probably too much for mknod to request
+        *    but we have to do query and setpathinfo so requesting
+        *    less could fail (unless we want to request getatr and setatr
+        *    permissions (only).  At least for POSIX we do not have to
+        *    request so much.
+        */
        int desiredAccess = GENERIC_READ | GENERIC_WRITE;
        __u16 fileHandle;
        struct cifs_sb_info *cifs_sb;
@@ -174,13 +253,43 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
        }
 
        mode &= ~current->fs->umask;
+       if (oplockEnabled)
+               oplock = REQ_OPLOCK;
 
-       if (nd && (nd->flags & LOOKUP_OPEN)) {
-               int oflags = nd->intent.open.flags;
+       if (nd && (nd->flags & LOOKUP_OPEN))
+               oflags = nd->intent.open.flags;
+       else
+               oflags = FMODE_READ;
+
+       if (tcon->unix_ext && (tcon->ses->capabilities & CAP_UNIX) &&
+           (CIFS_UNIX_POSIX_PATH_OPS_CAP &
+                       le64_to_cpu(tcon->fsUnixInfo.Capability))) {
+               rc = cifs_posix_open(full_path, &newinode, inode->i_sb,
+                                    mode, oflags, &oplock, &fileHandle, xid);
+               /* EIO could indicate that (posix open) operation is not
+                  supported, despite what server claimed in capability
+                  negotation.  EREMOTE indicates DFS junction, which is not
+                  handled in posix open */
+
+               if ((rc == 0) && (newinode == NULL))
+                       goto cifs_create_get_file_info; /* query inode info */
+               else if (rc == 0) /* success, no need to query */
+                       goto cifs_create_set_dentry;
+               else if ((rc != -EIO) && (rc != -EREMOTE) &&
+                        (rc != -EOPNOTSUPP)) /* path not found or net err */
+                       goto cifs_create_out;
+               /* else fallthrough to retry, using older open call, this is
+                  case where server does not support this SMB level, and
+                  falsely claims capability (also get here for DFS case
+                  which should be rare for path not covered on files) */
+       }
 
+       if (nd && (nd->flags & LOOKUP_OPEN)) {
+               /* if the file is going to stay open, then we
+                  need to set the desired access properly */
                desiredAccess = 0;
                if (oflags & FMODE_READ)
-                       desiredAccess |= GENERIC_READ;
+                       desiredAccess |= GENERIC_READ; /* is this too little? */
                if (oflags & FMODE_WRITE) {
                        desiredAccess |= GENERIC_WRITE;
                        if (!(oflags & FMODE_READ))
@@ -199,8 +308,6 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
 
        /* BB add processing to set equivalent of mode - e.g. via CreateX with
           ACLs */
-       if (oplockEnabled)
-               oplock = REQ_OPLOCK;
 
        buf = kmalloc(sizeof(FILE_ALL_INFO), GFP_KERNEL);
        if (buf == NULL) {
@@ -233,116 +340,112 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
        }
        if (rc) {
                cFYI(1, ("cifs_create returned 0x%x", rc));
-       } else {
-               /* If Open reported that we actually created a file
-               then we now have to set the mode if possible */
-               if ((tcon->unix_ext) && (oplock & CIFS_CREATE_ACTION)) {
-                       struct cifs_unix_set_info_args args = {
+               goto cifs_create_out;
+       }
+
+       /* If Open reported that we actually created a file
+          then we now have to set the mode if possible */
+       if ((tcon->unix_ext) && (oplock & CIFS_CREATE_ACTION)) {
+               struct cifs_unix_set_info_args args = {
                                .mode   = mode,
                                .ctime  = NO_CHANGE_64,
                                .atime  = NO_CHANGE_64,
                                .mtime  = NO_CHANGE_64,
                                .device = 0,
-                       };
+               };
 
-                       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID) {
-                               args.uid = (__u64) current_fsuid();
-                               if (inode->i_mode & S_ISGID)
-                                       args.gid = (__u64) inode->i_gid;
-                               else
-                                       args.gid = (__u64) current_fsgid();
-                       } else {
-                               args.uid = NO_CHANGE_64;
-                               args.gid = NO_CHANGE_64;
-                       }
-                       CIFSSMBUnixSetInfo(xid, tcon, full_path, &args,
-                               cifs_sb->local_nls,
-                               cifs_sb->mnt_cifs_flags &
-                                       CIFS_MOUNT_MAP_SPECIAL_CHR);
+               if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID) {
+                       args.uid = (__u64) current_fsuid();
+                       if (inode->i_mode & S_ISGID)
+                               args.gid = (__u64) inode->i_gid;
+                       else
+                               args.gid = (__u64) current_fsgid();
                } else {
-                       /* BB implement mode setting via Windows security
-                          descriptors e.g. */
-                       /* CIFSSMBWinSetPerms(xid,tcon,path,mode,-1,-1,nls);*/
-
-                       /* Could set r/o dos attribute if mode & 0222 == 0 */
+                       args.uid = NO_CHANGE_64;
+                       args.gid = NO_CHANGE_64;
                }
+               CIFSSMBUnixSetInfo(xid, tcon, full_path, &args,
+                       cifs_sb->local_nls,
+                       cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
+       } else {
+               /* BB implement mode setting via Windows security
+                  descriptors e.g. */
+               /* CIFSSMBWinSetPerms(xid,tcon,path,mode,-1,-1,nls);*/
 
-               /* server might mask mode so we have to query for it */
-               if (tcon->unix_ext)
-                       rc = cifs_get_inode_info_unix(&newinode, full_path,
-                                                inode->i_sb, xid);
-               else {
-                       rc = cifs_get_inode_info(&newinode, full_path,
-                                                buf, inode->i_sb, xid,
-                                                &fileHandle);
-                       if (newinode) {
-                               if (cifs_sb->mnt_cifs_flags &
-                                   CIFS_MOUNT_DYNPERM)
-                                       newinode->i_mode = mode;
-                               if ((oplock & CIFS_CREATE_ACTION) &&
-                                   (cifs_sb->mnt_cifs_flags &
-                                    CIFS_MOUNT_SET_UID)) {
-                                       newinode->i_uid = current_fsuid();
-                                       if (inode->i_mode & S_ISGID)
-                                               newinode->i_gid =
-                                                       inode->i_gid;
-                                       else
-                                               newinode->i_gid =
-                                                       current_fsgid();
-                               }
+               /* Could set r/o dos attribute if mode & 0222 == 0 */
+       }
+
+cifs_create_get_file_info:
+       /* server might mask mode so we have to query for it */
+       if (tcon->unix_ext)
+               rc = cifs_get_inode_info_unix(&newinode, full_path,
+                                             inode->i_sb, xid);
+       else {
+               rc = cifs_get_inode_info(&newinode, full_path, buf,
+                                        inode->i_sb, xid, &fileHandle);
+               if (newinode) {
+                       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DYNPERM)
+                               newinode->i_mode = mode;
+                       if ((oplock & CIFS_CREATE_ACTION) &&
+                           (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID)) {
+                               newinode->i_uid = current_fsuid();
+                               if (inode->i_mode & S_ISGID)
+                                       newinode->i_gid = inode->i_gid;
+                               else
+                                       newinode->i_gid = current_fsgid();
                        }
                }
+       }
 
-               if (rc != 0) {
-                       cFYI(1, ("Create worked, get_inode_info failed rc = %d",
-                                rc));
-               } else
-                       setup_cifs_dentry(tcon, direntry, newinode);
-
-               if ((nd == NULL /* nfsd case - nfs srv does not set nd */) ||
-                       (!(nd->flags & LOOKUP_OPEN))) {
-                       /* mknod case - do not leave file open */
-                       CIFSSMBClose(xid, tcon, fileHandle);
-               } else if (newinode) {
-                       struct cifsFileInfo *pCifsFile =
-                          kzalloc(sizeof(struct cifsFileInfo), GFP_KERNEL);
-
-                       if (pCifsFile == NULL)
-                               goto cifs_create_out;
-                       pCifsFile->netfid = fileHandle;
-                       pCifsFile->pid = current->tgid;
-                       pCifsFile->pInode = newinode;
-                       pCifsFile->invalidHandle = false;
-                       pCifsFile->closePend     = false;
-                       init_MUTEX(&pCifsFile->fh_sem);
-                       mutex_init(&pCifsFile->lock_mutex);
-                       INIT_LIST_HEAD(&pCifsFile->llist);
-                       atomic_set(&pCifsFile->wrtPending, 0);
-
-                       /* set the following in open now
+cifs_create_set_dentry:
+       if (rc == 0)
+               setup_cifs_dentry(tcon, direntry, newinode);
+       else
+               cFYI(1, ("Create worked, get_inode_info failed rc = %d", rc));
+
+       /* nfsd case - nfs srv does not set nd */
+       if ((nd == NULL) || (!(nd->flags & LOOKUP_OPEN))) {
+               /* mknod case - do not leave file open */
+               CIFSSMBClose(xid, tcon, fileHandle);
+       } else if (newinode) {
+               struct cifsFileInfo *pCifsFile =
+                       kzalloc(sizeof(struct cifsFileInfo), GFP_KERNEL);
+
+               if (pCifsFile == NULL)
+                       goto cifs_create_out;
+               pCifsFile->netfid = fileHandle;
+               pCifsFile->pid = current->tgid;
+               pCifsFile->pInode = newinode;
+               pCifsFile->invalidHandle = false;
+               pCifsFile->closePend     = false;
+               init_MUTEX(&pCifsFile->fh_sem);
+               mutex_init(&pCifsFile->lock_mutex);
+               INIT_LIST_HEAD(&pCifsFile->llist);
+               atomic_set(&pCifsFile->wrtPending, 0);
+
+               /* set the following in open now
                                pCifsFile->pfile = file; */
-                       write_lock(&GlobalSMBSeslock);
-                       list_add(&pCifsFile->tlist, &tcon->openFileList);
-                       pCifsInode = CIFS_I(newinode);
-                       if (pCifsInode) {
-                               /* if readable file instance put first in list*/
-                               if (write_only) {
-                                       list_add_tail(&pCifsFile->flist,
-                                               &pCifsInode->openFileList);
-                               } else {
-                                       list_add(&pCifsFile->flist,
-                                               &pCifsInode->openFileList);
-                               }
-                               if ((oplock & 0xF) == OPLOCK_EXCLUSIVE) {
-                                       pCifsInode->clientCanCacheAll = true;
-                                       pCifsInode->clientCanCacheRead = true;
-                                       cFYI(1, ("Exclusive Oplock inode %p",
-                                               newinode));
-                               } else if ((oplock & 0xF) == OPLOCK_READ)
-                                       pCifsInode->clientCanCacheRead = true;
+               write_lock(&GlobalSMBSeslock);
+               list_add(&pCifsFile->tlist, &tcon->openFileList);
+               pCifsInode = CIFS_I(newinode);
+               if (pCifsInode) {
+                       /* if readable file instance put first in list*/
+                       if (write_only) {
+                               list_add_tail(&pCifsFile->flist,
+                                             &pCifsInode->openFileList);
+                       } else {
+                               list_add(&pCifsFile->flist,
+                                        &pCifsInode->openFileList);
                        }
-                       write_unlock(&GlobalSMBSeslock);
+                       if ((oplock & 0xF) == OPLOCK_EXCLUSIVE) {
+                               pCifsInode->clientCanCacheAll = true;
+                               pCifsInode->clientCanCacheRead = true;
+                               cFYI(1, ("Exclusive Oplock inode %p",
+                                       newinode));
+                       } else if ((oplock & 0xF) == OPLOCK_READ)
+                               pCifsInode->clientCanCacheRead = true;
                }
+               write_unlock(&GlobalSMBSeslock);
        }
 cifs_create_out:
        kfree(buf);