s390/qdio: make thinint registration symmetric
authorJulian Wiedmann <jwi@linux.ibm.com>
Wed, 30 Sep 2020 08:09:07 +0000 (10:09 +0200)
committerVasily Gorbik <gor@linux.ibm.com>
Tue, 9 Feb 2021 14:57:04 +0000 (15:57 +0100)
tiqdio_add_device() adds the device to the tiq_list of eligible targets
for a data IRQ, which gets walked on each QDIO Adapter Interrupt to
inspect their DSCIs.

But currently the tiqdio_add_device() / tiqdio_remove_device() calls
are not symmetric - the device is removed within qdio_shutdown(),
but only added by qdio_activate().
So depending on the call sequence and encountered errors, we might
be trying to remove a list entry in qdio_shutdown() that was never even
added to the list. This required additional INIT_LIST_HEAD() calls to
ensure that the list entry was always in a consistent state.

All drivers now fence the IRQ delivery via qdio_start_irq() /
qdio_stop_irq(), so we can nicely integrate this tiq_list management
with the other steps needed for QDIO Adapter IRQ (de-)registration
(qdio_establish_thinint() / qdio_shutdown_thinint()).
As the naming suggests these get called during qdio_establish() and
qdio_shutdown(), with proper symmetry and roll-back after errors.

With this we longer need to worry about misplaced list removals, and
thus can clean up the list API abuse (INIT_LIST_HEAD() should not be
called on list entries).

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
Reviewed-by: Benjamin Block <bblock@linux.ibm.com>
Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
drivers/s390/cio/qdio.h
drivers/s390/cio/qdio_main.c
drivers/s390/cio/qdio_thinint.c

index 4889681..4a4e4de 100644 (file)
@@ -363,8 +363,6 @@ extern u64 last_ai_time;
 /* prototypes for thin interrupt */
 int qdio_establish_thinint(struct qdio_irq *irq_ptr);
 void qdio_shutdown_thinint(struct qdio_irq *irq_ptr);
 /* prototypes for thin interrupt */
 int qdio_establish_thinint(struct qdio_irq *irq_ptr);
 void qdio_shutdown_thinint(struct qdio_irq *irq_ptr);
-void tiqdio_add_device(struct qdio_irq *irq_ptr);
-void tiqdio_remove_device(struct qdio_irq *irq_ptr);
 int qdio_thinint_init(void);
 void qdio_thinint_exit(void);
 int test_nonshared_ind(struct qdio_irq *);
 int qdio_thinint_init(void);
 void qdio_thinint_exit(void);
 int test_nonshared_ind(struct qdio_irq *);
index 2cc489c..3b1e8ed 100644 (file)
@@ -986,7 +986,6 @@ int qdio_shutdown(struct ccw_device *cdev, int how)
         */
        qdio_set_state(irq_ptr, QDIO_IRQ_STATE_STOPPED);
 
         */
        qdio_set_state(irq_ptr, QDIO_IRQ_STATE_STOPPED);
 
-       tiqdio_remove_device(irq_ptr);
        qdio_shutdown_queues(irq_ptr);
        qdio_shutdown_debug_entries(irq_ptr);
 
        qdio_shutdown_queues(irq_ptr);
        qdio_shutdown_debug_entries(irq_ptr);
 
@@ -1104,7 +1103,6 @@ int qdio_allocate(struct ccw_device *cdev, unsigned int no_input_qs,
        if (rc)
                goto err_queues;
 
        if (rc)
                goto err_queues;
 
-       INIT_LIST_HEAD(&irq_ptr->entry);
        cdev->private->qdio_data = irq_ptr;
        qdio_set_state(irq_ptr, QDIO_IRQ_STATE_INACTIVE);
        return 0;
        cdev->private->qdio_data = irq_ptr;
        qdio_set_state(irq_ptr, QDIO_IRQ_STATE_INACTIVE);
        return 0;
@@ -1287,9 +1285,6 @@ int qdio_activate(struct ccw_device *cdev)
                goto out;
        }
 
                goto out;
        }
 
-       if (is_thinint_irq(irq_ptr))
-               tiqdio_add_device(irq_ptr);
-
        /* wait for subchannel to become active */
        msleep(5);
 
        /* wait for subchannel to become active */
        msleep(5);
 
index d495bf0..e1b9236 100644 (file)
@@ -66,22 +66,6 @@ static void put_indicator(u32 *addr)
        atomic_dec(&ind->count);
 }
 
        atomic_dec(&ind->count);
 }
 
-void tiqdio_add_device(struct qdio_irq *irq_ptr)
-{
-       mutex_lock(&tiq_list_lock);
-       list_add_rcu(&irq_ptr->entry, &tiq_list);
-       mutex_unlock(&tiq_list_lock);
-}
-
-void tiqdio_remove_device(struct qdio_irq *irq_ptr)
-{
-       mutex_lock(&tiq_list_lock);
-       list_del_rcu(&irq_ptr->entry);
-       mutex_unlock(&tiq_list_lock);
-       synchronize_rcu();
-       INIT_LIST_HEAD(&irq_ptr->entry);
-}
-
 static inline int references_shared_dsci(struct qdio_irq *irq_ptr)
 {
        return irq_ptr->dsci == &q_indicators[TIQDIO_SHARED_IND].ind;
 static inline int references_shared_dsci(struct qdio_irq *irq_ptr)
 {
        return irq_ptr->dsci == &q_indicators[TIQDIO_SHARED_IND].ind;
@@ -186,10 +170,15 @@ int qdio_establish_thinint(struct qdio_irq *irq_ptr)
        DBF_HEX(&irq_ptr->dsci, sizeof(void *));
 
        rc = set_subchannel_ind(irq_ptr, 0);
        DBF_HEX(&irq_ptr->dsci, sizeof(void *));
 
        rc = set_subchannel_ind(irq_ptr, 0);
-       if (rc)
+       if (rc) {
                put_indicator(irq_ptr->dsci);
                put_indicator(irq_ptr->dsci);
+               return rc;
+       }
 
 
-       return rc;
+       mutex_lock(&tiq_list_lock);
+       list_add_rcu(&irq_ptr->entry, &tiq_list);
+       mutex_unlock(&tiq_list_lock);
+       return 0;
 }
 
 void qdio_shutdown_thinint(struct qdio_irq *irq_ptr)
 }
 
 void qdio_shutdown_thinint(struct qdio_irq *irq_ptr)
@@ -197,6 +186,11 @@ void qdio_shutdown_thinint(struct qdio_irq *irq_ptr)
        if (!is_thinint_irq(irq_ptr))
                return;
 
        if (!is_thinint_irq(irq_ptr))
                return;
 
+       mutex_lock(&tiq_list_lock);
+       list_del_rcu(&irq_ptr->entry);
+       mutex_unlock(&tiq_list_lock);
+       synchronize_rcu();
+
        /* reset adapter interrupt indicators */
        set_subchannel_ind(irq_ptr, 1);
        put_indicator(irq_ptr->dsci);
        /* reset adapter interrupt indicators */
        set_subchannel_ind(irq_ptr, 1);
        put_indicator(irq_ptr->dsci);