On Thu, Jan 31, 2019 at 03:24:01PM +0000, Bernard Metzler wrote:
> >> +static void siw_netdev_unregistered(struct work_struct *work)
> >> +{
> >> + struct siw_device *sdev = container_of(work, struct siw_device,
> >> + netdev_unregister);
> >> +
> >> + struct siw_qp_attrs qp_attrs;
> >> + struct list_head *pos, *tmp;
> >> +
> >> + memset(&qp_attrs, 0, sizeof(qp_attrs));
> >> + qp_attrs.state = SIW_QP_STATE_ERROR;
> >> +
> >> + /*
> >> + * Mark all current QP's of this device dead
> >> + */
> >> + list_for_each_safe(pos, tmp, &sdev->qp_list) {
> >> + struct siw_qp *qp = list_entry(pos, struct siw_qp, devq);
> >> +
> >> + down_write(&qp->state_lock);
> >> + (void) siw_qp_modify(qp, &qp_attrs, SIW_QP_ATTR_STATE);
> >> + up_write(&qp->state_lock);
> >> + }
> >
> >Don't put code around ib_unregister. If you need to do things use a
> >callback at the right moment in the unregister flow. You might need
> >to
> >add a callback (see my latest series)
>
> I thought by having that executed in a work queue context only
> we'd be safe. I have to bring down all related QP's if a network device
> suddenly disappears... Looks like I have to better understand the
> intended unregister flow.
If it is important it should be done during module unload and link
removal, if it is not important, it shouldn't be done :)
The netdev will not fully unregister until the IB device unregisters
because of the set_netdev thing.
rxe and siw may need to support 'device disassociate' to be fully
correct.
> >You should also largely run the whole thing through clang-format
> >
>
> Will do.
>
> I used sparse and checkpatch so far. I thought that would
> make it for kernel patches... (obviously lots of
> RDMA midlayer code made it even w/o those checks ;) ).
It is good, take all of these tools with a grain of salt, they are
sort of 90% useful
I run every patch though checkpatch as I apply it.
Jason
