Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6

On Wed, 2009-07-01 at 06:01 -0500, Shirish Pargaonkar wrote:
> 
> I think the problem is, why EEXIST since file does not exist.  The
> looping is I guess
> because mktemp then attempts to create another file and receives EEXIST and it
> goes on forever.
> So the basic issues is why EEXIST since mktemp is attempting a file
> which does not
> exist on the server.

The problem is that the lookup is returning a positive dentry since it
just created the file. This makes the VFS turn around and return -EEXIST
(see the second O_EXCL check in do_filp_open). So we do have to skip the
create on lookup for O_EXCL or it won't work.

I think what's needed is something like this patch. It seems to fix the
testcase for me. That said, this is not adequately regression tested and
should not be committed until it is.

On that note, I'd like to reiterate my earlier sentiment that all of
this create-on-lookup code was committed prematurely and should be
backed out until it's more fully baked.

Just my USD$.02...

-- 
Jeff Layton <[email protected]>

_______________________________________________
linux-cifs-client mailing list
[email protected]
https://lists.samba.org/mailman/listinfo/linux-cifs-client

>From 7edbbd8dfa17b7865a25c97f3b586fefbf2a73b3 Mon Sep 17 00:00:00 2001
From: Jeff Layton <[email protected]>
Date: Wed, 1 Jul 2009 07:12:03 -0400
Subject: [PATCH] cifs: fix regression with O_EXCL creates and optimize away lookup

Signed-off-by: Jeff Layton <[email protected]>
---
 fs/cifs/dir.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 7dc6b74..0ac4859 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -643,6 +643,15 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
 			}
 	}
 
+	/*
+	 * O_EXCL: optimize away the lookup, but don't hash the dentry. Let
+	 * the VFS handle the create.
+	 */
+	if (nd->flags & LOOKUP_EXCL) {
+		d_instantiate(direntry, NULL);
+		return 0;
+	}
+
 	/* can not grab the rename sem here since it would
 	deadlock in the cases (beginning of sys_rename itself)
 	in which we already have the sb rename sem */
-- 
1.6.0.6