Re: [Xenomai-core] nucleus/pipe.c patch: check for lingering close at xnpipe_connect()
On Thu, 2009-09-10 at 12:49 -0400, Andreas Glatz wrote: > Hi, > > > > > > Whenever possible, please post patches inline (I had to manually copy it > > to allow commenting). > > Sure, no problem. > > > > > > You cannot bluntly call release() here. You may not run in Linux context > > while that handler demands this. > > It's also called in xnpipe_disconnect() (if I follow 'goto cleanup' and > the user-space isn't connected). So is there a different policy here? > I mean should xnpipe_connect() just be called from rt context whereas > xnpipe_disconnect() has to be called from non-rt context? > > > > Moreover, releasing the state is the > > job of the NRT user that opened it and obviously still has a hand on it. > > Yeah, that's the tricky part. I left that part out yet ... > > > > > (And the locking is imbalanced here, but that is shadowed by the other > > flaws.) > > Agree. > > If we had that new pipe behaviour it could significantly > speed-up debugging for us because we wouldn't have to stop (and > eventually start) all NRT applications before restarting the RT > application. > > Thanks for commenting on my quick and dirty patch. I think, now > I know where to continue. > Allowing the RT side to be re-opened multiple times won't fly, I'm afraid. The reason we have the linger-on-close behavior is to prevent the NRT side to refer to stale memory whenever the RT side preempts and disconnects. Excluding RT from preempting the write/read calls could cause high latencies (NRT may sleep anyway, so locking would not even be a safe option there). Maybe you should move the RT_PIPE descriptor in a separate, standalone module from your driver to keep the connection alive, and ignore the -EBUSY status upon failed attempt to connect to the same minor from your driver? That descriptor is shareable in kernel space so there is no restriction in using it from another module, and this would allow your driver module to be unloaded without actually tearing down the data channel with your userland app. > Andreas > > ___ > Xenomai-core mailing list > Xenomai-core@gna.org > https://mail.gna.org/listinfo/xenomai-core -- Philippe. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] nucleus/pipe.c patch: check for lingering close at xnpipe_connect()
Hi, > > Whenever possible, please post patches inline (I had to manually copy it > to allow commenting). Sure, no problem. > > You cannot bluntly call release() here. You may not run in Linux context > while that handler demands this. It's also called in xnpipe_disconnect() (if I follow 'goto cleanup' and the user-space isn't connected). So is there a different policy here? I mean should xnpipe_connect() just be called from rt context whereas xnpipe_disconnect() has to be called from non-rt context? > Moreover, releasing the state is the > job of the NRT user that opened it and obviously still has a hand on it. Yeah, that's the tricky part. I left that part out yet ... > > (And the locking is imbalanced here, but that is shadowed by the other > flaws.) Agree. If we had that new pipe behaviour it could significantly speed-up debugging for us because we wouldn't have to stop (and eventually start) all NRT applications before restarting the RT application. Thanks for commenting on my quick and dirty patch. I think, now I know where to continue. Andreas ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] nucleus/pipe.c patch: check for lingering close at xnpipe_connect()
Andreas Glatz wrote:
> Hi,
>
> Currently, you can destroy RT_PIPE while a non-rt application is
> connected. If that happens the pipe won't be fully closed but
> some parts will be leftover for cleanup when closing the RT_PIPE
> from the non-rt application (lingering close).
>
> For us it would be very convenient if we didn't have to close
> the non-rt side before restarting the rt-side (currently, the
> rt-side complains bc the non-rt side didn't resolve the lingering
> close).
>
> Here is a patch where the rt-side resolves the lingering close
> when connecting to the RT_PIPE. I tested it and it seems to work
> fine. Do you have any concerns?
>
> Andreas
>
Whenever possible, please post patches inline (I had to manually copy it
to allow commenting).
> --- pipe.orig.c 2009-09-09 17:23:10.0 -0400
> +++ pipe.c2009-09-09 17:32:48.0 -0400
> @@ -309,17 +309,35 @@
> int xnpipe_connect(int minor, struct xnpipe_operations *ops, void *xstate)
> {
> struct xnpipe_state *state;
> - int need_sched = 0, ret;
> + int need_sched = 0, test_lclose = 0, ret;
> spl_t s;
>
> - minor = xnpipe_minor_alloc(minor);
> - if (minor < 0)
> + ret = xnpipe_minor_alloc(minor);
> + if (ret >= 0)
> + minor = ret;
> + else
> + if(ret == -EBUSY)
> + test_lclose = 1;
> + else
> return minor;
>
> state = &xnpipe_states[minor];
>
> xnlock_get_irqsave(&nklock, s);
>
> + if(test_lclose) {
> + if(testbits(state->status, XNPIPE_KERN_LCLOSE)) {
> + clrbits(state->status, XNPIPE_KERN_LCLOSE);
> + xnlock_put_irqrestore(&nklock, s);
> + state->ops.release(state->xstate);
You cannot bluntly call release() here. You may not run in Linux context
while that handler demands this. Moreover, releasing the state is the
job of the NRT user that opened it and obviously still has a hand on it.
(And the locking is imbalanced here, but that is shadowed by the other
flaws.)
> + } else {
> + xnlock_put_irqrestore(&nklock, s);
> + return -EBUSY;
> + }
> + }
> +
> + xnlock_get_irqsave(&nklock, s);
> +
> ret = xnpipe_set_ops(state, ops);
> if (ret) {
> xnlock_put_irqrestore(&nklock, s);
IIUC, you are looking for a way to keep a pipe open on the NRT side
while the RT user is disappearing and re-appearing? Not sure if this is
feasible at all with current xnpipe. But if it is, I think you rather
have to introduce some kind of reset on the affected xnpipe_state.
Jan
signature.asc
Description: OpenPGP digital signature
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
