Re: lookup intent patch

[Shirish Pargaonkar <shirishpargaonkar@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3392 bytes --]

On Thu, Mar 19, 2009 at 3:36 PM, Shirish Pargaonkar
<shirishpargaonkar@gmail.com> wrote:
> On Fri, Feb 27, 2009 at 2:34 PM, Shirish Pargaonkar
> <shirishpargaonkar@gmail.com> wrote:
>> While there are pending issues related to broken Samba server posix
>> code/behaviour,
>> I thought I will run by you the patch for lookup intent nonetheless.
>>
>> It worked on 2.6.29-rc3 kernel with some of the testing I have done, I
>> just downloaded
>> the git sources from Steve's tree and ported the patch, that is what this is.
>> I am building those sources and should boot with that kernel and test
>> the patch and post
>> the results here.
>>
>> Regards,
>>
>> Shirish
>>
>
> Here is the second version of the lookup intent patch that takes into
> consideration the samba posix open bug.
>
> I ran modified connectathon test1 to create 5000 files.
> I am not sure whether existing code when does not send SET_PATH_INFO
> with info level
> SMB_SET_FILE_END_OF_FILE_INFO2  for O_TRUNC flag to a file open/creat
> is a bug or not,
> looking into it.
>
>
> With the cthon test1 test, the files are created using
> creat(<file_name>, 0666) meaning flags of
> O_WRONLY, O_CREAT, O_TRUNC.
>
> With the current code, the O_TRUNC is not honoured i.e. I do not
> see SMB_SET_FILE_END_OF_FILE_INFO2 info level going over the wire,
> hence the faster creation of the files.
>
> I changed the code in test1.c by changing the call
>  creat(name, CHMOD_RW)
> to
>  open(name, O_WRONLY | O_CREAT, CHMOD_RW)
> and I can see open with lookup intent is definitely faster (28%).
>
>
>
> ---------------------- lookup intent and cifs_posix_open with creat
> --------------------
>
> + date
> Thu Mar 19 11:26:03 CDT 2009
> + ./test1 -f
> ./test1: File and directory creation test
>  test1 files 2  created 5000 files 1 directories 1 levels deep
>        ./test1 ok.
> + date
> Thu Mar 19 11:26:29 CDT 2009
>
>
> SMB_POSIX_OPEN
> SMB_SET_FILE_END_OF_FILE_INFO2
> SMB_SET_FILE_UNIX_BASIC
>
>
> ---------------------- no lookup intent and cifs_posix_open with creat
> ------------------
>
> + date
> Thu Mar 19 11:28:43 CDT 2009
> + ./test1 -f
>
> ./test1: File and directory creation test
>  test1 files 2  created 5000 files 1 directories 1 levels deep
>        ./test1 ok.
> + date
> Thu Mar 19 11:29:01 CDT 2009
>
> SMB_POSIX_OPEN
>
> -----------------------------------------------------------------------------
>
>
>
>
> ---------------------- lookup intent and cifs_posix_open with open
> --------------------
>
> + date
> Thu Mar 19 15:19:32 CDT 2009
> + ./test1 -f
> ./test1: File and directory creation test
>  test1 files 2  created 5000 files 0 directories 1 levels deep
>        ./test1 ok.
> + date
> Thu Mar 19 15:19:45 CDT 2009
>
>
> ---------------------- no lookup intent and cifs_posix_open with open
> ------------------
>
> + date
> Thu Mar 19 15:21:07 CDT 2009
> + ./test1 -f
> ./test1: File and directory creation test
>  test1 files 2  created 5000 files 0 directories 1 levels deep
>        ./test1 ok.
> + date
> Thu Mar 19 15:21:25 CDT 2009
>
>
> -----------------------------------------------------------------------------
>

Here is the third iteration of the lookup intent patch for cifs, this
time mode is set with taking umaks into consideration.

Regards,

Shirish

[-- Attachment #2: li.3.patch --]
[-- Type: application/octet-stream, Size: 7967 bytes --]

diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index f9b6f68..011d0ac 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -129,12 +129,67 @@ cifs_bp_rename_retry:
 	return full_path;
 }
 
+static void
+cifs_fill_fileinfo(struct inode *newinode, __u16 fileHandle, 
+			struct cifsTconInfo *tcon, bool write_only)
+{
+	int oplock = 0;
+	struct cifsFileInfo *pCifsFile;
+	struct cifsInodeInfo *pCifsInode;
+
+	cERROR(1, ("cifs_fill_fileinfo enter\n"));
+
+	pCifsFile = kzalloc(sizeof(struct cifsFileInfo), GFP_KERNEL);
+
+	if (pCifsFile == NULL)
+		return;
+
+	if (oplockEnabled)
+		oplock = REQ_OPLOCK;
+
+	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);
+	cERROR(1, ("cifs_fill_fileinfo exit\n"));
+}
+
 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;
+	bool write_only = false;
 	FILE_UNIX_BASIC_INFO *presp_data;
 	__u32 posix_flags = 0;
 	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
@@ -172,6 +227,8 @@ int cifs_posix_open(char *full_path, struct inode **pinode,
 	if (oflags & O_DIRECT)
 		posix_flags |= SMB_O_DIRECT;
 
+        if (!(oflags & FMODE_READ))
+                write_only = true;
 
 	rc = CIFSPOSIXCreate(xid, cifs_sb->tcon, posix_flags, mode,
 			pnetfid, presp_data, &oplock, full_path,
@@ -198,6 +255,8 @@ int cifs_posix_open(char *full_path, struct inode **pinode,
 
 	posix_fill_in_inode(*pinode, presp_data, 1);
 
+	cifs_fill_fileinfo(*pinode, *pnetfid, cifs_sb->tcon, write_only);
+
 posix_open_ret:
 	kfree(presp_data);
 	return rc;
@@ -239,7 +298,6 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
 	char *full_path = NULL;
 	FILE_ALL_INFO *buf = NULL;
 	struct inode *newinode = NULL;
-	struct cifsInodeInfo *pCifsInode;
 	int disposition = FILE_OVERWRITE_IF;
 	bool write_only = false;
 
@@ -410,44 +468,8 @@ cifs_create_set_dentry:
 		/* 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);
+			cifs_fill_fileinfo(newinode, fileHandle,
+					cifs_sb->tcon, write_only);
 	}
 cifs_create_out:
 	kfree(buf);
@@ -580,13 +602,16 @@ int cifs_mknod(struct inode *inode, struct dentry *direntry, int mode,
 	return rc;
 }
 
-
 struct dentry *
 cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
 	    struct nameidata *nd)
 {
 	int xid;
 	int rc = 0; /* to get around spurious gcc warning, set to zero here */
+	int oplock = 0;
+	int mode;
+	__u16 fileHandle = 0;
+	bool posix_open = false;
 	struct cifs_sb_info *cifs_sb;
 	struct cifsTconInfo *pTcon;
 	struct inode *newInode = NULL;
@@ -632,12 +657,26 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
 	}
 	cFYI(1, ("Full path: %s inode = 0x%p", full_path, direntry->d_inode));
 
-	if (pTcon->unix_ext)
-		rc = cifs_get_inode_info_unix(&newInode, full_path,
-					      parent_dir_inode->i_sb, xid);
-	else
+	if (pTcon->unix_ext) {
+		if (!(nd->flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY)) &&
+				(nd->flags & LOOKUP_OPEN)) {
+			if (!((nd->intent.open.flags & O_CREAT ) &&
+					(nd->intent.open.flags & O_EXCL))) {
+				mode = nd->intent.open.create_mode &
+						~current->fs->umask;
+				rc = cifs_posix_open(full_path, &newInode,
+					parent_dir_inode->i_sb, mode,
+					nd->intent.open.flags, &oplock,
+					&fileHandle, xid);
+				posix_open = true;
+			}
+		}
+		if (!posix_open || ((rc == -EINVAL) || (rc == -EOPNOTSUPP)))
+			rc = cifs_get_inode_info_unix(&newInode, full_path,
+		 				parent_dir_inode->i_sb, xid);
+	} else
 		rc = cifs_get_inode_info(&newInode, full_path, NULL,
-					 parent_dir_inode->i_sb, xid, NULL);
+				parent_dir_inode->i_sb, xid, NULL);
 
 	if ((rc == 0) && (newInode != NULL)) {
 		if (pTcon->nocase)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 81747ac..4a7c886 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -284,35 +284,34 @@ int cifs_open(struct inode *inode, struct file *file)
 	cifs_sb = CIFS_SB(inode->i_sb);
 	tcon = cifs_sb->tcon;
 
-	if (file->f_flags & O_CREAT) {
-		/* search inode for this file and fill in file->private_data */
-		pCifsInode = CIFS_I(file->f_path.dentry->d_inode);
-		read_lock(&GlobalSMBSeslock);
-		list_for_each(tmp, &pCifsInode->openFileList) {
-			pCifsFile = list_entry(tmp, struct cifsFileInfo,
-					       flist);
-			if ((pCifsFile->pfile == NULL) &&
-			    (pCifsFile->pid == current->tgid)) {
-				/* mode set in cifs_create */
-
-				/* needed for writepage */
-				pCifsFile->pfile = file;
-
-				file->private_data = pCifsFile;
-				break;
-			}
-		}
-		read_unlock(&GlobalSMBSeslock);
-		if (file->private_data != NULL) {
-			rc = 0;
-			FreeXid(xid);
-			return rc;
-		} else {
-			if (file->f_flags & O_EXCL)
-				cERROR(1, ("could not find file instance for "
-					   "new file %p", file));
+	/* search inode for this file and fill in file->private_data */
+	pCifsInode = CIFS_I(file->f_path.dentry->d_inode);
+	read_lock(&GlobalSMBSeslock);
+	list_for_each(tmp, &pCifsInode->openFileList) {
+		pCifsFile = list_entry(tmp, struct cifsFileInfo,
+				       flist);
+		if ((pCifsFile->pfile == NULL) &&
+		    (pCifsFile->pid == current->tgid)) {
+			/* mode set in cifs_create */
+
+			/* needed for writepage */
+			pCifsFile->pfile = file;
+
+			file->private_data = pCifsFile;
+			break;
 		}
 	}
+	read_unlock(&GlobalSMBSeslock);
+
+	if (file->private_data != NULL) {
+		rc = 0;
+		FreeXid(xid);
+		return rc;
+	} else {
+		if ((file->f_flags & O_CREAT) && (file->f_flags & O_EXCL))
+			cERROR(1, ("could not find file instance for "
+				   "new file %p", file));
+	}
 
 	full_path = build_path_from_dentry(file->f_path.dentry);
 	if (full_path == NULL) {