Re: [PATCH v4 1/5] firmware: stratix10-svc: Add mutex lock and unlock in stratix10 memory allocation/free

[Mahesh Rao]

Hi Matthew,

On 10-06-2025 09:36 pm, Matthew Gerlach wrote:
> On 6/10/25 8:37 AM, Mahesh Rao via B4 Relay wrote:
> > From: Mahesh Rao <mahesh.rao@xxxxxxxxxx>
> >
> > This commit adds a mutex lock to protect the
> > stratix10_svc_allocate_memory and
> > stratix10_svc_free_memory functions to ensure
> > thread safety when allocating and freeing memory.
> > This prevents potential race conditions and ensures
> > synchronization.
> >
> > Signed-off-by: Mahesh Rao <mahesh.rao@xxxxxxxxxx>
> > Reviewed-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxx>
> > ---
> >   drivers/firmware/stratix10-svc.c | 30 +++++++++++++++++++++++-------
> >   1 file changed, 23 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/ 
> > stratix10-svc.c
> > index 
> > e3f990d888d71829f0ab22b8a59aa7af0316bea0..955468555738b2031dfcf6dc4db7dbf11ccc482c 100644
> > --- a/drivers/firmware/stratix10-svc.c
> > +++ b/drivers/firmware/stratix10-svc.c
> > @@ -1,6 +1,7 @@
> >   // SPDX-License-Identifier: GPL-2.0
> >   /*
> >    * Copyright (C) 2017-2018, Intel Corporation
> > + * Copyright (C) 2025, Altera Corporation
> >    */
> >   #include <linux/completion.h>
> > @@ -171,6 +172,10 @@ struct stratix10_svc_chan {
> >   static LIST_HEAD(svc_ctrl);
> >   static LIST_HEAD(svc_data_mem);
> > +/* svc_mem_lock protects access to the svc_data_mem list for
> > + * concurrent multi-client operations
> > + */
> > +static DEFINE_MUTEX(svc_mem_lock);
> >   /**
> >    * svc_pa_to_va() - translate physical address to virtual address
> > @@ -182,14 +187,17 @@ static LIST_HEAD(svc_data_mem);
> >   static void *svc_pa_to_va(unsigned long addr)
> >   {
> >       struct stratix10_svc_data_mem *pmem;
> > +    void *ret = NULL;
> >       pr_debug("claim back P-addr=0x%016x\n", (unsigned int)addr);
> > +    mutex_lock(&svc_mem_lock);
> >       list_for_each_entry(pmem, &svc_data_mem, node)
> > -        if (pmem->paddr == addr)
> > -            return pmem->vaddr;
> > -
> > -    /* physical address is not found */
> > -    return NULL;
> > +        if (pmem->paddr == addr) {
> > +            /* physical address is found */
> > +            ret = pmem->vaddr;
>
> I think you want a "break;" here.
>
>
> Matthew Geralch

Will make the change.
> > +        }
> > +    mutex_unlock(&svc_mem_lock);
> > +    return ret;
> >   }
> >   /**
> > @@ -990,6 +998,7 @@ int stratix10_svc_send(struct stratix10_svc_chan