Commit 12debc4248a4a7f1873e47cda2cdd7faca80b099

Authored by David Howells
Committed by Linus Torvalds
1 parent 755aedc159

iget: remove iget() and the read_inode() super op as being obsolete

Remove the old iget() call and the read_inode() superblock operation it uses
as these are really obsolete, and the use of read_inode() does not produce
proper error handling (no distinction between ENOMEM and EIO when marking an
inode bad).

Furthermore, this removes the temptation to use iget() to find an inode by
number in a filesystem from code outside that filesystem.

iget_locked() should be used instead.  A new function is added in an earlier
patch (iget_failed) that is to be called to mark an inode as bad, unlock it
and release it should the get routine fail.  Mark iget() and read_inode() as
being obsolete and remove references to them from the documentation.

Typically a filesystem will be modified such that the read_inode function
becomes an internal iget function, for example the following:

	void thingyfs_read_inode(struct inode *inode)
	{
		...
	}

would be changed into something like:

	struct inode *thingyfs_iget(struct super_block *sp, unsigned long ino)
	{
		struct inode *inode;
		int ret;

		inode = iget_locked(sb, ino);
		if (!inode)
			return ERR_PTR(-ENOMEM);
		if (!(inode->i_state & I_NEW))
			return inode;

		...
		unlock_new_inode(inode);
		return inode;
	error:
		iget_failed(inode);
		return ERR_PTR(ret);
	}

and then thingyfs_iget() would be called rather than iget(), for example:

	ret = -EINVAL;
	inode = iget(sb, ino);
	if (!inode || is_bad_inode(inode))
		goto error;

becomes:

	inode = thingyfs_iget(sb, ino);
	if (IS_ERR(inode)) {
		ret = PTR_ERR(inode);
		goto error;
	}

Note that is_bad_inode() does not need to be called.  The error returned by
thingyfs_iget() should render it unnecessary.

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Showing 5 changed files with 9 additions and 41 deletions Side-by-side Diff

Documentation/filesystems/Locking
... ... @@ -90,7 +90,6 @@
90 90 prototypes:
91 91 struct inode *(*alloc_inode)(struct super_block *sb);
92 92 void (*destroy_inode)(struct inode *);
93   - void (*read_inode) (struct inode *);
94 93 void (*dirty_inode) (struct inode *);
95 94 int (*write_inode) (struct inode *, int);
96 95 void (*put_inode) (struct inode *);
... ... @@ -114,7 +113,6 @@
114 113 BKL s_lock s_umount
115 114 alloc_inode: no no no
116 115 destroy_inode: no
117   -read_inode: no (see below)
118 116 dirty_inode: no (must not sleep)
119 117 write_inode: no
120 118 put_inode: no
... ... @@ -133,7 +131,6 @@
133 131 quota_read: no no no (see below)
134 132 quota_write: no no no (see below)
135 133  
136   -->read_inode() is not a method - it's a callback used in iget().
137 134 ->remount_fs() will have the s_umount lock if it's already mounted.
138 135 When called from get_sb_single, it does NOT have the s_umount lock.
139 136 ->quota_read() and ->quota_write() functions are both guaranteed to
Documentation/filesystems/porting
... ... @@ -34,8 +34,8 @@
34 34  
35 35 Make them ->alloc_inode and ->destroy_inode in your super_operations.
36 36  
37   -Keep in mind that now you need explicit initialization of private data -
38   -typically in ->read_inode() and after getting an inode from new_inode().
  37 +Keep in mind that now you need explicit initialization of private data
  38 +typically between calling iget_locked() and unlocking the inode.
39 39  
40 40 At some point that will become mandatory.
41 41  
... ... @@ -173,10 +173,10 @@
173 173 newly created inode to allow the test function to succeed. 'data' is
174 174 passed as an opaque value to both test and set functions.
175 175  
176   -When the inode has been created by iget5_locked(), it will be returned with
177   -the I_NEW flag set and will still be locked. read_inode has not been
178   -called so the file system still has to finalize the initialization. Once
179   -the inode is initialized it must be unlocked by calling unlock_new_inode().
  176 +When the inode has been created by iget5_locked(), it will be returned with the
  177 +I_NEW flag set and will still be locked. The filesystem then needs to finalize
  178 +the initialization. Once the inode is initialized it must be unlocked by
  179 +calling unlock_new_inode().
180 180  
181 181 The filesystem is responsible for setting (and possibly testing) i_ino
182 182 when appropriate. There is also a simpler iget_locked function that
Documentation/filesystems/vfs.txt
... ... @@ -203,8 +203,6 @@
203 203 struct inode *(*alloc_inode)(struct super_block *sb);
204 204 void (*destroy_inode)(struct inode *);
205 205  
206   - void (*read_inode) (struct inode *);
207   -
208 206 void (*dirty_inode) (struct inode *);
209 207 int (*write_inode) (struct inode *, int);
210 208 void (*put_inode) (struct inode *);
... ... @@ -242,15 +240,6 @@
242 240 ->alloc_inode was defined and simply undoes anything done by
243 241 ->alloc_inode.
244 242  
245   - read_inode: this method is called to read a specific inode from the
246   - mounted filesystem. The i_ino member in the struct inode is
247   - initialized by the VFS to indicate which inode to read. Other
248   - members are filled in by this method.
249   -
250   - You can set this to NULL and use iget5_locked() instead of iget()
251   - to read inodes. This is necessary for filesystems for which the
252   - inode number is not sufficient to identify an inode.
253   -
254 243 dirty_inode: this method is called by the VFS to mark an inode dirty.
255 244  
256 245 write_inode: this method is called when the VFS needs to write an
... ... @@ -308,9 +297,9 @@
308 297  
309 298 quota_write: called by the VFS to write to filesystem quota file.
310 299  
311   -The read_inode() method is responsible for filling in the "i_op"
312   -field. This is a pointer to a "struct inode_operations" which
313   -describes the methods that can be performed on individual inodes.
  300 +Whoever sets up the inode is responsible for filling in the "i_op" field. This
  301 +is a pointer to a "struct inode_operations" which describes the methods that
  302 +can be performed on individual inodes.
314 303  
315 304  
316 305 The Inode Object
... ... @@ -928,8 +928,6 @@
928 928 * @set: callback used to initialize a new struct inode
929 929 * @data: opaque data pointer to pass to @test and @set
930 930 *
931   - * This is iget() without the read_inode() portion of get_new_inode().
932   - *
933 931 * iget5_locked() uses ifind() to search for the inode specified by @hashval
934 932 * and @data in the inode cache and if present it is returned with an increased
935 933 * reference count. This is a generalized version of iget_locked() for file
... ... @@ -965,8 +963,6 @@
965 963 * iget_locked - obtain an inode from a mounted file system
966 964 * @sb: super block of file system
967 965 * @ino: inode number to get
968   - *
969   - * This is iget() without the read_inode() portion of get_new_inode_fast().
970 966 *
971 967 * iget_locked() uses ifind_fast() to search for the inode specified by @ino in
972 968 * the inode cache and if present it is returned with an increased reference
... ... @@ -1241,8 +1241,6 @@
1241 1241 struct inode *(*alloc_inode)(struct super_block *sb);
1242 1242 void (*destroy_inode)(struct inode *);
1243 1243  
1244   - void (*read_inode) (struct inode *);
1245   -
1246 1244 void (*dirty_inode) (struct inode *);
1247 1245 int (*write_inode) (struct inode *, int);
1248 1246 void (*put_inode) (struct inode *);
... ... @@ -1766,18 +1764,6 @@
1766 1764 extern struct inode * iget5_locked(struct super_block *, unsigned long, int (*test)(struct inode *, void *), int (*set)(struct inode *, void *), void *);
1767 1765 extern struct inode * iget_locked(struct super_block *, unsigned long);
1768 1766 extern void unlock_new_inode(struct inode *);
1769   -
1770   -static inline struct inode *iget(struct super_block *sb, unsigned long ino)
1771   -{
1772   - struct inode *inode = iget_locked(sb, ino);
1773   -
1774   - if (inode && (inode->i_state & I_NEW)) {
1775   - sb->s_op->read_inode(inode);
1776   - unlock_new_inode(inode);
1777   - }
1778   -
1779   - return inode;
1780   -}
1781 1767  
1782 1768 extern void __iget(struct inode * inode);
1783 1769 extern void iget_failed(struct inode *);