Messages in this thread Patch in this message | | | Date | Wed, 09 Jul 1997 11:05:08 -0400 | From | Bill Hawes <> | Subject | Re: linux-2.1.44 on i586: Immediate crash on boot |
| |
Linus Torvalds wrote:
> Does this first pre-45 look better than plain 44?
The new inode code looks pretty good. I like the new list primitives; IMHO Linux needs a standardized set of list operations.
You left some massive race conditions in clear_inode and iput though -- see my recent patches against 2.0.30 for example. The attached patch takes care of these.
The question of whether clear_inode can be called with count == 0 needs to be resolved. As it presently stands a filesystem without a put function will allow the inode count to go to 0 while remaining on the inuse list. This is OK, except that when you want to reuse it you need to call clear_inode, whcih reintroduces race conditions ...
One minor nit -- you init the inode semaphore in init_once, but then again in clean_inode. Once should be enough?
Regards, Bill--- fs/inode.c.old Wed Jul 9 07:30:36 1997 +++ fs/inode.c Wed Jul 9 10:37:55 1997 @@ -184,14 +184,29 @@ /* * This is called by the filesystem to tell us * that the inode is no longer useful. We just - * terminate it with extreme predjudice. + * terminate it with extreme prejudice. */ void clear_inode(struct inode *inode) { - truncate_inode_pages(inode, 0); - wait_on_inode(inode); - if (IS_WRITABLE(inode) && inode->i_sb && inode->i_sb->dq_op) - inode->i_sb->dq_op->drop(inode); + int count; + + while ((count = atomic_read(&(inode)->i_count)) == 1) { + if (inode->i_nrpages) + truncate_inode_pages(inode, 0); + else if (test_bit(I_LOCK, &inode->i_state)) + __wait_on_inode(inode); + else if (IS_WRITABLE(inode) && inode->i_sb && + inode->i_sb->dq_op) { + set_bit(I_LOCK, &inode->i_state); + inode->i_sb->dq_op->drop(inode); + unlock_inode(inode); + } + else break; + } + if (count > 1) { +printk("clear_inode: back in use count=%d\n", count); + return; + } spin_lock(&inode_lock); inode->i_state = 0; @@ -456,23 +471,39 @@ void iput(struct inode *inode) { - if (inode) { - if (inode->i_pipe) - wake_up_interruptible(&PIPE_WAIT(*inode)); + if (!inode) + return; + + if (inode->i_pipe) + wake_up_interruptible(&PIPE_WAIT(*inode)); - /* - * Last user dropping the inode? - */ - if (atomic_read(&inode->i_count) == 1) { + /* + * Last user dropping the inode? + */ + while (atomic_read(&inode->i_count) == 1) { + if (test_bit(I_LOCK, &inode->i_state)) + __wait_on_inode(inode); + else if (test_and_clear_bit(I_DIRTY, &inode->i_state)) { + set_bit(I_LOCK, &inode->i_state); + write_inode(inode); + unlock_inode(inode); + } + else if (IS_WRITABLE(inode) && inode->i_sb && + inode->i_sb->dq_op) { + set_bit(I_LOCK, &inode->i_state); + inode->i_sb->dq_op->drop(inode); + unlock_inode(inode); + } + else if (inode->i_sb && inode->i_sb->s_op) { void (*put)(struct inode *); - if (inode->i_sb && inode->i_sb->s_op) { - put = inode->i_sb->s_op->put_inode; - if (put) - put(inode); - } + put = inode->i_sb->s_op->put_inode; + if (put) + put(inode); + break; } - atomic_dec(&inode->i_count); + else break; } + atomic_dec(&inode->i_count); } int bmap(struct inode * inode, int block) | |