spin_lock -> spin_lock_irqsave and matching unlock
authorDan Cross <crossd@gmail.com>
Mon, 10 Apr 2017 18:47:46 +0000 (14:47 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Mon, 17 Apr 2017 18:07:36 +0000 (14:07 -0400)
I discovered a kernel panic when the mlx driver tried
to spinlock an IRQ-saving lock.  This is the result of
a manual audit to look for such bugs and repair them.
Is it all correct?  Who knows.  These drivers are
behemoths and actively searching through all of them
and verifing actual correctness of all locking calls in
all contexts is a big job that, frankly, none of us
have adequate time for at the moment.

Spinlock debugging FTW for pointing the problem out in
the first place.  Arguably, we ought to have separate
structure types for the different kinds of locks so
that this could be a compile time error.

Change-Id: I314c285672d15a5b43e0c2b0cb70b9b259c6c437
Signed-off-by: Dan Cross <crossd@gmail.com>
Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/drivers/net/bnx2x/bnx2x_main.c
kern/drivers/net/mlx4/en_netdev.c
kern/drivers/net/mlx4/mlx4_en.h
kern/drivers/net/mlx4/qp.c
kern/drivers/net/mlx4/resource_tracker.c
kern/drivers/net/mlx4/srq.c
kern/drivers/net/mlx4u/qp.c
kern/drivers/net/mlx4u/srq.c

index 1026113..0ac3e12 100644 (file)
@@ -12155,7 +12155,7 @@ static int bnx2x_init_bp(struct bnx2x *bp)
        qlock_init(&bp->fw_mb_mutex);
        qlock_init(&bp->drv_info_mutex);
        bp->drv_info_mng_owner = false;
-       spinlock_init_irqsave(&bp->stats_lock);
+       spinlock_init(&bp->stats_lock);
        sem_init(&bp->stats_sema, 1);
 
        INIT_DELAYED_WORK(&bp->sp_task, bnx2x_sp_task);
index 75dcc53..bdf20e8 100644 (file)
@@ -2862,7 +2862,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 
        priv = netdev_priv(dev);
        memset(priv, 0, sizeof(struct mlx4_en_priv));
-       spinlock_init_irqsave(&priv->stats_lock);
+       spinlock_init(&priv->stats_lock);
        INIT_WORK(&priv->rx_mode_task, mlx4_en_do_set_rx_mode);
        INIT_WORK(&priv->watchdog_task, mlx4_en_restart);
        INIT_WORK(&priv->linkstate_task, mlx4_en_linkstate);
@@ -2874,7 +2874,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 #endif
 #ifdef CONFIG_RFS_ACCEL
        INIT_LIST_HEAD(&priv->filters);
-       spinlock_init_irqsave(&priv->filters_lock);
+       spinlock_init(&priv->filters_lock);
 #endif
 
        priv->dev = dev;
index faabb0b..e53b70e 100644 (file)
@@ -618,7 +618,7 @@ static inline struct mlx4_cqe *mlx4_en_get_cqe(void *buf, int idx, int cqe_sz)
 #ifdef CONFIG_NET_RX_BUSY_POLL
 static inline void mlx4_en_cq_init_lock(struct mlx4_en_cq *cq)
 {
-       spinlock_init_irqsave(&cq->poll_lock);
+       spinlock_init(&cq->poll_lock);
        cq->state = MLX4_EN_CQ_STATE_IDLE;
 }
 
index 31805d5..69e4c7d 100644 (file)
@@ -48,13 +48,13 @@ void mlx4_qp_event(struct mlx4_dev *dev, uint32_t qpn, int event_type)
        struct mlx4_qp_table *qp_table = &mlx4_priv(dev)->qp_table;
        struct mlx4_qp *qp;
 
-       spin_lock(&qp_table->lock);
+       spin_lock_irqsave(&qp_table->lock);
 
        qp = __mlx4_qp_lookup(dev, qpn);
        if (qp)
                atomic_inc(&qp->refcount);
 
-       spin_unlock(&qp_table->lock);
+       spin_unlock_irqsave(&qp_table->lock);
 
        if (!qp) {
                mlx4_dbg(dev, "Async event for none existent QP %08x\n", qpn);
index d9c68be..56461c2 100644 (file)
@@ -497,7 +497,7 @@ int mlx4_init_resource_tracker(struct mlx4_dev *dev)
                    !res_alloc->allocated)
                        goto no_mem_err;
 
-               spinlock_init_irqsave(&res_alloc->alloc_lock);
+               spinlock_init(&res_alloc->alloc_lock);
                for (t = 0; t < dev->persist->num_vfs + 1; t++) {
                        struct mlx4_active_ports actv_ports =
                                mlx4_get_active_ports(dev, t);
@@ -831,14 +831,14 @@ int mlx4_get_slave_from_resource_id(struct mlx4_dev *dev,
 
        if (type == RES_QP)
                id &= 0x7fffff;
-       spin_lock(mlx4_tlock(dev));
+       spin_lock_irqsave(mlx4_tlock(dev));
 
        r = find_res(dev, id, type);
        if (r) {
                *slave = r->owner;
                err = 0;
        }
-       spin_unlock(mlx4_tlock(dev));
+       spin_unlock_irqsave(mlx4_tlock(dev));
 
        return err;
 }
index 75ca5b8..e850238 100644 (file)
@@ -42,13 +42,13 @@ void mlx4_srq_event(struct mlx4_dev *dev, uint32_t srqn, int event_type)
        struct mlx4_srq_table *srq_table = &mlx4_priv(dev)->srq_table;
        struct mlx4_srq *srq;
 
-       spin_lock(&srq_table->lock);
+       spin_lock_irqsave(&srq_table->lock);
 
        srq = radix_tree_lookup(&srq_table->tree, srqn & (dev->caps.num_srqs - 1));
        if (srq)
                atomic_inc(&srq->refcount);
 
-       spin_unlock(&srq_table->lock);
+       spin_unlock_irqsave(&srq_table->lock);
 
        if (!srq) {
                mlx4_warn(dev, "Async event for bogus SRQ %08x\n", srqn);
index 99b704e..4a3c160 100644 (file)
@@ -933,13 +933,13 @@ static void mlx4_ib_unlock_cqs(struct mlx4_ib_cq *send_cq, struct mlx4_ib_cq *re
 {
        if (send_cq == recv_cq) {
                __release(&recv_cq->lock);
-               spin_unlock(&send_cq->lock);
+               spin_unlock_irqsave(&send_cq->lock);
        } else if (send_cq->mcq.cqn < recv_cq->mcq.cqn) {
-               spin_unlock(&recv_cq->lock);
-               spin_unlock(&send_cq->lock);
+               spin_unlock_irqsave(&recv_cq->lock);
+               spin_unlock_irqsave(&send_cq->lock);
        } else {
-               spin_unlock(&send_cq->lock);
-               spin_unlock(&recv_cq->lock);
+               spin_unlock_irqsave(&send_cq->lock);
+               spin_unlock_irqsave(&recv_cq->lock);
        }
 }
 
@@ -2384,9 +2384,9 @@ static int mlx4_wq_overflow(struct mlx4_ib_wq *wq, int nreq, struct ib_cq *ib_cq
                return 0;
 
        cq = to_mcq(ib_cq);
-       spin_lock(&cq->lock);
+       spin_lock_irqsave(&cq->lock);
        cur = wq->head - wq->tail;
-       spin_unlock(&cq->lock);
+       spin_unlock_irqsave(&cq->lock);
 
        return cur + nreq >= wq->max_post;
 }
@@ -2948,7 +2948,7 @@ out:
                qp->sq_next_wqe = ind;
        }
 
-       spin_unlock_irqrestore(&qp->sq.lock, flags);
+       spin_unlock_irqsave(&qp->sq.lock, flags);
 
        return err;
 }
@@ -3035,7 +3035,7 @@ out:
                *qp->db.db = cpu_to_be32(qp->rq.head & 0xffff);
        }
 
-       spin_unlock_irqrestore(&qp->rq.lock, flags);
+       spin_unlock_irqsave(&qp->rq.lock, flags);
 
        return err;
 }
index e0461fb..dfa930f 100644 (file)
@@ -93,7 +93,7 @@ struct ib_srq *mlx4_ib_create_srq(struct ib_pd *pd,
                return ERR_PTR(-ENOMEM);
 
        mutex_init(&srq->mutex);
-       spin_lock_init(&srq->lock);
+       spinlock_init_irqsave(&srq->lock);
        srq->msrq.max    = roundup_pow_of_two(init_attr->attr.max_wr + 1);
        srq->msrq.max_gs = init_attr->attr.max_sge;
 
@@ -296,7 +296,10 @@ void mlx4_ib_free_srq_wqe(struct mlx4_ib_srq *srq, int wqe_index)
 {
        struct mlx4_wqe_srq_next_seg *next;
 
-       /* always called with interrupts disabled. */
+       /* Always called with interrupts disabled.  This lets us
+        * lock with spin_lock and unlock with spin_unlock; otherwise,
+        * we would use the _irqsave versions.  Tread carefully when
+        * modifying this code. */
        spin_lock(&srq->lock);
 
        next = get_wqe(srq, srq->tail);