IRC channel logs

2021-03-07.log

back to list of logs

<damo22>hehe
<damo22>fixed
<youpi>damo22: can't pci_port be passed as a parameter to enum_devices ?
<youpi>I don't see anything else needing it beyond enum_devices
<youpi>ah, it's already passed as root port, but can't pci_system_hurd_create just release it?
<damo22>erm, not sure
<damo22>it has tricky recursion
<damo22>it would need to be another param i guess
<youpi>ah, when recursing you pass another port ok
<youpi>Mmm, but you overwrite the cwd_port parameter anyway?
<youpi>i.e. you just need to keep the root port at each call, and not pass the cwd_port ?
<damo22>ive had tons of iterations on this, i forget how it works
<youpi>since it's the "parent" path that grows
<damo22>youre probably right
<damo22>i had it working with and without dir_readdir at one point, so when i cleaned it up i probably overlooked it
<damo22>do you want me to remove the global port?
<youpi>yes please
<youpi>global variables are usually ugly :)
<youpi>for the sort part, discussion would probably be needed with upstream. I'm not sure to understand: are you saying that you are getting duplicated entries if you don't sort entries?
<youpi>pci-arbiter is not supposed to provide duplicated entries like that
<youpi>it looks to me like another bug that shall be fixed
<damo22>yes there are dups if the list is not sorted
<damo22>but when you sort both the x86 ones and the hurd ones, no more dups
<damo22>so it kind of makes sense to sort inline
<youpi>"both" ?
<damo22>the x86 method list
<youpi>are you probing both with the x86 and the hurd methods ?
<youpi>I mean, upstream will frown quite a bit if only one backend needs such kind of sorting, while usually backends simply provide the proper list already
<damo22>yes, because the static arbiter uses the x86 method, and exposes hurdish method, then the rumpdisk probes using hurdish?
<youpi>they aren't in the same process
<youpi>so rumpdisk will have its own list, from the hurd method
<youpi>there is no reason for duplicates here
<youpi>it'll just get the list from the the static arbiter
<youpi>which got it from x86
<damo22>yes but the duplicates arise from the pcifs.c being stupid
<youpi>how so?
<damo22>assuming sorted list
<youpi>can't we provide a sorted list?
<damo22>yes we can sort in pcifs.c
<youpi>no I mean can't we just always have it sorted, from the start?
<damo22>but its easier to have it upstream
<youpi>how is it that it's not keeping in the right order ?
<damo22>ahh
<damo22>the x86 method has some recursion that does not generate the list in order or something
<damo22>i think
<youpi>I don't see how that coujldn't have posed problems before already
<youpi>and now pose problem that we'd have not seen before
<damo22>it was coincidence that qemu does not have issues
<damo22>but it affects real hw
<damo22>we need to have the list in order in libpciaccess
<youpi>that I understand
<damo22>or every time we use it, sort them
<damo22>but that is bad
<youpi>what I don't understand is how we'd end up in non-sorted order
<youpi>pci_system_x86_scan_bus does probe in order
<youpi>so I don't see how x86 could be building a non-ordered list
<damo22>i dont know, it gets out of order
<damo22>somewhere
<youpi>then something is fishy
<youpi>and that's to be fixed rather than brown-tape it
<damo22>maybe the hurd method of recursion does not work
<damo22>generates them out of order
<damo22>in libpciaccess
<damo22>it did my head in
<damo22>i hate recursion
<youpi>put prints, and you'll see how the recursion proceeds
<damo22>all i know is, when i sort the devices after they are generated for both backends, everything works nice
<damo22>Asking all remaining processes to terminate...killall5: can't read from 5/stat
<damo22>killall5: can't read from 6/stat
<youpi>pci-arbiter creates its entries in the order provided by pci_devices_next()
<youpi>so I believe at least the hurd method doesn't need the sort
<youpi>but I see in the x86 this:
<youpi>case PCI_HDRTYPE_BRIDGE: pci_system_x86_scan_bus (secbus);
<damo22>yes
<youpi>I guess possibly your hardware has a bridge, and that indeed will mix up buses
<youpi>so the x86 backed would then need sorting after probing everything
<damo22>YES
<youpi>that I guess upstream would understand and accept
<damo22>ok cool
<damo22>ive split patches into two
<damo22>and removed globals
<damo22>will send shortly
***Server sets mode: +nt
***Server sets mode: +nt
<youpi>- .destroy = pci_system_x86_destroy,
<youpi>don't we want to keep it?
<damo22>we cant unless we have a global for the port
<damo22>how would i pass in the pci_port?
<youpi>? x86_destroy doesn't use the pci_port ?
<youpi>it just calls ioperm
<damo22>ideally we device_close ?
<youpi>? are we talking about the same thing ?
<youpi>I'm talking about pci_system_x86_destroy
<damo22>yeah
<youpi>not the hurd backend at all
<damo22>oh
<youpi>well, the hurd_pci_methods structure
<youpi>but the x86 part of it
<damo22>i removed it because i had another method there
<damo22>that calls device_close
<youpi>in the previous patch yes, but now you can restore it
<damo22>but i couldnt work out how to close it without the port passed in
<damo22>right
<youpi>I mean restore leaving it as it was :)
<damo22>ok i can do that
<damo22>does it even matter? do we need to close io ports on hurd?
<damo22>i suppose if we lock them in kernel it matters
<youpi>yes, and also to let the process avoid keeping i/o privileges after closing libpciaccess
<damo22>ok
<damo22>sent
<youpi>(I guess you tested that e.g. netdde is still working fine with this updated libpciaccess?)
<damo22>yeah i booted with settrans /servers/bus/pci /hurd/pci-arbiter.static recompiled with my changes
<damo22>and shelled into the box
<damo22>linked with new libpciaccess.a
<damo22>but i have not tested the dynamic lib
<damo22>with existing pci-arbier
<damo22>with existing pci-arbiter linked dynamically to new libpciaccess, i get :fsysopts: /servers/socket/2: -i /dev/eth0 -a 0.0.0.0 -m 255.0.0.0: (ipc/mig) server died
<damo22>Error getting interfaces; Translator died
<youpi>settrans the translator with -a to get messages of the translator
<damo22>root@zamhurd:~# settrans -a /servers/bus/pci /hurd/pci-arbiter
<damo22>root@zamhurd:~#
<youpi>I mean the translator that dies
<damo22>ok
<youpi>apparently here it's pfinet that dies, probably out of netdde dying as well
<youpi>so settrans netdde by hand
<damo22>if netdde is already running i get settrans: /dev/netdde: Device or resource busy
<youpi>you can kill it or use -fg along -a
<damo22>netdde runs fine
<youpi>then settrans pfinet by hand as well
<damo22>/hurd/pfinet: device_open on /dev/eth0: (os/device) no such device
<damo22>but /dev/eth0 is a devnode to /hurd/netdde
<damo22>/dev/netdde
<youpi>did netdde not find the ethernet card?
<youpi>does lspci work ?
<damo22>lspci works
<damo22>00:02.0 Ethernet controller: Intel Corporation 82574L Gigabit Network Connection
<youpi>does netdde find it?
<youpi>when starting it by hand
<damo22>i cant tell but last line is l4dde26_register_rx_callback: New rx callback @ 0x1047630.
<damo22>it probes a lot but does not mention eth0
<damo22>yeah it fails to probe
<damo22>works with pci-arbiter.static
<damo22>(using my latest branch)
<damo22>maybe the path needs to be _SERVERS_BUS_PCI instead of "." for the fallback path
<damo22>so there could be another conditional in there
<damo22>(in libpciaccess)
<youpi>no, it's a relative path since you use file_name_lookup_under
<damo22>yea but the "under" pathway might fail and then it uses file_name_lookup only?
<damo22>i mean in that case
<youpi>?
<damo22>if there is no pci "device"
<youpi>it just looks up in the dir port
<youpi>I mean
<youpi>pci_system_hurd_create does file_name_lookup (_SERVERS_BUS_PCI
<youpi>and then it passes that to enum_devices
<youpi>and enum_devices uses relative paths from there
<damo22>yeah but parent is "."
<youpi>so what?
<damo22>ok
<youpi>that's relative to the root
<youpi>_under does relative lookup
<damo22>i see
<youpi>relative to the root port it's given
<youpi>its a openat() basically
<damo22>ok so i dont know why this is failing
<damo22>lspci -A hurd works
<damo22>but only as root!
<youpi>iirc that's what we have currently
<damo22>netdde uses libpciaccess.so.0
<damo22>i thought it might be a symlink thing but it looks okay
<damo22>could it be machdev changes?
<damo22>because now there is a new thread for machdev
<damo22>but netdde was not updated
<damo22>and i did not install the libmachdev.so
<damo22>maybe netdde needs a pthread_exit(NULL)
<damo22>Version: 0.0.20200330-6
<damo22>that is my version of netdde
<youpi>ah yes it needs to be refreshed, see my latest commit in the incubator
<youpi>(actually, your change that I commited)
<damo22>i think the pci entries are broken in netfs_impl.c and only got fixed in my version of the arbiter
<damo22>there is something fishy going on in the existing arbiter that does not play nice with the updated libpciaccess
<damo22>it could even be the count variable that you pointed out
<damo22>because we are requesting exactly one directory entry at a time and looping over them
<damo22>ok have submitted upstream to libpciaccess
<damo22>ok i split my patches for pci-arbiter and found a subset that can be applied as is
<damo22>and it fixes the bugs
<damo22>with updated libpciaccess
***rekado_ is now known as rekado
***TigerbotHesh is now known as Guest90648