On 2021-08-12 7:55 a.m., Koenig, Christian wrote: > Hi James, > > Evan seems to have understood how this all works together. > > See while any begin/end use critical section is active the work should not be active. > > When you handle only one ring you can just call cancel in begin use and schedule in end use. But when you have more than one ring you need a lock or counter to prevent concurrent work items to be started. > > Michelle's idea to use mod_delayed_work is a bad one because it assumes that the delayed work is still running. It merely assumes that the work may already have been scheduled before. Admittedly, I missed the cancel_delayed_work_sync calls for patch 2. While I think it can still have some effect when there's a single work item for multiple rings, as described by James, it's probably negligible, since presumably the time intervals between ring_begin_use and ring_end_use are normally much shorter than a second. So, while patch 2 is at worst a no-op (since mod_delayed_work is the same as schedule_delayed_work if the work hasn't been scheduled yet), I'm fine with dropping it. > Something similar applies to the first patch I think, There are no cancel work calls in that case, so the commit log is accurate TTBOMK. I noticed this because current mutter Git main wasn't able to sustain 60 fps on Navi 14 with a simple glxgears -fullscreen. mutter was dropping frames because its CPU work for a frame update occasionally took up to 3 ms, instead of the normal 2-300 microseconds. sysprof showed a lot of cycles spent in the functions which enable/disable GFXOFF in the HW. > so when this makes a difference it is actually a bug. There was certainly a bug though, which patch 1 fixes. :) -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer
