[bug report] rcu: Kill rnp->ofl_seq and use only rcu_state.ofl_lock for exclusion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello David Woodhouse,

Commit 82980b1622d9 ("rcu: Kill rnp->ofl_seq and use only
rcu_state.ofl_lock for exclusion") from Feb 16, 2021 (linux-next),
leads to the following Smatch static checker warning:

	kernel/rcu/tree.c:1844 rcu_gp_init()
	warn: mixing irq and irqsave

        kernel/rcu/srcutree.c:910 srcu_gp_end()
        warn: mixing irq and irqsave

kernel/rcu/tree.c
    1782 static noinline_for_stack bool rcu_gp_init(void)
    1783 {
    1784         unsigned long flags;
    1785         unsigned long oldmask;
    1786         unsigned long mask;
    1787         struct rcu_data *rdp;
    1788         struct rcu_node *rnp = rcu_get_root();
    1789         bool start_new_poll;
    1790 
    1791         WRITE_ONCE(rcu_state.gp_activity, jiffies);
    1792         raw_spin_lock_irq_rcu_node(rnp);
    1793         if (!rcu_state.gp_flags) {
    1794                 /* Spurious wakeup, tell caller to go back to sleep.  */
    1795                 raw_spin_unlock_irq_rcu_node(rnp);
    1796                 return false;
    1797         }
    1798         WRITE_ONCE(rcu_state.gp_flags, 0); /* Clear all flags: New GP. */
    1799 
    1800         if (WARN_ON_ONCE(rcu_gp_in_progress())) {
    1801                 /*
    1802                  * Grace period already in progress, don't start another.
    1803                  * Not supposed to be able to happen.
    1804                  */
    1805                 raw_spin_unlock_irq_rcu_node(rnp);
    1806                 return false;
    1807         }
    1808 
    1809         /* Advance to a new grace period and initialize state. */
    1810         record_gp_stall_check_time();
    1811         /* Record GP times before starting GP, hence rcu_seq_start(). */
    1812         rcu_seq_start(&rcu_state.gp_seq);
    1813         ASSERT_EXCLUSIVE_WRITER(rcu_state.gp_seq);
    1814         start_new_poll = rcu_sr_normal_gp_init();
    1815         trace_rcu_grace_period(rcu_state.name, rcu_state.gp_seq, TPS("start"));
    1816         rcu_poll_gp_seq_start(&rcu_state.gp_seq_polled_snap);
    1817         raw_spin_unlock_irq_rcu_node(rnp);
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Enables IRQs

    1818 
    1819         /*
    1820          * The "start_new_poll" is set to true, only when this GP is not able
    1821          * to handle anything and there are outstanding users. It happens when
    1822          * the rcu_sr_normal_gp_init() function was not able to insert a dummy
    1823          * separator to the llist, because there were no left any dummy-nodes.
    1824          *
    1825          * Number of dummy-nodes is fixed, it could be that we are run out of
    1826          * them, if so we start a new pool request to repeat a try. It is rare
    1827          * and it means that a system is doing a slow processing of callbacks.
    1828          */
    1829         if (start_new_poll)
    1830                 (void) start_poll_synchronize_rcu();
    1831 
    1832         /*
    1833          * Apply per-leaf buffered online and offline operations to
    1834          * the rcu_node tree. Note that this new grace period need not
    1835          * wait for subsequent online CPUs, and that RCU hooks in the CPU
    1836          * offlining path, when combined with checks in this function,
    1837          * will handle CPUs that are currently going offline or that will
    1838          * go offline later.  Please also refer to "Hotplug CPU" section
    1839          * of RCU's Requirements documentation.
    1840          */
    1841         WRITE_ONCE(rcu_state.gp_state, RCU_GP_ONOFF);
    1842         /* Exclude CPU hotplug operations. */
    1843         rcu_for_each_leaf_node(rnp) {
--> 1844                 local_irq_save(flags);
                         ^^^^^^^^^^^^^^^^^^^^^
It doesn't make sense to mix irq and irqsave.  Either it can be called
with IRQs disabled or it can't...

    1845                 arch_spin_lock(&rcu_state.ofl_lock);
    1846                 raw_spin_lock_rcu_node(rnp);
    1847                 if (rnp->qsmaskinit == rnp->qsmaskinitnext &&
    1848                     !rnp->wait_blkd_tasks) {
    1849                         /* Nothing to do on this leaf rcu_node structure. */
    1850                         raw_spin_unlock_rcu_node(rnp);
    1851                         arch_spin_unlock(&rcu_state.ofl_lock);
    1852                         local_irq_restore(flags);
    1853                         continue;
    1854                 }
    1855 
    1856                 /* Record old state, apply changes to ->qsmaskinit field. */
    1857                 oldmask = rnp->qsmaskinit;
    1858                 rnp->qsmaskinit = rnp->qsmaskinitnext;
    1859 
    1860                 /* If zero-ness of ->qsmaskinit changed, propagate up tree. */
    1861                 if (!oldmask != !rnp->qsmaskinit) {
    1862                         if (!oldmask) { /* First online CPU for rcu_node. */
    1863                                 if (!rnp->wait_blkd_tasks) /* Ever offline? */
    1864                                         rcu_init_new_rnp(rnp);
    1865                         } else if (rcu_preempt_has_tasks(rnp)) {
    1866                                 rnp->wait_blkd_tasks = true; /* blocked tasks */
    1867                         } else { /* Last offline CPU and can propagate. */
    1868                                 rcu_cleanup_dead_rnp(rnp);
    1869                         }
    1870                 }
    1871 
    1872                 /*
    1873                  * If all waited-on tasks from prior grace period are
    1874                  * done, and if all this rcu_node structure's CPUs are
    1875                  * still offline, propagate up the rcu_node tree and
    1876                  * clear ->wait_blkd_tasks.  Otherwise, if one of this
    1877                  * rcu_node structure's CPUs has since come back online,
    1878                  * simply clear ->wait_blkd_tasks.
    1879                  */
    1880                 if (rnp->wait_blkd_tasks &&
    1881                     (!rcu_preempt_has_tasks(rnp) || rnp->qsmaskinit)) {
    1882                         rnp->wait_blkd_tasks = false;
    1883                         if (!rnp->qsmaskinit)
    1884                                 rcu_cleanup_dead_rnp(rnp);
    1885                 }
    1886 
    1887                 raw_spin_unlock_rcu_node(rnp);
    1888                 arch_spin_unlock(&rcu_state.ofl_lock);
    1889                 local_irq_restore(flags);
    1890         }
    1891         rcu_gp_slow(gp_preinit_delay); /* Races with CPU hotplug. */
    1892 
    1893         /*
    1894          * Set the quiescent-state-needed bits in all the rcu_node
    1895          * structures for all currently online CPUs in breadth-first
    1896          * order, starting from the root rcu_node structure, relying on the
    1897          * layout of the tree within the rcu_state.node[] array.  Note that
    1898          * other CPUs will access only the leaves of the hierarchy, thus
    1899          * seeing that no grace period is in progress, at least until the
    1900          * corresponding leaf node has been initialized.
    1901          *
    1902          * The grace period cannot complete until the initialization
    1903          * process finishes, because this kthread handles both.
    1904          */
    1905         WRITE_ONCE(rcu_state.gp_state, RCU_GP_INIT);
    1906         rcu_for_each_node_breadth_first(rnp) {
    1907                 rcu_gp_slow(gp_init_delay);
    1908                 raw_spin_lock_irqsave_rcu_node(rnp, flags);
    1909                 rdp = this_cpu_ptr(&rcu_data);
    1910                 rcu_preempt_check_blocked_tasks(rnp);
    1911                 rnp->qsmask = rnp->qsmaskinit;
    1912                 WRITE_ONCE(rnp->gp_seq, rcu_state.gp_seq);
    1913                 if (rnp == rdp->mynode)
    1914                         (void)__note_gp_changes(rnp, rdp);
    1915                 rcu_preempt_boost_start_gp(rnp);
    1916                 trace_rcu_grace_period_init(rcu_state.name, rnp->gp_seq,
    1917                                             rnp->level, rnp->grplo,
    1918                                             rnp->grphi, rnp->qsmask);
    1919                 /* Quiescent states for tasks on any now-offline CPUs. */
    1920                 mask = rnp->qsmask & ~rnp->qsmaskinitnext;
    1921                 rnp->rcu_gp_init_mask = mask;
    1922                 if ((mask || rnp->wait_blkd_tasks) && rcu_is_leaf_node(rnp))
    1923                         rcu_report_qs_rnp(mask, rnp, rnp->gp_seq, flags);
    1924                 else
    1925                         raw_spin_unlock_irq_rcu_node(rnp);
    1926                 cond_resched_tasks_rcu_qs();
    1927                 WRITE_ONCE(rcu_state.gp_activity, jiffies);
    1928         }
    1929 
    1930         // If strict, make all CPUs aware of new grace period.
    1931         if (IS_ENABLED(CONFIG_RCU_STRICT_GRACE_PERIOD))
    1932                 on_each_cpu(rcu_strict_gp_boundary, NULL, 0);
    1933 
    1934         return true;
    1935 }

regards,
dan carpenter




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux