Hi -
On 2016-05-10 at 10:40 Dan Cross <[email protected]> wrote:
> This is hokey in the way we use krefs, due to racey code
> inherited from Plan 9. But it works well enough now that
> ipconfig runs and gets us a DHCP lease.
>
> Change-Id: I966a41f90a2d129eb9f07d9a7993fda990af1dbb
> Google-Bug-Id: 28667887
Do you all need this metadata in the commit log? I don't see any
Google-Bug-Id's in upstream Linux's commit log...
> /*
> + * TODO: Change this to a proper release.
> + * At the moment, this is difficult since link removal
> + * requires access to more than just the kref/struct Iplink.
> + * E.g., the self and interface pointers.
> + */
> +static void link_release(struct kref *kref)
> +{
> + (void)kref;
What's this doing? If the function does nothing, we can just have no
statements.
> @@ -1027,7 +1038,25 @@ remselfcache(struct Fs *f, struct Ipifc *ifc, struct
> Iplifc *lifc, uint8_t * a)
> if (link == NULL)
> panic("remselfcache");
>
> - if (kref_refcnt(&link->ref) > 1)
> + /*
> + * TODO: The check for 'refcnt > 0' here is awkward. It
> + * exists so that 'remselfcache' can be called concurrently.
> + * In the original plan 9 code, the 'goto out' branch was
> + * taken if the decremented reference count was exactly zero.
> + * In other threads this could become -1, which plan 9 didn't
> + * care about since the logic would be skipped over, and since
> + * 'iplinkfree' won't _actually_ free the link for five seconds
> + * (see comments in that function), and since all of the actual
> + * link manipulation happens in the thread where the reference
> + * is exactly equal to zero. But on Akaros, a negative kref
> + * will panic; hence checking for a positive ref count before
> + * decrementing. This entire mechanism is dubious. But Since
> + * this function is protected by a lock this is probably OK for
> + * the time being.
> + *
> + * However, it is a terrible design and we should fix it.
Agreed. I almost thing that if we're not going to use krefs properly, then we
shouldn't use them at all. Is link->ref just a counter then? But agreed, this
whole thing is a mess, and this commit is better than nothing.
> + */
> + if (kref_refcnt(&link->ref) > 0 && kref_put(&link->ref) != 0)
> goto out;
> +++ b/kern/src/net/iproute.c
> +static void route_release(struct kref *kref)
> +{
> + (void)kref;
Same as above.
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.
