On 2/24/20 7:46 PM, Christian König wrote:
Am 23.02.20 um 17:54 schrieb Thomas Hellström (VMware):
On 2/23/20 4:45 PM, Christian König wrote:
Am 21.02.20 um 18:12 schrieb Daniel Vetter:
[SNIP]
Yeah the Great Plan (tm) is to fully rely on ww_mutex slowly
degenerating
into essentially a global lock. But only when there's actual
contention
and thrashing.
Yes exactly. A really big problem in TTM is currently that we drop
the lock after evicting BOs because they tend to move in again
directly after that.
From practice I can also confirm that there is exactly zero benefit
from dropping locks early and reacquire them for example for the VM
page tables. That's just makes it more likely that somebody needs to
roll back and this is what we need to avoid in the first place.
If you have a benchmarking setup available it would be very
interesting for future reference to see how changing from WD to WW
mutexes affects the roll back frequency. WW is known to cause
rollbacks much less frequently but there is more work associated with
each rollback.
Not of hand. To be honest I still have a hard time to get a grip on
the difference between WD and WW from the algorithm point of view. So
I can't judge that difference at all.
OK. I don't think a detailed understanding of the algorithms is strictly
necessary, though. If we had had a good testcase we'd just have to
change DEFINE_WD_CLASS in dma-resv.c to DEFINE_WW_CLASS and benchmark
relevant figures.
Contention on BO locks during command submission is perfectly fine
as long as this is as lightweight as possible while we don't have
trashing. When we have trashing multi submission performance is best
archived to just favor a single process to finish its business and
block everybody else.
Hmm. Sounds like we need a per-manager ww_rwsem protecting manager
allocation, taken in write-mode then there's thrashing. In read-mode
otherwise. That would limit the amount of "unnecessary" locks we'd
have to keep and reduce unwanted side-effects, (see below):
Well per-manager (you mean per domain here don't you?)
yes.
doesn't sound like that useful because we rarely use only one domain,
Well the difference to keeping all locks would boil down to:
"Instead of keeping all locks of all bos we evict from thrashing
domains, keep locks of all thrashing domains in write mode". This means
that for domains that are not thrashing, we'd just keep read locks.
but I'm actually questioning for quite a while if the per BO lock
scheme was the right approach.
See from the performance aspect the closest to ideal solution I can
think of would be a ww_rwsem per user of a resource.
In other words we don't lock BOs, but instead a list of all their
users and when you want to evict a BO you need to walk that list and
inform all users that the BO will be moving.
During command submission you then have the fast path which rather
just grabs the read side of the user lock and check if all BOs are
still in the expected place.
If some BOs were evicted you back off and start the slow path, e.g.
maybe even copy additional data from userspace then grab the write
side of the lock etc.. etc...
That approach is similar to what we use in amdgpu with the per-VM BOs,
but goes a step further. Problem is that we are so used to per BO
locks in the kernel that this is probably not doable any more.
I think we need some-sort of per-bo lock to protect bo metadata. But
yes, relying solely on them to resolve other resource (domain)
contention may not be (or rather probably isn't) the right choice.
Because of this I would actually vote for forbidding to release
individual ww_mutex() locks in a context.
Yes, I see the problem.
But my first reaction is that this might have undersirable
side-effects. Let's say somebody wanted to swap the evicted BOs out?
Please explain further, I off hand don't see the problem here.
Lets say thread A evicts a lot of thread B's bos, and keeps the locks of
those bos for a prolonged time. Then thread C needs memory and wants to
swap out thread B's bos. It can't, or at least not during a certain
delay because thread A unnecessarily holds the locks.
In general I actually wanted to re-work TTM in a way that BOs in the
SYSTEM/SWAPABLE domain are always backed by a shmem file instead of
the struct page array we currently have.
That would probably work well if there are no SYSTEM+write-combined
users anymore. Typically in the old AGP days, you wouldn't change
caching mode when evicting from write-combine AGP to SYSTEM because of
the dead slow wbinvd() operation.
Or cpu-writes to them causing faults, that might also block the
mm_sem, which in turn blocks hugepaged?
Mhm, I also only have a higher level view how hugepaged works so why
does it grabs the mm_sem on the write side?
If I understand it correctly, it's needed when collapsing PMD
directories to huge PMD pages. But this was merely an example. For this
particular case the RETRY mechanism in the TTM fault handler we've
discussed before will try reasonably hard to release the mmap_sem when
sleeping on a bo lock.
Thanks,
Thomas
Thanks,
Christian.
Still it's a fairly simple solution to a problem that seems otherwise
hard to solve efficiently.
Thanks,
Thomas
Regards,
Christian.
-Daniel
