Re: [PATCH for-next v2 1/3] RDMA/bnxt_re: Add more flags in device init and uninit path

From: Selvin Xavier <selvin.xavier@broadcom.com>
To: Jason Gunthorpe <jgg@mellanox.com>
Cc: Doug Ledford <dledford@redhat.com>, linux-rdma@vger.kernel.org
Subject: Re: [PATCH for-next v2 1/3] RDMA/bnxt_re: Add more flags in device init and uninit path
Date: Tue, 25 Feb 2020 15:44:07 +0530	[thread overview]
Message-ID: <CA+sbYW2saCOoRs_E83J0raaMQ-=zaLUygYYk3NSkq4ZdMEcs9w@mail.gmail.com> (raw)
In-Reply-To: <20200224134828.GI26318@mellanox.com>

On Mon, Feb 24, 2020 at 7:18 PM Jason Gunthorpe <jgg@mellanox.com> wrote:
>
> On Mon, Feb 24, 2020 at 02:49:53AM -0800, Selvin Xavier wrote:
> > Add more flags for better granularity in in device init/uninit path
> >
> > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> >  drivers/infiniband/hw/bnxt_re/bnxt_re.h | 16 +++++++++----
> >  drivers/infiniband/hw/bnxt_re/main.c    | 41 ++++++++++++++++-----------------
> >  2 files changed, 31 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/bnxt_re/bnxt_re.h b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> > index c736e82..0babc66 100644
> > +++ b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> > @@ -135,11 +135,17 @@ struct bnxt_re_dev {
> >  #define BNXT_RE_FLAG_IBDEV_REGISTERED                1
> >  #define BNXT_RE_FLAG_GOT_MSIX                        2
> >  #define BNXT_RE_FLAG_HAVE_L2_REF             3
> > -#define BNXT_RE_FLAG_RCFW_CHANNEL_EN         4
> > -#define BNXT_RE_FLAG_QOS_WORK_REG            5
> > -#define BNXT_RE_FLAG_RESOURCES_ALLOCATED     7
> > -#define BNXT_RE_FLAG_RESOURCES_INITIALIZED   8
> > -#define BNXT_RE_FLAG_ISSUE_ROCE_STATS          29
> > +#define BNXT_RE_FLAG_ALLOC_RCFW                      4
> > +#define BNXT_RE_FLAG_NET_RING_ALLOC          5
> > +#define BNXT_RE_FLAG_RCFW_CHANNEL_EN         6
> > +#define BNXT_RE_FLAG_ALLOC_CTX                       7
> > +#define BNXT_RE_FLAG_STATS_CTX_ALLOC         8
> > +#define BNXT_RE_FLAG_STATS_CTX2_ALLOC                9
> > +#define BNXT_RE_FLAG_RCFW_CHANNEL_INIT               10
> > +#define BNXT_RE_FLAG_QOS_WORK_REG            11
> > +#define BNXT_RE_FLAG_RESOURCES_ALLOCATED     12
> > +#define BNXT_RE_FLAG_RESOURCES_INITIALIZED   13
> > +#define BNXT_RE_FLAG_ISSUE_ROCE_STATS                29
>
> The error unwind is better than adding these endless flags.
>
> It is hard to understand the flow with all this needless conditional,
> really if you want to change this I'd delete the test_bit scheme and
> stick to the normal goto error unwind.
The existing code was doing few  error unwinding. I thought it would
be simple to collect everything under bnxt_re_ib_unreg function and
cleanup with these flags. I am ok with both the approaches. Maybe,
I can drop this patch and post the remaining series.

>
> Jason