IRC channel logs

2020-08-02.log

back to list of logs

***Server sets mode: +nt
***Server sets mode: +nt
<damo22>youpi: did you see this? https://lists.gnu.org/archive/html/bug-hurd/2020-08/msg00000.html
<junlingm>just send in 3 patches to implement a new interface for user space irq handling. Any comments?
<junlingm>To the bugs-hurd list.
<junlingm>the new scheme is to open an irq0 - irq15 device, make a block read, which returns when an interrupt occurs, and ack the interrupt by writing to the device.
<damo22>what was wrong with the old one?
<junlingm>that each device must implement the device_intr_register and device_intr_ack functions, even though they are not the irq devices.
<damo22>libmachdev is a library that implements the mach device interface
<damo22>it only needs to be implemented once in there
<junlingm>ok, but every translator has implemented the two functions, right?
<damo22>no i dont think so
<junlingm>of course they are all stubs returning not supported. But etill.
<damo22>i havent seen your patches yet
<junlingm>For example, in devnode: http://git.savannah.gnu.org/cgit/hurd/hurd.git/tree/devnode/devnode.c#n248
<junlingm>it probably requires moderator permission to be sent to the list?
<damo22>idk
<damo22>maybe you need to subscribe
<junlingm>I did subcribe, anyway, I will wait fora day. It they have not appeared, I will send again.
<damo22>well i did make it an irq "device" but i think i also made it part of the device RPCs
<damo22>to use IRQ currently you have to open the "irq" device
<damo22>but it will refuse to use the RPCs unless the device is called "irq"
<junlingm>yes I know. I liked the idea that we open an irq device. I just wonder why other devices have to implement these two functions.
<damo22>well because its a new kind of operation
<junlingm>here is the second case of implement these two functions in eth-multiplexer. http://git.savannah.gnu.org/cgit/hurd/hurd.git/tree/eth-multiplexer/device_impl.c#n205
<junlingm>but they are only meaningful to irq.
<damo22>yeah but its not a write or read op
<jrtc27>why can you not just make it a different device type?
<jrtc27>surely mach allows different sets of RPCs for different device types?
<junlingm>I am actually thinking about suing device_set_status to ack, and read would be meaningful for an irq.
<jrtc27>(plus some generic ones shared between them)
<junlingm>using
<damo22>jrtc27: im not sure that is a true statement
<jrtc27>well
<jrtc27>you can have a universal namespace for IDs
<jrtc27>but anything not implemented automatically return as such
<junlingm>me neither. But I am new. So what do I know?
<junlingm>So how to declare a new device class?
<damo22>junlingm: thank you anyway for your patches, but before you rush ahead to write code, its probably a good idea to discuss what is required on the mailing list
<junlingm>Oh, another advantage(?) of using a read is that the trranslater just need to do a while loop for blocking read.
<junlingm>Sdamo22: sure. I taked about it here. You probably did not see the conversation. I will send to the list next time.
<junlingm>damo22: ^^^
<damo22>:) no problem
<damo22>but then we will have N devices, one per irq
<damo22>currently its multiplexed with a parameter, isnt it
<junlingm>Well I did the same as your patch, used only irqdev, but each time you do an op, there is a minor device number, which specifies irq number.
<damo22>ok
<junlingm>but, in the userspace, it probably makes sense to have /dev/irq0 - /dev/irq15 to back it up.
<jrtc27>do you really want all those device nodes for every possible IRQ?
<jrtc27>things get out of hand once you get beyond simple devices like an 8259 or the APIC
<junlingm>good question. In the future when we move to IOAPIC and LAPIC, we cannot possibly have 2000 devices.
<jrtc27>this is why the interface is the way it is
<junlingm>mach device server automatically does multiplexing though. it is like blk devices. There would be a new device_t structure created each time an irq* is opened, but there is really only one device backing it up.
<damo22>the way i wrote the current implementation, each irq has an indirection that is multiplexed to one device
<damo22>so you can have different numbers of irqs based on which architecture is running
<damo22>and they can be mapped differently to the hardware at compile time
<junlingm>I reused your user_intr_t to represent each irq line. If a device is not opened, there would be no device_t or user_intr_t created.
<damo22>with mine, it appends to a linked list when a new irq is opened i think
<damo22>not opened, used
<damo22>you only open "irq" once
<junlingm>here is the first part of the patch, where I moved your deliver_user_intr to a linux interrupt handler function. https://pastebin.com/embed_js/2de91KHe
<damo22>we are trying to deprecate the linux/ code
<damo22>ideally it will be removed completely so the kernel will have no drivers in it
<junlingm>I know. It just centralizes the interrupt handling. so when we move over, it will be a whole move.
<junlingm>anyway, I certainly do not insist that my patch is useful, I just wanted to address the conceptual problem that device_intr_register and device_intr_ack must be attached to all devices.
<damo22>im not experienced enough to review your code, try to ensure it does appear on the ML and it will be reviewd
<junlingm>there are probably bettr way to solve the problem then moving to read/write ops.
<junlingm>in fact, as I said earlier here, I was playing with EHCI. Now I can get async list working, I wanted to write a device node translator, but was wondering why I need to write device_intr_* even if my code does not provide irq service.
<damo22>USB does have interrupts
<damo22>if youre not using them it doesnt mean they dont exist
<junlingm>it does have interrupt endpoints, but they are just period transactions.
<junlingm>true. Maybe I am a purist in that sense :) That is why I said there might be other solutions. My patch may not be the right solutions.
***retropikzel_ is now known as retropikzel
<damo22>i cant seem to install bootstrapped rumpdisk as a translator
<damo22>it doesnt fail but it doesnt work
<damo22>i tried this http://git.zammit.org/hurd-sv.git/commit/?id=c68af43dbd602d39713449e7375d511e99d57733
<damo22>root@zamhurd:~# ls -la /dev/rumpdisk
<damo22>ls: cannot access '/dev/rumpdisk': Not a directory
<damo22>i expected it to be a CHRDEV
<damo22>do i need to implement io_S.h ?
<damo22>trivfs_S_io_stat
<youpi>damo22: yes, not processed yet
<youpi>jrtc27, damo22: RPC interfaces are meant to separate out sets of operations to implement, so you don't have you put in stubs, the intr RPCs could have gone to another interface, but we can as well just use device_read/write and achieve the same
<youpi>damo22: for being able to ls it you'd need to implement some things yes
<youpi>personally I'd say don't bother much
<youpi>implementing stat so it should be as some meaningful node should be enough
<damo22>youpi: well i thought not being able to ls it was an issue because it isnt letting me mount another partition on rumpdisk
<damo22>doesnt it need to have stat ability so it can be looked up?
<youpi>damo22: I don't remember what libdiskfs will require exactly
<youpi>but seeing it as a block device probably won't harm :)
<damo22>char device perhaps?
<youpi>I'm wondering if libstore can be persuaded to use device_read/write instead of io_read/write
<youpi>no, block device
<youpi>it really is a block device
<damo22>it provides a number of block devices
<youpi>ah you are talking about rumpdisk, sorry
<youpi>I'd still tend to rather think of it as a block device
<damo22>ok
<youpi>character device would still mean you can open()/read() it
<youpi>and get a stream of characters
<damo22>i see
<damo22>i thought everything that wasnt an actual block device but was special got char
<youpi>that's a common thing to do yes
<youpi>be here behind it's a series of block devices, so better advertise as block device
<damo22>ok sure
<damo22>IF_BLK or somthing
<damo22>youpi: file_set_translator, do i use _HURD_BLKDEV ?
<youpi>yeah
<damo22>or implement stat?
<damo22>the latter being to change the io_stat to S_IFBLK
<youpi>if ext2fs provides the stat RPC it's fine to just let it do
<damo22>hmm doesnt look like it
<youpi>damo22: that said, libtrivfs should already coping with this
<youpi>similarly to the way it does for netdde
<damo22>libtrivfs/io-stat.c
<damo22>err = file_set_translator (node, 0, FS_TRANS_SET, 0, NULL, 0, rumpdisk_port, MACH_MSG_TYPE_COPY_SEND);
<damo22>err does not backtrace but the translator i installed doesnt do anything
<youpi>what is rumpdisk_port coming from?
<damo22>created a port on trivfs_cntl_class and a send right on it
<damo22>i did that right after arranging the shutdown notification
<youpi>"doesn't do anything": that's not precise enough :)
<damo22>mm maybe it is doing *something* but cant open the same device twice?
<youpi>does it return an error?
<youpi>something else?
<youpi>trivfs doesn't care
<youpi>can you stat it? ls -l it?
<youpi>you can e.g. for a start make sure whether trivfs_S_fsys_getroot is going on as expected
<damo22>ls -l /dev/rumpdisk says not a directory
<youpi>and staT?
<damo22>didnt try stat
<damo22>cannot statx /dev/rumpdisk, not a directory
<youpi>also look with rpctrace what RPCs actually get called and what they return
<damo22>dir_lookup ("dev/rumpdisk" 64 0) = 0x4000002d (Operation not supported)
<youpi>is that the first dir_lookup call?
<youpi>put another way: is that really the RPC to rumpdisk, or to ext2fs ?
<youpi>normally what happens is that an RPC is done to ext2fs first with "dev/rumpdisk", and it returns a return saying to retry on the rumpdisk port, with the empty path
<damo22>task59(pid636)->vm_map (19771392 8192 0 0 76<--67(pid636) 221184 32 3 7 1) = 0x3 ((os/kern) no space available)
<damo22>that was in the middle somewhere
<youpi>that's not completely unexpected
<youpi>there are loading steps that get this kind of thing
<youpi>I'm wondering whether you'd need to fill fsys->underlying
<youpi>see trivfs_startup
<youpi>we call it without the underlying node at first (since we can't provide any initially, that's expected)
<youpi>but we might have to fill it later, for dir_lookup, io_stat etc. to be able to work
<youpi>notably trivfs_S_fsys_getroot uses it
<youpi>and that's what ext2fs would use on the control port to get access to the trivial translator FS
<youpi>I'd say to just put mach_prints to make sure what actually is happening within ext2fs and rump
<damo22>ok
<youpi>at some point you'll want to understand exactly how this is working anyway
<damo22>thanks, will do
<jrtc27>junlingm: as I said to AlmuHS the other day, when you're submitting patches for review, _please_ properly follow the style guide rather than having things like highly non-conforming whitespace
<AlmuHS>hi, a quick question. In which file is "kfree()" declared? I have a warning advice about kfree() is implicity declared in apic.c. But I don't know what #include I might to put here
<AlmuHS>I'm searching It with grep, but I don't find the declaration
<AlmuHS>ok, I found It in kern/kalloc.h . But It has different parameters than the mine
<AlmuHS>I was using kfree(old_list); , but with this function this might be kfree((vm_offset_t) old_list, sizeof(old_list));
<junlingm>AlmuHS: yes I realized that. I am using BBEdit, it somehow mixes tabs with spaces, and I don't know how to change that yet. Will fix it.
<junlingm>AlmuHS: yes the interface is kfree(vm_offset_t, size_t)
<AlmuHS>junlingm: check if your editor allows to show tabs and spaces graphically
<AlmuHS>in CodeBlocks, by example, this option is in the Editor configuration
<junlingm>ok. I will check. Maybe I will just change to Atom.
<AlmuHS>CodeBlocks is a good editor for C and C++
<AlmuHS>although configure the compilation using Makefile It's a few lazy
<junlingm>AlmuHS: here is the declaration for kfree in kern/kalloc.h:34, http://git.savannah.gnu.org/cgit/hurd/gnumach.git/tree/kern/kalloc.h#n34
<AlmuHS>thanks
<AlmuHS>I don't like the "implicit declarations" ;)
<junlingm>AlmuHS: Good idea. Will look into it. What is the guideline for spaces in hurd? tabs only? spaces only? mixed tabs and spaces? or as long as consistent?
<youpi>AlmuHS: you're right, not having the declaration means potential miscall
<youpi>the size parameter definitely is needed
<AlmuHS>yes, I've just fix the kfree call
<youpi>junlingm: consistent with the file you are patching, CMU mach style otherwise
<AlmuHS>I'm doing some little refactorizations to make the code simpler
<junlingm>on my patches? I don't see them on teh mailing list yet. I might have mistakenly send them using a different email address from the one I subscribed with.
<AlmuHS>I will send the refactorizations when I finish and test them ;)
<youpi>junlingm: your patches got on the list, several times :)
<junlingm>AlumHS: I can't seem to find the gnumach coding style document. Do you have a link or something?
<AlmuHS>see FreeBSD style. It's pretty similar
<youpi>there was a discussion about it the other day on IRC
<junlingm>It did! I did not receive it from the list myself.
<youpi>AlmuHS: would have an actual proper link that we can put on the wiki?
<youpi>junlingm: mailing lists are often configured not to send back mails to senders
<junlingm>ok. But yes, a link would be helpful.
<junlingm>youpi: oh.
<AlmuHS>jrtc27 send me a man page with the FreeBSD coding style. I go to search It
<AlmuHS>this https://www.freebsd.org/cgi/man.cgi?query=style%289%29
<AlmuHS>after the refactor , the code seems faster over Qemu
<AlmuHS>I've removed the if(ptr == NULL) checks in many functions which had been checked this in its caller
<junlingm>Thanks AlmuHS.
<AlmuHS>if I check apic != NULL in acpi_apic_init() before call acpi_apic_setup(), It makes not sense check if(apic == NULL) again in acpi_apic_setup()
<AlmuHS>I go to push the latest changes
<AlmuHS> https://github.com/AlmuHS/GNUMach_SMP/commit/1ddfd5de239b506849b7e1cc487e7db2f8f65e40
<AlmuHS> https://github.com/AlmuHS/GNUMach_SMP/commit/736d84268d129a44814a984ede933bc19632f8da
<AlmuHS> https://github.com/AlmuHS/GNUMach_SMP/commit/bd452eb2662c722f55169c9ebccc9599d57311fd
<junlingm>in hurd, do we have a queue implementation similar to gnumach? or we have to roll our own?
<AlmuHS>queue as data structure?
<junlingm>yes.
<AlmuHS>I'm not sure
<AlmuHS>now I'm testing my latest modifications over real hardware. Using the Thinkpad T60 again
<AlmuHS>in fact, over real hardware, now my code is faster
<AlmuHS>... or not. I've just notice that I forget do "git pull" before compile :-/
<AlmuHS>now yes: after pull the latest changes, now I can confirm that my code is faster
<AlmuHS>although the microkernel keeps freezed in exec loading most times yet
<AlmuHS>well, the freezed is Hurd, not Mach, is not?
<gnu_srs2>Hi, maybe a newbie question, but anyway: How do I restart a translator being replaced in /hurd (except rebooting)?
<jrtc27>showtrans foo
<jrtc27>settrans -pk foo <output>
<jrtc27>for an active translator
<jrtc27>see /var/lib/dpkg/info/hurd.postinst for how it restarts the networking translators (involves some pfinet-specific mangling)
<jrtc27>hm, except, that's for passive translators
<jrtc27>probably -ag for active?
<AlmuHS>in Debian wiki, there are an article about Hurd translator
<AlmuHS>wait a minutes
<AlmuHS>here: https://www.debian.org/ports/hurd/hurd-doc-translator
<AlmuHS>jrtc27 and gnu_srs2 check this https://www.debian.org/ports/hurd/hurd-doc-translator
<AlmuHS>I've removed the printf which prints a line in the cpus list, and now the Hurd freeze in exec translator is less common
<AlmuHS>in real hardware, I said
<jrtc27>so put it back? reliable reproducibility is useful for debugging such issues.. :)
<jrtc27>but yes, adding/removing print statements is an extremely common way to perturb timings such that race conditions do/don't show up
<AlmuHS>i've removed almost prints, except the ACPI tables and the CPU/IOAPIC lists
<AlmuHS>*most