linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: hch@infradead.org, viro@ftp.linux.org.uk, torvalds@osdl.org,
	akpm@osdl.org
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	dhowells@redhat.com
Subject: [PATCH 00/30] Remove iget() and read_inode()
Date: Mon, 01 Oct 2007 14:09:21 +0100	[thread overview]
Message-ID: <20071001130921.29339.72876.stgit@warthog.procyon.org.uk> (raw)



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


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.


David

             reply	other threads:[~2007-10-01 13:09 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-01 13:09 David Howells [this message]
2007-10-01 13:09 ` [PATCH 01/30] IGET: Introduce a function to register iget failure David Howells
2007-10-01 13:09 ` [PATCH 02/30] IGET: Use iget_failed() in AFS David Howells
2007-10-01 13:09 ` [PATCH 03/30] IGET: Use iget_failed() in GFS2 David Howells
2007-10-01 13:09 ` [PATCH 04/30] IGET: Mark iget() and read_inode() as being obsolete David Howells
2007-10-01 13:09 ` [PATCH 05/30] IGET: Stop AFFS from using iget() and read_inode() David Howells
2007-10-01 13:09 ` [PATCH 06/30] IGET: Stop autofs " David Howells
2007-10-01 13:09 ` [PATCH 07/30] IGET: Stop BEFS " David Howells
2007-10-01 17:39   ` Zach Brown
2007-10-01 17:44     ` Linus Torvalds
2007-10-01 18:06       ` Christoph Hellwig
2007-10-01 18:18         ` Zach Brown
2007-10-01 18:19         ` Linus Torvalds
2007-10-02 12:32         ` David Howells
2007-10-02 13:02           ` Dave Kleikamp
2007-10-02 13:24           ` David Howells
2007-10-02 14:16             ` Dave Kleikamp
2007-10-01 13:10 ` [PATCH 08/30] IGET: Stop BFS " David Howells
2007-10-01 13:10 ` [PATCH 09/30] IGET: Stop CIFS " David Howells
2007-10-01 13:10 ` [PATCH 10/30] IGET: Stop EFS " David Howells
2007-10-01 13:10 ` [PATCH 11/30] IGET: Stop EXT2 " David Howells
2007-10-02 10:06   ` Jan Kara
2007-10-02 12:58   ` David Howells
2007-10-01 13:10 ` [PATCH 12/30] IGET: Stop EXT3 " David Howells
2007-10-02 10:24   ` Jan Kara
2007-10-02 10:28   ` Jan Kara
2007-10-02 13:16   ` David Howells
2007-10-01 13:10 ` [PATCH 13/30] IGET: Stop EXT4 " David Howells
2007-10-02 12:26   ` Jan Kara
2007-10-02 13:37   ` David Howells
2007-10-01 13:10 ` [PATCH 14/30] IGET: Stop FAT " David Howells
2007-10-01 13:10 ` [PATCH 15/30] IGET: Stop FreeVXFS " David Howells
2007-10-01 13:10 ` [PATCH 16/30] IGET: Stop FUSE " David Howells
2007-10-01 13:10 ` [PATCH 17/30] IGET: Stop HFSPLUS " David Howells
2007-10-01 13:10 ` [PATCH 18/30] IGET: Stop ISOFS from using read_inode() David Howells
2007-10-01 13:10 ` [PATCH 19/30] IGET: Stop JFFS2 from using iget() and read_inode() David Howells
2007-10-01 13:11 ` [PATCH 20/30] IGET: Stop JFS " David Howells
2007-10-01 18:44   ` Dave Kleikamp
2007-10-01 13:11 ` [PATCH 21/30] IGET: Stop the MINIX filesystem " David Howells
2007-10-01 13:11 ` [PATCH 22/30] IGET: Stop PROCFS " David Howells
2007-10-01 13:11 ` [PATCH 23/30] IGET: Stop QNX4 " David Howells
2007-10-01 13:11 ` [PATCH 24/30] IGET: Stop ROMFS " David Howells
2007-10-01 13:11 ` [PATCH 25/30] IGET: Stop the SYSV filesystem " David Howells
2007-10-01 13:11 ` [PATCH 26/30] IGET: Stop UFS " David Howells
2007-10-01 13:11 ` [PATCH 27/30] IGET: Stop OPENPROMFS " David Howells
2007-10-01 13:11 ` [PATCH 28/30] IGET: Stop HOSTFS " David Howells
2007-10-01 13:11 ` [PATCH 29/30] IGET: Stop HPPFS " David Howells
2007-10-01 13:11 ` [PATCH 30/30] IGET: Remove iget() and the read_inode() super op as being obsolete David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20071001130921.29339.72876.stgit@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=akpm@osdl.org \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@osdl.org \
    --cc=viro@ftp.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).