On Sun, Dec 09, 2018 at 01:53:15PM -0700, Jason Gunthorpe wrote:
> On Sun, Dec 09, 2018 at 06:58:57PM +0200, Leon Romanovsky wrote:
> > On Sun, Dec 09, 2018 at 09:54:58AM -0700, Jason Gunthorpe wrote:
> > > On Sun, Dec 09, 2018 at 12:44:40PM +0200, Leon Romanovsky wrote:
> > > > -static void handle_atomics(struct mlx5_ib_qp *qp, struct mlx5_cqe64 *cqe64,
> > > > - u16 tail, u16 head)
> > > > -{
> > > > - u16 idx;
> > > > -
> > > > - do {
> > > > - idx = tail & (qp->sq.wqe_cnt - 1);
> > > > - handle_atomic(qp, cqe64, idx);
> > > > - if (idx == head)
> > > > - break;
> > > > -
> > > > - tail = qp->sq.w_list[idx].next;
> > > > - } while (1);
> > > > - tail = qp->sq.w_list[idx].next;
> > > > - qp->sq.last_poll = tail;
> > >
> > > This line is the only thing that has an external impact, is it OK to
> > > delete it?
> >
> > I hope so, no new bugs were found.
>
> The only place this variable gets read is during the
> MLX5_DEVICE_STATE_INTERNAL_ERROR flow (ie sw_send_comp) that is making
> fake WC's - somehow I doubt testing of
> MLX5_DEVICE_STATE_INTERNAL_ERROR includes generating ATOMICs and
> trying to abort the device..
>
> So I'm worried this will break something.. I'd rather see a patch
> fixing a compilation bug keep the loop and rename the function??
>
> Or at least the commit message should explain why it isn't necessary
> to keep last_poll updated..
We (Maor and I) looked again on the change and sw_send_comp() and it
looks like that function needs extra love to be awesome. I'll respin
this patch together with cleanup in sw_send_comp/sw_recv_comp.
Thanks
>
> Jason
Attachment:
signature.asc
Description: PGP signature
