On Mon, 12 Apr 2021 19:35:09 -0700 Dexuan Cui wrote:
> + apc->port_st_save = apc->port_is_up;
> + apc->port_is_up = false;
> + apc->start_remove = true;
> +
> + /* Ensure port state updated before txq state */
> + smp_wmb();
> +
> + netif_tx_disable(ndev);
In your napi poll method there is no barrier between port_is_up check
and netif_tx_queue_stopped().
> + netif_carrier_off(ndev);
> +
> + /* No packet can be transmitted now since apc->port_is_up is false.
> + * There is still a tiny chance that mana_poll_tx_cq() can re-enable
> + * a txq because it may not timely see apc->port_is_up being cleared
> + * to false, but it doesn't matter since mana_start_xmit() drops any
> + * new packets due to apc->port_is_up being false.
> + *
> + * Drain all the in-flight TX packets
> + */
> + for (i = 0; i < apc->num_queues; i++) {
> + txq = &apc->tx_qp[i].txq;
> +
> + while (atomic_read(&txq->pending_sends) > 0)
> + usleep_range(1000, 2000);
> + }
> + /* All cleanup actions should stay after rtnl_lock(), otherwise
> + * other functions may access partially cleaned up data.
> + */
> + rtnl_lock();
> +
> + mana_detach(ndev);
> +
> + unregister_netdevice(ndev);
> +
> + rtnl_unlock();
I find the resource management somewhat strange. Why is mana_attach()
and mana_detach() called at probe/remove time, and not when the
interface is brought up? Presumably when the user ifdowns the interface
there is no point holding the resources? Your open/close methods are
rather empty.
> + if ((eq_addr & PAGE_MASK) != eq_addr)
> + return -EINVAL;
> +
> + if ((cq_addr & PAGE_MASK) != cq_addr)
> + return -EINVAL;
> +
> + if ((rq_addr & PAGE_MASK) != rq_addr)
> + return -EINVAL;
> +
> + if ((sq_addr & PAGE_MASK) != sq_addr)
> + return -EINVAL;
PAGE_ALIGNED()
