IRC channel logs

2024-11-23.log

back to list of logs

<damo22>youpi: are device interrupt handlers supposed to detect whether the interrupt is for them?
<youpi>yes
<damo22>then we need irqhelp handler to return int not void
<youpi>it's supposed to be just a notification that possibly the device status should be checked
<youpi>why?
<damo22>so we dont ack unless the irq was for them
<youpi>it's really supposed to be just a notification, and an ack once the driver is happy with it
<youpi>all drivers sharing the same irq get the same notification and have to check the device
<damo22>ok
<youpi>(their respective device)
<damo22>etno found that rumpdisk is getting irq 10 and 11 interrupts
<damo22>i dont see how that is possi ble
<damo22>mach_msg_server ignores the return value from demuxer
<damo22>so we dont even need return 1 or 0
<damo22>it uses ->RetCode
<damo22>is it okay if it always detects MIG_NO_REPLY?
<damo22>netbsd interrupt handlers have a return value of 0 if they didnt handle the interrupt
<damo22>if they didnt handle the interrupt, why do we ack it?
<damo22>s/handle/claim
<damo22>maybe rump will claim it later?
<youpi>the irq controller needs to know when it can re-enable the interrupt again
<youpi>which is after the drivers have told the devices that they have handled the irq
<youpi>it's especially important for level-triggered interrupt
<youpi>the in-kernel interrupt handler can't re-enable the interrupt before drivers have acked it, otherwise the interrupt would raise immediately again
<youpi>handlers returning whether they handled the interrupt is not really useful: possibly you can't know whether the event is for just one device or several
<youpi>so simpler to just notify them all and wait for all of them to ack it
<damo22>what if they get stuck waiting for each other?
<damo22>is that possible?
<youpi>each other? why?
<youpi>the driver would just check whether the device has something to tell
<youpi>if nothing, then do nothing and just ack the irq
<damo22>ok
<youpi>if the devices indeed keep telling they have things to tell, well then sure you'll keep handling them, but that's only what you want: manage what's there
<damo22>we need to debug this more
<damo22>etno discovered if you run another rump_init() on a different linked set of drivers it crashes the main rump
<damo22>or tries to hog the irq
<damo22>and the disk driver interrupts are no longer delivered
<damo22>something is still fishy with interrupt sharing i think
<damo22>we probably need to link pci-userspace with irqhelp
<damo22>and then debug from there
<damo22>supposing an interrupt happens and it gets delivered, i assume multiple device drivers may have handlers for this irq, and both receive the interrupt. if the first one is still handling it and the second one acks it first, the kernel may reenable the irq before the first driver has had a chance to ack it, then acks it... what stops the second ack from acking the next interrupt?
<damo22>ie, the next interrupt could be lost
<damo22>there will be N acks per interrupt where N is the number of drivers sharing this irq
<damo22>in irq_acknowledge() in gnumach, shouldnt the function return before calling __enable_irq() if there are unacked irqs still in the system?
<damo22>so then, it will not reenable the irq until ALL drivers have handled this interrupt
<Pellescours>if you do that, is there a risk that you wait forever ?
<damo22>no because the interrupt thread detects that
<damo22>s/that/dead drivers
<damo22>hmmm :
<damo22>[ 3.2900050] unmounting done
<damo22>irq handler [11]: release a dead delivery port f6600ed8 entry f5c5b928
<damo22>irq handler [11]: still 2 unacked irqs in entry f5c5b928
<damo22>hang
<damo22>maybe it can skip enable_irq() if e->n_unacked > N where N is the number of sharing drivers
<damo22>since there might be a pending interrupt for each driver
<damo22>but should not be more than that
<Pellescours>shared irq is something that should be solved by msi right?
<damo22>msi makes more confusion
<damo22>we need to fix ordinary interrupts first
<etno>damo22: in a shared IRQ scenario, if one handler processes the signal and determines that it has sthg to do, then it will ack to the IRQ ctrl and if the other device had triggered during that time, then the IRQ will trigger again.
<etno>If both kick concurrently (the devices trigger IRQs very close in time), then the first handler will ack, and the IRQ will kick again concurrently with the second handler acking. But since the second IRQ signal is about to be delivered, and both handlers have acked for their own device, then the thing gets stuck.
<etno>Youpi: in this case ^^, I think that we need some atomicity somewhere.
<etno>Also, damo22 's proposal about a return value could allow us to detect spurious interrupts and ack them in gnumach.
<etno>A sequential IRQ signal delivery brings significant drawbacks, though.
<etno>youpi: I said "atomicity" above when I should have used "sequential" instead
<etno>Oh! Maybe we could cheat to keep signal delivery concurrent. If the IRQ trigger message has a bool return code, then we count the positive acks and acknowledge to the controller that many times (even if in the future). Similar to a semaphore.
<youpi>damo22: yes, that's what I meant: only re-enable interrupts once all drivers have acked
<youpi>that's why we have a n_unacked field
<youpi>to count how many times drivers have acked (v.s. how many times notifications were sent)
<damo22>but during an irq_acknowledge() it enables interrupts always
<youpi>note that __enable_irq does *not* systematically re-enable, it counts as well
<youpi>since it's supposed to be usable by in-kernel drivers too
<youpi>where sharing can happen too
<youpi>n_unacked is a way for the kernel/user layer know how many times it should re-enable if the user-level driver crashes
<youpi>etno: we do already have "atomicity"
<youpi>when the first device raises the interrupt, the irq gets disabled, until the drivers acks it
<youpi>if devices raise it in the meanwhile, that's fine, that irq will get notified later
<youpi>unless the other drivers happen to get notified of the first raise after that, thus handle the device, and ack the interrupt
<youpi>for edge-triggered interrupts, the interrupt will raise again, even if all drivers have managed the devices
<youpi>for level-triggered interrupts, the interrupt will not raise again, that's fine: if all drivers have managed the devices the interrupt will not raise again and that's fine ; if one driver didn't manage its device, the interrupt will raise again
<damo22>dont we need to count how many user drivers there are per irq?
<youpi>we do
<youpi>it's the list
<youpi>we have one item per user-level driver
<damo22>yes but its not easy to get the number of shared drivers per irq
<damo22>you need to count them
<youpi>that's done in __disable_irq
<youpi>we call __disable_irq once for each user-land drive
<etno>youpi: I hadn't noticed the counting of acks. Thanks for shedding light on this 👍
<damo22>ok
<damo22>then i am confused why i get this message irq handler [11]: still 2 unacked irqs in entry .... and then it hangs
<damo22>do we need to reset tot_num_intr to 0?
<damo22>when we empty out the n_unacked?
<damo22>wont that outer loop run forever otherwise?
<damo22>while (del || irqtab.tot_num_intr);...
<youpi>Mmm, we probably need to decrement tot_num_intr, yes
<youpi>ah, no, tot_num_intr is not decremented in irq_acknowledge
<youpi>it's along interrupts-- that it's supposed to be decremented
<youpi>so it's already done, it's not releated to n_unacked
<damo22>so what if there are tot_num_intr > 0 and the unacked have been completely emptied by a del = 1
<damo22>i think that loop will never break if the queue iteration has nothing to iterate
<youpi>if del = 1, n_unacked is still the number of times it was supposed to be acked by userland
<youpi>ah sorry I was still thinking about the number of times to call __enable_irq
<youpi>for tot_num_intr it seems in del = 1 we should decrement it e->interrupts times indeed
<youpi>to account for all the interrupts that couldn't be delivered
<damo22>yeahhh
<damo22> /* Account for all interrupts that could not be delivered */
<damo22> irqtab.tot_num_intr -= e->interrupts;
<damo22> e->interrupts = 0; ?
<youpi>yes
<damo22>fixed
<damo22>no more hang
<damo22>$ sudo RUMP_NCPUS=1 RUMP_VERBOSE=1 ./test-rumpusb
<damo22>[ 3.1600050] unmounting done
<damo22>irq handler [11]: release a dead delivery port f662ea00 entry f5c53848
<damo22>irq handler [11]: still 2 unacked irqs in entry f5c53848
<etno>\o/
<damo22>i'll sleep on it and send patch tomorrow
<etno>The original goal is to make a USB server around rumpkernel (ugen), and implement a filesystem in the spirit of pci-arbiter to expose devices. Libusb would then provide low level access to bind USB-device drivers.
<damo22>rumpusbdisk currently bakes the host controller and the mass storage driver into a monolithic usb stack
<damo22>so it would be nice to be able to split it in two
<Pellescours>Just to see the state of SMP, I tried to remove the gnumach commit that create a 2 cpu pool to have all cpus used at boot. And just to notify that it hang during the rumpdisk boot. I checked quickly with the debugger and I see all rumpdisk threads in mach_msg_continue or mach_msg_receive_continue. (I don’t know exactly what it means but SMP is not ready yet for all cpus at boot)
<solid_black>hello!
<Pellescours>hello
<azert>etno: I don’t think your plan is perfect
<azert>I think that the best would be to make the usb driver pluggable, that is with dynamically load able modules, without relying solely on libusb for drivers
<azert>Of course a bugged driver would be able to bring down the full usb system, but I don’t think that libusb exposes the full potential
<azert>Pellescours: you could try to put rumpdisk alone on the multicpu pool, just to check that it is really his fault
<azert>I think the idea was to move one boot process after one to that pool and debug them one by one until it stopped deadlocking, and then get rid of the hack
<Pellescours>yes, but I didn’t get news of this since long time, and so I just wanted to see his state
<etno>azert: thanks for the idea. Having USB drivers as individual processes is the kind of things that make the Hurd appealing to me.
<etno>Both implementations can exist.
<azert>I think your plan is still exciting
<azert>What I’m worried is that you’ll end up with the full netbsd usb stack in one process + libusb
<azert>Since you’ll definitely want to reuse netbsd drivers
<azert>At that point, having dynamic loading will be an improvement
<azert>For instance, I don’t think there is any libusb driver for usb storage
<azert>Same for keyboard and mice, we definitely want these three
<azert>etno: you might want to check into vhci and usbip as another way to break the usb driver apart instead of the libusb thingy
<etno>azert: Rump is a good source of drivers, I agree. However, libusb provides a rather standardized interface, and while this causes a difficulty in the short term, I believe that the overall benefit will be worthwhile.
<azert>I agree!
<azert>But why there is no storage device at all over the whole internet running with libusb as a backend???
<etno>I would say that it will be slow, and also because Linux exists ? :-D
<azert>VHCI and USBIP should be able to do the job: one root device that enumerate all the connected devices and export them out with usbip or same protocol implemented over Mach ports. Then drivers implemented by VHCI reusing netbsd drivers at first
<azert>Maybe it is because of that! Maybe it’s just slow. Somehow I doubt it
<azert>The slowness factor is dubious: people do things that are much slower such as sending usb packets over WiFi from webcams literally all the time!
<azert>But it’s possible that your plan will develop into something better of course
<etno>I will have to read about vhci. I don't know what this is.
<azert>So please don’t listen to me and go for it if you feel the energy
<azert>VHCI is a virtual usb host controller
<azert>Basically an usb host controller without any hardware
<azert>Where you can emulate devices. What people really do is to transfer usb packets over the internet network
<azert>Netbsd 10 implements it
<etno>> So please don’t listen to me and go for it if you feel the energy
<etno>It is always good to bounce ideas !
<azert>Ok, another crazy idea: the netbsd internal interfaces are quite stable
<azert>Instead of libusb, one could think of porting that interface for the usb device nodes
<azert> https://man.netbsd.org/usbdi.9
<azert>Well.. probably the vhci thing is cleaner if one’s wants multiple rump servers to speak to each other
<azert> http://vhci.a-singer.de/vhci.pdf
<etno>It is maybe possible to have the devices behave like in netbsd, and glue a libusb on top
<azert>etno: you don’t want to reimplement usb mass storage
<azert>And nobody ever did it on libusb for a simple reason: usb is the easy part, you’ll have to reimplement the scsi interface on top of that
<azert>So you need a way to reuse usb drivers, absolutely
<azert>Changing topic, I’ve seen that hacker that had an interview with some of you a few months ago, Kent Overstreet, on the news recently
<azert>Looks like he’s getting mobbed by the Linux maintainers. Poor guy that sucks, considering how much he is investing in his stuff filesystem
<azert>Reminds me a bit of another extrovert guy that tried to push a different fs on Linux and ended up in prison for life
<gfleury>youpi: my patch for *pthread_setcancelstate* has a bug. Will investigate further more.
<azert>Filesystems and operating systems are so interconnected that it shouldn’t be trivial to make one without another
<gfleury>azert: hello, sad to see what happens to Kent also
<gfleury>azert: which guy end UP in prison?
<azert>Reiser
<azert>Hope this time it works better..
<gfleury>Hmm let me check internet
<azert>Reiser4 was a much more advanced design and got nowhere near being included in mainline
<azert>gfleury: https://arstechnica.com/gadgets/2024/01/convicted-murderer-filesystem-creator-writes-of-regrets-to-linux-list/
<gfleury>thanks
<azert>Here the full letter, quite insightful https://lore.kernel.org/lkml/b98b29cf-27d9-49e0-b10b-1848399badfd@kittens.ph/T/#u
<gfleury>azert: thank you, this story is touching
<azert>indeed
<azert> https://www.phoronix.com/news/ReiserFS-Deleted-Linux-6.13 <— two days ago! Coincidence?