On Wed, Sep 16, 2020 at 11:12:02AM -0300, Jason Gunthorpe wrote:
> On Wed, Sep 16, 2020 at 03:44:29PM +0300, Leon Romanovsky wrote:
> > On Wed, Sep 16, 2020 at 09:04:40AM -0300, Jason Gunthorpe wrote:
> > > On Wed, Sep 16, 2020 at 01:37:10PM +0300, Leon Romanovsky wrote:
> > > > It depends on how you want to treat errors from rdma_read_gid_attr_ndev_rcu().
> > > > Current check allows us to ensure that any error returned by this call is
> > > > handled.
> > > >
> > > > Otherwise we will find ourselves with something like this:
> > > > ndev = rdma_read_gid_attr_ndev_rcu(gid_attr);
> > > > if (IS_ERR(ndev)) {
> > > > if (rdma_protocol_roce())
> > > > goto error;
> > > > if (ERR_PTR(ndev) != -ENODEV)
> > > > goto error;
> > > > }
> > >
> > > Isn't it just
> > >
> > > if (IS_ERR(ndev)) {
> > > if (ERR_PTR(ndev) != -ENODEV)
> > > goto error;
> > > index = -1;
> > > }
> > >
> > > Which seems fine and clear enough
> >
> > It is a problem if roce device returned -ENODEV.
>
> Can it happen? RCU I suppose, but I think this is an issue in
> rdma_read_gid_attr_ndev_rcu() - it should not return ENODEV if the RCU
> shows the gid_attr is being concurrently destroyed
>From RoCE point of view, it is a problem if device is destroyed or gid
not valid, the different returned values won't change much. For the IB,
we don't care.
>
> Jason
