Messages in this thread | | | From | Bartosz Golaszewski <> | Date | Thu, 28 Sep 2023 20:23:36 +0200 | Subject | Re: [PATCH v2 02/11] firmware: qcom: scm: add a dedicated SCM memory allocator |
| |
On Thu, Sep 28, 2023 at 8:19 PM Jeff Johnson <quic_jjohnson@quicinc.com> wrote: > > On 9/28/2023 2:20 AM, Bartosz Golaszewski wrote: > > +void *qcom_scm_mem_alloc(size_t size, gfp_t gfp) > > +{ > > + struct qcom_scm_mem_chunk *chunk; > > + unsigned long vaddr; > > there are places below where you unnecessarily typecast this to its > given type >
Ah, it's a leftover from when this variable was of type void *. Thanks.
> > + int ret; > > + > > + if (!size) > > + return ZERO_SIZE_PTR; > > + > > + size = roundup(size, 1 << PAGE_SHIFT); > > + > > + chunk = kzalloc(sizeof(*chunk), gfp); > > + if (!chunk) > > + return NULL; > > + > > + vaddr = gen_pool_alloc(qcom_scm_mem.pool, size); > > + if (!vaddr) { > > + kfree(chunk); > > + return NULL; > > + } > > + > > + chunk->paddr = gen_pool_virt_to_phys(qcom_scm_mem.pool, > > + (unsigned long)vaddr); > > unnecessary typecast? > > > + chunk->size = size; > > + > > + scoped_guard(spinlock_irqsave, &qcom_scm_mem.lock) { > > my first exposure to this infrastructure..very cool now that I've > wrapped my head around it! This helped for those also new to this: > https://lwn.net/Articles/934679/ >
It's amazing but be careful with it. I used it wrong in one place and got yelled at by Linus Torvalds personally already. :)
Bartosz
> > + ret = radix_tree_insert(&qcom_scm_mem.chunks, vaddr, chunk); > > + if (ret) { > > + gen_pool_free(qcom_scm_mem.pool, (unsigned long)vaddr, > > unnecessary typecast? > > > + chunk->size); > > + kfree(chunk); > > + return NULL; > > + } > > + } > > + > > + return (void *)vaddr; > > +} >
| |