07:33 karolherbst[d]: i509vcb[d]: oh, it's more about the kernel asking running applications to free memory they strictly don't need, e.g. caches.
13:09 conan_kudo[d]: karolherbst[d]: it is theoretically possible with systemd-oomd
13:09 conan_kudo[d]: it was something that I had talked to them about a few years ago
13:10 karolherbst[d]: right, but it would send signals to all the processes, no?
13:10 karolherbst[d]: or would we have a better way of doing that?
13:11 karolherbst[d]: I'm sure systemd could add an API for services linking to libsystemd or whatever, but that won't really fly in the general case
13:11 conan_kudo[d]: a D-Bus oriented API or a varlink one would probably make more sense
13:11 conan_kudo[d]: adding an API requires figuring out what we want out of it first
13:12 conan_kudo[d]: which was the challenge we had when trying to figure this out three years ago
13:12 karolherbst[d]: though could also just kill applications not respecting the signal
13:13 karolherbst[d]: a bit drastic, but mhhh
13:13 karolherbst[d]: the thing is rather, how would we want mesa to make use of it
13:13 karolherbst[d]: like.. mesa is a lib
13:13 karolherbst[d]: and I'm sure mesa starting to require d-bus would cause quite the shitstorm
13:14 magic_rb[d]: Yeah please dont
13:15 karolherbst[d]: ~~linux, the only OS without a desktop bus everybody accepts being there~~
13:16 blockofalumite[d]: karolherbst[d]: T- the what
13:16 blockofalumite[d]: Yea, please don't
13:16 karolherbst[d]: I mean.. on any other OS that system works over something like d-bus
13:16 magic_rb[d]: karolherbst[d]: I do get the need for dbus, but also, i dont know if dbus is the right solution here
13:16 magic_rb[d]: Like i also get the need for systemd, and i still dislike it
13:16 karolherbst[d]: wanna use signals?
13:17 karolherbst[d]: wanna have a custom protocl?
13:17 tiredchiku[d]: carrier pigeons
13:17 blockofalumite[d]: I need to properly delve into dbus someday
13:17 karolherbst[d]: dbus is there, it's IPC, but yeah....
13:17 blockofalumite[d]: tiredchiku[d]: Sounds like a rad name for a new system bus
13:17 blockofalumite[d]: libcpigeon
13:17 conan_kudo[d]:sighs
13:17 karolherbst[d]: mesa could also just link against libsystemd instead probably
13:18 conan_kudo[d]: I wish we had a unified kernel+user IPC
13:18 karolherbst[d]: ~~you mean d-bus~~
13:18 blockofalumite[d]: karolherbst[d]: Please no, what about Void, Alpine, Artix, Devuan
13:18 tiredchiku[d]: karolherbst[d]: <a:Pepefight:857348186119864320>
13:18 blockofalumite[d]: FreeBSD
13:18 karolherbst[d]: blockofalumite[d]: they can stay 10 years behind for all I care
13:18 conan_kudo[d]: blockofalumite[d]: Alpine would probably adopt systemd at some point in the near future
13:18 tiredchiku[d]: karolherbst[d]: won't that break a *lot* of compatibility
13:18 karolherbst[d]: tiredchiku[d]: yes
13:18 conan_kudo[d]: karolherbst[d]: ~~I mean Binder~~
13:18 karolherbst[d]: though
13:18 blockofalumite[d]: karolherbst[d]: Not using systemd is not being behind, it's having a different philosophy
13:18 karolherbst[d]: it could be optional
13:18 tiredchiku[d]: cringe
13:19 karolherbst[d]: conan_kudo[d]: right....
13:19 karolherbst[d]: though then the question is what it would give you.. there were ideas to get dbus into the kernel, but that kinda stalled for reasons and dbus-broker solved most of hte issues
13:19 conan_kudo[d]: then dbus-broker itself stalled
13:19 karolherbst[d]: mhh, does it need more work though?
13:20 conan_kudo[d]: as we load more things on the bus, it definitely needs more perf work
13:20 karolherbst[d]: I see
13:20 conan_kudo[d]: karolherbst[d]: it would give us a richer, unified IPC that even supports namespaces and is per-seat for "free"
13:20 karolherbst[d]: blockofalumite[d]: I would agree if an alternative would be as advanced as systemd
13:20 karolherbst[d]: and I don't mean launchd
13:21 conan_kudo[d]: and launchd requires the Mach kernel IPC
13:21 tiredchiku[d]: the solution to every problem, is rm -rf
13:21 karolherbst[d]: conan_kudo[d]: right...
13:21 blockofalumite[d]: karolherbst[d]: Again, different philosophy. Many of us *don't* want our init system to be millions of lines of code
13:21 tiredchiku[d]: this conversation has happened on this server at least 5 times before
13:21 karolherbst[d]: yeah...
13:21 tiredchiku[d]: the differing philosophies one
13:21 tiredchiku[d]: no point running the laps again
13:22 karolherbst[d]: I mean, I see the idea, the thing is, we might want to get notified of OOM situations in a consistent way
13:22 tiredchiku[d]: also, Discord channel #nouveau🔗
13:22 blockofalumite[d]: tiredchiku[d]: I don't agree there
13:22 karolherbst[d]: and there are only so many options we have
13:22 karolherbst[d]: people are allowed to come up with alternatives
13:22 tiredchiku[d]: blockofalumite[d]: I mean, it's established no one's budging from their position here
13:22 karolherbst[d]: but people won't wait on them
13:22 conan_kudo[d]: Linux is the only major operating system with no unified IPC by default, which is honestly really frustrating
13:23 blockofalumite[d]: There's a lot of other frustrating things about Linux heh
13:23 karolherbst[d]: so if it's a systemd API or a dbus API the first that emerges, it's going to get used
13:23 karolherbst[d]: it's really that simple
13:23 conan_kudo[d]: the systemd API already kind of exists
13:23 conan_kudo[d]: but I don't think it's the "right shape" for what we want out of it
13:23 karolherbst[d]: part of the service stuff?
13:23 conan_kudo[d]: yeah
13:23 karolherbst[d]: I see
13:23 blockofalumite[d]: karolherbst[d]: Yea, not by everyone tho, yipee, all hail fragmentation
13:23 blockofalumite[d]: As if we already don't have such problems with Wayland vs Portals x3
13:24 conan_kudo[d]: the portals largely don't overlap with Wayland though
13:24 karolherbst[d]: anyway...
13:24 karolherbst[d]: the one thing I always liked about frameworks like cocoa on macos is, that there are callbacks for all sorts of notifications just available, because it was the defacto API to use
13:24 tiredchiku[d]: Discord channel #nouveau🔗
13:24 karolherbst[d]: and there are a looot of great things in there
13:24 conan_kudo[d]: karolherbst[d]: yeah, it's similar on Windows too
13:25 blockofalumite[d]: Are we in the right channel here
13:25 conan_kudo[d]: the API design hurts my brain, but the concept is solid
13:25 karolherbst[d]: and mesa getting notified of "you are running out or (V)RAM" would be one interesting event to get notified on
13:25 conan_kudo[d]: absolutely
13:25 conan_kudo[d]: and it's not like the system doesn't know about it
13:25 conan_kudo[d]: it's just nothing to connect that information to create an action
13:25 karolherbst[d]: blockofalumite[d]: we uhm.. went a bit too far from the original topic
13:25 karolherbst[d]: conan_kudo[d]: yeah
13:26 karolherbst[d]: and the obvious choice is to use something something systemd, or signals or d-bus atm
13:26 karolherbst[d]: there isn't really any other option
13:26 karolherbst[d]: signals are pain, so I'm sure it's not going to be signals
13:26 conan_kudo[d]: signals probably doesn't work because of the whole namespace problem
13:26 karolherbst[d]: yeah.. and having multiple handlers for hte same signal and ...
13:26 karolherbst[d]: and the C api is kinda scuffed
13:26 conan_kudo[d]: crossing namespace boundaries is really hard with signals
13:27 conan_kudo[d]: well, doing so with any kind of fidelity anyway
13:27 conan_kudo[d]: it works with systemd because it's often the ns manager
13:27 karolherbst[d]: yeah...
13:27 karolherbst[d]: you could even only notify a special slice or something
13:27 karolherbst[d]: or seat
13:27 conan_kudo[d]: absolutely
13:27 conan_kudo[d]: that's part of the oomd model
13:28 karolherbst[d]: yeah...
13:28 karolherbst[d]: oomd handling it wouldn't be the worst place here as it's monitoring resource usage already
13:28 karolherbst[d]: might just need some more code to deal with VRAM
13:28 conan_kudo[d]: for dbus, there usually is a mediation mechanism that rewrites and bridges between the ns boundary, but how that works varies across mediation systems
13:28 conan_kudo[d]: yup
13:28 karolherbst[d]: systemd usually communicates through d-bus already anyway
13:28 conan_kudo[d]: it would actually be helpful for desktops overall
13:28 tiredchiku[d]: this conversation is still better suited for Discord channel #tech-talk
13:28 karolherbst[d]: and if mesa could just register some callback for oom events
13:28 tiredchiku[d]: I think
13:29 conan_kudo[d]: this matters for nouveau just like any other GPU driver
13:29 karolherbst[d]: maybe somebody should prototype something and we'll discuss it with others on dri-devel
13:29 karolherbst[d]: but like
13:29 tiredchiku[d]: which is why
13:30 karolherbst[d]: that's a lot of work
13:30 tiredchiku[d]: anyway, am gonna go play something
13:30 karolherbst[d]: have fun
13:30 conan_kudo[d]: karolherbst[d]: maybe we should talk to Anita or Daan and see if they have some ideas on how to proto this
13:30 karolherbst[d]: yeah, maybe
13:31 karolherbst[d]: the issue is we don't have a common VRAM usage UAPI, so that's gonna be a bit of a problem
13:31 karolherbst[d]: it's all userspace APIs for now
13:33 conan_kudo[d]: that's probably something to fix then
13:33 conan_kudo[d]: it's one of those things that is frustrating about the DRM API, it should be more uniform than it actually is
13:34 conan_kudo[d]: admittedly I have similar frustrations with VFS
13:42 blockofalumite[d]: karolherbst[d]: Now if only I would properly understand what the discussion is about so I could participate
13:43 karolherbst[d]: blockofalumite[d]: like... if the system runs out of VRAM (or RAM) it should be able to tell applications to release resources it doesn't strictly need, e.g. caches
13:43 blockofalumite[d]: From what I gathered, it's about asking processes that are using the GPU to free VRAM when the VRAM gets full?
13:43 karolherbst[d]: other OS, especially mobile ones, have it as part of their main toolkit and API
13:44 blockofalumite[d]: karolherbst[d]: Mm, yea, that does sound quite useful
13:44 karolherbst[d]: but system libs, e.g. mesa, also do cache resources internally
13:44 karolherbst[d]: and it would be kinda silly that each application to call some GL/CK/VK API to flush those caches
13:45 karolherbst[d]: and I'm pretty sure that other OS already handle those things internally as well
13:45 blockofalumite[d]: Can't the app just register a callback? And if it needs handling from the kernel, the kernel could have a kernel socket to poll
13:45 karolherbst[d]: yeah, but like.. do you want every application to register a callback and call `gtk_please_flush_caches` or `qt_please_flush_internal_caches` or....
13:45 karolherbst[d]: what about applications not doing it?
13:45 blockofalumite[d]: I mean I would presume that the toolkit would handle that
13:46 karolherbst[d]: yeah.. maybe _that_ would be good enough, but then it needs vk/gl/etc extensions
13:46 karolherbst[d]: which _might_ be a way
13:46 blockofalumite[d]: Yea, a VK/GL extension imo is the best way
13:46 conan_kudo[d]: mesa _is_ the toolkit from my PoV
13:46 karolherbst[d]: but then every app has to do it
13:46 conan_kudo[d]: it's what applications link to for this
13:46 karolherbst[d]: and we could get the same result without apps having to do that
13:46 conan_kudo[d]: so Mesa itself should do it
13:47 blockofalumite[d]: I am thinking, from the kernel side, have a socket, and have mesa subscribe to the socket and flush driver caches
13:47 blockofalumite[d]: For app-local caches, aka on top of mesa, have a VK and GL ext to register a callback, because every app needs to have its own custom code cuz only the app knows what its caches are
13:47 blockofalumite[d]: Have higher-level toolkits like GTK or QT that use VK or GL internally just register to this callback and internally drop their own caches
13:48 blockofalumite[d]: That's what I would do, anyway
13:48 conan_kudo[d]: so now we're back to needing kernel IPC
13:48 blockofalumite[d]: I mean we already have smth like that, no? Take eg. FUSE, that uses a kernel socket
13:48 conan_kudo[d]: not really no
13:48 karolherbst[d]: yeah, so people would just use whatever systemd/dbus would provide and for people not liking systemd/dbus to add an alternative a bit later
13:48 blockofalumite[d]: I don't follow there
13:48 karolherbst[d]: we won't add a custom protocol for that stuff
13:49 karolherbst[d]: or rather, I doubt anybody would want to do it
13:49 karolherbst[d]: systemd-oomd would want to provide something probably anyway at some point, and that's probably going to get used
13:49 blockofalumite[d]: I don't follow there
13:49 blockofalumite[d]: What would a userspace IPC be for?
13:49 karolherbst[d]: that's the socket you mention above
13:50 karolherbst[d]: something needs to broadcast the "low RAM" event across multiple processes anyway
13:50 karolherbst[d]: and oomd could e.g. also do it only for certain seats or be more fine grained here
13:50 karolherbst[d]: some high prio service won't need to flush, where low prio stuff will have to
13:50 blockofalumite[d]: karolherbst[d]: I don't follow
13:50 karolherbst[d]: you can add a loot of policy stuff on top of that
13:51 blockofalumite[d]: The kernel has to manage this, how does a userspace IPC help
13:51 karolherbst[d]: why does the kernel have to?
13:51 blockofalumite[d]: What else would ?
13:51 karolherbst[d]: systemd-oomd?
13:51 blockofalumite[d]: How would that come to know information about VRAM?
13:51 karolherbst[d]: it has all the knowledge
13:51 karolherbst[d]: blockofalumite[d]: some UAPI
13:51 karolherbst[d]: the same as for RAM
13:51 blockofalumite[d]: That it would poll?
13:52 karolherbst[d]: could also do it event based if you wanted to
13:52 conan_kudo[d]: it already has to for everything else
13:52 karolherbst[d]: but yeah.. linux ain't that great for async events like this, but that's a different thing to fix anyway :ferrisUpsideDown:
13:52 conan_kudo[d]: it listens for events through udev but also polls for things as it needs to
13:52 blockofalumite[d]: karolherbst[d]: I don't follow there
13:53 blockofalumite[d]: Through what API? And why can the other processes not do the same?
13:53 karolherbst[d]: like you can have a blocking syscall which unblocks once there is new data or something, then it's async
13:53 karolherbst[d]: blockofalumite[d]: policies
13:53 blockofalumite[d]: Wdym by policies
13:53 karolherbst[d]: maybe youj don't want every process to flush caches
13:53 karolherbst[d]: maybe you have high prio and low prio ones
13:53 karolherbst[d]: maybe you have high and low prio sessions
13:54 karolherbst[d]: maybe you want certain applications to flush at 25% free, others only at 5%
13:54 blockofalumite[d]: Why not have that be config that mesa pulls ?
13:54 conan_kudo[d]: with namespace boundaries, security rules, and priority models, it gets hard to do this right without a central manager
13:54 karolherbst[d]: blockofalumite[d]: then you need an IPC anyway
13:54 conan_kudo[d]: e.g. you probably don't want flushing for a renderfarm
13:54 blockofalumite[d]: karolherbst[d]: I don't follow, it could just read a config file
13:54 karolherbst[d]: it won't be a config file
13:54 karolherbst[d]: because it relies on runtime information
13:55 conan_kudo[d]: right, it's dynamic
13:55 karolherbst[d]: I wished things were that simple, but they really aren't
13:55 karolherbst[d]: of course, you _could_ have a simple base model
13:55 karolherbst[d]: but once you want more fine grained and advanced control it gets complicated
13:55 conan_kudo[d]: really once you are multiprocess it gets complex
13:55 conan_kudo[d]: your multiprocess model can make this very hard
13:55 blockofalumite[d]: I mean, you could have each app handle it, where the callback only tells it *hey, memory is used above xyz, you might wanna do smth depending on your priority*
13:55 karolherbst[d]: we'll probably have more and more system services using the GPU
13:56 conan_kudo[d]: all the simple cases rely on cooperative modeling, but modern systems are preemptive/preemptible
13:56 karolherbst[d]: and you don't really want a random user app to annoy it
13:56 blockofalumite[d]: conan_kudo[d]: This already relies on all apps being cooperative and dropping the caches on request
13:56 karolherbst[d]: blockofalumite[d]: then you need a handshake mechanism to tell that certain instance how important it is
13:56 blockofalumite[d]: Yep, and that's for each app to figure out
13:57 karolherbst[d]: blockofalumite[d]: not if mesa handles it internally without the knowledge of the app
13:57 conan_kudo[d]: blockofalumite[d]: well, it doesn't have to be... the model I'm thinking of is that the system and user can preempt applications through policy and mesa itself
13:57 blockofalumite[d]: karolherbst[d]: Is it about driver caches or app caches?
13:57 karolherbst[d]: blockofalumite[d]: driver
13:57 conan_kudo[d]: app caches are a harder problem, but rarely a VRAM issue
13:58 karolherbst[d]: yeah.. apps that care could also subscribe to those events if they bother, but many won't
13:58 karolherbst[d]: toolkits could also do it for internal caches
13:58 conan_kudo[d]: if mesa does it, nobody else really needs to
13:58 karolherbst[d]: for RAM it already kinda exists
13:58 conan_kudo[d]: yup
13:58 karolherbst[d]: so yeah.. VRAM is the bigger concern here, but some applications might also cache GPU memory allocations
13:58 karolherbst[d]: probably some compute ones
13:59 blockofalumite[d]: I'd rather we had a custom protocol than to depend on systemd or dbus, so that no patches or complex compat layers are required x333
13:59 karolherbst[d]: having a central authority in control of policies and fine grained control ain't a bad idea, and we already have it, so it makes sense to expand on that
13:59 karolherbst[d]: blockofalumite[d]: yeah... but as I said, people will do it via systemd first, and then others are free to add custom stuff
14:00 karolherbst[d]: I'm not saying it, because it's the best/more pragmatic approach, I'm saying it, because that's how it will go down anyway
14:00 conan_kudo[d]: other platforms are mostly just reimplementing the interfaces with their own implementations, which is perfectly fine too
14:00 conan_kudo[d]: systemd takes care to fully define their interfaces specifically for that
14:00 blockofalumite[d]: As long as mesa doesn't hard-depend on dbus or systemd, and it's possible to use smth *other* than systemd-oomd for this, I don't mind.
14:00 blockofalumite[d]: Fwiw, I won't be having systemd-oomd run even on my systemd systems, the default config causes mayhem and I don't want to spend weeks trying to make it not do that
14:00 karolherbst[d]: mesa can abstract that away once something new emerges
14:01 karolherbst[d]: will have to do it anyway for other OS
14:01 conan_kudo[d]: I'm sure Microsoft would backend it with their own API too 🙂
14:01 karolherbst[d]: yeah
14:01 conan_kudo[d]: even though technically Windows does it automatically at a level lower than even Mea
14:01 karolherbst[d]: oh really
14:02 karolherbst[d]: but like the mesa stuff still caches anyway
14:02 karolherbst[d]: mhh maybe the d3d12 driver doesn't...
14:02 karolherbst[d]: but yeah.. "mesa depending on systemd for..." is gonna be a hot topic
14:02 blockofalumite[d]: What is the status of D3D in Mesa ?
14:03 karolherbst[d]: removed mostly
14:03 conan_kudo[d]: karolherbst[d]: it was something I observed back in the day when I used mesa-based ICDs 🙂
14:03 karolherbst[d]: there is a d3d12 driver, for implementing other APIS on top of d3d12
14:03 conan_kudo[d]: gallium nine is still there
14:03 blockofalumite[d]: Oooooh, so like smth like Zink?
14:03 karolherbst[d]: yeah
14:04 karolherbst[d]: currently it's for GL, maybe vaapi/vdpau as well?
14:04 conan_kudo[d]: there's just not much point anymore to have a D3D front for mesa
14:04 karolherbst[d]: yeah...
14:04 blockofalumite[d]: It'd be interesting cuz it'd allow some open source Windows apps to run on Linux
14:04 karolherbst[d]: nine was great at a time
14:05 conan_kudo[d]: with D3D being reimplemented on top of Vulkan with DXVK and other things, it's a lot easier to just do D3D->VK->Mesa
14:05 karolherbst[d]: blockofalumite[d]: the use case is WSL mostly
14:05 conan_kudo[d]: blockofalumite[d]: dxvk-native is what you want then
14:05 blockofalumite[d]: conan_kudo[d]: Can one use DXVK et al outside of Proton/Wine?
14:05 conan_kudo[d]: yes
14:05 conan_kudo[d]: DXVK has a "native" target for Linux and has had that for a few years now
14:05 conan_kudo[d]: it's packaged in Fedora
14:06 blockofalumite[d]: Ah, interesting. Does it support D3D12?
14:06 karolherbst[d]: does that rely on libwine?
14:06 conan_kudo[d]: blockofalumite[d]: Yes.
14:06 conan_kudo[d]: karolherbst[d]: No.
14:06 blockofalumite[d]: I see mentions of D3D8, D3D9, D3D10 and D3D11 but not D3D12 on their readme
14:06 tiredchiku[d]: conan_kudo[d]: no
14:06 tiredchiku[d]: that's vkd3d-proton
14:06 karolherbst[d]: conan_kudo[d]: okay.. so it provides the d3d API, but you still target Linux elsewhere?
14:07 conan_kudo[d]: Yes
14:07 karolherbst[d]: I see
14:07 conan_kudo[d]: tiredchiku[d]: oh yeah, you're right, it's v8~v11
14:07 blockofalumite[d]: What does one use for D3D12?
14:07 tiredchiku[d]: and there's no vkd3d-proton-native
14:08 blockofalumite[d]: It'd be interesting to try to get some source-level Windows -> Linux compat thing
14:08 magic_rb[d]: iirc portal 2 now uses dxvk internally, they recently switched it from opengl to vulkan, which means they ripped out the old opengl renderer and now use dx11/10/9 with dxvk
14:08 karolherbst[d]: funky
14:09 blockofalumite[d]: blockofalumite[d]: ~~I also wish we had better tooling for debugging things running in Wine but meh~~
14:09 karolherbst[d]: ~~there is this gdb script~~
14:09 blockofalumite[d]: Noooooo
14:09 conan_kudo[d]: tiredchiku[d]: it also probably doesn't matter
14:09 tiredchiku[d]: magic_rb[d]: yeah, dxvk-native
14:09 tiredchiku[d]: even tf2 does now
14:10 conan_kudo[d]: it's becoming a thing even with indie games
14:10 blockofalumite[d]: blockofalumite[d]: I really want a good debugger
14:10 blockofalumite[d]: Am I the only one who really does not vibe with neither gdb nor lldb?
14:10 karolherbst[d]: the issue is that CLI based debuggers are always kinda bad
14:11 blockofalumite[d]: tiredchiku[d]: Interesting
14:11 blockofalumite[d]: I wonder if it'd be possible to port some open source Windows games to Linux with this
14:11 karolherbst[d]: GUI ones give you a 10x better experience imho
14:11 tiredchiku[d]: blockofalumite[d]: it should be
14:11 blockofalumite[d]: Yea, except we got no GUI debuggers x3
14:11 karolherbst[d]: ~~vscode~~
14:11 magic_rb[d]: karolherbst[d]: no >:( youll pry my gdb-fu from my cold dead hands
14:11 karolherbst[d]: though
14:11 karolherbst[d]: uhh. there was this qt one
14:11 conan_kudo[d]: there used to be a lot of gui-based gdb frontends
14:11 blockofalumite[d]: ~~I wanna to work on a tool to track ioctls for debugging Mesa~~
14:11 karolherbst[d]: qt-creator
14:11 conan_kudo[d]: yes, Qt Creator has integrated gdb frontend
14:12 karolherbst[d]: the only ones which I used was qt-creator and vscode, but vscode is scuffed a bit
14:12 karolherbst[d]: well..
14:12 karolherbst[d]: it depends
14:12 magic_rb[d]: tiredchiku[d]: with that we could possibly get somewhat native builds of stalker games, dxvk-native + parts of libwine?
14:12 karolherbst[d]: qt-creator was solid, maybe I should use it again, but like...
14:12 blockofalumite[d]: blockofalumite[d]: Idk if anyone would like the tool outside of me, but might as well mention it. The idea is to integrate with a format for static typing ioctls, and also with debuginfod, and allow the exploration of structs and stuff passed around in ioctls. As a GUI debugger.
14:13 karolherbst[d]: I wonder if there is a gui strace tool
14:13 blockofalumite[d]: I am primarily writing that so that I have an easier time understanding how Mesa interacts with the kernel uAPI but yea
14:13 blockofalumite[d]: Yea, this is like strace but... Better suited for this, as strace can only get me so far
14:14 karolherbst[d]: ~~write a gdb breakpoint macro to dump the stuff in a csv file~~
14:14 blockofalumite[d]: I can't really break on syscalls in gdb
14:14 blockofalumite[d]: It's pretty borked
14:15 karolherbst[d]: oh no
14:15 blockofalumite[d]: I need to break on both sys enter and sys exit
14:15 karolherbst[d]: write a ptrace tool yourself
14:15 blockofalumite[d]: Yea, and this is that ptrace tool
14:15 blockofalumite[d]: x333
14:15 karolherbst[d]: I see
14:15 blockofalumite[d]: Except I kinda wanna go all out and make it GUI and all
14:15 blockofalumite[d]: I am really unsure if I can autogenerate info about which ioctls a driver accepts on an inode
14:16 karolherbst[d]: ~~copy it from strace~~
14:16 blockofalumite[d]: strace doesn't know
14:16 karolherbst[d]: huh
14:16 karolherbst[d]: ohh mhh
14:16 blockofalumite[d]: Yea x3
14:16 conan_kudo[d]: karolherbst[d]: I wish there was... But alas there isn't.
14:16 karolherbst[d]: you know what?
14:16 karolherbst[d]: I have an idea
14:17 blockofalumite[d]: The idea is that it will figure out what inode it is, then figure out the driver, and read some static database to figure out what are the types.
14:17 blockofalumite[d]: It will then ask debuginfod for the actual type information.
14:17 karolherbst[d]: make ioctls declarative in the kernel and generate code instead
14:17 blockofalumite[d]: Yea, I wish we had that
14:18 tiredchiku[d]: magic_rb[d]: possibly
14:18 blockofalumite[d]: We have the IO/IOW/IOR/IORW stuff as a basic attempt, but it's all over the place, because a driver registers a structure depending on what kinda driver it is, and somewhere this struct has an fops pointer which then has an ioctls pointer, but every kind of driver has it different
14:18 blockofalumite[d]: So it's hard to statically figure out which registered inode cdev corresponds to which ioctl table
14:18 karolherbst[d]: 🦊
14:18 karolherbst[d]: oh right, that reminds me...
14:20 karolherbst[d]: somebody should prepare and submit the s/fops/🦊 path to linux for next year
14:24 karolherbst[d]: *patch
14:24 blockofalumite[d]: Take the xe driver.
14:24 blockofalumite[d]: xe_module.c first iterates over a list of function pointers, one of those then calls pci_register_driver with a `struct pci_driver` which then points to a function pointer, which does crap with a `struct drm_driver` that only then points to a `drm_ioctl_desc[]` array. Making code to statically find the `drm_ioctl_desc` array and merge it together with the core drm ioctls would be a nightmare
14:25 blockofalumite[d]: So, if someone would make declarative ioctls that bind to a cdev
14:25 blockofalumite[d]: And get it mainlined
14:25 blockofalumite[d]: I'd worship them
14:25 karolherbst[d]: I think that person needs to be you 🙃 though not sure if that's something upstream even wants
14:26 blockofalumite[d]: Yea, the second part is exactly what I am worried about.
14:26 blockofalumite[d]: I'd do it myself but idk if upstream would want it
14:26 blockofalumite[d]: Esp. cuz it'd be refactoring for a *lot* of drivers, meaning that the two systems would probably have to live side by side
14:27 blockofalumite[d]: To be frank, it would be a huge step forward as it would allow folks to write userspace tooling for debugging ioctls, as well as generating documentation for ioctls, and other stuff. Currently, most ioctls are undocumented...
14:28 karolherbst[d]: yeah.. I think if one would come up with a prototype on how it's all useful that might help
14:29 blockofalumite[d]: I mean, the use case here is mostly developer experience. That is, being able to write general tools that allow debugging of ioctls instead of having guesswork that's often wrong
14:30 blockofalumite[d]: And being able to tie a signature to the ioctls, allowing a debugger to inspect the params easily I suppose
14:30 karolherbst[d]: sure, but I think there needs to be patches to those tools to show how useful that all would be first
14:30 blockofalumite[d]: Meaning, I'd maintain a fork of the kernel where that is done, and write the tooling around to showcase it ?
14:30 blockofalumite[d]: Sure, I can do that
14:31 karolherbst[d]: well.. or patch strace
14:31 blockofalumite[d]: I mean, strace is limited so it won't really show the capabilities of this that well
14:32 karolherbst[d]: but yeah.. I think if the demo would show it for a single driver, or a handful of basic IOCTLS and a tool autogenerating parsers for it, that would be kinda cool
14:32 blockofalumite[d]: All that would happen for example would be instead of this:
14:32 blockofalumite[d]: ioctl(3, DRM_IOCTL_I915_GETPARAM, 0x7fffffffc8d0) = 0
14:32 blockofalumite[d]: ioctl(3, DRM_IOCTL_SYNCOBJ_CREATE, 0x7fffffffc8b0) = 0
14:32 blockofalumite[d]: ioctl(3, _IOC(_IOC_READ|_IOC_WRITE, 0x64, 0xc3, 0x28), 0x7fffffffc890) = 0
14:32 blockofalumite[d]: ioctl(3, DRM_IOCTL_GET_CAP, 0x7fffffffc8a0) = 0
14:32 blockofalumite[d]: We would have all of the ioctl requests figured out properly, and eg strings displayed properly as well as numbers
14:32 karolherbst[d]: yeah..
14:32 blockofalumite[d]: karolherbst[d]: Autogenerating parsers ?
14:32 karolherbst[d]: like.. code that reads the struct and displays it
14:33 blockofalumite[d]: Like, the declarative struct in the kernel driver, or
14:33 karolherbst[d]: I mean for tools
14:33 blockofalumite[d]: I am thinking of exposing the ioctl signature database in debugfs
14:33 karolherbst[d]: so you throw in the declared ioctls and code displaying it falls out
14:33 karolherbst[d]: blockofalumite[d]: sadly it's not that simple
14:33 karolherbst[d]: like
14:33 karolherbst[d]: some things are versioned
14:33 blockofalumite[d]: Wdym
14:33 karolherbst[d]: and your tool wants to work properly on different kernels
14:34 blockofalumite[d]: karolherbst[d]: Unsure I follow there
14:34 karolherbst[d]: some IOCTLS have versions in them
14:34 karolherbst[d]: and depending on it, the used struct changes
14:34 blockofalumite[d]: Yea, but that changes the request id no ?
14:34 karolherbst[d]: doesn't have to
14:34 blockofalumite[d]: Then like
14:34 blockofalumite[d]: The running kernel knows which struct it has
14:34 karolherbst[d]: some ioctls are also multiplexer
14:34 karolherbst[d]: so some value in the request changes how you have to parse it
14:35 blockofalumite[d]: I mean, that'd just be a union
14:35 karolherbst[d]: sure
14:35 karolherbst[d]: but
14:35 blockofalumite[d]: There has to be some enter function
14:35 karolherbst[d]: you can also only display the correct variant
14:35 blockofalumite[d]: Unsure I follow
14:35 karolherbst[d]: the point is, that values in the request can change how you have to parse the data
14:36 karolherbst[d]: and what structs to use
14:37 blockofalumite[d]: Hmm, I mean that's true, but the goal of this is to provide a C signature for the ioctls, any further encoding of this (an externally tagged union) should imo be specific to the userspace tooling
14:37 blockofalumite[d]: I don't think it'd be a good idea to come up with a complex type system that can encode all of this
14:37 blockofalumite[d]: Because yea, it's possible, I would also probably be able to figure one out, but I think that it'd be too complex and too much additional burden
14:37 karolherbst[d]: then userspace tooling will have to end up writing all the ioctl parsers themselves, because they need to parse that complexity
14:38 blockofalumite[d]: They don't have to, I mean unions exist in C too and gdb + debuginfod don't have data for encoding which type it is
14:38 karolherbst[d]: just a list of ioctls in a file isn't necessarily that helpful if you can also just spend 2 minutes and resolve that with a bit of copy/paste
14:38 blockofalumite[d]: I don't follow
14:38 karolherbst[d]: blockofalumite[d]: yeah, but displaying all union variants all at once, especially when nested is not helpful
14:39 blockofalumite[d]: It's way more helpful than `0x7fffffffc870`
14:39 karolherbst[d]: if the kernel should change the way how to declare ioctls, there needs to be like a real benefit
14:39 karolherbst[d]: blockofalumite[d]: you can just include the uapi headers
14:39 blockofalumite[d]: karolherbst[d]: I don't follow
14:39 karolherbst[d]: and write the decoder
14:39 blockofalumite[d]: What?
14:40 blockofalumite[d]: I don't get at all what you are saying, let's take a step back x3
14:40 karolherbst[d]: I mean.. the tool (e.g. strace) can just add some code to display the value instead of simply printing the address
14:40 karolherbst[d]: that can already happen today
14:40 blockofalumite[d]: How would it do that if it doesn't know what the value is supposed to be ?
14:40 karolherbst[d]: you add code so it does know it
14:40 karolherbst[d]: the ioctl is stable
14:40 karolherbst[d]: so you just check it once
14:40 karolherbst[d]: and write the code
14:41 karolherbst[d]: like.. it already knows it's `DRM_IOCTL_I915_GETPARAM`
14:41 blockofalumite[d]: karolherbst[d]: Sure, and there is a *lot* of ioctls
14:41 karolherbst[d]: it can just pull the header for the uapi and use the struct there
14:41 karolherbst[d]: blockofalumite[d]: yeah
14:41 karolherbst[d]: so
14:41 karolherbst[d]: that's why that should be possible to autogenerate
14:41 blockofalumite[d]: karolherbst[d]: And that's what i am proposing
14:41 blockofalumite[d]: Declarative ioctls, have the database generated in debugfs
14:43 karolherbst[d]: not sure you need it in debugfs, because you can also just pull the files out of git
14:43 karolherbst[d]: and allow more advanced use cases
14:43 blockofalumite[d]: Creating a more complex type system to be able to encode externally tagged unions is imo not great, through sure I could future-proof the format
14:43 karolherbst[d]: the thing is
14:44 karolherbst[d]: if you don't have that, or take into account that for the future, tools will have to write their own code anyway
14:44 blockofalumite[d]: karolherbst[d]: I'd want one userspace tool to support multiple kernel versions without having to be updated
14:44 karolherbst[d]: you only need the most recent one
14:44 karolherbst[d]: but yeah...
14:44 blockofalumite[d]: I really don't follow what you are proposing
14:45 karolherbst[d]: I mean.. I'm saying that whatever you come up with, it probably will have to support tagged unions for it to be really useful
14:45 karolherbst[d]: because otherwise you'll end up writing the code instead fo display those properly
14:45 blockofalumite[d]: Why would it not be useful without tagged unions? I think it'd be better to take things step by step
14:45 blockofalumite[d]: Before we force maintainers to add info about what tags
14:46 blockofalumite[d]: Let's just only switch to declarative ioctls
14:46 karolherbst[d]: maybe, but it needs to be taken into account, that it's gonna happen sooner or later
14:46 blockofalumite[d]: Sure, I can allow attributes
14:46 karolherbst[d]: though my concern would be, just declaring the ioctls won't be enough to get agreement here
14:46 blockofalumite[d]: (Like serde has in Rust for example)
14:46 blockofalumite[d]: Why so ?
14:47 karolherbst[d]: because there wouldn't be a huge benefit for anybody
14:47 karolherbst[d]: though it might be good enough for a lot of ioctls
14:47 blockofalumite[d]: karolherbst[d]: What do you mean, I mean I already outlined the benefits
14:47 blockofalumite[d]: I am so confused
14:48 blockofalumite[d]: DWARF info also doesn't have externally tagged unions
14:48 blockofalumite[d]: Is it useless?
14:48 karolherbst[d]: the thing is.. people only really have to write the code once per ioctl and they are done, and for tagged unions they'll probably want to then anyway if it's not already handled by the auto generated code
14:49 karolherbst[d]: blockofalumite[d]: though in userspace code you rarely hit nested unions
14:49 karolherbst[d]: there are some really really cursed ioctls out there
14:50 blockofalumite[d]: karolherbst[d]: Sure, but why spend the time manually going through every driver and keeping the local ioctl database up-to-date
14:50 blockofalumite[d]: I mean sure, that is *an* option
14:50 karolherbst[d]: I don't want to say that having something basic wouldn't already be good, I'm mostly concerned if it's enough to get a buy in from relevant folks
14:50 blockofalumite[d]: And what I originally thought that I'd have to do
14:51 blockofalumite[d]: The idea is DWARF info for ioctls + attributes
14:51 blockofalumite[d]: The attributes could be docs for an uapi doc generator, or it could be externally tagged unions
14:51 blockofalumite[d]: But imo starting small, and just *getting* the system in, before additional attributes are defined, would be good
14:52 karolherbst[d]: yeah, it would certainly be a starting point and something to show, the question is more if it's enough for people to see it merged or not
14:52 karolherbst[d]: of course you shouldn't do the full thing before asking for review
14:52 blockofalumite[d]: Yea... I mean for sure both the old system and the new system would have to exist
14:53 blockofalumite[d]: As for the debugfs, the idea is, why would I have to locally pull in the database? And then ensure I have the right versions of things, etc.
14:53 blockofalumite[d]: To me it seems better to have smth like debuginfod, where you locally request the debug info and not have to care about versioning
14:53 blockofalumite[d]: That's why I would shove the database at runtime into debugfs
14:54 blockofalumite[d]: Or, well, literally anywhere I suppose, but debugfs seems the most suitable
14:54 blockofalumite[d]: Imagine it like `/proc/config.gz`, that's better than having to go to your distro and DL the config tarball (if they ship one)
14:55 karolherbst[d]: the uapi is stable no matter what the config is
14:55 karolherbst[d]: and it can't break
14:55 blockofalumite[d]: A distro could add new ioctls
14:55 karolherbst[d]: no
14:55 karolherbst[d]: well
14:55 blockofalumite[d]: Wdym no
14:55 karolherbst[d]: yes, but...
14:56 blockofalumite[d]: Yea
14:56 blockofalumite[d]: That's why you have to fetch the database from the distro
14:56 karolherbst[d]: it's more for like very specific niche distros
14:56 karolherbst[d]: but yeah..
14:56 blockofalumite[d]: Not really...? It's like
14:56 blockofalumite[d]: If you have out-of-tree modules
14:56 karolherbst[d]: yeah.. but they might not want the info to be shown in the first place
14:56 blockofalumite[d]: That's new ioctls you need to deal with
14:56 karolherbst[d]: but anyway
14:56 blockofalumite[d]: Sure, they might not, but many will want it
14:56 karolherbst[d]: as far as upstream is concerned: out of tree modules don't exist
14:57 karolherbst[d]: and no argument based on out-of-tree matters at all
14:57 blockofalumite[d]: I guess it might be better to have the distro ship the database ?
14:57 karolherbst[d]: if your reason is "because out-of-tree modules exist" it won't be listened to upstream
14:57 blockofalumite[d]: Okay, even with in-tree, I think it makes sense to not have to vendor the database
14:57 blockofalumite[d]: It makes no sense to have to update the tool just because the kernel had an update
14:58 karolherbst[d]: if it's in-tree, the uapi is stable
14:58 blockofalumite[d]: Yes, it's stable, and ?
14:58 karolherbst[d]: across all config and builds
14:58 karolherbst[d]: there is just a single uapi
14:58 blockofalumite[d]: The tool was built, say, Jan 2024. Since then, the kernel had new ioctls introduced. Why would the tool devs have to bump the tool version just to update the vendored database?
14:58 karolherbst[d]: and if you would fetch the info from e.g. 6.11, it has to be match whatever is in 5.10
14:59 karolherbst[d]: just with new stuff on top
14:59 blockofalumite[d]: Yes
14:59 blockofalumite[d]: And the new stuff matters too
14:59 blockofalumite[d]: If there was no change to the tool, why should the tool have to update
14:59 karolherbst[d]: right, but then the argument is, "you need to rebuild the tool ones" vs "we need to add a new debugfs/procfs API just to support the info at runtime"
14:59 karolherbst[d]: and I'd expect people to lean towards the former
14:59 karolherbst[d]: *once
15:00 karolherbst[d]: and you might be able to do more things with that info at compile time than at runtime
15:00 blockofalumite[d]: Sigh, I hate dealing with this, I want to work on cool tech, not do political lobbying x3
15:00 blockofalumite[d]: Sure, the distro could just ship the database I suppose.
15:00 blockofalumite[d]: That way the tool doesn't have to update
15:01 karolherbst[d]: yeah.. I don't know for sure how the discussion would go, only that those points might be brought up and you probably will need to have good arguments
15:01 karolherbst[d]: the tool could live inside the kernel
15:01 karolherbst[d]: so it just gets built alongside it
15:01 blockofalumite[d]: The main argument against having the distro distribute it is because the kernel on-disk and kernel in-ram might mismatch
15:01 karolherbst[d]: or rather.. the parser/decoder part of it
15:01 blockofalumite[d]: But that's a distro problem ig, so probably would just get disregarded
15:02 karolherbst[d]: blockofalumite[d]: well, that's not a concern if you built against the latest and greatest, because it's backwards compatible
15:02 blockofalumite[d]: It could be a concern
15:02 blockofalumite[d]: It's just another thing that a distro has to deal with ig
15:02 blockofalumite[d]: But meh
15:02 blockofalumite[d]: Not like we don't already have a fuckton of such things to deal with, what's one more x3
15:02 karolherbst[d]: you could have `libuapi_decoder.so` be something build from linux kernel git directly, so it's just in the same package
15:03 blockofalumite[d]: I mean
15:03 blockofalumite[d]: Why do that ?
15:03 karolherbst[d]: there are already enough tools in the kernel tree
15:03 karolherbst[d]: the packaging already exists
15:03 blockofalumite[d]: You mean like
15:03 blockofalumite[d]: Have the database *inside* the .so ?
15:03 karolherbst[d]: something inside `tools/` and yeah
15:03 blockofalumite[d]: And provide a C API to interact with it
15:03 karolherbst[d]: yep
15:04 karolherbst[d]: or it's a database file in whatever format which gets generated as part of the kernel build system
15:04 blockofalumite[d]: Yea database file is what I had in mind
15:05 karolherbst[d]: you already have files like that already
15:05 karolherbst[d]: `/usr/include/linux/`
15:05 karolherbst[d]: it's also the greatest and latest
15:05 karolherbst[d]: and you have the 6.11 files even if you run e.g. 6.8
15:06 blockofalumite[d]: And I just thought that it'd be okay to have the file exposed in debugfs the way that config.gz is exposed in procfs, but meh, ig distro is okay
15:06 karolherbst[d]: config.gz makes sense because it can be different in each distro/machine
15:06 karolherbst[d]: this database will be the same everywhere
15:06 karolherbst[d]: (mostly(
15:06 blockofalumite[d]: If we ignore reality (out-of-tree) yea x3
15:06 karolherbst[d]: or asahi linux
15:06 blockofalumite[d]: AHAHAH
15:07 karolherbst[d]: but yeah.. they can ship their own database file anyway
15:07 blockofalumite[d]: Yea
15:07 blockofalumite[d]: Ig it's good enough to just have the db file shipped
15:08 karolherbst[d]: on every kernel-headers update distros need to rebuild stuff already anyway, one more tool won't really matter much
15:08 blockofalumite[d]: Yea
15:08 blockofalumite[d]: I mean there'd need to be a standard location for this
15:08 blockofalumite[d]: So we'd have everyone else
15:08 blockofalumite[d]: And then NixOS
15:09 blockofalumite[d]: 🤣
15:09 karolherbst[d]: could be part of kernel-headers 😄
15:09 blockofalumite[d]: Hmm, I suppose... I mean the app would need to load it at runtime
15:09 karolherbst[d]: mhh
15:09 karolherbst[d]: maybe, though I'd guess if it's e.g. a json file, tools could also just generate more code at compile time to have a decoder and all of that
15:10 karolherbst[d]: but yeah...
15:10 blockofalumite[d]: I was hoping for a binary format, smth like DWARF
15:10 blockofalumite[d]: But JSOn could work
15:10 karolherbst[d]: I'm sure python folks might want to make use of it as well
15:10 karolherbst[d]: or well
15:10 karolherbst[d]: allow for scripting
15:11 blockofalumite[d]: I mean
15:11 blockofalumite[d]: There's stuff like https://github.com/eliben/pyelftools
15:11 karolherbst[d]: heh
15:11 karolherbst[d]: but anyway, those are more or less technical details and one has to see what works
15:11 blockofalumite[d]: Yea
15:12 blockofalumite[d]: Tbh it'd be mega cool if I could get this into upstream but idk x333
15:12 karolherbst[d]: but like.. having a lib one can use and it would parse the ioctl including tagged unions and all that stuff would be super useful for tools like strace and valgrind
15:12 blockofalumite[d]: Yea, I mean one can just write such a lib out of kernel
15:12 karolherbst[d]: could also extend gdb and have conditional breakpoints
15:12 blockofalumite[d]: Wdym
15:13 karolherbst[d]: like break on a ioctl being submitted having specific values
15:13 karolherbst[d]: though...
15:13 karolherbst[d]: could also break earlier
15:14 blockofalumite[d]: I don't follow how that relates ig
15:14 karolherbst[d]: gdb has conditional breakpoints and you can break on parameters being a special value
15:15 karolherbst[d]: `break ioctl IOCTL_IDENTIFIER if params->some_field == 5` or something could be added
15:15 blockofalumite[d]: Conditional breakpoints that make the debugee's performance die
15:15 blockofalumite[d]: karolherbst[d]: Ah, what for
15:15 karolherbst[d]: I use conditional breakpoints all the time
15:15 karolherbst[d]: they are usefull if your think gets called million of times
15:15 karolherbst[d]: *thing
15:15 blockofalumite[d]: Yea, I know
15:15 blockofalumite[d]: But like what is the addition
15:16 karolherbst[d]: not sure any of this would work on ioctl itself like that
15:16 karolherbst[d]: though
15:16 karolherbst[d]: you could add a cast in your conditional I guess
15:16 blockofalumite[d]: karolherbst[d]: I mean
15:16 blockofalumite[d]: You can't even non-conditionally break on an ioctl properly
15:17 blockofalumite[d]: You have to break on the glibc function and pray
15:17 karolherbst[d]: right...
15:18 blockofalumite[d]: Syscall breaking is pretty borked
15:18 blockofalumite[d]: For some reason
15:18 blockofalumite[d]: Idk if nobody bothered or if there's a technical reason
15:18 karolherbst[d]: maybe the former
15:18 blockofalumite[d]: Possible
15:18 blockofalumite[d]: I think that it breaks on syscall exit
15:18 blockofalumite[d]: Because I can't seem to get the correct values
15:19 blockofalumite[d]: So I am guessing that the userspace buffers are trashed or something
15:19 blockofalumite[d]: I honestly don't know why I get nonsense
15:19 karolherbst[d]: ioctls are weird
15:19 blockofalumite[d]: Yea
15:19 blockofalumite[d]: And it's annoying to try to get gdb to pull in stuff
15:19 blockofalumite[d]: For half the kernel uapi structs it just screams "no such symbol"
15:19 blockofalumite[d]: And I have no idea how to make it pull in symbols
15:22 blockofalumite[d]: I really wish we had a debugger that didn't do that kinda stuff
15:22 blockofalumite[d]: One which would let me inspect values properly
15:28 karolherbst[d]: I usually break where the ioctl gets used
15:28 karolherbst[d]: and always compile with -O0 -g3
15:29 blockofalumite[d]: I do, too
15:29 blockofalumite[d]: But for some reason it just
15:29 blockofalumite[d]: I should move to some non-bridged channel for this
15:29 blockofalumite[d]: Discord channel #tech-talk
17:22 magic_rb[d]: ive just confirmed that on stable mesa, ghostrunner 2 runs without any problems, well it runs very badly, but it runs
17:23 magic_rb[d]: also, if any of you remember me crying about random segfaults, turns out it was a hardware issue, which somehow the proprietary nvidia driver could handle? the connection between my mainboard and a daughter board which has usbc with display out on it, had a bad ribbon cable connectior
17:23 magic_rb[d]: and so the cable marked as USB, which I assume is responsible for the USB traffic was intermittently disconnecting, which somehow the proprietary nvidia driver didnt mind nearly as much
17:24 magic_rb[d]: while nouveau would immediately segfault and dip
17:24 magic_rb[d]: ive tested both vulkan and zink to work completely fine
21:15 samantas5855[d]: asdqueerfromeu[d]: interesting
21:15 samantas5855[d]: I didn't know nvk supported ray tracing
21:16 samantas5855[d]: I mean lighting loooks broken sometimes
21:16 samantas5855[d]: but still impressive