DragonFly kernel List (threaded) for 2008-01
Re: Patch for inode SLIST conversion
:That's the perfect solution!
:Patch appended. _lwkt_trytokref needs the same change, as it might
:acquire the token as well. I think setting lastref = NULL in
:lwkt_reltoken is also not absolutely neccessary, but it's not wrong
:either, so I did it.
Looks pretty good. It's almost ready for commit. I see one bug and
two implementation issues in lwkt_getalltokens().
The 'if (tok->t_lastref != refs) tok->t_lastref = NULL' case should
only occur inside the 'if (tok->t_owner != td)' conditional. That
is, if the same thread has acquired the same token several times
(each with its own ref), only the most recent acquisition (the first
one found on the td_toks list) should have the t_lastref logic.
Otherwise the remaining acquisitions will cause it to be NULL'd out
The first implementation issue is the UP version. The non-SMP code
does not have a lwkt_getalltokens() procedure so the staleness is
not being tracked at all. We need to create a UP lwkt_getalltokens()
procedure as well whos only job is to run through and check t_lastref.
We may have to implement t_owner for UP too.
The second implementation issue has to do with recursive acquisition
of the same token. The current t_lastref implementation will cause
the deeper acquisition to 'stale-out' the higher level acquisition.
It could be argued that if you have procedure A acquire a token and
it calls procedure B which, unknown to procedure A also needs the token,
then procedure A should detect the temporary loss of control over
the structures represented by the token by seeing that it has become
stale. Your current implementation has this effect.
Alternatively we might want to allow recursive acquisition to NOT
indicate staleness. To do this we would have to augment lwkt_reltoken()
to detect that t_count != 0 and run the rest of the td_toks list to
find the next higher level up on the stack. If t_lastref was pointing
at the ref being released, it would be repointed at the next ref in the
recursion instead of being NULL'd out.
This would mean that we would have to implement a getalltokens/relalltokens
procedure for the UP case as well as the SMP case, plus also implement
t_owner and t_count for the UP case. This is fairly simple to do...
we would basically just remove the SMP conditionals from most of the code
and make the SMP code the same as the UP code, with some minor differences
in that the UP code doesn't need the spinlock and getalltokens can't
'fail' to acquire the token.
This will require some surgery, would you like to have a go at it?
Despite the work needed it won't actually complicate the code any more,
it's more on the order of a rearrangement.