> +static void mana_link_state_handle(struct work_struct *w)
> +{
> + struct mana_context *ac =
> + container_of(w, struct mana_context, link_change_work.work);
> + struct mana_port_context *apc;
> + struct net_device *ndev;
> + bool link_up;
> + int i;
Since you don't need ac here, i would postpone the assignment into the
body of the function, so keeping with reverse christmass tree.
> +
> + if (!rtnl_trylock()) {
> + schedule_delayed_work(&ac->link_change_work, 1);
> + return;
> + }
Is there a deadlock you are trying to avoid here? Why not wait for the
lock?
> +
> + if (ac->link_event == HWC_DATA_HW_LINK_CONNECT)
> + link_up = true;
> + else if (ac->link_event == HWC_DATA_HW_LINK_DISCONNECT)
> + link_up = false;
> + else
> + goto out;
> +
> + /* Process all ports */
> + for (i = 0; i < ac->num_ports; i++) {
> + ndev = ac->ports[i];
> + if (!ndev)
> + continue;
> +
> + apc = netdev_priv(ndev);
> +
> + if (link_up) {
> + netif_carrier_on(ndev);
> +
> + if (apc->port_is_up)
> + netif_tx_wake_all_queues(ndev);
> +
> + __netdev_notify_peers(ndev);
> + } else {
> + if (netif_carrier_ok(ndev)) {
> + netif_tx_disable(ndev);
> + netif_carrier_off(ndev);
> + }
> + }
It is odd this is asymmetric. Up and down should really be opposites.
> @@ -3500,6 +3548,8 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
> int err;
> int i;
>
> + cancel_delayed_work_sync(&ac->link_change_work);
I don't know delayed work too well. Is this sufficient when the work
requeues itself because it cannot get RTNL?
Andrew
