[PATCH 1/1] mm: change inlined allocation helpers to account at the call site

Kent Overstreet kent.overstreet at linux.dev
Thu Apr 4 22:28:43 UTC 2024


On Thu, Apr 04, 2024 at 03:17:43PM -0700, Suren Baghdasaryan wrote:
> On Thu, Apr 4, 2024 at 10:08 AM Suren Baghdasaryan <surenb at google.com> wrote:
> >
> > On Thu, Apr 4, 2024 at 10:04 AM Matthew Wilcox <willy at infradead.org> wrote:
> > >
> > > On Thu, Apr 04, 2024 at 09:54:04AM -0700, Suren Baghdasaryan wrote:
> > > > +++ b/include/linux/dma-fence-chain.h
> > > > @@ -86,10 +86,7 @@ dma_fence_chain_contained(struct dma_fence *fence)
> > > >   *
> > > >   * Returns a new struct dma_fence_chain object or NULL on failure.
> > > >   */
> > > > -static inline struct dma_fence_chain *dma_fence_chain_alloc(void)
> > > > -{
> > > > -     return kmalloc(sizeof(struct dma_fence_chain), GFP_KERNEL);
> > > > -};
> > > > +#define dma_fence_chain_alloc()      kmalloc(sizeof(struct dma_fence_chain), GFP_KERNEL)
> > >
> > > You've removed some typesafety here.  Before, if I wrote:
> > >
> > >         struct page *page = dma_fence_chain_alloc();
> > >
> > > the compiler would warn me that I've done something stupid.  Now it
> > > can't tell.  Suggest perhaps:
> > >
> > > #define dma_fence_chain_alloc()                                           \
> > >         (struct dma_fence_chain *)kmalloc(sizeof(struct dma_fence_chain), \
> > >                                                 GFP_KERNEL)
> > >
> > > but maybe there's a better way of doing that.  There are a few other
> > > occurrences of the same problem in this monster patch.
> >
> > Got your point.
> 
> Ironically, checkpatch generates warnings for these type casts:
> 
> WARNING: unnecessary cast may hide bugs, see
> http://c-faq.com/malloc/mallocnocast.html
> #425: FILE: include/linux/dma-fence-chain.h:90:
> + ((struct dma_fence_chain *)kmalloc(sizeof(struct dma_fence_chain),
> GFP_KERNEL))
> 
> I guess I can safely ignore them in this case (since we cast to the
> expected type)?

Correct, it's not hiding bugs in this case, it's adding type safety.

checkpatch is definitely not authoritative, you really have to use your
own judgement with it



More information about the Linux-security-module-archive mailing list