[PATCH] CIFS: Do not overwrite aops elements

[Dave Kleikamp <shaggy@austin.ibm.com>]

On Fri, 2006-05-12 at 04:20 -0700, Andrew Morton wrote:
> Christoph Hellwig <hch@lst.de> wrote:
> >
> > Same as with already do with the file operations:
> >  keep them in .rodata and prevents people from doing runtime patching.
> 
> It would occasionally be nice to be the second person to compile something.
> 
> fs/cifs/inode.c: In function `cifs_get_inode_info_unix':
> fs/cifs/inode.c:187: warning: assignment of read-only member `readpages'
> fs/cifs/inode.c: In function `cifs_get_inode_info':
> fs/cifs/inode.c:525: warning: assignment of read-only member `readpages'
> 
> Steve, you should have separate sets of a_ops and select the appropriate
> one for inode->i_data.a_ops, rather than scribbling on cifs_addr_ops.

This patch has been compile-tested only.

cifs should not be overwriting an element of the aops structure, since
the structure is shared by all cifs inodes.  Instead define a separate
aops structure to suit each purpose.

I also took the liberty of replacing a hard-coded 4096 with PAGE_CACHE_SIZE

Signed-off-by: Dave Kleikamp <shaggy@austin.ibm.com>

diff -urp linux-2.6.17-rc4/fs/cifs/cifsfs.h linux/fs/cifs/cifsfs.h
--- linux-2.6.17-rc4/fs/cifs/cifsfs.h	2006-05-12 12:46:27.000000000 -0500
+++ linux/fs/cifs/cifsfs.h	2006-05-12 12:49:45.000000000 -0500
@@ -33,6 +33,7 @@
 #endif
 
 extern const struct address_space_operations cifs_addr_ops;
+extern const struct address_space_operations cifs_addr_ops_smallbuf;
 
 /* Functions related to super block operations */
 extern struct super_operations cifs_super_ops;
diff -urp linux-2.6.17-rc4/fs/cifs/file.c linux/fs/cifs/file.c
--- linux-2.6.17-rc4/fs/cifs/file.c	2006-05-12 12:46:27.000000000 -0500
+++ linux/fs/cifs/file.c	2006-05-12 12:50:34.000000000 -0500
@@ -1955,3 +1955,19 @@ const struct address_space_operations ci
 	/* .sync_page = cifs_sync_page, */
 	/* .direct_IO = */
 };
+
+/*
+ * cifs_readpages requires the server to support a buffer large enough to
+ * contain the header plus one complete page of data.  Otherwise, we need
+ * to leave cifs_readpages out of the address space operations.
+ */
+const struct address_space_operations cifs_addr_ops_smallbuf = {
+	.readpage = cifs_readpage,
+	.writepage = cifs_writepage,
+	.writepages = cifs_writepages,
+	.prepare_write = cifs_prepare_write,
+	.commit_write = cifs_commit_write,
+	.set_page_dirty = __set_page_dirty_nobuffers,
+	/* .sync_page = cifs_sync_page, */
+	/* .direct_IO = */
+};
diff -urp linux-2.6.17-rc4/fs/cifs/inode.c linux/fs/cifs/inode.c
--- linux-2.6.17-rc4/fs/cifs/inode.c	2006-05-12 10:31:20.000000000 -0500
+++ linux/fs/cifs/inode.c	2006-05-12 12:52:08.000000000 -0500
@@ -180,11 +180,12 @@ int cifs_get_inode_info_unix(struct inod
 			else /* not direct, send byte range locks */ 
 				inode->i_fop = &cifs_file_ops;
 
-			inode->i_data.a_ops = &cifs_addr_ops;
 			/* check if server can support readpages */
 			if(pTcon->ses->server->maxBuf < 
-			    4096 + MAX_CIFS_HDR_SIZE)
-				inode->i_data.a_ops->readpages = NULL;
+			    PAGE_CACHE_SIZE + MAX_CIFS_HDR_SIZE)
+				inode->i_data.a_ops = &cifs_addr_ops_smallbuf;
+			else
+				inode->i_data.a_ops = &cifs_addr_ops;
 		} else if (S_ISDIR(inode->i_mode)) {
 			cFYI(1, ("Directory inode"));
 			inode->i_op = &cifs_dir_inode_ops;
@@ -519,10 +520,11 @@ int cifs_get_inode_info(struct inode **p
 			else /* not direct, send byte range locks */
 				inode->i_fop = &cifs_file_ops;
 
-			inode->i_data.a_ops = &cifs_addr_ops;
 			if(pTcon->ses->server->maxBuf < 
-			     4096 + MAX_CIFS_HDR_SIZE)
-				inode->i_data.a_ops->readpages = NULL;
+			     PAGE_CACHE_SIZE + MAX_CIFS_HDR_SIZE)
+				inode->i_data.a_ops = &cifs_addr_ops_smallbuf;
+			else
+				inode->i_data.a_ops = &cifs_addr_ops;
 		} else if (S_ISDIR(inode->i_mode)) {
 			cFYI(1, ("Directory inode"));
 			inode->i_op = &cifs_dir_inode_ops;
diff -urp linux-2.6.17-rc4/fs/cifs/readdir.c linux/fs/cifs/readdir.c
--- linux-2.6.17-rc4/fs/cifs/readdir.c	2006-05-12 10:31:20.000000000 -0500
+++ linux/fs/cifs/readdir.c	2006-05-12 13:11:47.000000000 -0500
@@ -21,6 +21,7 @@
  *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  */
 #include <linux/fs.h>
+#include <linux/pagemap.h>
 #include <linux/stat.h>
 #include <linux/smp_lock.h>
 #include "cifspdu.h"
@@ -215,11 +216,13 @@ static void fill_in_inode(struct inode *
 		else
 			tmp_inode->i_fop = &cifs_file_ops;
 
-		tmp_inode->i_data.a_ops = &cifs_addr_ops;
 		if((cifs_sb->tcon) && (cifs_sb->tcon->ses) &&
 		   (cifs_sb->tcon->ses->server->maxBuf <
-			4096 + MAX_CIFS_HDR_SIZE))
-			tmp_inode->i_data.a_ops->readpages = NULL;
+			PAGE_CACHE_SIZE + MAX_CIFS_HDR_SIZE))
+			tmp_inode->i_data.a_ops = &cifs_addr_ops_smallbuf;
+		else
+			tmp_inode->i_data.a_ops = &cifs_addr_ops;
+
 		if(isNewInode)
 			return; /* No sense invalidating pages for new inode
 				   since have not started caching readahead file
@@ -338,11 +341,12 @@ static void unix_fill_in_inode(struct in
 		else
 			tmp_inode->i_fop = &cifs_file_ops;
 
-		tmp_inode->i_data.a_ops = &cifs_addr_ops;
 		if((cifs_sb->tcon) && (cifs_sb->tcon->ses) &&
 		   (cifs_sb->tcon->ses->server->maxBuf < 
-			4096 + MAX_CIFS_HDR_SIZE))
-			tmp_inode->i_data.a_ops->readpages = NULL;
+			PAGE_CACHE_SIZE + MAX_CIFS_HDR_SIZE))
+			tmp_inode->i_data.a_ops = &cifs_addr_ops_smallbuf;
+		else
+			tmp_inode->i_data.a_ops = &cifs_addr_ops;
 
 		if(isNewInode)
 			return; /* No sense invalidating pages for new inode since we

-- 
David Kleikamp
IBM Linux Technology Center