| Message ID | 20250211163109.12200-2-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] dma-buf: fix incorrect dma-fence documentation | expand |
On Tue, Feb 11, 2025 at 05:31:06PM +0100, Christian König wrote: > There isn't much worse than documentation giving an incorrect advise. > Grabbing a spinlock while interrupts are disabled usually means that you > must also disable interrupts for all other uses of this spinlock. > > Otherwise really hard to debug issues can occur. So fix that invalid > documentation. > > Signed-off-by: Christian König <christian.koenig@amd.com> Oops :-/ Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch> > --- > include/linux/dma-fence.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h > index e7ad819962e3..e230af0d123f 100644 > --- a/include/linux/dma-fence.h > +++ b/include/linux/dma-fence.h > @@ -169,8 +169,8 @@ struct dma_fence_ops { > * implementation know that there is another driver waiting on the > * signal (ie. hw->sw case). > * > - * This function can be called from atomic context, but not > - * from irq context, so normal spinlocks can be used. > + * This is called with irq's disabled, so only spinlocks which also > + * disable irq's can be used. > * > * A return value of false indicates the fence already passed, > * or some failure occurred that made it impossible to enable > -- > 2.34.1 >
On 2/11/25 19:31, Christian König wrote: > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h > index e7ad819962e3..e230af0d123f 100644 > --- a/include/linux/dma-fence.h > +++ b/include/linux/dma-fence.h > @@ -169,8 +169,8 @@ struct dma_fence_ops { > * implementation know that there is another driver waiting on the > * signal (ie. hw->sw case). > * > - * This function can be called from atomic context, but not > - * from irq context, so normal spinlocks can be used. > + * This is called with irq's disabled, so only spinlocks which also > + * disable irq's can be used. Nit: The description sounds a bit cryptic to me. I'd add ".. so only spinlocks which disable IRQ's can be used in the code outside of this callback". Note that I removed the word 'also' because disabling IRQs isn't necessary for a spinlock taken within interrupt context because IRQs are already disabled there.
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index e7ad819962e3..e230af0d123f 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -169,8 +169,8 @@ struct dma_fence_ops { * implementation know that there is another driver waiting on the * signal (ie. hw->sw case). * - * This function can be called from atomic context, but not - * from irq context, so normal spinlocks can be used. + * This is called with irq's disabled, so only spinlocks which also + * disable irq's can be used. * * A return value of false indicates the fence already passed, * or some failure occurred that made it impossible to enable
There isn't much worse than documentation giving an incorrect advise. Grabbing a spinlock while interrupts are disabled usually means that you must also disable interrupts for all other uses of this spinlock. Otherwise really hard to debug issues can occur. So fix that invalid documentation. Signed-off-by: Christian König <christian.koenig@amd.com> --- include/linux/dma-fence.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)