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

Re: [PATCH v5 3/4] xen/console: remove max_init_domid dependency



Hi Jan,

On 04/06/2025 13:55, Jan Beulich wrote:
On 31.05.2025 01:19, dmkhn@xxxxxxxxx wrote:
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -2461,6 +2461,39 @@ void domid_free(domid_t domid)
      spin_unlock(&domid_lock);
  }
+/*
+ * Find the ID of the next possible console owner domain.
+ *
+ * @return Domain ID: DOMID_XEN or non-system domain IDs within
+ * the range of [0..DOMID_FIRST_RESERVED-1].
+ */
+domid_t domid_find_with_input_allowed(domid_t hint)
+{
+    domid_t domid = DOMID_XEN;
+
+    if ( hint < DOMID_FIRST_RESERVED )
+    {
+        struct domain *d;
+
+        rcu_read_lock(&domlist_read_lock);
+
+        for ( d = domid_to_domain(hint);

If the domain with ID "hint" went away, what is being switched to changes
compared to behavior prior to this patch, if I'm not mistaken. While this
_may_ be acceptable, not saying so in the description is imo a no-go.

+              d && get_domain(d) && d->domain_id < DOMID_FIRST_RESERVED;

What's the DOMID_FIRST_RESERVED check for? And where's the put_domain()
for the get_domain() here?

+              d = rcu_dereference(d->next_in_list) )
+        {
+            if ( d->console.input_allowed )
+            {
+                domid = d->domain_id;
+                break;
+            }
+        }
+
+        rcu_read_unlock(&domlist_read_lock);
+    }
+
+    return domid;
+}

My concern remains: With many domains, the loop here may take quite a few
iterations. That's even more concerning because it regresses right away in
environments where along with boot-time created domains (eligible for
console focus) later further domains are created (non of which are eligible
for console focus). That is, the step from last boot-time created back to
DOMID_XEN may now take excessively long.

+1. I vaguely recall making the same concern earlier in the review. One possibility would be to have a list of domains where the console is supported. I don't think it would be necessary to have the list sorted as I view the console switching a debug facility.

Cheers,

--
Julien Grall




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.