From: Dan Carpenter <dan.carpenter@linaro.org>
To: dwmw@amazon.co.uk, paulmck@kernel.org
Cc: rcu@vger.kernel.org
Subject: [bug report] rcu: Kill rnp->ofl_seq and use only rcu_state.ofl_lock for exclusion
Date: Wed, 8 May 2024 15:29:12 +0300 [thread overview]
Message-ID: <febb13ab-a4bb-48b4-8e97-7e9f7749e6da@moroto.mountain> (raw)
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
next reply other threads:[~2024-05-08 12:29 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-08 12:29 Dan Carpenter [this message]
2024-05-08 14:33 ` [bug report] rcu: Kill rnp->ofl_seq and use only rcu_state.ofl_lock for exclusion Paul E. McKenney
2024-05-08 14:55 ` Dan Carpenter
2024-05-08 17:32 ` Paul E. McKenney
2024-05-08 17:38 ` Dan Carpenter
2024-05-08 23:50 ` Paul E. McKenney
2024-05-09 6:46 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=febb13ab-a4bb-48b4-8e97-7e9f7749e6da@moroto.mountain \
--to=dan.carpenter@linaro.org \
--cc=dwmw@amazon.co.uk \
--cc=paulmck@kernel.org \
--cc=rcu@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).