Re: [PATCH for-next 1/2] RDMA/efa: Move host info set to first ucontext allocation

[Leon Romanovsky <leon@kernel.org>]

On Tue, Jan 05, 2021 at 02:22:23PM +0200, Gal Pressman wrote:
> On 05/01/2021 13:21, Leon Romanovsky wrote:
> > On Tue, Jan 05, 2021 at 12:43:25PM +0200, Gal Pressman wrote:
> >> Downstream patch will require the userspace version which is passed as
> >> part of ucontext allocation. Move the host info set there and make sure
> >> it's only called once (on the first allocation).
> >>
> >> Reviewed-by: Firas JahJah <firasj@amazon.com>
> >> Reviewed-by: Leonid Feschuk <lfesch@amazon.com>
> >> Signed-off-by: Gal Pressman <galpress@amazon.com>
> >> ---
> >>  drivers/infiniband/hw/efa/efa.h       | 7 +++++++
> >>  drivers/infiniband/hw/efa/efa_main.c  | 4 +---
> >>  drivers/infiniband/hw/efa/efa_verbs.c | 3 +++
> >>  3 files changed, 11 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/infiniband/hw/efa/efa.h b/drivers/infiniband/hw/efa/efa.h
> >> index e5d9712e98c4..9c9cd5867489 100644
> >> --- a/drivers/infiniband/hw/efa/efa.h
> >> +++ b/drivers/infiniband/hw/efa/efa.h
> >> @@ -45,6 +45,11 @@ struct efa_stats {
> >>       atomic64_t keep_alive_rcvd;
> >>  };
> >>
> >> +enum {
> >> +     EFA_FLAGS_HOST_INFO_SET_BIT,
> >> +     EFA_FLAGS_NUM,
> >> +};
> >> +
> >>  struct efa_dev {
> >>       struct ib_device ibdev;
> >>       struct efa_com_dev edev;
> >> @@ -62,6 +67,7 @@ struct efa_dev {
> >>       struct efa_irq admin_irq;
> >>
> >>       struct efa_stats stats;
> >> +     DECLARE_BITMAP(flags, EFA_FLAGS_NUM);
> >>  };
> >
> > Why do you need such over-engineering?
> > What is wrong with old school "u8 flag"?
>
> The main reason is for the atomic test_and_set_bit() usage, otherwise it would
> be an atomic flag, not u8 flag.

But efa_dev can be opened with different applications and they can have
different user space versions, but you are setting this info for the
first caller only. How will it help for the debug?

Thanks