linux-next: manual merge of the vfs tree with the cifs tree

linux-next: manual merge of the vfs tree with the cifs tree

[Stephen Rothwell]

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

Hi all,

Today's linux-next merge of the vfs tree got a conflict in:

  fs/cifs/file.c

between commit:

  4c7b707d72f5 ("cifs: revalidate mapping when we open files for SMB1 POSIX")

from the cifs tree and commit:

  4d66952a2032 ("cifs: have cifs_fattr_to_inode() refuse to change type on live inode")

from the vfs tree.

I fixed it up (I thknk - see below) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc fs/cifs/file.c
index 042e24aad410,78266f0e0595..000000000000
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@@ -165,8 -165,7 +165,8 @@@ int cifs_posix_open(char *full_path, st
  			goto posix_open_ret;
  		}
  	} else {
 +		cifs_revalidate_mapping(*pinode);
- 		cifs_fattr_to_inode(*pinode, &fattr);
+ 		rc = cifs_fattr_to_inode(*pinode, &fattr);
  	}
  
  posix_open_ret:

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

linux-next: manual merge of the vfs tree with the cifs tree

[Stephen Rothwell]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in:

  fs/cifs/cifsfs.c

between commit:

  56c1d814aadf ("CIFS: Remove some obsolete comments")

from the cifs tree and commit:

  51085a1f913a ("cifs: use C99 syntax for inode_operations initializer")

from the vfs tree.

This was entirely expected.  Al, if its easy, you could just drop that
patch from the vfs tree.

I fixed it up (I just used the version from the cifs tree) and can
carry the fix as necessary. This is now fixed as far as linux-next is
concerned, but any non trivial conflicts should be mentioned to your
upstream maintainer when your tree is submitted for merging.  You may
also want to consider cooperating with the maintainer of the conflicting
tree to minimise any particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

Re: linux-next: manual merge of the vfs tree with the cifs tree

[Jeff Layton]

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

On Tue, 13 May 2014 13:16:16 +1000
Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi Al,
> 
> Today's linux-next merge of the vfs tree got a conflict in
> fs/cifs/cifsfs.c between commit 485c4e72d0f1 ("cifs: revalidate mapping
> prior to satisfying aio_read request") from the cifs tree and commits
> aad4f8bb42af ("switch simple generic_file_aio_read() users to
> ->read_iter()") and 3dae8750c368 ("cifs: switch to ->write_iter()")
> from the vfs tree.
> 
> I fixed it up (I dropped the cifs commit, so this will need a better
> fix ...) and can carry the fix as necessary.
> 

Thanks Stephen,

Steve, what we probably need to do is replace 485c4e72d0f1 with this
patch. Only tested for compilation so far, but it should do the right
thing. I'll resend to the list once I've had a chance to test it.

--------------------------[snip]-------------------------

[PATCH] cifs: revalidate mapping prior to satisfying read_iter request

Before satisfying a read with cache=loose, we should always check
that the pagecache is valid.

Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Jeff Layton <jlayton@poochiereds.net>
---
 fs/cifs/cifsfs.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 078c171c4f1b..149b09ce148d 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -729,6 +729,19 @@ out_nls:
 	goto out;
 }
 
+static ssize_t
+cifs_loose_read_iter(struct kiocb *iocb, struct iov_iter *to)
+{
+	ssize_t rc;
+	struct inode *inode = file_inode(iocb->ki_filp);
+
+	rc = cifs_revalidate_mapping(inode);
+	if (rc)
+		return rc;
+
+	return generic_file_read_iter(iocb, to);
+}
+
 static ssize_t cifs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 {
 	struct inode *inode = file_inode(iocb->ki_filp);
@@ -885,7 +898,7 @@ const struct inode_operations cifs_symlink_inode_ops = {
 const struct file_operations cifs_file_ops = {
 	.read = new_sync_read,
 	.write = new_sync_write,
-	.read_iter = generic_file_read_iter,
+	.read_iter = cifs_loose_read_iter,
 	.write_iter = cifs_file_write_iter,
 	.open = cifs_open,
 	.release = cifs_close,
@@ -943,7 +956,7 @@ const struct file_operations cifs_file_direct_ops = {
 const struct file_operations cifs_file_nobrl_ops = {
 	.read = new_sync_read,
 	.write = new_sync_write,
-	.read_iter = generic_file_read_iter,
+	.read_iter = cifs_loose_read_iter,
 	.write_iter = cifs_file_write_iter,
 	.open = cifs_open,
 	.release = cifs_close,
-- 
1.9.0




[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

linux-next: manual merge of the vfs tree with the cifs tree

[Stephen Rothwell]

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

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in
fs/cifs/cifsfs.c between commit 485c4e72d0f1 ("cifs: revalidate mapping
prior to satisfying aio_read request") from the cifs tree and commits
aad4f8bb42af ("switch simple generic_file_aio_read() users to
->read_iter()") and 3dae8750c368 ("cifs: switch to ->write_iter()")
from the vfs tree.

I fixed it up (I dropped the cifs commit, so this will need a better
fix ...) and can carry the fix as necessary.

-- 
Cheers,
Stephen Rothwell                    sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

linux-next: manual merge of the vfs tree with the cifs tree

[Stephen Rothwell]

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

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in fs/cifs/dir.c
between commit 76023fd85bfd ("CIFS: Rename Get/FreeXid and make them work
with unsigned int") from the cifs tree and commit 070ac6cd4b0a ("cifs:
implement i_op->atomic_open()") from the vfs tree.

I fixed it up (see below) and can carry the fix as necessary.

P.S. Steve. that cifs tree commit has a bad email address for you in your
Signed-off-by line and committer (Steven French <sfrench@w500smf.(none)>).
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/cifs/dir.c
index 9d73354,a180265..0000000
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@@ -133,29 -133,40 +133,40 @@@ cifs_bp_rename_retry
  	return full_path;
  }
  
+ /*
+  * Don't allow the separator character in a path component.
+  * The VFS will not allow "/", but "\" is allowed by posix.
+  */
+ static int
+ check_name(struct dentry *direntry)
+ {
+ 	struct cifs_sb_info *cifs_sb = CIFS_SB(direntry->d_sb);
+ 	int i;
+ 
+ 	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIX_PATHS)) {
+ 		for (i = 0; i < direntry->d_name.len; i++) {
+ 			if (direntry->d_name.name[i] == '\\') {
+ 				cFYI(1, "Invalid file name");
+ 				return -EINVAL;
+ 			}
+ 		}
+ 	}
+ 	return 0;
+ }
+ 
+ 
  /* Inode operations in similar order to how they appear in Linux file fs.h */
  
- int
- cifs_create(struct inode *inode, struct dentry *direntry, umode_t mode,
- 		struct nameidata *nd)
+ static int cifs_do_create(struct inode *inode, struct dentry *direntry,
 -			  int xid, struct tcon_link *tlink, unsigned oflags,
++			  unsigned int xid, struct tcon_link *tlink, unsigned oflags,
+ 			  umode_t mode, __u32 *oplock, __u16 *fileHandle,
+ 			  int *created)
  {
  	int rc = -ENOENT;
- 	unsigned int xid;
  	int create_options = CREATE_NOT_DIR;
- 	__u32 oplock = 0;
- 	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;
- 	struct tcon_link *tlink;
- 	struct cifs_tcon *tcon;
+ 	int desiredAccess;
+ 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+ 	struct cifs_tcon *tcon = tlink_tcon(tlink);
  	char *full_path = NULL;
  	FILE_ALL_INFO *buf = NULL;
  	struct inode *newinode = NULL;
@@@ -321,37 -355,136 +355,136 @@@ cifs_create_get_file_info
  	}
  
  cifs_create_set_dentry:
- 	if (rc == 0)
- 		d_instantiate(direntry, newinode);
- 	else
+ 	if (rc != 0) {
  		cFYI(1, "Create worked, get_inode_info failed rc = %d", rc);
+ 		goto out;
+ 	}
+ 	d_drop(direntry);
+ 	d_add(direntry, newinode);
  
- 	if (newinode && nd) {
- 		struct cifsFileInfo *pfile_info;
- 		struct file *filp;
+ 	/* ENOENT for create?  How weird... */
+ 	rc = -ENOENT;
+ 	if (!newinode) {
+ 		CIFSSMBClose(xid, tcon, *fileHandle);
+ 		goto out;
+ 	}
+ 	rc = 0;
  
- 		filp = lookup_instantiate_filp(nd, direntry, generic_file_open);
- 		if (IS_ERR(filp)) {
- 			rc = PTR_ERR(filp);
- 			CIFSSMBClose(xid, tcon, fileHandle);
- 			goto cifs_create_out;
- 		}
+ out:
+ 	kfree(buf);
+ 	kfree(full_path);
+ 	return rc;
+ }
  
- 		pfile_info = cifs_new_fileinfo(fileHandle, filp, tlink, oplock);
- 		if (pfile_info == NULL) {
- 			fput(filp);
- 			CIFSSMBClose(xid, tcon, fileHandle);
- 			rc = -ENOMEM;
- 		}
- 	} else {
+ int
+ cifs_atomic_open(struct inode *inode, struct dentry *direntry,
+ 		 struct file *file, unsigned oflags, umode_t mode,
+ 		 int *opened)
+ {
+ 	int rc;
 -	int xid;
++	unsigned int xid;
+ 	struct tcon_link *tlink;
+ 	struct cifs_tcon *tcon;
+ 	__u16 fileHandle;
+ 	__u32 oplock;
+ 	struct file *filp;
+ 	struct cifsFileInfo *pfile_info;
+ 
+ 	/* Posix open is only called (at lookup time) for file create now.  For
+ 	 * opens (rather than creates), because we do not know if it is a file
+ 	 * or directory yet, and current Samba no longer allows us to do posix
+ 	 * open on dirs, we could end up wasting an open call on what turns out
+ 	 * to be a dir. For file opens, we wait to call posix open till
+ 	 * cifs_open.  It could be added to atomic_open in the future but the
+ 	 * performance tradeoff of the extra network request when EISDIR or
+ 	 * EACCES is returned would have to be weighed against the 50% reduction
+ 	 * in network traffic in the other paths.
+ 	 */
+ 	if (!(oflags & O_CREAT)) {
+ 		struct dentry *res = cifs_lookup(inode, direntry, 0);
+ 		if (IS_ERR(res))
+ 			return PTR_ERR(res);
+ 
+ 		return finish_no_open(file, res);
+ 	}
+ 
+ 	rc = check_name(direntry);
+ 	if (rc)
+ 		return rc;
+ 
 -	xid = GetXid();
++	xid = get_xid();
+ 
+ 	cFYI(1, "parent inode = 0x%p name is: %s and dentry = 0x%p",
+ 	     inode, direntry->d_name.name, direntry);
+ 
+ 	tlink = cifs_sb_tlink(CIFS_SB(inode->i_sb));
+ 	filp = ERR_CAST(tlink);
+ 	if (IS_ERR(tlink))
+ 		goto free_xid;
+ 
+ 	tcon = tlink_tcon(tlink);
+ 
+ 	rc = cifs_do_create(inode, direntry, xid, tlink, oflags, mode,
+ 			    &oplock, &fileHandle, opened);
+ 
+ 	if (rc)
+ 		goto out;
+ 
+ 	rc = finish_open(file, direntry, generic_file_open, opened);
+ 	if (rc) {
  		CIFSSMBClose(xid, tcon, fileHandle);
+ 		goto out;
  	}
  
- cifs_create_out:
- 	kfree(buf);
- 	kfree(full_path);
+ 	pfile_info = cifs_new_fileinfo(fileHandle, filp, tlink, oplock);
+ 	if (pfile_info == NULL) {
+ 		CIFSSMBClose(xid, tcon, fileHandle);
+ 		fput(filp);
+ 		rc = -ENOMEM;
+ 	}
+ 
+ out:
+ 	cifs_put_tlink(tlink);
+ free_xid:
 -	FreeXid(xid);
++	free_xid(xid);
+ 	return rc;
+ }
+ 
+ int cifs_create(struct inode *inode, struct dentry *direntry, umode_t mode,
+ 		bool excl)
+ {
+ 	int rc;
 -	int xid = GetXid();
++	int xid = get_xid();
+ 	/*
+ 	 * 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.
+ 	 */
+ 	unsigned oflags = O_EXCL | O_CREAT | O_RDWR;
+ 	struct tcon_link *tlink;
+ 	__u16 fileHandle;
+ 	__u32 oplock;
+ 	int created = FILE_CREATED;
+ 
+ 	cFYI(1, "cifs_create parent inode = 0x%p name is: %s and dentry = 0x%p",
+ 	     inode, direntry->d_name.name, direntry);
+ 
+ 	tlink = cifs_sb_tlink(CIFS_SB(inode->i_sb));
+ 	rc = PTR_ERR(tlink);
+ 	if (IS_ERR(tlink))
+ 		goto free_xid;
+ 
+ 	rc = cifs_do_create(inode, direntry, xid, tlink, oflags, mode,
+ 			    &oplock, &fileHandle, &created);
+ 	if (!rc)
+ 		CIFSSMBClose(xid, tlink_tcon(tlink), fileHandle);
+ 
  	cifs_put_tlink(tlink);
+ free_xid:
 -	FreeXid(xid);
 +	free_xid(xid);
+ 
  	return rc;
  }
  
@@@ -488,22 -621,17 +621,17 @@@ mknod_out
  
  struct dentry *
  cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
- 	    struct nameidata *nd)
+ 	    unsigned int flags)
  {
 -	int xid;
 +	unsigned int xid;
  	int rc = 0; /* to get around spurious gcc warning, set to zero here */
  	struct cifs_sb_info *cifs_sb;
  	struct tcon_link *tlink;
  	struct cifs_tcon *pTcon;
- 	struct cifsFileInfo *cfile;
  	struct inode *newInode = NULL;
  	char *full_path = NULL;
- 	struct file *filp;
  
 -	xid = GetXid();
 +	xid = get_xid();
  
  	cFYI(1, "parent inode = 0x%p name is: %s and dentry = 0x%p",
  	      parent_dir_inode, direntry->d_name.name, direntry);

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]