On Thu, 2010-10-28 at 09:46 +0200, Philippe Gerum wrote: > On Thu, 2010-10-28 at 09:31 +0200, Jan Kiszka wrote: > > Am 28.10.2010 07:17, Philippe Gerum wrote: > > > On Tue, 2010-10-26 at 21:33 +0200, Jan Kiszka wrote: > > >> Am 26.10.2010 07:22, Jan Kiszka wrote: > > >>> Will come up with two patches for stable, one for I-pipe and one for > > >>> Xenomai, later today. Then we can discuss which cases I'm missing. > > >> > > >> While meditating over my approach (which turned out to be less trivial > > >> as expected - of course), I also reconsidered your current patches. The > > >> concerns I had (forwarding of spurious IRQ to Linux) turned out to be > > >> harmless (Linux will ignore such few spurious events). > > >> > > > > > > That is not even an issue if you consider the sequence to be > > > xnarch_disable_irq then ipipe_control (new version, doing a critical > > > entry to flip the irq mode). > > > > When you want to support shared IRQs, xnarch_disable_irq is tabu. I > > suppose you meant some my_device_disable_irqs(). > > No, it is perfectly valid provided you made sure that no handler > remained on the shared list. There is absolutely no reason to keep a > line unmasked if no device is supposed to be active on it. Hence the > release sequence described earlier. > > > > > > > > >> Still, the approach to sync via shutting down the line for the current > > >> domain before xnintr_irq_detach doesn't work for us. It only works if > > >> xnintr_irq_detach actually detaches from the line, but it breaks if > > >> there are users remaining. > > >> > > >> We need intrlock to check if we are the last user while removing > > >> ourselves from the list. And we cannot postpone line detaching after the > > >> critical section as we may otherwise race with the next registration on > > >> that line. IOW, I don't see how to solve the issue without moving the > > >> drain after the detach and making the detach safer instead. > > >> > > >> Do you agree? > > >> > > > > > > I agree this is not trivial, for sure. To keep things simple, I would > > > introduce a new "teardown" flag to freeze the descriptor, thus avoiding > > > further attachments, while xnintr_detach can probe the shared list for > > > lingering users, and eventually call xnarch_disable_irq > > > +xnarch_ignore_irq+xnarch_release_irq in sequence with all locks > > > dropped, if empty. > > > > > > The only adverse effect I can see ATM would be some concurrent caller of > > > xnintr_detach() blocked on the teardown flag on another CPU, albeit it > > > _could_ have joined the bandwagon, attaching the irq, in case the shared > > > list proved to remain active (and thus xnarch_release_irq was not > > > called). But this may also look like a simple way to prevent live > > > locking of interrupt descriptors. YMMV. > > > > This sounds like it's best discussed based on patches. > > > > Likely, yes. I'll have a look when time allows.
The following patches implements the teardown approach. The basic idea is: - neither break nor improve old setups with legacy I-pipe patches not providing the revised ipipe_control_irq call. - fix the SMP race when detaching interrupts. The last patch also fixes two other issues: - do not alter the irq descriptor (e.g. cookie and stats) if the attachment fails early - do not set irq affinity before the validity checks, and set it only for the first handler introduced in the shared list. http://git.xenomai.org/?p=xenomai-rpm.git;a=commit;h=ea771aec61c399dc85e14d3de571cffaa5b9b1e7 http://git.xenomai.org/?p=xenomai-rpm.git;a=commit;h=4fa4643f8e59ca0528b059f82e97c0b37419e62e > > > Jan > > > -- Philippe. _______________________________________________ Xenomai-help mailing list [email protected] https://mail.gna.org/listinfo/xenomai-help
