| Message ID | 20220120132747.2348-2-christian.koenig@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/9] dma-buf: consolidate dma_fence subclass checking | expand |
On 1/20/22 14:27, Christian König wrote: > Consolidate the wrapper functions to check for dma_fence > subclasses in the dma_fence header. > > This makes it easier to document and also check the different > requirements for fence containers in the subclasses. > > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > include/linux/dma-fence-array.h | 15 +------------ > include/linux/dma-fence-chain.h | 3 +-- > include/linux/dma-fence.h | 38 +++++++++++++++++++++++++++++++++ > 3 files changed, 40 insertions(+), 16 deletions(-) > > diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h > index 303dd712220f..fec374f69e12 100644 > --- a/include/linux/dma-fence-array.h > +++ b/include/linux/dma-fence-array.h > @@ -45,19 +45,6 @@ struct dma_fence_array { > struct irq_work work; > }; > > -extern const struct dma_fence_ops dma_fence_array_ops; > - > -/** > - * dma_fence_is_array - check if a fence is from the array subsclass > - * @fence: fence to test > - * > - * Return true if it is a dma_fence_array and false otherwise. > - */ > -static inline bool dma_fence_is_array(struct dma_fence *fence) > -{ > - return fence->ops == &dma_fence_array_ops; > -} > - > /** > * to_dma_fence_array - cast a fence to a dma_fence_array > * @fence: fence to cast to a dma_fence_array > @@ -68,7 +55,7 @@ static inline bool dma_fence_is_array(struct dma_fence *fence) > static inline struct dma_fence_array * > to_dma_fence_array(struct dma_fence *fence) > { > - if (fence->ops != &dma_fence_array_ops) > + if (!fence || !dma_fence_is_array(fence)) > return NULL; > > return container_of(fence, struct dma_fence_array, base); > diff --git a/include/linux/dma-fence-chain.h b/include/linux/dma-fence-chain.h > index 54fe3443fd2c..ee906b659694 100644 > --- a/include/linux/dma-fence-chain.h > +++ b/include/linux/dma-fence-chain.h > @@ -49,7 +49,6 @@ struct dma_fence_chain { > spinlock_t lock; > }; > > -extern const struct dma_fence_ops dma_fence_chain_ops; > > /** > * to_dma_fence_chain - cast a fence to a dma_fence_chain > @@ -61,7 +60,7 @@ extern const struct dma_fence_ops dma_fence_chain_ops; > static inline struct dma_fence_chain * > to_dma_fence_chain(struct dma_fence *fence) > { > - if (!fence || fence->ops != &dma_fence_chain_ops) > + if (!fence || !dma_fence_is_chain(fence)) > return NULL; > > return container_of(fence, struct dma_fence_chain, base); > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h > index 1ea691753bd3..775cdc0b4f24 100644 > --- a/include/linux/dma-fence.h > +++ b/include/linux/dma-fence.h > @@ -587,4 +587,42 @@ struct dma_fence *dma_fence_get_stub(void); > struct dma_fence *dma_fence_allocate_private_stub(void); > u64 dma_fence_context_alloc(unsigned num); > > +extern const struct dma_fence_ops dma_fence_array_ops; > +extern const struct dma_fence_ops dma_fence_chain_ops; > + > +/** > + * dma_fence_is_array - check if a fence is from the array subclass > + * @fence: the fence to test > + * > + * Return true if it is a dma_fence_array and false otherwise. > + */ > +static inline bool dma_fence_is_array(struct dma_fence *fence) > +{ > + return fence->ops == &dma_fence_array_ops; > +} > + > +/** > + * dma_fence_is_chain - check if a fence is from the chain subclass > + * @fence: the fence to test > + * > + * Return true if it is a dma_fence_chain and false otherwise. > + */ > +static inline bool dma_fence_is_chain(struct dma_fence *fence) > +{ > + return fence->ops == &dma_fence_chain_ops; > +} > + > +/** > + * dma_fence_is_container - check if a fence is a container for other fences > + * @fence: the fence to test > + * > + * Return true if this fence is a container for other fences, false otherwise. > + * This is important since we can't build up large fence structure or otherwise > + * we run into recursion during operation on those fences. > + */ > +static inline bool dma_fence_is_container(struct dma_fence *fence) > +{ > + return dma_fence_is_array(fence) || dma_fence_is_chain(fence); > +} What's the strategy here moving forward if we add more dma_resv containers, or if a driver adds a container that similarly has risc of recursion? Should we perhaps add an ops bool for this, or require that all dma_resv containers that has this limitation be part of the dma-buf subsystem rather than driver-specific? Thanks, /Thomas > + > #endif /* __LINUX_DMA_FENCE_H */
Am 21.01.22 um 08:41 schrieb Thomas Hellström (Intel): > > On 1/20/22 14:27, Christian König wrote: >> Consolidate the wrapper functions to check for dma_fence >> subclasses in the dma_fence header. >> >> This makes it easier to document and also check the different >> requirements for fence containers in the subclasses. >> >> Signed-off-by: Christian König <christian.koenig@amd.com> >> --- >> include/linux/dma-fence-array.h | 15 +------------ >> include/linux/dma-fence-chain.h | 3 +-- >> include/linux/dma-fence.h | 38 +++++++++++++++++++++++++++++++++ >> 3 files changed, 40 insertions(+), 16 deletions(-) >> >> diff --git a/include/linux/dma-fence-array.h >> b/include/linux/dma-fence-array.h >> index 303dd712220f..fec374f69e12 100644 >> --- a/include/linux/dma-fence-array.h >> +++ b/include/linux/dma-fence-array.h >> @@ -45,19 +45,6 @@ struct dma_fence_array { >> struct irq_work work; >> }; >> -extern const struct dma_fence_ops dma_fence_array_ops; >> - >> -/** >> - * dma_fence_is_array - check if a fence is from the array subsclass >> - * @fence: fence to test >> - * >> - * Return true if it is a dma_fence_array and false otherwise. >> - */ >> -static inline bool dma_fence_is_array(struct dma_fence *fence) >> -{ >> - return fence->ops == &dma_fence_array_ops; >> -} >> - >> /** >> * to_dma_fence_array - cast a fence to a dma_fence_array >> * @fence: fence to cast to a dma_fence_array >> @@ -68,7 +55,7 @@ static inline bool dma_fence_is_array(struct >> dma_fence *fence) >> static inline struct dma_fence_array * >> to_dma_fence_array(struct dma_fence *fence) >> { >> - if (fence->ops != &dma_fence_array_ops) >> + if (!fence || !dma_fence_is_array(fence)) >> return NULL; >> return container_of(fence, struct dma_fence_array, base); >> diff --git a/include/linux/dma-fence-chain.h >> b/include/linux/dma-fence-chain.h >> index 54fe3443fd2c..ee906b659694 100644 >> --- a/include/linux/dma-fence-chain.h >> +++ b/include/linux/dma-fence-chain.h >> @@ -49,7 +49,6 @@ struct dma_fence_chain { >> spinlock_t lock; >> }; >> -extern const struct dma_fence_ops dma_fence_chain_ops; >> /** >> * to_dma_fence_chain - cast a fence to a dma_fence_chain >> @@ -61,7 +60,7 @@ extern const struct dma_fence_ops dma_fence_chain_ops; >> static inline struct dma_fence_chain * >> to_dma_fence_chain(struct dma_fence *fence) >> { >> - if (!fence || fence->ops != &dma_fence_chain_ops) >> + if (!fence || !dma_fence_is_chain(fence)) >> return NULL; >> return container_of(fence, struct dma_fence_chain, base); >> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h >> index 1ea691753bd3..775cdc0b4f24 100644 >> --- a/include/linux/dma-fence.h >> +++ b/include/linux/dma-fence.h >> @@ -587,4 +587,42 @@ struct dma_fence *dma_fence_get_stub(void); >> struct dma_fence *dma_fence_allocate_private_stub(void); >> u64 dma_fence_context_alloc(unsigned num); >> +extern const struct dma_fence_ops dma_fence_array_ops; >> +extern const struct dma_fence_ops dma_fence_chain_ops; >> + >> +/** >> + * dma_fence_is_array - check if a fence is from the array subclass >> + * @fence: the fence to test >> + * >> + * Return true if it is a dma_fence_array and false otherwise. >> + */ >> +static inline bool dma_fence_is_array(struct dma_fence *fence) >> +{ >> + return fence->ops == &dma_fence_array_ops; >> +} >> + >> +/** >> + * dma_fence_is_chain - check if a fence is from the chain subclass >> + * @fence: the fence to test >> + * >> + * Return true if it is a dma_fence_chain and false otherwise. >> + */ >> +static inline bool dma_fence_is_chain(struct dma_fence *fence) >> +{ >> + return fence->ops == &dma_fence_chain_ops; >> +} >> + >> +/** >> + * dma_fence_is_container - check if a fence is a container for >> other fences >> + * @fence: the fence to test >> + * >> + * Return true if this fence is a container for other fences, false >> otherwise. >> + * This is important since we can't build up large fence structure >> or otherwise >> + * we run into recursion during operation on those fences. >> + */ >> +static inline bool dma_fence_is_container(struct dma_fence *fence) >> +{ >> + return dma_fence_is_array(fence) || dma_fence_is_chain(fence); >> +} > > What's the strategy here moving forward if we add more dma_resv > containers, or if a driver adds a container that similarly has risc of > recursion? Should we perhaps add an ops bool for this, or require that > all dma_resv containers that has this limitation be part of the > dma-buf subsystem rather than driver-specific? Good question. I think that all containers which also implement the dma_fence interface should be part of the DMA-buf subsystem and not driver specific. Drivers just tend to reinvent something incorrectly. Where/How should we document that? Regards, Christian. > > Thanks, > /Thomas > > >> + >> #endif /* __LINUX_DMA_FENCE_H */
diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h index 303dd712220f..fec374f69e12 100644 --- a/include/linux/dma-fence-array.h +++ b/include/linux/dma-fence-array.h @@ -45,19 +45,6 @@ struct dma_fence_array { struct irq_work work; }; -extern const struct dma_fence_ops dma_fence_array_ops; - -/** - * dma_fence_is_array - check if a fence is from the array subsclass - * @fence: fence to test - * - * Return true if it is a dma_fence_array and false otherwise. - */ -static inline bool dma_fence_is_array(struct dma_fence *fence) -{ - return fence->ops == &dma_fence_array_ops; -} - /** * to_dma_fence_array - cast a fence to a dma_fence_array * @fence: fence to cast to a dma_fence_array @@ -68,7 +55,7 @@ static inline bool dma_fence_is_array(struct dma_fence *fence) static inline struct dma_fence_array * to_dma_fence_array(struct dma_fence *fence) { - if (fence->ops != &dma_fence_array_ops) + if (!fence || !dma_fence_is_array(fence)) return NULL; return container_of(fence, struct dma_fence_array, base); diff --git a/include/linux/dma-fence-chain.h b/include/linux/dma-fence-chain.h index 54fe3443fd2c..ee906b659694 100644 --- a/include/linux/dma-fence-chain.h +++ b/include/linux/dma-fence-chain.h @@ -49,7 +49,6 @@ struct dma_fence_chain { spinlock_t lock; }; -extern const struct dma_fence_ops dma_fence_chain_ops; /** * to_dma_fence_chain - cast a fence to a dma_fence_chain @@ -61,7 +60,7 @@ extern const struct dma_fence_ops dma_fence_chain_ops; static inline struct dma_fence_chain * to_dma_fence_chain(struct dma_fence *fence) { - if (!fence || fence->ops != &dma_fence_chain_ops) + if (!fence || !dma_fence_is_chain(fence)) return NULL; return container_of(fence, struct dma_fence_chain, base); diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 1ea691753bd3..775cdc0b4f24 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -587,4 +587,42 @@ struct dma_fence *dma_fence_get_stub(void); struct dma_fence *dma_fence_allocate_private_stub(void); u64 dma_fence_context_alloc(unsigned num); +extern const struct dma_fence_ops dma_fence_array_ops; +extern const struct dma_fence_ops dma_fence_chain_ops; + +/** + * dma_fence_is_array - check if a fence is from the array subclass + * @fence: the fence to test + * + * Return true if it is a dma_fence_array and false otherwise. + */ +static inline bool dma_fence_is_array(struct dma_fence *fence) +{ + return fence->ops == &dma_fence_array_ops; +} + +/** + * dma_fence_is_chain - check if a fence is from the chain subclass + * @fence: the fence to test + * + * Return true if it is a dma_fence_chain and false otherwise. + */ +static inline bool dma_fence_is_chain(struct dma_fence *fence) +{ + return fence->ops == &dma_fence_chain_ops; +} + +/** + * dma_fence_is_container - check if a fence is a container for other fences + * @fence: the fence to test + * + * Return true if this fence is a container for other fences, false otherwise. + * This is important since we can't build up large fence structure or otherwise + * we run into recursion during operation on those fences. + */ +static inline bool dma_fence_is_container(struct dma_fence *fence) +{ + return dma_fence_is_array(fence) || dma_fence_is_chain(fence); +} + #endif /* __LINUX_DMA_FENCE_H */
Consolidate the wrapper functions to check for dma_fence subclasses in the dma_fence header. This makes it easier to document and also check the different requirements for fence containers in the subclasses. Signed-off-by: Christian König <christian.koenig@amd.com> --- include/linux/dma-fence-array.h | 15 +------------ include/linux/dma-fence-chain.h | 3 +-- include/linux/dma-fence.h | 38 +++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 16 deletions(-)