Messages in this thread | | | Date | Sat, 01 Jun 2002 12:19:36 -0700 | From | Andrew Morton <> | Subject | Re: [patch 12/16] fix race between writeback and unlink |
| |
Linus Torvalds wrote: > > On Sat, 1 Jun 2002, Andrew Morton wrote: > > > > So run __iget prior to dropping inode_lock. > > This part looks horrible: > > + spin_unlock(&inode_lock); > + iput(inode); > + spin_lock(&inode_lock);
Yup. The inode refcounting APIs are really awkward. Note how I recently added dopey code in ext2_put_inode() to only drop the prealloc window on the "final" iput().
> Why not just split up the code inside iput(), and then just do > > if (atomic_dec(&inode->i_count)) > final_iput(inode); > > where final_iput() _wants_ the spinlock held already? > > That's basically what "iput()" will end up doing, except for that > "put_inode()" thing, which is just a horrible hack anyway. > > So get rid of "put_inode()", and replace it with a new one that takes the > place of the > > if (!inode->i_nlink) { > ... delete .. > } else { > .. free .. > } > > and makes that one be a "i_op->drop_inode" thing that defaults to the > current "delete if i_nlink is zero, free it if i_nlink is not zero and > nobody uses it". > > The general VFS layer really shouldn't have assigned that strogn a meaning > to "i_nlink" anyway, it's not for the VFS layer to decide (and it only > causes problems for any non-UNIX-on-a-disk filesystems). >
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.
- - 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/
| |