On Thu, Feb 16, 2023 at 13:48 Stanislaw Gruszka <stanislaw.gruszka@xxxxxxxxxxxxxxx> wrote:
> > The same mutex lock/unlock and counter decrementing in
> > hl_release_dmabuf() is already done in the memhash_node_export_put()
> > helper function.
> >
> > Signed-off-by: Tomer Tayar <ttayar@xxxxxxxxx>
> > Reviewed-by: Oded Gabbay <ogabbay@xxxxxxxxxx>
> > Signed-off-by: Oded Gabbay <ogabbay@xxxxxxxxxx>
> > ---
> > drivers/accel/habanalabs/common/memory.c | 89 ++++++++++++----------
> --
> > 1 file changed, 43 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/accel/habanalabs/common/memory.c
> b/drivers/accel/habanalabs/common/memory.c
> > index e6474d38afc4..7b3809853dd5 100644
> > --- a/drivers/accel/habanalabs/common/memory.c
> > +++ b/drivers/accel/habanalabs/common/memory.c
> > @@ -1779,6 +1779,47 @@ static void hl_unmap_dmabuf(struct
> dma_buf_attachment *attachment,
> > kfree(sgt);
> > }
> >
> > +static struct hl_vm_hash_node *memhash_node_export_get(struct hl_ctx
> *ctx, u64 addr)
> > +{
> > + struct hl_device *hdev = ctx->hdev;
> > + struct hl_vm_hash_node *hnode;
> > +
> > + /* get the memory handle */
> > + mutex_lock(&ctx->mem_hash_lock);
> > + hash_for_each_possible(ctx->mem_hash, hnode, node, (unsigned
> long)addr)
> > + if (addr == hnode->vaddr)
> > + break;
> > +
> > + if (!hnode) {
>
> This looks suspicious, I think hnode can be not-NULL here and has
> hnode->vaddr different than searched addr, in case there is
> hash collision and we iterate over hlist where there is no
> searched addr. Not 100% sure about that though.
>
> I think would be better to provide helper like this:
>
> hash_for_each_possible(ctx->mem_hash, hnode, node, (unsigned
> long)addr)
> if (addr == hnode->vaddr)
> return hnode;
> return NULL;
>
> which is basically standard way how hash_for_each_possible() used.
>
>
> Regards
> Stanislaw
Thanks Stanislaw, we will add such a helper and use it here and in another place with a similar pattern.
If that is okay, we will do it in another patch, as this one only moves an existing function for code reuse.
