[linux-cifs-client] [PATCH] cifs: have cifs_create use POSIX create if it's available (try #2)

Jeff Layton jlayton at redhat.com
Mon Feb 9 21:27:59 GMT 2009


This reduces the number of calls to the server when creating files, and
also makes the force_create_mode option in samba work correctly.

It also refactors some of the error handling in this function to
reduce layers of indentation.

This is the second attempt at this patch. The first patch had a
bug in the open flag conversion routines and didn't try to enable
O_DIRECT on direct I/O mounts.

Signed-off-by: Jeff Layton <jlayton at redhat.com>
---
 fs/cifs/cifsproto.h |    2 +
 fs/cifs/dir.c       |  283 +++++++++++++++++++++++++++++---------------------
 fs/cifs/inode.c     |    2 +-
 3 files changed, 167 insertions(+), 120 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 71cc5e4..4a71753 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -91,6 +91,8 @@ extern u64 cifs_UnixTimeToNT(struct timespec);
 extern __le64 cnvrtDosCifsTm(__u16 date, __u16 time);
 extern struct timespec cnvrtDosUnixTm(__u16 date, __u16 time);
 
+extern void posix_fill_in_inode(struct inode *tmp_inode,
+				FILE_UNIX_BASIC_INFO *pData, int isNewInode);
 extern struct inode *cifs_new_inode(struct super_block *sb, unsigned long inum);
 extern int cifs_get_inode_info(struct inode **pinode,
 			const unsigned char *search_path,
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 964aad0..21d2545 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -152,11 +152,13 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
 	int oplock = 0;
 	/* BB below access is too much for the mknod to request */
 	int desiredAccess = GENERIC_READ | GENERIC_WRITE;
+	__u32 posix_flags = SMB_O_CREAT | SMB_O_RDWR | SMB_O_TRUNC;
 	__u16 fileHandle;
 	struct cifs_sb_info *cifs_sb;
 	struct cifsTconInfo *tcon;
 	char *full_path = NULL;
 	FILE_ALL_INFO *buf = NULL;
+	FILE_UNIX_BASIC_INFO *pinfo = NULL;
 	struct inode *newinode = NULL;
 	struct cifsInodeInfo *pCifsInode;
 	int disposition = FILE_OVERWRITE_IF;
@@ -169,32 +171,47 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
 
 	full_path = build_path_from_dentry(direntry);
 	if (full_path == NULL) {
-		FreeXid(xid);
-		return -ENOMEM;
+		rc = -ENOMEM;
+		goto cifs_create_out;
 	}
 
 	mode &= ~current->fs->umask;
 
+	/* convert lookup intent to flags for create */
 	if (nd && (nd->flags & LOOKUP_OPEN)) {
 		int oflags = nd->intent.open.flags;
 
-		desiredAccess = 0;
-		if (oflags & FMODE_READ)
-			desiredAccess |= GENERIC_READ;
-		if (oflags & FMODE_WRITE) {
-			desiredAccess |= GENERIC_WRITE;
-			if (!(oflags & FMODE_READ))
-				write_only = true;
+		if ((oflags & (FMODE_READ | FMODE_WRITE)) ==
+			      (FMODE_READ | FMODE_WRITE)) {
+			desiredAccess = (GENERIC_READ | GENERIC_WRITE);
+			posix_flags = SMB_O_CREAT | SMB_O_RDWR;
+		} else if (oflags & FMODE_READ) {
+			desiredAccess = GENERIC_READ;
+			posix_flags = SMB_O_CREAT | SMB_O_RDONLY;
+		} else if (oflags & FMODE_WRITE) {
+			desiredAccess = GENERIC_WRITE;
+			posix_flags = SMB_O_CREAT | SMB_O_WRONLY;
+			write_only = true;
 		}
 
-		if ((oflags & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL))
+		if ((oflags & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL)) {
 			disposition = FILE_CREATE;
-		else if ((oflags & (O_CREAT | O_TRUNC)) == (O_CREAT | O_TRUNC))
+			posix_flags |= SMB_O_EXCL;
+		} else if ((oflags & (O_CREAT | O_TRUNC)) ==
+			   (O_CREAT | O_TRUNC)) {
 			disposition = FILE_OVERWRITE_IF;
-		else if ((oflags & O_CREAT) == O_CREAT)
+			posix_flags |= SMB_O_TRUNC;
+		} else if ((oflags & O_CREAT) == O_CREAT)
 			disposition = FILE_OPEN_IF;
 		else
 			cFYI(1, ("Create flag not set in create function"));
+
+		if (oflags & O_APPEND)
+			posix_flags |= SMB_O_APPEND;
+		if (oflags & O_SYNC)
+			posix_flags |= SMB_O_SYNC;
+		if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DIRECT_IO)
+			posix_flags |= SMB_O_DIRECT;
 	}
 
 	/* BB add processing to set equivalent of mode - e.g. via CreateX with
@@ -202,11 +219,39 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
 	if (oplockEnabled)
 		oplock = REQ_OPLOCK;
 
+	/* try POSIX create first */
+	if (tcon->unix_ext) {
+		pinfo = kzalloc(sizeof(FILE_UNIX_BASIC_INFO), GFP_KERNEL);
+		if (pinfo == NULL) {
+			rc = -ENOMEM;
+			goto cifs_create_out;
+		}
+		rc = CIFSPOSIXCreate(xid, tcon, posix_flags, mode, &fileHandle,
+				     pinfo, &oplock, full_path,
+				     cifs_sb->local_nls,
+				     cifs_sb->mnt_cifs_flags &
+						CIFS_MOUNT_MAP_SPECIAL_CHR);
+		if (!rc) {
+			/* check for valid pinfo */
+			if (pinfo->Type == cpu_to_le32(-1))
+				goto cifs_create_reval;
+			newinode = cifs_new_inode(inode->i_sb, (unsigned long)
+							pinfo->UniqueId);
+			if (!newinode) {
+				rc = -ENOMEM;
+				goto cifs_create_out;
+			}
+
+			posix_fill_in_inode(newinode, pinfo, 1);
+			goto cifs_create_set_dentry;
+		} else if (rc != -EOPNOTSUPP)
+			goto cifs_create_out;
+	}
+
 	buf = kmalloc(sizeof(FILE_ALL_INFO), GFP_KERNEL);
 	if (buf == NULL) {
-		kfree(full_path);
-		FreeXid(xid);
-		return -ENOMEM;
+		rc = -ENOMEM;
+		goto cifs_create_out;
 	}
 
 	/*
@@ -231,123 +276,123 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
 			&fileHandle, &oplock, buf, cifs_sb->local_nls,
 			cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
 	}
-	if (rc) {
-		cFYI(1, ("cifs_create returned 0x%x", rc));
+
+	if (rc)
+		goto cifs_create_out;
+
+	/*
+	 * If old-style 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);
 	} 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 = {
-				.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();
+		/* 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 */
+	}
+
+	/* server might mask mode so we have to query for it */
+cifs_create_reval:
+	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)
-					args.gid = (__u64) inode->i_gid;
+					newinode->i_gid = inode->i_gid;
 				else
-					args.gid = (__u64) current_fsgid();
-			} else {
-				args.uid = NO_CHANGE_64;
-				args.gid = NO_CHANGE_64;
+					newinode->i_gid = current_fsgid();
 			}
-			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);*/
-
-			/* Could set r/o dos attribute if mode & 0222 == 0 */
 		}
+	}
 
-		/* 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();
-				}
-			}
-		}
+cifs_create_set_dentry:
+	if (rc != 0) {
+		cFYI(1, ("Create worked, get_inode_info failed rc = %d", rc));
+	} else {
+		setup_cifs_dentry(tcon, direntry, newinode);
+	}
 
-		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
-				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_unlock(&GlobalSMBSeslock);
+	/* if nd is NULL 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);
+
+		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_unlock(&GlobalSMBSeslock);
 	}
 cifs_create_out:
 	kfree(buf);
+	kfree(pinfo);
 	kfree(full_path);
 	FreeXid(xid);
+	cFYI(1, ("cifs_create returned %d", rc));
 	return rc;
 }
 
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 4987c50..d4b73fa 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1045,7 +1045,7 @@ out_reval:
 	return rc;
 }
 
-static void posix_fill_in_inode(struct inode *tmp_inode,
+void posix_fill_in_inode(struct inode *tmp_inode,
 	FILE_UNIX_BASIC_INFO *pData, int isNewInode)
 {
 	struct cifsInodeInfo *cifsInfo = CIFS_I(tmp_inode);
-- 
1.5.5.6



More information about the linux-cifs-client mailing list