backport iget_locked from 2.5/2.6
PATCH 1/1 backport iget_locked from 2.5/2.6
Provide an iget variant without unlocking the inode and ->read_inode call. This is needed for XFS and IIRC the reiserfs folks wanted it, too.
Tested in 2.5 for more than half a year and in 2.4-ac/-aa, and the varoius vendor trees for a long time.
fs/inode.c +18 -23
@@ -834,6 +839,20 @@return inode; } +void unlock_new_inode (struct inode *inode) +{ +/* +* This is special! We do not need the spinlock +* when clearing I_LOCK, because we're guaranteed +* that nobody else tries to do anything about the +* state of the inode when it is locked, as we +* just created it (so there can be no old holders +* that haven't tested I_LOCK). +*/ + inode->i_state &= ~(I_LOCK |I_NEW ); +wake_up (&inode->i_wait); +} +/* * This is called without the inode lock held.. Be careful. *
@@ -856,31 +875,13 @@list_add (&inode->i_list, &inode_in_use);list_add (&inode->i_hash, head); inode->i_ino = ino; - inode->i_state =I_LOCK ; + inode->i_state =I_LOCK |I_NEW ;spin_unlock (&inode_lock); -/* reiserfs specific hack right here. We don't -** want this to last, and are looking for VFS changes -** that will allow us to get rid of it. -** -- [email protected] -*/ -if (sb->s_op->read_inode2) { - sb->s_op->read_inode2 (inode, opaque) ; - }else { - sb->s_op->read_inode (inode); - } -/* -* This is special! We do not need the spinlock -* when clearing I_LOCK, because we're guaranteed -* that nobody else tries to do anything about the -* state of the inode when it is locked, as we -* just created it (so there can be no old holders -* that haven't tested I_LOCK). +* Return the locked inode with I_NEW set, the +* caller is responsible for filling in the contents */ - inode->i_state &= ~I_LOCK ; -wake_up (&inode->i_wait); -return inode; }
@@ -960,8 +961,7 @@return inode; } - -struct inode *iget4 (struct super_block *sb,unsigned long ino,find_inode_t find_actor,void *opaque) +struct inode *iget4_locked (struct super_block *sb,unsigned long ino,find_inode_t find_actor,void *opaque) {struct list_head * head = inode_hashtable +hash (sb,ino);struct inode * inode;
include/linux/fs.h +38 -2
@@ -966,6 +969,7 @@#define I_LOCK 8#define I_FREEING 16#define I_CLEAR 32 +#define I_NEW 64#define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES)
@@ -1391,12 +1396,47 @@extern void force_delete (struct inode *);extern struct inode *igrab (struct inode *);extern ino_t iunique (struct super_block *,ino_t ); +extern void unlock_new_inode (struct inode *);typedef int (*find_inode_t )(struct inode *,unsigned long ,void *); -extern struct inode *iget4 (struct super_block *,unsigned long ,find_inode_t ,void *); + +extern struct inode *iget4_locked (struct super_block *,unsigned long , +find_inode_t ,void *); + +static inline struct inode *iget4 (struct super_block *sb,unsigned long ino, +find_inode_t find_actor,void *opaque) +{ +struct inode *inode =iget4_locked (sb, ino, find_actor, opaque); + +if (inode && (inode->i_state &I_NEW )) { +/* +* reiserfs-specific kludge that is expected to go away ASAP. +*/ +if (sb->s_op->read_inode2) + sb->s_op->read_inode2 (inode, opaque); +else + sb->s_op->read_inode (inode); +unlock_new_inode (inode); + } + +return inode; +} +static inline struct inode *iget (struct super_block *sb,unsigned long ino) { -return iget4 (sb, ino,NULL ,NULL ); +struct inode *inode =iget4_locked (sb, ino,NULL ,NULL ); + +if (inode && (inode->i_state &I_NEW )) { + sb->s_op->read_inode (inode); +unlock_new_inode (inode); + } + +return inode; +} + +static inline struct inode *iget_locked (struct super_block *sb,unsigned long ino) +{ +return iget4_locked (sb, ino,NULL ,NULL ); }extern void clear_inode (struct inode *);
kernel/ksyms.c +2 -1
@@ -143,7 +143,8 @@EXPORT_SYMBOL (fget);EXPORT_SYMBOL (igrab);EXPORT_SYMBOL (iunique); -EXPORT_SYMBOL (iget4); +EXPORT_SYMBOL (iget4_locked); +EXPORT_SYMBOL (unlock_new_inode);EXPORT_SYMBOL (iput);EXPORT_SYMBOL (inode_init_once);EXPORT_SYMBOL (force_delete);
Helllo!
Quoted 3 lines · Christoph Hellwig
Provide an iget variant without unlocking the inode and ->read_inode call. This is needed for XFS and IIRC the reiserfs folks wanted it, too.
No. This patch is useless for our purposes. (and coda/nfs ). We wanted to get rid of a race where inode allocation and filling fs specific parts are done non-atomically.
The patch below does not achieve this. We still fill inode private part outside of inode_lock locked region.
Bye,
Oleg
Quoted 2 lines · Oleg Drokin
The patch below does not achieve this. We still fill inode private part outside of
inode_locklocked region.
-ENOPATCH :)
Hello!
Quoted 1 line
Mail-Followup-To: Christoph Hellwig <[email protected]>,
Hm, very interesting header, I'd say. No wonder I'm getting errors replying to your emails.
Quoted 3 lines · Christoph Hellwig
Quoted 2 lines
The patch below does not achieve this. We still fill inode private part outside of
inode_locklocked region.
-ENOPATCH:)
I meant the patch in the email you sent and to which I answered originally ;)
Bye,
Oleg
Quoted 6 lines · Oleg Drokin
Hello!
Quoted 1 line
Mail-Followup-To: Christoph Hellwig <[email protected]>,
Hm, very interesting header, I'd say. No wonder I'm getting errors replying to your emails.
Well, I got the same from you although I though only in the Cc line which I removed.
Quoted 6 lines
Quoted 4 lines
Quoted 3 lines
The patch below does not achieve this. We still fill inode private part outside of
inode_locklocked region.
-ENOPATCH:)I meant the patch in the email you sent and to which I answered originally ;)
Sorry, I missed the 'not' when reading and though you had an alternate patch
Hello!
Quoted 5 lines · Christoph Hellwig
Quoted 3 lines
Quoted 1 line
Mail-Followup-To: Christoph Hellwig <[email protected]>,
Hm, very interesting header, I'd say. No wonder I'm getting errors replying to your emails.
Well, I got the same from you although I though only in the Cc line which I removed.
That's because I hit "reply" first time and have not looked at the CC list.
Quoted 6 lines
Quoted 4 lines
Quoted 3 lines
Quoted 2 lines
The patch below does not achieve this. We still fill inode private part outside of
inode_locklocked region.
-ENOPATCH:)I meant the patch in the email you sent and to which I answered originally ;)
Sorry, I missed the 'not' when reading and though you had an alternate patch
Actually, I have altenate patch (but no, I have not tried to attach it) ;)
I have full iget5_locked backport (that I was sending around some time ago). Only it will break every iget4 user not in the tree (by changing VFS API), so I guess it is not an option. But if people think it is good idea to adapt this change, I can easily resurrect that patch, of course.
Bye,
Oleg