Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> This is an attempt to fix the bugs found in the minor number management: >> - changing the bitmap requires atomic operations >> - clrbits/setbits work against xnarch_atomic_t, and that is only 32 bit >> wide on x86-64 > > I think we should rather fix xnarch_atomic_t to be the size of a long on > x86-64, than having to cope with xnarch_atomic_t of various sizes in the > code (there are probably other places where we depend on the size of > xnarch_atomic_t).
IIRC, not all Linux atomic ops exits for atomic64_t, so we would have to provide our own versions. Moreover, there might be a deeper reason for Linux staying at 32 bit atomics on x86-64. Performance? Just speculating now, but I wouldn't touch this for Xenomai without an urgent need. > > As for the find_first_bit, why not simply taking the nklock > xnpipe_minor_free ? This is a first masking section, so this should not > matter a lot. Yeah, probably the better approach - back to non-atomic ops: ---------> Instead of trying to perform the minor release lockless (and failing due to 32-bit atomic ops on x86-64), let's go back to nklock-protected bitmap handling. Signed-off-by: Jan Kiszka <[email protected]> --- ksrc/nucleus/pipe.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ksrc/nucleus/pipe.c b/ksrc/nucleus/pipe.c index d7409da..d201f5d 100644 --- a/ksrc/nucleus/pipe.c +++ b/ksrc/nucleus/pipe.c @@ -78,9 +78,8 @@ static inline int xnpipe_minor_alloc(int minor) static inline void xnpipe_minor_free(int minor) { /* May be called with nklock free. */ - clrbits(xnpipe_bitmap[minor / BITS_PER_LONG], - 1UL << (minor % BITS_PER_LONG)); - xnarch_memory_barrier(); + __clrbits(xnpipe_bitmap[minor / BITS_PER_LONG], + 1UL << (minor % BITS_PER_LONG)); } static inline void xnpipe_enqueue_wait(struct xnpipe_state *state, int mask) @@ -414,7 +413,9 @@ cleanup: } else { xnlock_put_irqrestore(&nklock, s); state->ops.release(state->xstate); + xnlock_get_irqsave(&nklock, s); xnpipe_minor_free(minor); + xnlock_put_irqrestore(&nklock, s); } if (need_sched) @@ -622,8 +623,8 @@ int xnpipe_flush(int minor, int mode) clrbits((__state)->status, XNPIPE_KERN_LCLOSE); \ xnlock_put_irqrestore(&nklock, (__s)); \ (__state)->ops.release((__state)->xstate); \ - xnpipe_minor_free(xnminor_from_state(__state)); \ xnlock_get_irqsave(&nklock, (__s)); \ + xnpipe_minor_free(xnminor_from_state(__state)); \ } \ } while(0) _______________________________________________ Xenomai-core mailing list [email protected] https://mail.gna.org/listinfo/xenomai-core
