On Wed, 21 Mar 2018, Chris Wilson <ch...@chris-wilson.co.uk> wrote: > Quoting Michel Thierry (2018-03-21 17:01:12) >> On 3/21/2018 3:46 AM, Mika Kuoppala wrote: >> > Chris Wilson <ch...@chris-wilson.co.uk> writes: >> >> - /* The write will be ordered by the uncached read (itself >> >> - * a memory barrier), so we do not need another in the form >> >> - * of a locked instruction. The race between the interrupt >> >> - * handler and the split test/clear is harmless as we order >> >> - * our clear before the CSB read. If the interrupt arrived >> >> - * first between the test and the clear, we read the updated >> >> - * CSB and clear the bit. If the interrupt arrives as we read >> >> - * the CSB or later (i.e. after we had cleared the bit) the >> >> bit >> >> - * is set and we do a new loop. >> >> - */ >> >> - __clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted); >> >> + /* Clear before reading to catch new interrupts */ >> >> + clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted); >> >> + smp_mb__after_atomic(); >> >> Checkpatch wants a comment for the memory barrier... Are we being strict >> about it? (https://patchwork.freedesktop.org/series/40359/) > > There's a comment for it not two lines above! Silly perl script.
Sure, it's nowhere near perfect. But I do like to get the reminder about this, "hey don't forget to document your memory barriers, locks, etc.". It does mean we can't use checkpatch for gating, but I think it can make the reviewer's life easier to be able to just point at the results, and ask the author to fix the relevant stuff. I think it's less tedious and less offensive than the reviewer doing the job manually. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
