On 24.08.22 11:03, Julien Grall wrote:
> Hi,
> 
> On 16/08/2022 07:40, Jan Beulich wrote:
>> On 16.08.2022 04:36, Penny Zheng wrote:
>>> +void free_domstatic_page(struct page_info *page)
>>> +{
>>> +    struct domain *d = page_get_owner(page);
>>> +    bool drop_dom_ref;
>>> +
>>> +    if ( unlikely(!d) )
>>> +    {
>>> +        ASSERT_UNREACHABLE();
>>> +        printk("The about-to-free static page %"PRI_mfn" must be owned by a 
>>> domain\n",
>>> +               mfn_x(page_to_mfn(page)));
>>> +        return;
>>> +    }
>>
>> For the message to be useful as a hint if the assertion triggers, it
>> wants printing ahead of the assertion. I also think it wants to be a
>> XENLOG_G_* kind of log level, so it would be rate limited by default
>> in release builds. Just to be on the safe side.
> 
> +1
> 
>> (I'm not in favor of
>> the log message in the first place, but I do know that Julien had
>> asked for one.)
> TBH, I think all ASSERT_UNREACHABLE() paths should be accompanied with a 
> printk(). This would also allow us to catch issue in production rather than in 
> only in debug.

What about something like the following then?

--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -40,6 +40,16 @@
      unlikely(ret_warn_on_);             \
  })

+#define WARN_ONCE() do {                \
+    static bool warned = false;         \
+                                        \
+    if ( !warned )                      \
+    {                                   \
+        warned = true;                  \
+        WARN();                         \
+    }                                   \
+} while (0)
+
  /* All clang versions supported by Xen have _Static_assert. */
  #if defined(__clang__) || \
      (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6))
@@ -63,7 +73,7 @@
  #define ASSERT_UNREACHABLE() assert_failed("unreachable")
  #else
  #define ASSERT(p) do { if ( 0 && (p) ) {} } while (0)
-#define ASSERT_UNREACHABLE() do { } while (0)
+#define ASSERT_UNREACHABLE() WARN_ONCE()
  #endif

  #define ABS(_x) ({                              \


Juergen