Re: [Patch v2 3/5] RDMA/mana_ib : Add error eq and notification from SoC

From: Simon Horman <simon.horman@corigine.com>
To: sharmaajay@linuxonhyperv.com
Cc: Jason Gunthorpe <jgg@ziepe.ca>, Leon Romanovsky <leon@kernel.org>,
	Dexuan Cui <decui@microsoft.com>, Wei Liu <wei.liu@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	linux-rdma@vger.kernel.org, linux-hyperv@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Ajay Sharma <sharmaajay@microsoft.com>
Subject: Re: [Patch v2 3/5] RDMA/mana_ib : Add error eq and notification from SoC
Date: Wed, 26 Jul 2023 14:25:43 +0200	[thread overview]
Message-ID: <ZMEQxzqhdbNRxAfE@corigine.com> (raw)
In-Reply-To: <1690343820-20188-4-git-send-email-sharmaajay@linuxonhyperv.com>

On Tue, Jul 25, 2023 at 08:56:58PM -0700, sharmaajay@linuxonhyperv.com wrote:

...

> diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
> index 2c4e3c496644..2ea24ba3065f 100644
> --- a/drivers/infiniband/hw/mana/main.c
> +++ b/drivers/infiniband/hw/mana/main.c
> @@ -504,3 +504,47 @@ int mana_ib_query_gid(struct ib_device *ibdev, u32 port, int index,
>  void mana_ib_disassociate_ucontext(struct ib_ucontext *ibcontext)
>  {
>  }
> +
> +void mana_ib_soc_event_handler(void *ctx, struct gdma_queue *queue,
> +				struct gdma_event *event)

Hi Ajay,

I wonder if this function should be static.
It seems to only be used in this file.

> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c

...

> @@ -435,44 +434,47 @@ static int mana_gd_register_irq(struct gdma_queue *queue,
>  	gc = gd->gdma_context;
>  	r = &gc->msix_resource;
>  	dev = gc->dev;
> +	msi_index = spec->eq.msix_index;
>  
>  	spin_lock_irqsave(&r->lock, flags);
>  
> -	msi_index = find_first_zero_bit(r->map, r->size);
> -	if (msi_index >= r->size || msi_index >= gc->num_msix_usable) {
> -		err = -ENOSPC;
> -	} else {
> -		bitmap_set(r->map, msi_index, 1);
> -		queue->eq.msix_index = msi_index;
> -	}
> -
> -	spin_unlock_irqrestore(&r->lock, flags);
> +	if (!spec->eq.msix_allocated) {
> +		msi_index = find_first_zero_bit(r->map, r->size);
>  
> -	if (err) {
> -		dev_err(dev, "Register IRQ err:%d, msi:%u rsize:%u, nMSI:%u",
> -			err, msi_index, r->size, gc->num_msix_usable);
> +			if (msi_index >= r->size ||
> +			    msi_index >= gc->num_msix_usable)
> +				err = -ENOSPC;
> +			else
> +				bitmap_set(r->map, msi_index, 1);

It looks like the indention of the lines above is off.
There seems to be one tab too many.

>  
> -		return err;
> +		if (err) {
> +			dev_err(dev, "Register IRQ err:%d, msi:%u rsize:%u, nMSI:%u",
> +				err, msi_index, r->size, gc->num_msix_usable);
> +				goto out;
> +		}
>  	}

...