Re: Odd code in iput() (since 2.1.60). What for?
On Mon, 22 Mar 1999, Stephen C. Tweedie wrote: > Hi, > > On Sat, 20 Mar 1999 15:46:18 -0500 (EST), Alexander Viro > <[EMAIL PROTECTED]> said: > > > Folks, could somebody recall why the check for I_DIRTY had been > > added to iput()? AFAICS it does nothing. If the inode is hashed and clean > > it's already on inode_in_use, otherwise we are in *big* trouble (the only > > reason for that might be crazy ->delete_inode() doing > > insert_inode_hash(). > > To maintain an LRU ordering for recently released inodes, I imagine. > However, right now there's nothing I can see in inode.c which actually > relies on that ordering: whenever we do a free_inodes(), we dump all > the inodes that we can. In the future, having a sane LRU ordering on > the in-use list may be valuable. Oh, OK. That makes sense. I missed the reodering side-effect. Thanks ;-) In FAT fixes (I'll post the them for testing tomorrow or on Wednesday) I actually needed a clear way to keep references to inode that wouldn't affect i_count and related behaviour. I did it - it looks so: 1. Whenever we are scheduling the inode for freeing (free_inodes, invalidate_inodes and two places in iput) we set I_FREEING in ->i_state. 2. I've added struct inode *igrab(struct inode*) that grabs the spinlock, checks for I_FREEING, if it is set - releases the spinlock and returns NULL, otherwise increases i_count, releases the spinlock, does wait_on_inode() and returns the inode. 3. If fs wants to manage private references to struct inode it can use igrab()/iput() to get/release a normal reference. It has to supply ->clear_inode() method and forget all private references to inode when it is called. Some simple internal locking may be needed within the filesystem, indeed. igrab() returning NULL should be processed as the case when the reference was already invalidated by foo_clear_inode() call. Changes to fs/inode.c are minimal (+10 lines) and do not affect existing filesystems. If fs wants to keep an internal hash indexed *not* by i_ino it can do it easily (e.g. FAT - we can keep i_ino constant over the whole life of in-core inode and forget about bothering with 'busy' directory slots. There go 20-odd races in FAT-derived filesystem + tons of ugly code working around other 20-odd races). In case of FAT all code needed to implement hash indexed by directory entry position + glue for clear interaction with icache took about 40 lines. *And* allowed to remove much more cruft. So there... Cheers, Al
Re: Odd code in iput() (since 2.1.60). What for?
Hi, On Sat, 20 Mar 1999 15:46:18 -0500 (EST), Alexander Viro <[EMAIL PROTECTED]> said: > Folks, could somebody recall why the check for I_DIRTY had been > added to iput()? AFAICS it does nothing. If the inode is hashed and clean > it's already on inode_in_use, otherwise we are in *big* trouble (the only > reason for that might be crazy ->delete_inode() doing > insert_inode_hash(). To maintain an LRU ordering for recently released inodes, I imagine. However, right now there's nothing I can see in inode.c which actually relies on that ordering: whenever we do a free_inodes(), we dump all the inodes that we can. In the future, having a sane LRU ordering on the in-use list may be valuable. --Stephen
Re: Odd code in iput() (since 2.1.60). What for?
On Sat, 20 Mar 1999, G. Allen Morris III wrote: > It is my understanding that iget() will return an inode even if > its link count is zero. You still have to call iput on the > inode to get i_count back to zero. And I don't think that > this inode would be DIRTY. Ahem... How does it matter? If the iput() in question doesn't drive i_count into 0 the code in question isn't executed at all. If you are giving up the last reference *and* nlink is zero - the thing will be removed from hash, ->delete_inode() will be called and unless it will rehash the sucker (which it shouldn't and doesn't) clear_inode() will be called and inode will got to freelist. Code in question not executed again. The only cases when it may be executed and be *not* a no-op: a) delete_inode() decides to link the sucker somewhere instead of killing it for good and rehashes it. Will work *only* if the inode will be also made clean. Doesn't look right for me, besides nobody actually tries to pull such stunts. b) inode we've called is clean, hashed and not on the main list (and we gave up the last in-core reference). In this case we transfer it to the main list. Not to mention the fact that it can never happen (AFAICS) it sounds really odd. > It may be that there should be an iget...() that does not return > deleted inodes. (I could use it in NFSD, to get at the inode cache.) It would be *wrong* (and mostly pointless) on normal filesystems and it can be trivially checked on any fs, anyway, so... Cheers, Al
Re: Odd code in iput() (since 2.1.60). What for?
It is my understanding that iget() will return an inode even if its link count is zero. You still have to call iput on the inode to get i_count back to zero. And I don't think that this inode would be DIRTY. It may be that there should be an iget...() that does not return deleted inodes. (I could use it in NFSD, to get at the inode cache.) Allen >>>Alexander Viro said: > Folks, could somebody recall why the check for I_DIRTY had been > added to iput()? AFAICS it does nothing. If the inode is hashed and clean > it's already on inode_in_use, otherwise we are in *big* trouble (the only > reason for that might be crazy ->delete_inode() doing insert_inode_hash(). > Was it a preparation for undelete or what?) I have to tweak inode.c for > FAT fixes and I'm not too happy about the perspective of tweaking code if > I don't understand its parts ;-/ Especially in such place. > Code in question had been added in 2.1.60 and I couldn't find any > discussion in archives ;-/ Could somebody help with it? > Cheers, > Al > > > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to [EMAIL PROTECTED] > Please read the FAQ at http://www.tux.org/lkml/ - G. Allen Morris III
Odd code in iput() (since 2.1.60). What for?
Folks, could somebody recall why the check for I_DIRTY had been added to iput()? AFAICS it does nothing. If the inode is hashed and clean it's already on inode_in_use, otherwise we are in *big* trouble (the only reason for that might be crazy ->delete_inode() doing insert_inode_hash(). Was it a preparation for undelete or what?) I have to tweak inode.c for FAT fixes and I'm not too happy about the perspective of tweaking code if I don't understand its parts ;-/ Especially in such place. Code in question had been added in 2.1.60 and I couldn't find any discussion in archives ;-/ Could somebody help with it? Cheers, Al
