Re: [PATCH 2/2] drm/scheduler: Remove obsolete spinlock.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 05/16/2018 09:16 AM, Lucas Stach wrote:
Am Mittwoch, den 16.05.2018, 15:10 +0200 schrieb Christian König:
Am 16.05.2018 um 15:00 schrieb Lucas Stach:
Am Mittwoch, den 16.05.2018, 14:32 +0200 schrieb Christian König:
Am 16.05.2018 um 14:28 schrieb Lucas Stach:
Am Mittwoch, den 16.05.2018, 14:08 +0200 schrieb Christian König:
Yes, exactly.

For normal user space command submission we should have tons of
locks
guaranteeing that (e.g. just the VM lock should do).

For kernel moves we have the mutex for the GTT windows which
protects
it.

The could be problems with the UVD/VCE queues to cleanup the
handles
when an application crashes.
FWIW, etnaviv is currently completely unlocked in this path, but I
believe that this isn't an issue as the sched fence seq numbers are
per
entity. So to actually end up with reversed seqnos one context has
to
preempt itself to do another submit, while the current one hasn't
returned from kernel space, which I believe is a fairly theoretical
issue. Is my understanding correct?
Yes. The problem is with the right timing this can be used to access
freed up memory.

If you then manage to place a page table in that freed up memory
taking
over the system is just a typing exercise.
Thanks. I believe we don't have this problem in etnaviv, as memory
referencing is tied to the job and will only be unreferenced on
free_job, but I'll re-check this when I've got some time.
Well that depends on how you use the sequence numbers.

If you use them for job completion check somewhere then you certainly
have a problem if job A gets the 1 and B the 2, but B completes before A.
We don't do this anymore. All the etnaviv internals are driven by the
fence signal callbacks.

At bare minimum that's still a bug we need to fix because it confuses
functions like dma_fence_is_later() and dma_fence_later().
Agreed. Also amending the documentation to state that
drm_sched_job_init() and drm_sched_entity_push_job() need to be called
under a common lock seems like a good idea.

I will respin the patch with added documentation.

Andrey

Regards,
Lucas

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux