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..
Jason
