LinuxLists.cc - [PATCH 00/31] Remove iget() and read

2007-10-10 21:45:36

Subject: [PATCH 00/31] Remove iget() and read_inode() [try #3]

Hi Christoph, Al,

Here's a set of patches that remove all calls to iget() and all read_inode()
functions. They should be removed for two reasons: firstly they don't lend
themselves to good error handling, and secondly their presence is a temptation
for code outside a filesystem to call iget() to access inodes within that
filesystem.

There are a few benefits to this:

(1) Error handling gets simpler as you can return an error code rather than
having to call is_bad_inode().

(2) You can now tell the difference between ENOMEM and EIO occurring in the
read_inode() path.

(3) The code should get smaller. iget() is an inline function that is
typically called 2-3 times per filesystem that uses it. By folding the
iget code into the read_inode code for each filesystem, it eliminates
some duplication.

A tarball of the patches can be retrieved from:

http://people.redhat.com/~dhowells/iget-remove.tar.bz2

Additionally, there are a couple of patches that introduce an ERR_CAST() macro
to be used instead of ERR_PTR(PTR_ERR(p)) constructs and apply it to all such
instances in the kernel.

Of the patches directly relevant to the subject:

The first patch adds a function, iget_failed() that is a canned piece of code
for killing an inode when the inode construction path fails.

The second and third patches makes AFS and GFS2 use iget_failed() rather than
interpolating the sequence directly.

The fourth patch marks iget() and read_inode() as being deprecated.

The final patch removes iget() and read_inode() completely.

Each of the other patches modify a filesystem that used iget() and read_inode()
to use iget_locked() instead. The standard procedure was to convert:

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

into:

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 call thingyfs_iget() rather than iget():

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;
}

There were exceptions; most notably it appeared FAT should be calling ilookup()
not iget().

Additionally, HPPFS and HOSTFS (UM-specific filesystems) really need checking:

hostfs_kern.c:

(*) hostfs_iget() should perhaps subsume init_inode() and hostfs_read_inode().

(*) It would appear that all hostfs inodes are the same inode because iget()
was being called with inode number 0 - which forms the lookup key.

hppfs_kern.c:

(*) The HPPFS inode retains a pointer to the proc dentry it is shadowing, but
whilst it does appear to retain a reference to it, it doesn't appear to
destroy the reference if the inode goes away.

(*) hppfs_iget() should perhaps subsume init_inode() and hppfs_read_inode().

(*) It would appear that all hppfs inodes are the same inode because iget()
was being called with inode number 0, which forms the lookup key.

In addition to the introduction of ERR_CAST, the ext2, ext3 and ext4 patches
have been fixed from Jan Kara's review.

David

2007-10-10 21:43:44

Subject: [PATCH 00/31] Remove iget() and read_inode() [try #3]

Subject: [PATCH 01/31] Add an ERR_CAST() macro to complement ERR_PTR and co. [try #3]

Subject: [PATCH 02/31] Convert ERR_PTR(PTR_ERR(p)) instances to ERR_CAST(p) [try #3]

Subject: [PATCH 03/31] IGET: Introduce a function to register iget failure [try #3]

Subject: [PATCH 04/31] IGET: Use iget_failed() in AFS [try #3]

Subject: [PATCH 05/31] IGET: Use iget_failed() in GFS2 [try #3]

Subject: [PATCH 06/31] IGET: Stop AFFS from using iget() and read_inode() [try #3]

Subject: [PATCH 07/31] IGET: Stop autofs from using iget() and read_inode() [try #3]

Subject: [PATCH 08/31] IGET: Stop BEFS from using iget() and read_inode() [try #3]

Subject: [PATCH 09/31] IGET: Stop BFS from using iget() and read_inode() [try #3]

Subject: [PATCH 10/31] IGET: Stop CIFS from using iget() and read_inode() [try #3]

Subject: [PATCH 11/31] IGET: Stop EFS from using iget() and read_inode() [try #3]

Subject: [PATCH 12/31] IGET: Stop EXT2 from using iget() and read_inode() [try #3]

Subject: [PATCH 13/31] IGET: Stop EXT3 from using iget() and read_inode() [try #3]

Subject: [PATCH 14/31] IGET: Stop EXT4 from using iget() and read_inode() [try #3]

Subject: [PATCH 15/31] IGET: Stop FAT from using iget() and read_inode() [try #3]

Subject: [PATCH 16/31] IGET: Stop FreeVXFS from using iget() and read_inode() [try #3]

Subject: [PATCH 17/31] IGET: Stop FUSE from using iget() and read_inode() [try #3]

Subject: [PATCH 18/31] IGET: Stop HFSPLUS from using iget() and read_inode() [try #3]

Subject: [PATCH 19/31] IGET: Stop ISOFS from using read_inode() [try #3]

Subject: [PATCH 20/31] IGET: Stop JFFS2 from using iget() and read_inode() [try #3]

Subject: [PATCH 21/31] IGET: Stop JFS from using iget() and read_inode() [try #3]

Subject: [PATCH 22/31] IGET: Stop the MINIX filesystem from using iget() and read_inode() [try #3]

Subject: [PATCH 24/31] IGET: Stop QNX4 from using iget() and read_inode() [try #3]

Subject: [PATCH 23/31] IGET: Stop PROCFS from using iget() and read_inode() [try #3]

Subject: [PATCH 26/31] IGET: Stop the SYSV filesystem from using iget() and read_inode() [try #3]

Subject: [PATCH 25/31] IGET: Stop ROMFS from using iget() and read_inode() [try #3]

Subject: [PATCH 27/31] IGET: Stop UFS from using iget() and read_inode() [try #3]

Subject: [PATCH 28/31] IGET: Stop OPENPROMFS from using iget() and read_inode() [try #3]

Subject: [PATCH 30/31] IGET: Stop HPPFS from using iget() and read_inode() [try #3]

Subject: [PATCH 29/31] IGET: Stop HOSTFS from using iget() and read_inode() [try #3]

Subject: [PATCH 31/31] IGET: Remove iget() and the read_inode() super op as being obsolete [try #3]

Subject: Re: [PATCH 09/31] IGET: Stop BFS from using iget() and read_inode() [try #3]

Subject: Re: [PATCH 09/31] IGET: Stop BFS from using iget() and read_inode() [try #3]