[RFC] iput() cleanup (was Re: [patch 12/16] fix race between writeback and unlink)

From: Linus Torvalds (torvalds@transmeta.com)
Date: Sun Jun 02 2002 - 23:27:52 EST


On Sat, 1 Jun 2002, Andrew Morton wrote:
>
> > Why not just split up the code inside iput(), and then just do
> >
> > if (atomic_dec(&inode->i_count))
> > final_iput(inode);
>
> Yes, I suspect all the inode refcounting, locking, I_FREEING, I_LOCK, etc
> could do with a spring clean. Make it a bit more conventional. I'll
> discuss with Al when he resurfaces.

This is a first cut at cleaning up "iput()" and getting rid of some of the
magic VFS-level behaviour of the i_nlink field which many filesystems do
not actually want - as shown by the number of "force_delete" users out
there.

It does not change any real behaviour, but it splits up the "iput()"
behaviour into several functions ("common_delete_inode()",
"common_forget_inode()" and "common_drop_inode()"), and adds a place for a
low-level filesystem to hook into the behaviour at inode drop time,
through the "drop_inode" superblock operation.

This part of the diff probably explains it best

+ drop_inode: called when the last access to the inode is dropped,
+ with the inode_lock spinlock held.
+
+ This method should be either NULL (normal unix filesystem
+ semantics) or "common_delete_inode" (for filesystems that do not
+ want to cache inodes - causing "delete_inode" to always be
+ called regardless of the value of i_nlink)
+
+ The "common_delete_inode()" behaviour is equivalent to the
+ old practice of using "force_delete" in the put_inode() case,
+ but does not have the races that the "force_delete()" approach
+ had.

This removes _most_ of the "put_inode()" users, although filesystems can
(and do) still use it for other things - notably to drop pre-allocations
etc.

Comments?

                        Linus

----
===== Documentation/filesystems/Locking 1.29 vs edited =====
--- 1.29/Documentation/filesystems/Locking	Fri May 31 18:18:14 2002
+++ edited/Documentation/filesystems/Locking	Sun Jun  2 20:58:05 2002
@@ -88,6 +88,7 @@
 	void (*read_inode) (struct inode *);
 	void (*write_inode) (struct inode *, int);
 	void (*put_inode) (struct inode *);
+	void (*drop_inode) (struct inode *);
 	void (*delete_inode) (struct inode *);
 	void (*put_super) (struct super_block *);
 	void (*write_super) (struct super_block *);
@@ -102,6 +103,7 @@
 read_inode:	yes				(see below)
 write_inode:	no
 put_inode:	no
+drop_inode:	no				!!!inode_lock!!!
 delete_inode:	no
 clear_inode:	no
 put_super:	yes	yes	maybe		(see below)
===== Documentation/filesystems/vfs.txt 1.1 vs edited =====
--- 1.1/Documentation/filesystems/vfs.txt	Tue Feb  5 09:40:36 2002
+++ edited/Documentation/filesystems/vfs.txt	Sun Jun  2 21:17:23 2002
@@ -178,6 +178,7 @@
 	void (*read_inode) (struct inode *);
 	void (*write_inode) (struct inode *, int);
 	void (*put_inode) (struct inode *);
+	void (*drop_inode) (struct inode *);
 	void (*delete_inode) (struct inode *);
 	int (*notify_change) (struct dentry *, struct iattr *);
 	void (*put_super) (struct super_block *);
@@ -203,6 +204,19 @@

put_inode: called when the VFS inode is removed from the inode cache. This method is optional + + drop_inode: called when the last access to the inode is dropped, + with the inode_lock spinlock held. + + This method should be either NULL (normal unix filesystem + semantics) or "common_delete_inode" (for filesystems that do not + want to cache inodes - causing "delete_inode" to always be + called regardless of the value of i_nlink) + + The "common_delete_inode()" behaviour is equivalent to the + old practice of using "force_delete" in the put_inode() case, + but does not have the races that the "force_delete()" approach + had.

delete_inode: called when the VFS wants to delete an inode

===== drivers/hotplug/pci_hotplug_core.c 1.14 vs edited ===== --- 1.14/drivers/hotplug/pci_hotplug_core.c Thu May 9 13:44:41 2002 +++ edited/drivers/hotplug/pci_hotplug_core.c Sun Jun 2 20:12:36 2002 @@ -290,7 +290,7 @@

static struct super_operations pcihpfs_ops = { statfs: simple_statfs, - put_inode: force_delete, + drop_inode: common_delete_inode, };

static int pcihpfs_fill_super(struct super_block *sb, void *data, int silent) ===== drivers/usb/core/inode.c 1.31 vs edited ===== --- 1.31/drivers/usb/core/inode.c Wed May 22 09:25:48 2002 +++ edited/drivers/usb/core/inode.c Sun Jun 2 20:13:58 2002 @@ -298,7 +298,7 @@

static struct super_operations usbfs_ops = { statfs: simple_statfs, - put_inode: force_delete, + drop_inode: common_delete_inode, };

static int usbfs_fill_super(struct super_block *sb, void *data, int silent) ===== fs/binfmt_misc.c 1.13 vs edited ===== --- 1.13/fs/binfmt_misc.c Thu May 23 09:08:34 2002 +++ edited/fs/binfmt_misc.c Sun Jun 2 19:48:46 2002 @@ -621,7 +621,7 @@

static struct super_operations s_ops = { statfs: simple_statfs, - put_inode: force_delete, + drop_inode: common_delete_inode, clear_inode: bm_clear_inode, };

===== fs/inode.c 1.60 vs edited ===== --- 1.60/fs/inode.c Fri May 31 18:18:09 2002 +++ edited/fs/inode.c Sun Jun 2 20:11:02 2002 @@ -782,6 +782,97 @@ spin_unlock(&inode_lock); }

+void common_delete_inode(struct inode *inode) +{ + struct super_operations *op = inode->i_sb->s_op; + + list_del(&inode->i_hash); + INIT_LIST_HEAD(&inode->i_hash); + list_del(&inode->i_list); + INIT_LIST_HEAD(&inode->i_list); + inode->i_state|=I_FREEING; + inodes_stat.nr_inodes--; + spin_unlock(&inode_lock); + + if (inode->i_data.nrpages) + truncate_inode_pages(&inode->i_data, 0); + + if (op && op->delete_inode) { + void (*delete)(struct inode *) = op->delete_inode; + if (!is_bad_inode(inode)) + DQUOT_INIT(inode); + /* s_op->delete_inode internally recalls clear_inode() */ + delete(inode); + } else + clear_inode(inode); + if (inode->i_state != I_CLEAR) + BUG(); + destroy_inode(inode); +} +EXPORT_SYMBOL(common_delete_inode); + +static void common_forget_inode(struct inode *inode) +{ + struct super_block *sb = inode->i_sb; + + if (!list_empty(&inode->i_hash)) { + if (!(inode->i_state & (I_DIRTY|I_LOCK))) { + list_del(&inode->i_list); + list_add(&inode->i_list, &inode_unused); + } + inodes_stat.nr_unused++; + spin_unlock(&inode_lock); + if (!sb || (sb->s_flags & MS_ACTIVE)) + return; + write_inode_now(inode, 1); + spin_lock(&inode_lock); + inodes_stat.nr_unused--; + list_del_init(&inode->i_hash); + } + list_del_init(&inode->i_list); + inode->i_state|=I_FREEING; + inodes_stat.nr_inodes--; + spin_unlock(&inode_lock); + if (inode->i_data.nrpages) + truncate_inode_pages(&inode->i_data, 0); + clear_inode(inode); + destroy_inode(inode); +} + +/* + * Normal UNIX filesystem behaviour: delete the + * inode when the usage count drops to zero, and + * i_nlink is zero. + */ +static void common_drop_inode(struct inode *inode) +{ + if (!inode->i_nlink) + common_delete_inode(inode); + else + common_forget_inode(inode); +} + +/* + * Called when we're dropping the last reference + * to an inode. + * + * Call the FS "drop()" function, defaulting to + * the legacy UNIX filesystem behaviour.. + * + * NOTE! NOTE! NOTE! We're called with the inode lock + * held, and the drop function is supposed to release + * the lock! + */ +static inline void iput_final(struct inode *inode) +{ + struct super_operations *op = inode->i_sb->s_op; + void (*drop)(struct inode *) = common_drop_inode; + + if (op && op->drop_inode) + drop = op->drop_inode; + drop(inode); +} + /** * iput - put an inode * @inode: inode to put @@ -793,77 +884,17 @@ void iput(struct inode *inode) { if (inode) { - struct super_block *sb = inode->i_sb; - struct super_operations *op = NULL; + struct super_operations *op = inode->i_sb->s_op;

if (inode->i_state == I_CLEAR) BUG();

- if (sb && sb->s_op) - op = sb->s_op; if (op && op->put_inode) op->put_inode(inode);

- if (!atomic_dec_and_lock(&inode->i_count, &inode_lock)) - return; - - if (!inode->i_nlink) { - list_del(&inode->i_hash); - INIT_LIST_HEAD(&inode->i_hash); - list_del(&inode->i_list); - INIT_LIST_HEAD(&inode->i_list); - inode->i_state|=I_FREEING; - inodes_stat.nr_inodes--; - spin_unlock(&inode_lock); - - if (inode->i_data.nrpages) - truncate_inode_pages(&inode->i_data, 0); - - if (op && op->delete_inode) { - void (*delete)(struct inode *) = op->delete_inode; - if (!is_bad_inode(inode)) - DQUOT_INIT(inode); - /* s_op->delete_inode internally recalls clear_inode() */ - delete(inode); - } else - clear_inode(inode); - if (inode->i_state != I_CLEAR) - BUG(); - } else { - if (!list_empty(&inode->i_hash)) { - if (!(inode->i_state & (I_DIRTY|I_LOCK))) { - list_del(&inode->i_list); - list_add(&inode->i_list, &inode_unused); - } - inodes_stat.nr_unused++; - spin_unlock(&inode_lock); - if (!sb || (sb->s_flags & MS_ACTIVE)) - return; - write_inode_now(inode, 1); - spin_lock(&inode_lock); - inodes_stat.nr_unused--; - list_del_init(&inode->i_hash); - } - list_del_init(&inode->i_list); - inode->i_state|=I_FREEING; - inodes_stat.nr_inodes--; - spin_unlock(&inode_lock); - if (inode->i_data.nrpages) - truncate_inode_pages(&inode->i_data, 0); - clear_inode(inode); - } - destroy_inode(inode); + if (atomic_dec_and_lock(&inode->i_count, &inode_lock)) + iput_final(inode); } -} - -void force_delete(struct inode *inode) -{ - /* - * Kill off unused inodes ... iput() will unhash and - * delete the inode if we set i_nlink to zero. - */ - if (atomic_read(&inode->i_count) == 1) - inode->i_nlink = 0; }

/** ===== fs/devfs/base.c 1.39 vs edited ===== --- 1.39/fs/devfs/base.c Tue May 14 09:27:48 2002 +++ edited/fs/devfs/base.c Sun Jun 2 20:15:00 2002 @@ -2576,7 +2576,7 @@

static struct super_operations devfs_sops = { - put_inode: force_delete, + drop_inode: common_delete_inode, clear_inode: devfs_clear_inode, statfs: simple_statfs, }; ===== fs/driverfs/inode.c 1.22 vs edited ===== --- 1.22/fs/driverfs/inode.c Fri May 31 18:18:10 2002 +++ edited/fs/driverfs/inode.c Sun Jun 2 19:49:16 2002 @@ -442,7 +442,7 @@

static struct super_operations driverfs_ops = { statfs: simple_statfs, - put_inode: force_delete, + drop_inode: common_delete_inode, };

static int driverfs_fill_super(struct super_block *sb, void *data, int silent) ===== fs/ncpfs/inode.c 1.19 vs edited ===== --- 1.19/fs/ncpfs/inode.c Thu May 23 09:08:34 2002 +++ edited/fs/ncpfs/inode.c Sun Jun 2 20:14:34 2002 @@ -84,7 +84,7 @@ { alloc_inode: ncp_alloc_inode, destroy_inode: ncp_destroy_inode, - put_inode: force_delete, + drop_inode: common_delete_inode, delete_inode: ncp_delete_inode, put_super: ncp_put_super, statfs: ncp_statfs, ===== fs/proc/inode.c 1.13 vs edited ===== --- 1.13/fs/proc/inode.c Mon May 20 06:38:28 2002 +++ edited/fs/proc/inode.c Sun Jun 2 19:59:25 2002 @@ -121,7 +121,7 @@ alloc_inode: proc_alloc_inode, destroy_inode: proc_destroy_inode, read_inode: proc_read_inode, - put_inode: force_delete, + drop_inode: common_delete_inode, delete_inode: proc_delete_inode, statfs: simple_statfs, }; ===== fs/ramfs/inode.c 1.19 vs edited ===== --- 1.19/fs/ramfs/inode.c Fri May 31 18:18:10 2002 +++ edited/fs/ramfs/inode.c Sun Jun 2 19:59:56 2002 @@ -277,7 +277,7 @@

static struct super_operations ramfs_ops = { statfs: simple_statfs, - put_inode: force_delete, + drop_inode: common_delete_inode, };

static int ramfs_fill_super(struct super_block * sb, void * data, int silent) ===== fs/smbfs/inode.c 1.23 vs edited ===== --- 1.23/fs/smbfs/inode.c Thu May 23 09:08:34 2002 +++ edited/fs/smbfs/inode.c Sun Jun 2 20:14:19 2002 @@ -94,7 +94,7 @@ { alloc_inode: smb_alloc_inode, destroy_inode: smb_destroy_inode, - put_inode: force_delete, + drop_inode: common_delete_inode, delete_inode: smb_delete_inode, put_super: smb_put_super, statfs: smb_statfs, ===== include/linux/fs.h 1.131 vs edited ===== --- 1.131/include/linux/fs.h Fri May 31 18:18:14 2002 +++ edited/include/linux/fs.h Sun Jun 2 20:49:29 2002 @@ -800,6 +800,7 @@ void (*dirty_inode) (struct inode *); void (*write_inode) (struct inode *, int); void (*put_inode) (struct inode *); + void (*drop_inode) (struct inode *); void (*delete_inode) (struct inode *); void (*put_super) (struct super_block *); void (*write_super) (struct super_block *); @@ -1183,10 +1184,10 @@

extern void inode_init_once(struct inode *); extern void iput(struct inode *); -extern void force_delete(struct inode *); extern struct inode * igrab(struct inode *); extern ino_t iunique(struct super_block *, ino_t); extern int inode_needs_sync(struct inode *inode); +extern void common_delete_inode(struct inode *inode);

extern struct inode * iget5_locked(struct super_block *, unsigned long, int (*test)(struct inode *, void *), int (*set)(struct inode *, void *), void *); extern struct inode * iget_locked(struct super_block *, unsigned long); ===== kernel/ksyms.c 1.95 vs edited ===== --- 1.95/kernel/ksyms.c Fri May 31 18:18:10 2002 +++ edited/kernel/ksyms.c Sun Jun 2 19:39:17 2002 @@ -140,7 +140,6 @@ EXPORT_SYMBOL(iunique); EXPORT_SYMBOL(iput); EXPORT_SYMBOL(inode_init_once); -EXPORT_SYMBOL(force_delete); EXPORT_SYMBOL(follow_up); EXPORT_SYMBOL(follow_down); EXPORT_SYMBOL(lookup_mnt); ===== mm/shmem.c 1.52 vs edited ===== --- 1.52/mm/shmem.c Fri May 31 18:18:13 2002 +++ edited/mm/shmem.c Sun Jun 2 19:46:02 2002 @@ -1483,7 +1483,7 @@ remount_fs: shmem_remount_fs, #endif delete_inode: shmem_delete_inode, - put_inode: force_delete, + drop_inode: common_delete_inode, put_super: shmem_put_super, };

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Fri Jun 07 2002 - 22:00:13 EST