Re: [Xenomai-core] [RFC][patch] per-process data.

[Philippe Gerum <rpm@xenomai.org>]

Gilles Chanteperdrix wrote:
> The attached version of the patch do that.

Fine with me, please merge as you see fit. Details follow:

> +++ include/nucleus/ppd.h	2006-05-10 14:27:11.000000000 +0200
> @@ -0,0 +1,26 @@
> +#ifndef PPD_H
> +#define PPD_H
> +

Given that ppds are shadow-specific, I would move ppd.h's contents to 
shadow.h directly.

> +static unsigned
> +xnppd_lookup_inner(xnqueue_t **pq, xnppd_holder_t **pholder, xnppd_key_t *key)
> +{
> +    unsigned bucket = jhash2((uint32_t *)&key->mm,
> +                             sizeof(key->mm)/sizeof(uint32_t), 0);

GCC is likely going to wine on 64-bit archs for attempting to forge a 
type-punned pointer.

> +    xnppd_holder_t *ppd = NULL;
>      xnfeatinfo_t finfo;
>      u_long featmis;
>      int muxid;
> @@ -981,18 +1115,39 @@
>         chance to call xnpod_init(). */
>  
>      if (muxtable[muxid].eventcb) {
> -        int err = muxtable[muxid].eventcb(XNSHADOW_CLIENT_ATTACH);
> +        xnlock_get_irqsave(&nklock, s);
> +        ppd = xnppd_lookup(muxid, curr->mm);
> +        xnlock_put_irqrestore(&nklocks, s);
>

Looks racy to release the nucleus lock that early before actually using 
the found value, if another thread from the same process manages to 
preempt the caller and interleave a binding request on its own (would be 
stupid, I agree, but hey, most of our day-to-day job is to implement 
LARTs pretending that we are "innovating" somehow, so...)

>  
> +/* Call with nklock locked irqs off. */
> +xnppd_holder_t *xnppd_get(unsigned muxid)
> +{
> +    if (xnpod_userspace_p())
> +        return xnppd_lookup(muxid - 1, current->mm);
> +
> +    return NULL;
> +}

Same remark than previously regarding shadow.h, all the xnppd_* services 
are truely shadow-related  services, so calling them 
xnshadow_ppd*something() would be more accurate.

-- 

Philippe.