diff options
author | David Howells <dhowells@redhat.com> | 2008-02-07 03:15:52 -0500 |
---|---|---|
committer | Linus Torvalds <torvalds@woody.linux-foundation.org> | 2008-02-07 11:42:29 -0500 |
commit | 12debc4248a4a7f1873e47cda2cdd7faca80b099 (patch) | |
tree | 1ad80b77d213ea09cb746d6e4d50c4316462a452 | |
parent | 755aedc15900ff7d83dd046f632af9a680b0c28f (diff) |
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>
-rw-r--r-- | Documentation/filesystems/Locking | 3 | ||||
-rw-r--r-- | Documentation/filesystems/porting | 12 | ||||
-rw-r--r-- | Documentation/filesystems/vfs.txt | 17 | ||||
-rw-r--r-- | fs/inode.c | 4 | ||||
-rw-r--r-- | include/linux/fs.h | 14 |
5 files changed, 9 insertions, 41 deletions
diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking index 37c10cba7177..42d4b30b1045 100644 --- a/Documentation/filesystems/Locking +++ b/Documentation/filesystems/Locking | |||
@@ -90,7 +90,6 @@ of the locking scheme for directory operations. | |||
90 | prototypes: | 90 | prototypes: |
91 | struct inode *(*alloc_inode)(struct super_block *sb); | 91 | struct inode *(*alloc_inode)(struct super_block *sb); |
92 | void (*destroy_inode)(struct inode *); | 92 | void (*destroy_inode)(struct inode *); |
93 | void (*read_inode) (struct inode *); | ||
94 | void (*dirty_inode) (struct inode *); | 93 | void (*dirty_inode) (struct inode *); |
95 | int (*write_inode) (struct inode *, int); | 94 | int (*write_inode) (struct inode *, int); |
96 | void (*put_inode) (struct inode *); | 95 | void (*put_inode) (struct inode *); |
@@ -114,7 +113,6 @@ locking rules: | |||
114 | BKL s_lock s_umount | 113 | BKL s_lock s_umount |
115 | alloc_inode: no no no | 114 | alloc_inode: no no no |
116 | destroy_inode: no | 115 | destroy_inode: no |
117 | read_inode: no (see below) | ||
118 | dirty_inode: no (must not sleep) | 116 | dirty_inode: no (must not sleep) |
119 | write_inode: no | 117 | write_inode: no |
120 | put_inode: no | 118 | put_inode: no |
@@ -133,7 +131,6 @@ show_options: no (vfsmount->sem) | |||
133 | quota_read: no no no (see below) | 131 | quota_read: no no no (see below) |
134 | quota_write: no no no (see below) | 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 | ->remount_fs() will have the s_umount lock if it's already mounted. | 134 | ->remount_fs() will have the s_umount lock if it's already mounted. |
138 | When called from get_sb_single, it does NOT have the s_umount lock. | 135 | When called from get_sb_single, it does NOT have the s_umount lock. |
139 | ->quota_read() and ->quota_write() functions are both guaranteed to | 136 | ->quota_read() and ->quota_write() functions are both guaranteed to |
diff --git a/Documentation/filesystems/porting b/Documentation/filesystems/porting index fbd3815a5f57..92b888d540a6 100644 --- a/Documentation/filesystems/porting +++ b/Documentation/filesystems/porting | |||
@@ -34,8 +34,8 @@ FOO_I(inode) (see in-tree filesystems for examples). | |||
34 | 34 | ||
35 | Make them ->alloc_inode and ->destroy_inode in your super_operations. | 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 - | 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(). | 38 | typically between calling iget_locked() and unlocking the inode. |
39 | 39 | ||
40 | At some point that will become mandatory. | 40 | At some point that will become mandatory. |
41 | 41 | ||
@@ -173,10 +173,10 @@ should be a non-blocking function that initializes those parts of a | |||
173 | newly created inode to allow the test function to succeed. 'data' is | 173 | newly created inode to allow the test function to succeed. 'data' is |
174 | passed as an opaque value to both test and set functions. | 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 | 176 | When the inode has been created by iget5_locked(), it will be returned with the |
177 | the I_NEW flag set and will still be locked. read_inode has not been | 177 | I_NEW flag set and will still be locked. The filesystem then needs to finalize |
178 | called so the file system still has to finalize the initialization. Once | 178 | the initialization. Once the inode is initialized it must be unlocked by |
179 | the inode is initialized it must be unlocked by calling unlock_new_inode(). | 179 | calling unlock_new_inode(). |
180 | 180 | ||
181 | The filesystem is responsible for setting (and possibly testing) i_ino | 181 | The filesystem is responsible for setting (and possibly testing) i_ino |
182 | when appropriate. There is also a simpler iget_locked function that | 182 | when appropriate. There is also a simpler iget_locked function that |
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt index 9d019d35728f..bd55038b56f5 100644 --- a/Documentation/filesystems/vfs.txt +++ b/Documentation/filesystems/vfs.txt | |||
@@ -203,8 +203,6 @@ struct super_operations { | |||
203 | struct inode *(*alloc_inode)(struct super_block *sb); | 203 | struct inode *(*alloc_inode)(struct super_block *sb); |
204 | void (*destroy_inode)(struct inode *); | 204 | void (*destroy_inode)(struct inode *); |
205 | 205 | ||
206 | void (*read_inode) (struct inode *); | ||
207 | |||
208 | void (*dirty_inode) (struct inode *); | 206 | void (*dirty_inode) (struct inode *); |
209 | int (*write_inode) (struct inode *, int); | 207 | int (*write_inode) (struct inode *, int); |
210 | void (*put_inode) (struct inode *); | 208 | void (*put_inode) (struct inode *); |
@@ -242,15 +240,6 @@ or bottom half). | |||
242 | ->alloc_inode was defined and simply undoes anything done by | 240 | ->alloc_inode was defined and simply undoes anything done by |
243 | ->alloc_inode. | 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 | dirty_inode: this method is called by the VFS to mark an inode dirty. | 243 | dirty_inode: this method is called by the VFS to mark an inode dirty. |
255 | 244 | ||
256 | write_inode: this method is called when the VFS needs to write an | 245 | write_inode: this method is called when the VFS needs to write an |
@@ -308,9 +297,9 @@ or bottom half). | |||
308 | 297 | ||
309 | quota_write: called by the VFS to write to filesystem quota file. | 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" | 300 | Whoever sets up the inode is responsible for filling in the "i_op" field. This |
312 | field. This is a pointer to a "struct inode_operations" which | 301 | is a pointer to a "struct inode_operations" which describes the methods that |
313 | describes the methods that can be performed on individual inodes. | 302 | can be performed on individual inodes. |
314 | 303 | ||
315 | 304 | ||
316 | The Inode Object | 305 | The Inode Object |
diff --git a/fs/inode.c b/fs/inode.c index 276ffd6b6fdd..53245ffcf93d 100644 --- a/fs/inode.c +++ b/fs/inode.c | |||
@@ -928,8 +928,6 @@ EXPORT_SYMBOL(ilookup); | |||
928 | * @set: callback used to initialize a new struct inode | 928 | * @set: callback used to initialize a new struct inode |
929 | * @data: opaque data pointer to pass to @test and @set | 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 | * iget5_locked() uses ifind() to search for the inode specified by @hashval | 931 | * iget5_locked() uses ifind() to search for the inode specified by @hashval |
934 | * and @data in the inode cache and if present it is returned with an increased | 932 | * and @data in the inode cache and if present it is returned with an increased |
935 | * reference count. This is a generalized version of iget_locked() for file | 933 | * reference count. This is a generalized version of iget_locked() for file |
@@ -966,8 +964,6 @@ EXPORT_SYMBOL(iget5_locked); | |||
966 | * @sb: super block of file system | 964 | * @sb: super block of file system |
967 | * @ino: inode number to get | 965 | * @ino: inode number to get |
968 | * | 966 | * |
969 | * This is iget() without the read_inode() portion of get_new_inode_fast(). | ||
970 | * | ||
971 | * iget_locked() uses ifind_fast() to search for the inode specified by @ino in | 967 | * iget_locked() uses ifind_fast() to search for the inode specified by @ino in |
972 | * the inode cache and if present it is returned with an increased reference | 968 | * the inode cache and if present it is returned with an increased reference |
973 | * count. This is for file systems where the inode number is sufficient for | 969 | * count. This is for file systems where the inode number is sufficient for |
diff --git a/include/linux/fs.h b/include/linux/fs.h index d202600d36bd..36b7abefacbe 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h | |||
@@ -1241,8 +1241,6 @@ struct super_operations { | |||
1241 | struct inode *(*alloc_inode)(struct super_block *sb); | 1241 | struct inode *(*alloc_inode)(struct super_block *sb); |
1242 | void (*destroy_inode)(struct inode *); | 1242 | void (*destroy_inode)(struct inode *); |
1243 | 1243 | ||
1244 | void (*read_inode) (struct inode *); | ||
1245 | |||
1246 | void (*dirty_inode) (struct inode *); | 1244 | void (*dirty_inode) (struct inode *); |
1247 | int (*write_inode) (struct inode *, int); | 1245 | int (*write_inode) (struct inode *, int); |
1248 | void (*put_inode) (struct inode *); | 1246 | void (*put_inode) (struct inode *); |
@@ -1767,18 +1765,6 @@ extern struct inode * iget5_locked(struct super_block *, unsigned long, int (*te | |||
1767 | extern struct inode * iget_locked(struct super_block *, unsigned long); | 1765 | extern struct inode * iget_locked(struct super_block *, unsigned long); |
1768 | extern void unlock_new_inode(struct inode *); | 1766 | extern void unlock_new_inode(struct inode *); |
1769 | 1767 | ||
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 | |||
1782 | extern void __iget(struct inode * inode); | 1768 | extern void __iget(struct inode * inode); |
1783 | extern void iget_failed(struct inode *); | 1769 | extern void iget_failed(struct inode *); |
1784 | extern void clear_inode(struct inode *); | 1770 | extern void clear_inode(struct inode *); |