Re: Adding attr, inode reference query

[Timothy Shimmin <tes@sgi.com>]

Dave Chinner wrote:
> On Tue, Oct 07, 2008 at 11:04:32AM +1100, Barry Naujok wrote:
>> I'm doing a bit of debugging with attr creation in xfs_repair which uses
>> libxfs which has it's own simple cache/ref counting/transaction mechanism
>> for inodes and buffers.
>>
>> I came across a refcounting issue when adding an extended attribute to an
>> inode, calling xfs_attr_set_int (indirectly in Phase 6):
>>   - if there are no extended attributes, a attr fork area is created within
>>     the inode (calling xfs_bmap_add_attrfork). After this call in libxfs,
>>     the inode is derefenced.
>>   - if extended attributes already exist, the inode isn't dereferenced
>>     after calling xfs_attr_set_int.
>>
>> I seem to have traced this down to xfs_bmap_add_attrfork not calling
>> xfs_trans_ihold after calling xfs_trans_ijoin like other similar functions.
>> BUT, it does call IHOLD(ip).
> 
> The difference between the two is kinda subtle. IHOLD() increments
> the reference count to ensure the transaction commit doesn't drop
> the last reference to the inode when it unlocks it and hence
> cause us to enter reclaim in the commit code.
> 
> OTOH, xfs_trans_ihold() holds the inode across the transaction
> commit so that it is still locked when xfs_trans_commit() completes.
> This is needed for rolling transactions to be able to continue
> across duplication and commit without needing to relock inodes.
> 
Oh okay.
Want a reference held in both cases, but don't always want it locked
after commit.
One way, we take an extra reference and then drop it at commit,
the other we just don't drop the reference at commit.

--Tim