On 31/08/23 03:49, Dixit, Ashutosh wrote: > On Wed, 30 Aug 2023 13:56:57 -0700, Rodrigo Vivi wrote: >> On Tue, Aug 29, 2023 at 10:33:02PM -0700, Dixit, Ashutosh wrote: >>> On Tue, 29 Aug 2023 22:15:43 -0700, Aravind Iddamsetty wrote: >>> Hi Aravind, >>> >>>> @@ -162,7 +162,7 @@ int xe_force_wake_get(struct xe_force_wake *fw, >>>> domain->id, ret); >>>> } >>>> fw->awake_domains |= woken; >>>> - mutex_unlock(&fw->lock); >>>> + spin_unlock(&fw->lock); >>> No need to change anything yet, but let's get some more opinion on this: is >>> it ok to (a) just replace the mutex with a spinlock in these force_wake >>> functions, or, (b) should we have a second set of functions to be called in >>> atomic context, say: xe_force_wake_get/put_atomic? So we should use (b) in >>> atomic contexts and everywhere else we just continue to use the previous >>> set of non-atomic functions? Or just converting the default set of >>> functions to use spin lock (as is done in this patch) is ok? >> It looks okay to me, >> Reviewed-by: Rodrigo Vivi > Still thinking about this, maybe some time (not part of this series) we > should do a power measurement comparison between mutex and spinlock and see > if there's an appreciable difference (unless we already know?). But till we > do that, this is fine, so this is also: > > Reviewed-by: Ashutosh Dixit Thanks Rodrigo and Ashutosh for your r-b. Regards, Aravind.