include/linux/dma-fence-chain.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Casting the return value of kmalloc() is unnecessary and can be
removed. Remove it and fix the following Coccinelle/coccicheck warning
reported by alloc_cast.cocci:
WARNING: casting value returned by memory allocation function to (struct dma_fence_chain *) is useless.
Compile-tested only.
Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com>
---
include/linux/dma-fence-chain.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/dma-fence-chain.h b/include/linux/dma-fence-chain.h
index ad9e2506c2f4..a9dc82e1b7f3 100644
--- a/include/linux/dma-fence-chain.h
+++ b/include/linux/dma-fence-chain.h
@@ -87,7 +87,7 @@ dma_fence_chain_contained(struct dma_fence *fence)
* Returns a new struct dma_fence_chain object or NULL on failure.
*/
#define dma_fence_chain_alloc() \
- ((struct dma_fence_chain *)kmalloc(sizeof(struct dma_fence_chain), GFP_KERNEL))
+ kmalloc(sizeof(struct dma_fence_chain), GFP_KERNEL)
/**
* dma_fence_chain_free
--
2.39.2On Sun, 30 Jun 2024 03:12:16 +0200 Thorsten Blum <thorsten.blum@toblux.com> wrote: > Casting the return value of kmalloc() is unnecessary and can be > removed. Remove it and fix the following Coccinelle/coccicheck warning > reported by alloc_cast.cocci: > > WARNING: casting value returned by memory allocation function to (struct dma_fence_chain *) is useless. > > Compile-tested only. > > ... > > --- a/include/linux/dma-fence-chain.h > +++ b/include/linux/dma-fence-chain.h > @@ -87,7 +87,7 @@ dma_fence_chain_contained(struct dma_fence *fence) > * Returns a new struct dma_fence_chain object or NULL on failure. > */ > #define dma_fence_chain_alloc() \ > - ((struct dma_fence_chain *)kmalloc(sizeof(struct dma_fence_chain), GFP_KERNEL)) > + kmalloc(sizeof(struct dma_fence_chain), GFP_KERNEL) > > /** > * dma_fence_chain_free No, I do think the cast is useful: struct page *page = dma_fence_chain_alloc(); will presently generate a warning. We want this. Your change will remove that useful warning. Unrelatedly: there is no earthly reason why this is implemented as a macro. A static inline function would be so much better. Why do we keep doing this.
On Mon, Jul 01, 2024 at 11:26:34PM -0700, Andrew Morton wrote: > No, I do think the cast is useful: > > struct page *page = dma_fence_chain_alloc(); > > will presently generate a warning. We want this. Your change will > remove that useful warning. > > > Unrelatedly: there is no earthly reason why this is implemented as a > macro. A static inline function would be so much better. Why do we > keep doing this. Agreed with all of the above. Adding the dmabuf maintainers.
Am 02.07.24 um 08:40 schrieb Christoph Hellwig: > On Mon, Jul 01, 2024 at 11:26:34PM -0700, Andrew Morton wrote: >> No, I do think the cast is useful: >> >> struct page *page = dma_fence_chain_alloc(); >> >> will presently generate a warning. We want this. Your change will >> remove that useful warning. >> >> >> Unrelatedly: there is no earthly reason why this is implemented as a >> macro. A static inline function would be so much better. Why do we >> keep doing this. > Agreed with all of the above. Adding the dmabuf maintainers. Thanks for adding me and I have to ask to be added on DMA-buf patches when initially sending them out. First of all: Yes that cast is intentionally there and yes that is intentionally a define and not an inline function. See this patch here which changed that: commit 2c321f3f70bc284510598f712b702ce8d60c4d14 Author: Suren Baghdasaryan <surenb@google.com> Date: Sun Apr 14 19:07:31 2024 -0700 mm: change inlined allocation helpers to account at the call site Main goal of memory allocation profiling patchset is to provide accounting that is cheap enough to run in production. To achieve that we inject counters using codetags at the allocation call sites to account every time allocation is made. This injection allows us to perform accounting efficiently because injected counters are immediately available as opposed to the alternative methods, such as using _RET_IP_, which would require counter lookup and appropriate locking that makes accounting much more expensive. This method requires all allocation functions to inject separate counters at their call sites so that their callers can be individually accounted. Counter injection is implemented by allocation hooks which should wrap all allocation functions. Inlined functions which perform allocations but do not use allocation hooks are directly charged for the allocations they perform. In most cases these functions are just specialized allocation wrappers used from multiple places to allocate objects of a specific type. It would be more useful to do the accounting at their call sites instead. Instrument these helpers to do accounting at the call site. Simple inlined allocation wrappers are converted directly into macros. More complex allocators or allocators with documentation are converted into _noprof versions and allocation hooks are added. This allows memory allocation profiling mechanism to charge allocations to the callers of these functions. Regards, Christian.
On Tue, 2 Jul 2024 09:13:35 +0200 Christian König <christian.koenig@amd.com> wrote: > yes that is > intentionally a define and not an inline function. > > See this patch here which changed that: > > commit 2c321f3f70bc284510598f712b702ce8d60c4d14 > Author: Suren Baghdasaryan <surenb@google.com> > Date: Sun Apr 14 19:07:31 2024 -0700 > > mm: change inlined allocation helpers to account at the call site Dang, yes, that was a regrettable change. But hardly the end of the world. I do think each such alteration should have included a comment to prevent people from going and cleaning them up.
On Tue, Jul 2, 2024 at 7:34 AM Andrew Morton <akpm@linux-foundation.org> wrote: > > On Tue, 2 Jul 2024 09:13:35 +0200 Christian König <christian.koenig@amd.com> wrote: > > > yes that is > > intentionally a define and not an inline function. > > > > See this patch here which changed that: > > > > commit 2c321f3f70bc284510598f712b702ce8d60c4d14 > > Author: Suren Baghdasaryan <surenb@google.com> > > Date: Sun Apr 14 19:07:31 2024 -0700 > > > > mm: change inlined allocation helpers to account at the call site > > Dang, yes, that was a regrettable change. But hardly the end of the > world. I do think each such alteration should have included a comment > to prevent people from going and cleaning them up. Sorry I missed this discussion. Yes, the definition was intentional and I will add comments for all the cases which were changed this way. Thanks, Suren. > >
On Tue, Jul 2, 2024 at 8:15 AM Suren Baghdasaryan <surenb@google.com> wrote: > > On Tue, Jul 2, 2024 at 7:34 AM Andrew Morton <akpm@linux-foundation.org> wrote: > > > > On Tue, 2 Jul 2024 09:13:35 +0200 Christian König <christian.koenig@amd.com> wrote: > > > > > yes that is > > > intentionally a define and not an inline function. > > > > > > See this patch here which changed that: > > > > > > commit 2c321f3f70bc284510598f712b702ce8d60c4d14 > > > Author: Suren Baghdasaryan <surenb@google.com> > > > Date: Sun Apr 14 19:07:31 2024 -0700 > > > > > > mm: change inlined allocation helpers to account at the call site > > > > Dang, yes, that was a regrettable change. But hardly the end of the > > world. I do think each such alteration should have included a comment > > to prevent people from going and cleaning them up. > > Sorry I missed this discussion. Yes, the definition was intentional > and I will add comments for all the cases which were changed this way. Posted https://lore.kernel.org/all/20240703174225.3891393-1-surenb@google.com/ adding clarifying comments. Thanks, Suren. > Thanks, > Suren. > > > > >
On Tue, Jul 02, 2024 at 12:33:57AM -0700, Andrew Morton wrote: > Dang, yes, that was a regrettable change. But hardly the end of the > world. I do think each such alteration should have included a comment > to prevent people from going and cleaning them up. .. or we should have never merged that utter mess ..
© 2016 - 2025 Red Hat, Inc.