[Haiku-commits] r21516 - haiku/trunk/src/add-ons/kernel/busses/usb

[Michael Lotz]

> Hi,
> 
> I think this patch might introduce some bugs.

Yes and I am fully aware of all the things you just mentioned. I had 
the very same thoughts while I was doing it. The patch was not designed 
to fix anything isochronous related, I actually was thinking about just 
disabling it completely. Please visit http://dev.haiku-os.org/ticket/1284 
to see why I had to do something. It is not acceptable that a kernel 
thread takes up so much of the CPU that the system becomes unusable. I 
did not notice this since I did not run Haiku on a host with UHCI after 
applying the patch. I only reviewed it from a code correctness 
standpoint and I was ok with that all compiled cleanly in the end.
Sorry but I have to assume that you did never run Haiku and tested your 
changes? This should have been pretty obvious to notice.

> I thought that having another thread would improve performances on a
> multiprocessor machine.

Yes, multi-threading usually helps when there is more than one 
execution unit. The problem here is that you need to synchronize the 
two threads and interrupt handler. The interrupt on complete does 
trigger the finisher thread. And you could do the same for the iso 
finisher thread too, but instead of adding another semaphore and 
duplicating the semaphore creation, handling and resetting, it just 
seemed much cleaner to only use one thread that is triggered by the 
existing semaphore and then have this thread execute both finisher 
functions.

My changes are not of a high quality as you pointed out, and are not 
there to stay. You can revert my changes completely and also add the 
thread again, but you have to fix that problem. If you can fix it with 
the extra thread this is fine by my, otherwise I'd advise you to think 
about just using the single threaded approach and adapt the isochronous 
finisher so that it works with this setup.

Regards
Michael