On 2015-11-28 at 18:09 'Davide Libenzi' via Akaros wrote:
> I was planning to use chan->aux to stick my perf device private data
> (IOW the session), but now I think I see the problem which Barrett was
> mentioning me about weird refcounting.
> In devclone() we simply do:
>
> new_channel->aux = old_channel->aux;
>
> Which is not going to be nice once both channels are closed.IMO the
> device needs a proper ->clone() opportunity (NULL allowed) to incref
> its own data, otherwise things gets messy.
Yeah, the 9ns rules for channels and refcnts are pretty messy. I have
a bunch of notes I've been meaning to clean up and put in
Documentation. But I still have some unsolved mysteries with this
code, and might be wrong on a bunch of things.
Here's the gist:
When it comes to walking and refcnts, I think of there being two types
of chans within 9ns. I call them "distinct" and "temporary." Temporary
chans are used during walks. I think that temp chans are not
closed. Distinct chans are the final output of a successful walk.
Those will definitely be closed by the device, once for every
attach or distinct walk.
There are two sources of distinct chans: walk and attach. So from a
device's perspective, if the walk() or attach() functions in struct dev
succeed, then we can be guaranteed that the chan will be closed with
the device's close() method.
Now, it is a little unclear to me whether or not temporary chans are
closed with a device's close method. I think they are not.
Rudimentary debugging says they aren't, and we can also attempt to
enforce that.
There's another issue with walk succeeding - it's not enough for the
walk to succeed, but the walk has to lead to a different chan object.
That's where the word 'distinct' comes from. It's possible for a
walk() to succeed, but end up at the same place it started (long
story). That's why I do the c == wq->clone check in eventfd. For
refcnting, we'll need to know how many times it'll be closed. if c ==
nc, then it will only get closed once. Maybe 'distinct' is a bad word
to use here, since the chan is still 'finalized', it's just a question
of how many times it'll be closed.
As far as devclone() goes, right after that we set the type == -1,
which means the chan has no official type (and that -1 means we won't
call our device's close either).
nc = devclone(c);
nc->type = -1; /* device doesn't know about this channel yet */
So simply doing a devclone won't trigger a device channel close. I
think the type = -1 is being used as a flag inside k/s/ns/dev.c to
differentiate between Temporary and Distinct chans.
Here's another wrinkle. All of this is separate from whether or not a
chan has had open() called on it. Device chans will have their close()
method called even if they haven't been opened. 9ns has a COPEN flag
(for c->flag) that devices can use to determine if they have been
opened or not. COPEN'd chans are distinct, AFAIK.
So there's COPEN'd chans. The un-COPEN'd chans are called "kernel
internal" (by me, in the code in a few places). The "kernel internal"
can be either "distinct" (definitely belonging to the device) or
temporary. And when a walk succeeds, there's a corner case where we
didn't walk to a new, distinct chan and in that case we only cclose
once.
Yikes.
I'll try to think about this a bit more and come up with some helpers
for device writers.
In the meantime, you might be able to ignore how devclone sets c->aux,
since you only need to worry about closes being called on distinct
chans that came from attach or walk.
Barret
--
You received this message because you are subscribed to the Google Groups
"Akaros" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
For more options, visit https://groups.google.com/d/optout.
