Re: [PATCH 1/3] Introduce btrfs_iget helper

David Woodhouse
2008-08-13T09:07:14Z

On Tue, 2008-08-12 at 14:46 -0400, Christoph Hellwig wrote:
> Please leave the unlock_new_inode to the caller and kill the is_new
> parameter. Gives much nicer to read code, and generally making sure the
> inode is 100% ready and correct before unlocking it is a good idea, too
> so that e.g. no one can actually access an inode marked bad due to a
> generation number mismatch before it's marked bad.

Unconvinced -- I don't like the idea that we'd sometimes return a locked
inode, sometimes not, and force the callers to handle that.

And the inode _is_ ready and correct. We don't mark inodes bad because
of a generation number mismatch -- we just return -ESTALE and drop our
refcount on the inode. The inode itself is fine and there's no issue
with it being unlocked.

The code in btrfs_lookup() which goes...
/* the inode and parent dir are two different roots */
if (new && root != sub_root) {
igrab(inode);
sub_root->inode = inode;
do_orphan = 1;
}
... should also be fine when the inode is unlocked, too.

--
David Woodhouse Open Source Technology Centre
David.Woodhouse@intel.com Intel Corporation