On 11/08/09 14:07, Frank Batschulat (Home) wrote:
> On Sat, 07 Nov 2009 23:10:13 +0100, Pavel Filipensky <Pavel.Filipensky at
> sun.com> wrote:
>
>
>> can I get a code review for: 6897605 Deadlock in nfs4_active_reclaim()
>> on r_lock if file and its XATTR are hashed in the same r4hashq_t
>>
>> Webrev:
>>
>> http://cr.opensolaris.org/~pavelf/6897605
>>
>> Background:
>>
>> If the system is low on memory, in can decide to reclaim memory from
>> "rnode4_cache".
>> This can lead to a deadlock if the rnode for a file uses extended
>> attributes and both
>> rnodes (file and its xattr) are hashed in the same r4hashq_t since the
>> nfsv4 code will ask for the same r_lock twice.
>>
>> The fix makes the reclaim less strict but safe - we avoid releasing of
>> xattr vnode.
>>
>
>
Hi Frank,
> I'm not much comfortable fix this fix. since this cached xattr dir vnode is
> accociated
> with the rnode we're going to deactivate - who's going to release the xattr
> vnode
> eventually ?
>
nfs4_active_data_reclaim() is not going to deactivate the 'main' rnode
which is pointing to the xattr dir vnode.
nfs4_active_data_reclaim() is only freeing various caches for the rnode,
the rnode itself remains active.
Since xattr dir vnode is one of the caches, we might decide not be so
thorough in freeing th ecaches
and keep this cache.
If the 'main' rnode is deactivated sometimes in the future, r_xattr_dir
will be correctly released,
e.g. in r4inactive() or in rp4_addfree() - no leaking vnode.
> have you considered using VN_RELE_ASYNC(xattr vnode) here instead, which would
> issue a taskq thread handling the VN_RELE() for the asynchronously ?
>
Thanks for this tip - I was not aware that there is an asynchronous
version of VN_RELE().
> of course since we're in a low memory situation, a taskq may also have
> difficulties
> being created, but at least it will eventually and we have this all
> consistant.
>
So we have 2 options:
1) remove VN_RELE(xattr)
pros: - simplify the code
cons: - less memory is freed
2) introduce VN_RELE_ASYNC(xattr)
pros: - more memory is reclaimed
cons: - slower code (extra work when the system is low on memory)
- adding a new code - need to create a new taskq (pool?)
Unless the ability to release as much memory as possible is critical, I
would prefer the simpler option - option #1.
Thanks,
Pavel
> ---
> frankB
>
