[linux-cifs-client,3/3] cifs: have cifs_create use POSIX create if it's available

Commit Message

Jeff Layton Feb. 11, 2009, 1:08 p.m. UTC

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 <[e-mail]>
---
 fs/cifs/cifsproto.h |    2 +
 fs/cifs/dir.c       |  283 +++++++++++++++++++++++++++++---------------------
 fs/cifs/inode.c     |    2 +-
 3 files changed, 167 insertions(+), 120 deletions(-)

Comments

Steve French Feb. 11, 2009, 4:41 p.m. UTC | #1

On Wed, Feb 11, 2009 at 7:08 AM, Jeff Layton <[e-mail]> wrote:

> 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.
>

This is great, but wondering if the posix create code should be split out to
a different function - calling out early from cifs_create to a new
posix_create (when the server supports posix extensions) to reduce the size
of this (now) 250+ line cifs_create function (as I had done in my earlier
start on posix_create last month).

Jeff Layton Feb. 11, 2009, 4:44 p.m. UTC | #2

On Wed, 11 Feb 2009 10:41:07 -0600
Steve French <[e-mail]> wrote:

> On Wed, Feb 11, 2009 at 7:08 AM, Jeff Layton <[e-mail]> wrote:
> 
> > 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.
> >
> 
> This is great, but wondering if the posix create code should be split out to
> a different function - calling out early from cifs_create to a new
> posix_create (when the server supports posix extensions) to reduce the size
> of this (now) 250+ line cifs_create function (as I had done in my earlier
> start on posix_create last month).
> 

Wouldn't hurt, though it may be a little while before I can get to it...

Steve French Feb. 11, 2009, 4:51 p.m. UTC | #3

On Wed, Feb 11, 2009 at 10:44 AM, Jeff Layton <[e-mail]> wrote:

> On Wed, 11 Feb 2009 10:41:07 -0600
> Steve French <[e-mail]> wrote:
>
> > On Wed, Feb 11, 2009 at 7:08 AM, Jeff Layton <[e-mail]> wrote:
> >
> > > 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.
> > >
> >
> > This is great, but wondering if the posix create code should be split out
> to
> > a different function - calling out early from cifs_create to a new
> > posix_create (when the server supports posix extensions) to reduce the
> size
> > of this (now) 250+ line cifs_create function (as I had done in my earlier
> > start on posix_create last month).
> >
>
> Wouldn't hurt, though it may be a little while before I can get to it...
>

Am looking at it now.

By the way, in your patch the line:

       FreeXid(xid);
+       cFYI(1, ("cifs_create returned %d", rc));
       return rc;

is not necessary.   FreeXid already includes the cFYI debug line.

Jeff Layton Feb. 11, 2009, 5:07 p.m. UTC | #4

On Wed, 11 Feb 2009 10:51:12 -0600
Steve French <[e-mail]> wrote:

> On Wed, Feb 11, 2009 at 10:44 AM, Jeff Layton <[e-mail]> wrote:
> 
> > On Wed, 11 Feb 2009 10:41:07 -0600
> > Steve French <[e-mail]> wrote:
> >
> > > On Wed, Feb 11, 2009 at 7:08 AM, Jeff Layton <[e-mail]> wrote:
> > >
> > > > 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.
> > > >
> > >
> > > This is great, but wondering if the posix create code should be split out
> > to
> > > a different function - calling out early from cifs_create to a new
> > > posix_create (when the server supports posix extensions) to reduce the
> > size
> > > of this (now) 250+ line cifs_create function (as I had done in my earlier
> > > start on posix_create last month).
> > >
> >
> > Wouldn't hurt, though it may be a little while before I can get to it...
> >
> 
> Am looking at it now.
> 
> By the way, in your patch the line:
> 
>        FreeXid(xid);
> +       cFYI(1, ("cifs_create returned %d", rc));
>        return rc;
> 
> is not necessary.   FreeXid already includes the cFYI debug line.
> 

Yeah I thought about that. There was an earlier cFYI that that
replaced. I just moved it down in the function and changed it to print
as a signed int rather than in hex. I'm ok with removing it altogether
though.

Shirish Pargaonkar Feb. 11, 2009, 7:05 p.m. UTC | #5

On Wed, Feb 11, 2009 at 10:41 AM, Steve French <[e-mail]> wrote:
>
>
> On Wed, Feb 11, 2009 at 7:08 AM, Jeff Layton <[e-mail]> wrote:
>>
>> 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.
>
> This is great, but wondering if the posix create code should be split out to
> a different function - calling out early from cifs_create to a new
> posix_create (when the server supports posix extensions) to reduce the size
> of this (now) 250+ line cifs_create function (as I had done in my earlier
> start on posix_create last month).
>
>
>
> --
> Thanks,
>
> Steve
>
> _______________________________________________
> linux-cifs-client mailing list
> [e-mail]
> https://lists.samba.org/mailman/listinfo/linux-cifs-client
>
>


Nitpicking here, I think it would be a good idea to carve out
posix_create function that cifs_create calls.
Then this posix_create function could also be called from cifs_lookup
when lookup intents are implemented.
And may be it should be called posix_open instead of posix_create, it
is all open anyway.

Regards,

Shirish

Jeff Layton Feb. 11, 2009, 9:16 p.m. UTC | #6

On Wed, 11 Feb 2009 13:05:32 -0600
Shirish Pargaonkar <[e-mail]> wrote:

> On Wed, Feb 11, 2009 at 10:41 AM, Steve French <[e-mail]> wrote:
> >
> >
> > On Wed, Feb 11, 2009 at 7:08 AM, Jeff Layton <[e-mail]> wrote:
> >>
> >> 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.
> >
> > This is great, but wondering if the posix create code should be split out to
> > a different function - calling out early from cifs_create to a new
> > posix_create (when the server supports posix extensions) to reduce the size
> > of this (now) 250+ line cifs_create function (as I had done in my earlier
> > start on posix_create last month).
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve
> >
> > _______________________________________________
> > linux-cifs-client mailing list
> > [e-mail]
> > https://lists.samba.org/mailman/listinfo/linux-cifs-client
> >
> >
> 
> 
> Nitpicking here, I think it would be a good idea to carve out
> posix_create function that cifs_create calls.
> Then this posix_create function could also be called from cifs_lookup
> when lookup intents are implemented.
> And may be it should be called posix_open instead of posix_create, it
> is all open anyway.
> 

I agree that they're very similar. It probably makes some sense to have
posix_create and posix_open share some code.

It's not clear to me that we need to do all of the stuff that we do in
cifs_open for the O_CREAT case anyway. That's probably reasonable as a
later cleanup and rethink on how it works.

Jeff Layton Feb. 11, 2009, 11:51 p.m. UTC | #7

On Wed, 11 Feb 2009 13:05:32 -0600
Shirish Pargaonkar <[e-mail]> wrote:

> On Wed, Feb 11, 2009 at 10:41 AM, Steve French <[e-mail]> wrote:
> >
> >
> > On Wed, Feb 11, 2009 at 7:08 AM, Jeff Layton <[e-mail]> wrote:
> >>
> >> 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.
> >
> > This is great, but wondering if the posix create code should be split out to
> > a different function - calling out early from cifs_create to a new
> > posix_create (when the server supports posix extensions) to reduce the size
> > of this (now) 250+ line cifs_create function (as I had done in my earlier
> > start on posix_create last month).
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve
> >
> > _______________________________________________
> > linux-cifs-client mailing list
> > [e-mail]
> > https://lists.samba.org/mailman/listinfo/linux-cifs-client
> >
> >
> 
> 
> Nitpicking here, I think it would be a good idea to carve out
> posix_create function that cifs_create calls.
> Then this posix_create function could also be called from cifs_lookup
> when lookup intents are implemented.
> And may be it should be called posix_open instead of posix_create, it
> is all open anyway.
> 

What's the advantage of doing the actual create on lookup rather than
in the create operation? It seems like doing the actual file creation
with the ->create op is a more logical scheme...

Steve French Feb. 12, 2009, 12:04 a.m. UTC | #8

It saves a round trip on query path info (a huge reduction in the number of
smbs sent)

On Wed, Feb 11, 2009 at 5:51 PM, Jeff Layton <[e-mail]> wrote:

> On Wed, 11 Feb 2009 13:05:32 -0600
> Shirish Pargaonkar <[e-mail]> wrote:
>
> > On Wed, Feb 11, 2009 at 10:41 AM, Steve French <[e-mail]>
> wrote:
> > >
> > >
> > > On Wed, Feb 11, 2009 at 7:08 AM, Jeff Layton <[e-mail]>
> wrote:
> > >>
> > >> 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.
> > >
> > > This is great, but wondering if the posix create code should be split
> out to
> > > a different function - calling out early from cifs_create to a new
> > > posix_create (when the server supports posix extensions) to reduce the
> size
> > > of this (now) 250+ line cifs_create function (as I had done in my
> earlier
> > > start on posix_create last month).
> > >
> > >
> > >
> > > --
> > > Thanks,
> > >
> > > Steve
> > >
> > > _______________________________________________
> > > linux-cifs-client mailing list
> > > [e-mail]
> > > https://lists.samba.org/mailman/listinfo/linux-cifs-client
> > >
> > >
> >
> >
> > Nitpicking here, I think it would be a good idea to carve out
> > posix_create function that cifs_create calls.
> > Then this posix_create function could also be called from cifs_lookup
> > when lookup intents are implemented.
> > And may be it should be called posix_open instead of posix_create, it
> > is all open anyway.
> >
>
> What's the advantage of doing the actual create on lookup rather than
> in the create operation? It seems like doing the actual file creation
> with the ->create op is a more logical scheme...
>
> --
> Jeff Layton <[e-mail]>
>

Jeff Layton Feb. 13, 2009, 2:44 p.m. UTC | #9

On Wed, 11 Feb 2009 11:44:26 -0500
Jeff Layton <[e-mail]> wrote:

> > This is great, but wondering if the posix create code should be split out to
> > a different function - calling out early from cifs_create to a new
> > posix_create (when the server supports posix extensions) to reduce the size
> > of this (now) 250+ line cifs_create function (as I had done in my earlier
> > start on posix_create last month).
> > 
> 
> Wouldn't hurt, though it may be a little while before I can get to it...
> 

I had a look at doing this and I'm not convinced that you're really
going gain much from trying to break the posix create code into a
separate function. There are a lot of things that will need to be
passed back and forth so the argument list is going to be pretty long...

The thing that *would* probably help would be to unify cifs_create and
cifs_open. i.e., turn them into wrappers around a common function. That
would also probably make it easier to call that function from the
lookup like you suggested. I think though, that that would be better
suited to a separate cleanup patch.

Would it be reasonable to take this patch as-is now, and look at
unifying those routines later?

Steve French Feb. 13, 2009, 8:56 p.m. UTC | #10

On Fri, Feb 13, 2009 at 8:44 AM, Jeff Layton <[e-mail]> wrote:

> On Wed, 11 Feb 2009 11:44:26 -0500
> Jeff Layton <[e-mail]> wrote:
>
> > > This is great, but wondering if the posix create code should be split
> out to
> > > a different function - calling out early from cifs_create to a new
> > > posix_create (when the server supports posix extensions) to reduce the
> size
> > > of this (now) 250+ line cifs_create function (as I had done in my
> earlier
> > > start on posix_create last month).
> > >
> >
> > Wouldn't hurt, though it may be a little while before I can get to it...
> >
>
> I had a look at doing this and I'm not convinced that you're really
> going gain much from trying to break the posix create code into a
> separate function. There are a lot of things that will need to be
> passed back and forth so the argument list is going to be pretty long...
>
> The thing that *would* probably help would be to unify cifs_create and
> cifs_open. i.e., turn them into wrappers around a common function. That
> would also probably make it easier to call that function from the
> lookup like you suggested. I think though, that that would be better
> suited to a separate cleanup patch.
>
> Would it be reasonable to take this patch as-is now, and look at
> unifying those routines later?
>
>
I have used some of your patch (testing it now), but changed a few things -
moved the posix_create call up to a distinct helper function and checked to
make sure that the server claimed support for posix extensions before
calling it (you were only checking if unix extensions were on).

Patch

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 446e62c..083dfc5 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -92,6 +92,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, __u64 *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..55eea50 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,
+						  &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 91d63f3..9c688af 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1052,7 +1052,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);