Hi Maxime,
> -----Original Message-----
> From: Maxime Coquelin <maxime.coque...@redhat.com>
> Sent: Tuesday, October 8, 2024 12:52 AM
> To: Vargas, Hernan <hernan.var...@intel.com>; dev@dpdk.org;
> gak...@marvell.com; t...@redhat.com
> Cc: Chautru, Nicolas <nicolas.chau...@intel.com>; Zhang, Qi Z
> <qi.z.zh...@intel.com>
> Subject: Re: [PATCH v2 05/10] baseband/acc: enhance SW ring alignment
>
>
>
> On 10/3/24 22:49, Hernan Vargas wrote:
> > Calculate the aligned total size required for queue rings, ensuring
> > that the size is a power of two for proper memory allocation.
> >
> > Signed-off-by: Hernan Vargas <hernan.var...@intel.com>
> > ---
> > drivers/baseband/acc/acc_common.h | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/baseband/acc/acc_common.h
> > b/drivers/baseband/acc/acc_common.h
> > index 0d1c26166ff2..8ac1ca001c1d 100644
> > --- a/drivers/baseband/acc/acc_common.h
> > +++ b/drivers/baseband/acc/acc_common.h
> > @@ -767,19 +767,20 @@ alloc_sw_rings_min_mem(struct rte_bbdev
> *dev, struct acc_device *d,
> > int i = 0;
> > uint32_t q_sw_ring_size = ACC_MAX_QUEUE_DEPTH *
> get_desc_len();
> > uint32_t dev_sw_ring_size = q_sw_ring_size * num_queues;
> > - /* Free first in case this is a reconfiguration */
> > + uint32_t alignment = q_sw_ring_size *
> rte_align32pow2(num_queues);
> > + /* Free first in case this is dev_sw_ring_size, q_sw_ring_size,
> > +socket); reconfiguration */
>
> There is a copy/paste mistake in the comment?
Thanks, yes. We missed it in the rebase somehow.
>
> > rte_free(d->sw_rings_base);
> >
> > /* Find an aligned block of memory to store sw rings */
> > while (i < ACC_SW_RING_MEM_ALLOC_ATTEMPTS) {
> > /*
> > * sw_ring allocated memory is guaranteed to be aligned to
> > - * q_sw_ring_size at the condition that the requested size is
> > + * alignment at the condition that the requested size is
>
> This comment is really unclear "aligned to alignment"
Unclear indeed when reading it again. Should be "aligned to the variable
`alignment` ..."
Ie the change is purely to use now the new variable `alignment` instead of
`queue_ring_size`
Thanks
>
> > * less than the page size
> > */
> > sw_rings_base = rte_zmalloc_socket(
> > dev->device->driver->name,
> > - dev_sw_ring_size, q_sw_ring_size, socket);
> > + dev_sw_ring_size, alignment, socket);
> >
> > if (sw_rings_base == NULL) {
> > rte_acc_log(ERR,
