CINXE.COM

<?xml version="1.0" encoding="UTF-8" standalone="yes"?> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"><html xmlns="http://www.w3.org/1999/xhtml"><head><meta http-equiv="Content-Type" content="text/html; charset=UTF-8" /><title>LKML: "tip-bot2 for Ingo Molnar": [tip: locking/urgent] headers/cleanup.h: Remove the if_not_guard() facility</title><link href="/css/message.css" rel="stylesheet" type="text/css" /><link href="/css/wrap.css" rel="alternate stylesheet" type="text/css" title="wrap" /><link href="/css/nowrap.css" rel="stylesheet" type="text/css" title="nowrap" /><link href="/favicon.ico" rel="shortcut icon" /><script src="/js/simple-calendar.js" type="text/javascript"></script><script src="/js/styleswitcher.js" type="text/javascript"></script><link rel="alternate" type="application/rss+xml" title="lkml.org : last 100 messages" href="/rss.php" /><link rel="alternate" type="application/rss+xml" title="lkml.org : last messages by "tip-bot2 for Ingo Molnar"" href="/groupie.php?aid=" /><script> var _paq = window._paq = window._paq || []; /* tracker methods like "setCustomDimension" should be called before "trackPageView" */ _paq.push(["setDoNotTrack", true]); _paq.push(["disableCookies"]); _paq.push(['trackPageView']); _paq.push(['enableLinkTracking']); (function() { var u="//m.lkml.org/"; _paq.push(['setTrackerUrl', u+'matomo.php']); _paq.push(['setSiteId', '1']); var d=document, g=d.createElement('script'), s=d.getElementsByTagName('script')[0]; g.async=true; g.src=u+'matomo.js'; s.parentNode.insertBefore(g,s); })(); </script></head><body onload="es.jasper.simpleCalendar.init();" itemscope="itemscope" itemtype="http://schema.org/BlogPosting"><table border="0" cellpadding="0" cellspacing="0"><tr><td width="180" align="center"><a href="/"><img style="border:0;width:135px;height:32px" src="/images/toprowlk.gif" alt="lkml.org" /></a></td><td width="32">聽</td><td class="nb"><div><a class="nb" href="/lkml"> [lkml]</a> 聽 <a class="nb" href="/lkml/2024"> [2024]</a> 聽 <a class="nb" href="/lkml/2024/12"> [Dec]</a> 聽 <a class="nb" href="/lkml/2024/12/7"> [7]</a> 聽 <a class="nb" href="/lkml/last100"> [last100]</a> 聽 <a href="/rss.php"><img src="/images/rss-or.gif" border="0" alt="RSS Feed" /></a></div><div>Views: <a href="#" class="nowrap" onclick="setActiveStyleSheet('wrap');return false;">[wrap]</a><a href="#" class="wrap" onclick="setActiveStyleSheet('nowrap');return false;">[no wrap]</a> 聽 <a class="nb" href="/lkml/mheaders/2024/12/7/186" onclick="this.href='/lkml/headers'+'/2024/12/7/186';">[headers]</a>聽 <a href="/lkml/bounce/2024/12/7/186">[forward]</a>聽 </div></td><td width="32">聽</td></tr><tr><td valign="top"><div class="es-jasper-simpleCalendar" baseurl="/lkml/"></div><div class="threadlist">Messages in this thread</div><ul class="threadlist"><li class="root"><a href="/lkml/2024/11/18/293">First message in thread</a></li><li><a href="/lkml/2024/11/20/1074">Linus Torvalds</a><ul><li><a href="/lkml/2024/12/6/461">Ingo Molnar</a><ul><li><a href="/lkml/2024/12/6/949">David Lechner</a></li><li class="origin"><a href="">"tip-bot2 for Ingo Molnar"</a></li></ul></li></ul></li></ul><div class="threadlist">Patch in this message</div><ul class="threadlist"><li><a href="/lkml/diff/2024/12/7/186/1">Get diff 1</a></li></ul></td><td width="32" rowspan="2" class="c" valign="top"><img src="/images/icornerl.gif" width="32" height="32" alt="/" /></td><td class="c" rowspan="2" valign="top" style="padding-top: 1em"><table><tr><td><table><tr><td class="lp">Date</td><td class="rp" itemprop="datePublished">Sat, 07 Dec 2024 10:22:28 -0000</td></tr><tr><td class="lp">From</td><td class="rp" itemprop="author">"tip-bot2 for Ingo Molnar" <></td></tr><tr><td class="lp">Subject</td><td class="rp" itemprop="name">[tip: locking/urgent] headers/cleanup.h: Remove the if_not_guard() facility</td></tr></table></td><td></td></tr></table><pre itemprop="articleBody">The following commit has been merged into the locking/urgent branch of tip:<br /><br />Commit-ID: b4d83c8323b0c4a899a996fed919cfe10720d289<br />Gitweb: <a href="https://git.kernel.org/tip/b4d83c8323b0c4a899a996fed919cfe10720d289">https://git.kernel.org/tip/b4d83c8323b0c4a899a996fed919cfe10720d289</a><br />Author: Ingo Molnar <mingo@kernel.org><br />AuthorDate: Fri, 06 Dec 2024 10:19:31 +01:00<br />Committer: Ingo Molnar <mingo@kernel.org><br />CommitterDate: Sat, 07 Dec 2024 11:15:14 +01:00<br /><br />headers/cleanup.h: Remove the if_not_guard() facility<br /><br />Linus noticed that the new if_not_guard() definition is fragile:<br /><br /> "This macro generates actively wrong code if it happens to be inside an<br /> if-statement or a loop without a block.<br /><br /> IOW, code like this:<br /><br /> for (iterate-over-something)<br /> if_not_guard(a)<br /> return -BUSY;<br /><br /> looks like will build fine, but will generate completely incorrect code."<br /><br />The reason is that the __if_not_guard() macro is multi-statement, so<br />while most kernel developers expect macros to be simple or at least<br />compound statements - but for __if_not_guard() it is not so:<br /><br /> #define __if_not_guard(_name, _id, args...) \<br /> BUILD_BUG_ON(!__is_cond_ptr(_name)); \<br /> CLASS(_name, _id)(args); \<br /> if (!__guard_ptr(_name)(&_id))<br /><br />To add insult to injury, the placement of the BUILD_BUG_ON() line makes<br />the macro appear to compile fine, but it will generate incorrect code<br />as Linus reported, for example if used within iteration or conditional<br />statements that will use the first statement of a macro as a loop body<br />or conditional statement body.<br /><br />[ I'd also like to note that the original submission by David Lechner did<br /> not contain the BUILD_BUG_ON() line, so it was safer than what we ended<br /> up committing. Mea culpa. ]<br /><br />It doesn't appear to be possible to turn this macro into a robust<br />single or compound statement that could be used in single statements,<br />due to the necessity to define an auto scope variable with an open<br />scope and the necessity of it having to expand to a partial 'if'<br />statement with no body.<br /><br />Instead of trying to work around this fragility, just remove the<br />construct before it gets used.<br /><br />Reported-by: Linus Torvalds <torvalds@linux-foundation.org><br />Signed-off-by: Ingo Molnar <mingo@kernel.org><br />Cc: David Lechner <dlechner@baylibre.com><br />Cc: Peter Zijlstra <peterz@infradead.org><br />Link: <a href="https://lore.kernel.org/r/Z1LBnX9TpZLR5Dkf@gmail.com">https://lore.kernel.org/r/Z1LBnX9TpZLR5Dkf@gmail.com</a><br />---<br /> include/linux/cleanup.h | 14 --------------<br /> 1 file changed, 14 deletions(-)<br /><br />diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h<br />index 966fcc5..ec00e3f 100644<br />--- a/include/linux/cleanup.h<br />+++ b/include/linux/cleanup.h<br />@@ -273,12 +273,6 @@ static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \<br /> * an anonymous instance of the (guard) class, not recommended for<br /> * conditional locks.<br /> *<br />- * if_not_guard(name, args...) { <error handling> }:<br />- * convenience macro for conditional guards that calls the statement that<br />- * follows only if the lock was not acquired (typically an error return).<br />- *<br />- * Only for conditional locks.<br />- *<br /> * scoped_guard (name, args...) { }:<br /> * similar to CLASS(name, scope)(args), except the variable (with the<br /> * explicit name 'scope') is declard in a for-loop such that its scope is<br />@@ -350,14 +344,6 @@ _label: \<br /> #define scoped_cond_guard(_name, _fail, args...) \<br /> __scoped_cond_guard(_name, _fail, __UNIQUE_ID(label), args)<br /> <br />-#define __if_not_guard(_name, _id, args...) \<br />- BUILD_BUG_ON(!__is_cond_ptr(_name)); \<br />- CLASS(_name, _id)(args); \<br />- if (!__guard_ptr(_name)(&_id))<br />-<br />-#define if_not_guard(_name, args...) \<br />- __if_not_guard(_name, __UNIQUE_ID(guard), args)<br />-<br /> /*<br /> * Additional helper macros for generating lock guards with types, either for<br /> * locks that don't have a native type (eg. RCU, preempt) or those that need a<br /></pre></td><td width="32" rowspan="2" class="c" valign="top"><img src="/images/icornerr.gif" width="32" height="32" alt="\" /></td></tr><tr><td align="right" valign="bottom"> 聽 </td></tr><tr><td align="right" valign="bottom">聽</td><td class="c" valign="bottom" style="padding-bottom: 0px"><img src="/images/bcornerl.gif" width="32" height="32" alt="\" /></td><td class="c">聽</td><td class="c" valign="bottom" style="padding-bottom: 0px"><img src="/images/bcornerr.gif" width="32" height="32" alt="/" /></td></tr><tr><td align="right" valign="top" colspan="2"> 聽 </td><td class="lm">Last update: 2024-12-07 11:23 聽聽 [from the cache]<br />漏2003-2020 <a href="http://blog.jasper.es/"><span itemprop="editor">Jasper Spaans</span></a>|hosted at <a href="https://www.digitalocean.com/?refcode=9a8e99d24cf9">Digital Ocean</a> and my Meterkast|<a href="http://blog.jasper.es/categories.html#lkml-ref">Read the blog</a></td><td>聽</td></tr></table><script language="javascript" src="/js/styleswitcher.js" type="text/javascript"></script></body></html>