[PATCH] kill ->i_op->revalidate()

kill ->i_op->revalidate()

Documentation/filesystems/Locking

@@ -42,9 +42,8 @@ prototypes:
 	int (*follow_link) (struct dentry *, struct nameidata *);
 	void (*truncate) (struct inode *);
 	int (*permission) (struct inode *, int);
-	int (*revalidate) (struct dentry *);
 	int (*setattr) (struct dentry *, struct iattr *);
-	int (*getattr) (struct dentry *, struct iattr *);
+	int (*getattr) (struct vfsmount *, struct dentry *, struct kstat *);
 	int (*setxattr) (struct dentry *, const char *, void *, size_t, int);
 	ssize_t (*getxattr) (struct dentry *, const char *, void *, size_t);
 	ssize_t (*listxattr) (struct dentry *, char *, size_t);
@@ -67,8 +66,7 @@ follow_link:	no	no
 truncate:	no	yes		(see below)
 setattr:	no	yes
 permission:	yes	no
-getattr:				(see below)
-revalidate:	no			(see below)
+getattr:	no	no
 setxattr:	no	yes
 getxattr:	no	yes
 listxattr:	no	yes
@@ -76,14 +74,11 @@ removexattr:	no	yes
 	Additionally, ->rmdir(), ->unlink() and ->rename() have ->i_sem on
 victim.
 	cross-directory ->rename() has (per-superblock) ->s_vfs_rename_sem.
-	->revalidate(), it may be called both with and without the i_sem
-on dentry->d_inode.
 	->truncate() is never called directly - it's a callback, not a
 method. It's called by vmtruncate() - library function normally used by
 ->setattr(). Locking information above applies to that call (i.e. is
 inherited from ->setattr() - vmtruncate() is used when ATTR_SIZE had been
 passed).
-	->getattr() is currently unused.
 
 See Documentation/filesystems/directory-locking for more detailed discussion
 of the locking scheme for directory operations.

Documentation/filesystems/porting

@@ -187,3 +187,14 @@ e.g.
                unlock_new_inode(inode);
        }
 
+---
+[recommended]
+
+->getattr() finally getting used.  See instances in nfs, minix, etc.
+
+---
+[mandatory]
+
+->revalidate() is gone.  If your filesystem had it - provide ->getattr()
+and let it call whatever you had as ->revlidate() + (for symlinks that
+had ->revalidate()) add calls in ->follow_link()/->readlink().

fs/bad_inode.c

@@ -60,7 +60,6 @@ struct inode_operations bad_inode_ops =
 	follow_link:	bad_follow_link,
 	truncate:	EIO_ERROR,
 	permission:	EIO_ERROR,
-	revalidate:	EIO_ERROR,
 	getattr:	EIO_ERROR,
 };
 

fs/coda/cnode.c

@@ -28,7 +28,7 @@ inline int coda_isnullfid(ViceFid *fid)
 static struct inode_operations coda_symlink_inode_operations = {
 	readlink:	page_readlink,
 	follow_link:	page_follow_link,
-	setattr:	coda_notify_change,
+	setattr:	coda_setattr,
 };
 
 /* cnode.c */

fs/coda/dir.c

@@ -72,8 +72,8 @@ struct inode_operations coda_dir_inode_operations =
 	mknod:		coda_mknod,
 	rename:		coda_rename,
 	permission:	coda_permission,
-        revalidate:	coda_revalidate_inode,
-	setattr:	coda_notify_change,
+        getattr:	coda_getattr,
+	setattr:	coda_setattr,
 };
 
 struct file_operations coda_dir_operations = {
@@ -147,7 +147,6 @@ static struct dentry *coda_lookup(struct inode *dir, struct dentry *entry)
 
 int coda_permission(struct inode *inode, int mask)
 {
-	umode_t mode = inode->i_mode;
         int error;
  
 	if (!mask)

fs/coda/inode.c

@@ -237,7 +237,15 @@ static void coda_clear_inode(struct inode *inode)
 	coda_cache_clear_inode(inode);
 }
 
-int coda_notify_change(struct dentry *de, struct iattr *iattr)
+int coda_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
+{
+	int err = coda_revalidate_inode(dentry);
+	if (!err)
+		generic_fillattr(dentry->d_inode, stat);
+	return err;
+}
+
+int coda_setattr(struct dentry *de, struct iattr *iattr)
 {
 	struct inode *inode = de->d_inode;
         struct coda_vattr vattr;
@@ -266,8 +274,8 @@ int coda_notify_change(struct dentry *de, struct iattr *iattr)
 
 struct inode_operations coda_file_inode_operations = {
 	permission:	coda_permission,
-	revalidate:	coda_revalidate_inode,
-	setattr:	coda_notify_change,
+	getattr:	coda_getattr,
+	setattr:	coda_setattr,
 };
 
 static int coda_statfs(struct super_block *sb, struct statfs *buf)

fs/coda/pioctl.c

@@ -32,7 +32,7 @@ static int coda_pioctl(struct inode * inode, struct file * filp,
 struct inode_operations coda_ioctl_inode_operations =
 {
 	permission:	coda_ioctl_permission,
-	setattr:	coda_notify_change,
+	setattr:	coda_setattr,
 };
 
 struct file_operations coda_ioctl_operations = {

fs/nfs/dir.c

@@ -64,8 +64,8 @@ struct inode_operations nfs_dir_inode_operations = {
 	mknod:		nfs_mknod,
 	rename:		nfs_rename,
 	permission:	nfs_permission,
-	revalidate:	nfs_revalidate,
-	setattr:	nfs_notify_change,
+	getattr:	nfs_getattr,
+	setattr:	nfs_setattr,
 };
 
 typedef u32 * (*decode_dirent_t)(u32 *, struct nfs_entry *, int);
@@ -357,7 +357,7 @@ static int nfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
 
 	lock_kernel();
 
-	res = nfs_revalidate(dentry);
+	res = nfs_revalidate_inode(NFS_SERVER(inode), inode);
 	if (res < 0) {
 		unlock_kernel();
 		return res;

fs/nfs/file.c

@@ -54,8 +54,8 @@ struct file_operations nfs_file_operations = {
 
 struct inode_operations nfs_file_inode_operations = {
 	permission:	nfs_permission,
-	revalidate:	nfs_revalidate,
-	setattr:	nfs_notify_change,
+	getattr:	nfs_getattr,
+	setattr:	nfs_setattr,
 };
 
 /* Hack for future NFS swap support */

fs/nfs/inode.c

@@ -730,7 +730,7 @@ __nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
 }
 
 int
-nfs_notify_change(struct dentry *dentry, struct iattr *attr)
+nfs_setattr(struct dentry *dentry, struct iattr *attr)
 {
 	struct inode *inode = dentry->d_inode;
 	struct nfs_fattr fattr;
@@ -744,7 +744,7 @@ nfs_notify_change(struct dentry *dentry, struct iattr *attr)
 	error = nfs_revalidate_inode(NFS_SERVER(inode),inode);
 	if (error) {
 #ifdef NFS_PARANOIA
-printk("nfs_notify_change: revalidate failed, error=%d\n", error);
+printk("nfs_setattr: revalidate failed, error=%d\n", error);
 #endif
 		goto out;
 	}
@@ -768,7 +768,7 @@ printk("nfs_notify_change: revalidate failed, error=%d\n", error);
 	 */
 	if (attr->ia_valid & ATTR_SIZE) {
 		if (attr->ia_size != fattr.size)
-			printk("nfs_notify_change: attr=%Ld, fattr=%Ld??\n",
+			printk("nfs_setattr: attr=%Ld, fattr=%Ld??\n",
 			       (long long) attr->ia_size, (long long)fattr.size);
 		vmtruncate(inode, attr->ia_size);
 	}
@@ -808,14 +808,13 @@ nfs_wait_on_inode(struct inode *inode, int flag)
 	return error;
 }
 
-/*
- * Externally visible revalidation function
- */
-int
-nfs_revalidate(struct dentry *dentry)
+int nfs_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
 {
 	struct inode *inode = dentry->d_inode;
-	return nfs_revalidate_inode(NFS_SERVER(inode), inode);
+	int err = nfs_revalidate_inode(NFS_SERVER(inode), inode);
+	if (!err)
+		generic_fillattr(inode, stat);
+	return err;
 }
 
 /*

fs/nfs/symlink.c

@@ -59,7 +59,9 @@ static char *nfs_getlink(struct inode *inode, struct page **ppage)
 	struct page *page;
 	u32 *p;
 
-	/* Caller revalidated the directory inode already. */
+	page = ERR_PTR(nfs_revalidate_inode(NFS_SERVER(inode), inode));
+	if (page)
+		goto read_failed;
 	page = read_cache_page(&inode->i_data, 0,
 				(filler_t *)nfs_symlink_filler, inode);
 	if (IS_ERR(page))
@@ -72,7 +74,7 @@ static char *nfs_getlink(struct inode *inode, struct page **ppage)
 
 getlink_read_error:
 	page_cache_release(page);
-	return ERR_PTR(-EIO);
+	page = ERR_PTR(-EIO);
 read_failed:
 	return (char*)page;
 }
@@ -107,6 +109,6 @@ static int nfs_follow_link(struct dentry *dentry, struct nameidata *nd)
 struct inode_operations nfs_symlink_inode_operations = {
 	readlink:	nfs_readlink,
 	follow_link:	nfs_follow_link,
-	revalidate:	nfs_revalidate,
-	setattr:	nfs_notify_change,
+	getattr:	nfs_getattr,
+	setattr:	nfs_setattr,
 };

fs/smbfs/dir.c

@@ -47,7 +47,7 @@ struct inode_operations smb_dir_inode_operations =
 	mkdir:		smb_mkdir,
 	rmdir:		smb_rmdir,
 	rename:		smb_rename,
-	revalidate:	smb_revalidate_inode,
+	getattr:	smb_getattr,
 	setattr:	smb_notify_change,
 };
 

fs/smbfs/file.c

@@ -398,6 +398,6 @@ struct file_operations smb_file_operations =
 struct inode_operations smb_file_inode_operations =
 {
 	permission:	smb_file_permission,
-	revalidate:	smb_revalidate_inode,
+	getattr:	smb_getattr,
 	setattr:	smb_notify_change,
 };

fs/smbfs/inode.c

@@ -611,6 +611,14 @@ smb_statfs(struct super_block *sb, struct statfs *buf)
 	return result;
 }
 
+int smb_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
+{
+	int err = smb_revalidate_inode(dentry);
+	if (!err)
+		generic_fillattr(dentry->d_inode, stat);
+	return err;
+}
+
 int
 smb_notify_change(struct dentry *dentry, struct iattr *attr)
 {

fs/smbfs/proto.h

@@ -49,6 +49,7 @@ extern void smb_get_inode_attr(struct inode *inode, struct smb_fattr *fattr);
 extern void smb_set_inode_attr(struct inode *inode, struct smb_fattr *fattr);
 extern void smb_invalidate_inodes(struct smb_sb_info *server);
 extern int smb_revalidate_inode(struct dentry *dentry);
+extern int smb_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat);
 extern int smb_notify_change(struct dentry *dentry, struct iattr *attr);
 /* file.c */
 extern struct address_space_operations smb_file_aops;

fs/stat.c

@@ -14,18 +14,6 @@
 
 #include <asm/uaccess.h>
 
-/*
- * Revalidate the inode. This is required for proper NFS attribute caching.
- */
-static __inline__ int
-do_revalidate(struct dentry *dentry)
-{
-	struct inode *inode = dentry->d_inode;
-	if (inode->i_op->revalidate)
-		return inode->i_op->revalidate(dentry);
-	return 0;
-}
-
 void generic_fillattr(struct inode *inode, struct kstat *stat)
 {
 	stat->dev = kdev_t_to_nr(inode->i_dev);
@@ -46,16 +34,11 @@ void generic_fillattr(struct inode *inode, struct kstat *stat)
 
 static int do_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
 {
-	int res = 0;
 	struct inode *inode = dentry->d_inode;
 
 	if (inode->i_op->getattr)
 		return inode->i_op->getattr(mnt, dentry, stat);
 
-	res = do_revalidate(dentry);
-	if (res)
-		return res;
-
 	generic_fillattr(inode, stat);
 	if (!stat->blksize) {
 		struct super_block *s = inode->i_sb;
@@ -247,8 +230,7 @@ asmlinkage long sys_readlink(const char * path, char * buf, int bufsiz)
 		struct inode * inode = nd.dentry->d_inode;
 
 		error = -EINVAL;
-		if (inode->i_op && inode->i_op->readlink &&
-		    !(error = do_revalidate(nd.dentry))) {
+		if (inode->i_op && inode->i_op->readlink) {
 			UPDATE_ATIME(inode);
 			error = inode->i_op->readlink(nd.dentry, buf, bufsiz);
 		}

include/linux/coda_linux.h

@@ -40,7 +40,8 @@ int coda_flush(struct file *f);
 int coda_release(struct inode *i, struct file *f);
 int coda_permission(struct inode *inode, int mask);
 int coda_revalidate_inode(struct dentry *);
-int coda_notify_change(struct dentry *, struct iattr *);
+int coda_getattr(struct vfsmount *, struct dentry *, struct kstat *);
+int coda_setattr(struct dentry *, struct iattr *);
 int coda_isnullfid(ViceFid *fid);
 
 /* global variables */

include/linux/fs.h

@@ -781,7 +781,6 @@ struct inode_operations {
 	int (*follow_link) (struct dentry *, struct nameidata *);
 	void (*truncate) (struct inode *);
 	int (*permission) (struct inode *, int);
-	int (*revalidate) (struct dentry *);
 	int (*setattr) (struct dentry *, struct iattr *);
 	int (*getattr) (struct vfsmount *mnt, struct dentry *, struct kstat *);
 	int (*setxattr) (struct dentry *, const char *, void *, size_t, int);

include/linux/nfs_fs.h

@@ -234,12 +234,12 @@ extern int nfs_inode_is_stale(struct inode *, struct nfs_fh *,
 extern struct inode *nfs_fhget(struct dentry *, struct nfs_fh *,
 				struct nfs_fattr *);
 extern int __nfs_refresh_inode(struct inode *, struct nfs_fattr *);
-extern int nfs_revalidate(struct dentry *);
+extern int nfs_getattr(struct vfsmount *, struct dentry *, struct kstat *);
 extern int nfs_permission(struct inode *, int);
 extern int nfs_open(struct inode *, struct file *);
 extern int nfs_release(struct inode *, struct file *);
 extern int __nfs_revalidate_inode(struct nfs_server *, struct inode *);
-extern int nfs_notify_change(struct dentry *, struct iattr *);
+extern int nfs_setattr(struct dentry *, struct iattr *);
 
 /*
  * linux/fs/nfs/file.c