xfs
[Top] [All Lists]

Re: [UPDATED RFC] Create with EA initial work

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [UPDATED RFC] Create with EA initial work
From: Niv Sardi <xaiki@xxxxxxx>
Date: Wed, 23 Jul 2008 15:35:23 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20080722043851.GA7244@xxxxxxxxxxxxx> (Christoph Hellwig's message of "Tue, 22 Jul 2008 00:38:51 -0400")
References: <1214196150-5427-1-git-send-email-xaiki@xxxxxxx> <1215675545-2707-1-git-send-email-xaiki@xxxxxxx> <20080722043851.GA7244@xxxxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Gnus/5.110011 (No Gnus v0.11) Emacs/23.0.60 (i486-pc-linux-gnu)
Christoph Hellwig <hch@xxxxxxxxxxxxx> writes:
> On Thu, Jul 10, 2008 at 05:39:01PM +1000, Niv Sardi wrote:
>> There is a bug in this, that I can see with the Parent Pointers code
>> going on top of it (that will be posted soon), it is basically calling
>> xfs_attr_set_int_trans() in xfs_create() just before the last
>> commit. For some reason, the first call to xfs_roll_trans (after
>> xfs_bmap_add_attrfork_trans()) will complain about the inode being
>> unlocked after xfs_trans_commit(). I understand I need to call
>> xfs_trans_ihold(ip) on it, but we already do in xfs_create() so I
>> think I must be missing something else??? any ideas ?
>
> There are multiple ways to deal with inodes linked to transactions.
>
> In all cases it needs to be linked into the transaction by
> xfs_trans_ijoin, or the opencoded equivalent for a new inode in
> xfs_trans_iget.

Shouldn't the inode already be in the transaction ? we just created it
and added it to the directory ?

xfs_create()
  xfs_dir_ialloc(tp, dp, ip)
    xfs_ialloc(tp, dp, ip)
      xfs_trans_iget(tp, ip)

and it has a i_transp after dir_ialloc.

And is why don't we use ijoin in iget as we copy the exact same code a
few lines later ? (like in [0])

>  Then you can use xfs_trans_ihold to make sure on transaction commit
> the inode reference count is not dropped and the inode is not
> unlocked, or simply grab a reference to the inode and let the
> transaction commit handler unlock it and decrement the reference
> count.  The latter is what's used by xfs_create and the former is what
> the attr code does, and as far as I can see the only things what works
> with xfs_attr_rolltrans.

hum, that seemed to work on my UML, shall we require callers to do all
the ihold, ijoin stuff or should we do that (or at least check it) from
the new attr*_trans code ?

[0] 

Attachment: txtZhqz0r4pC8.txt
Description: Text document

<Prev in Thread] Current Thread [Next in Thread>