13:54 <bbrezillon> robmur01: Re: panfrost_job_push() making all BO mappings active => this is already what we do, and I don't think there's any problem in panfrost regarding page table allocations, at least not until we decide with want to implement a VM_BIND-like ioctl()

13:58 <bbrezillon> but if we want to add a VM_BIND ioctl to pancsf, mapping/unmapping operations will be deferred to a drm_sched queue, and executed in the run_job() path, where we're not allowed to allocate in blocking mode (the shrinker / sched deadlock thing you were discussing with robclark)

13:59 <robmur01> that's what I mean - job_push can preallocate pagetables by just "faulting in" PTEs matching the granularity the VM_BIND wants, that way the subsequent map() can be known not to need to allocate

14:02 <robmur01> if necessary, you can also prevent unmap from freeing tables by just splitting up into less than the size of the next block up

14:02 <bbrezillon> uh, except the SUBMIT ioctl() no longer gets passed a list of BOs

14:02 <bbrezillon> (in pancsf, I mean)

14:03 <bbrezillon> oh, nevermind, you meant the VM_BIND job

14:10 <bbrezillon> if pre-faulting pages doesn't do anything when something is already mapped in that range, I guess that could work. That's assuming we don't free page tables when unmapping a region that's later mapped by another VM_BIND operation, which forces us to keep track of all queued operations.

14:27 <bbrezillon> robmur01: just want to make sure I get it correctly. For the pre-allocation to happen, we need to get rid of this https://elixir.bootlin.com/linux/latest/source/drivers/iommu/io-pgtable-arm.c#L484, and call the map function with iommu_prot=0, right?

15:11 <robmur01> or just map *something* and immediately unmap it again

15:13 <robmur01> I am assuming this is at a point where any previous users of the given VA range have already been kicked out

15:25 <bbrezillon> well, that would mean adding an ordering constraint on VM_BIND operations. vkQueueBindSparse() is asynchronous by nature, and we don't know the state of the VM at job execution time (bind jobs have explicit deps, and the state of the VM might have changed when we get to execute the VM_BIND operation)

15:26 <bbrezillon> one option to make sure we always have a correct VM state at job submission time is to: 1/ have only one binding queue 2/ execute synchronous binds on this queue

15:27 <bbrezillon> but ideally, I'd like to relax that constraint :-/

15:28 <bbrezillon> I guess exposing just one sparse binding queue is fine, but forcing synchronous binds to wait on async ones is not great

15:34 <bbrezillon> maybe that's just a non-issue, because if something was mapped already, we can figure it out before doing the dummy map/unmap dance to get things pre-allocated

15:36 <bbrezillon> it still feels a bit hack-ish to have to map/unmap in that case, but oh well

15:38 <robmur01> ah, OK, that makes things more tricky - say the previous user had a 2MB block where VM_BIND wants 4k pages; a table would definitely need allocating, but can't be "faulted in" until the previous mapping is actually removed

15:39 <bbrezillon> robmur01: that still requires some flag to prevent freeing a page table on an unmap, if we know other bind ops targetting the range covered by this page table are queued

15:40 <bbrezillon> robmur01: and there's this 2MB vs 4k granularity thing, but I thought you assumed we'd use 4k granularity for everything to avoid splits in the unmap path

15:44 <robmur01> guess I've been assuming that that would only apply to sparse mappings, and we'd still have "normal" BOs simply mapped for their lifetime, which could use blocks

15:46 <robmur01> if you never use blocks ever at all, and split unmaps at less than 2MB, then io-pgtable will essentially never free any tables, so any VA which has been used at least once will be reusable without allocation

15:47 <robmur01> it's churning VAs back and forth between the two granularities that poses most of the awkwardness

15:57 <robmur01> TBH I'm starting to think at this point it might just be easier to map with GFP_NOWAIT instead of GFP_KERNEL :)

15:58 <robmur01> it would seem horribly wrong if that still had a shrinker dependency somehow...

15:59 <robmur01> and if memory really is that low then just letting jobs fail until reclaim has had a chance to run by itself doesn't seem *too* terrible

16:02 <bbrezillon> robmur01: mind joining #dri-devel, so we can discuss it with people knowing what can and can't be done in the run_job() path?

16:08 <robmur01> oh alright then... it's already 4PM so I don't think today is a "finish the PMU driver" day anyway :P