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

Re: [PATCH v5 04/13] xen/spinlock: add rspin_[un]lock_irq[save|restore]()



On 18.03.24 17:08, Jan Beulich wrote:
On 18.03.2024 17:05, Jürgen Groß wrote:
On 18.03.24 16:59, Jan Beulich wrote:
On 18.03.2024 16:55, Jürgen Groß wrote:
On 18.03.24 15:43, Jan Beulich wrote:
On 14.03.2024 08:20, Juergen Gross wrote:
Instead of special casing rspin_lock_irqsave() and
rspin_unlock_irqrestore() for the console lock, add those functions
to spinlock handling and use them where needed.

Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with two remarks:

--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -475,15 +475,31 @@ void _rspin_lock(rspinlock_t *lock)
        lock->recurse_cnt++;
    }
+unsigned long _rspin_lock_irqsave(rspinlock_t *lock)
+{
+    unsigned long flags;
+
+    local_irq_save(flags);
+    _rspin_lock(lock);
+
+    return flags;
+}
+
    void _rspin_unlock(rspinlock_t *lock)
    {
        if ( likely(--lock->recurse_cnt == 0) )
        {
            lock->recurse_cpu = SPINLOCK_NO_CPU;
-        spin_unlock(lock);
+        _spin_unlock(lock);

This looks like an unrelated change. I think I can guess the purpose, but
it would be nice if such along-the-way changes could be mentioned in the
description.

I think it would be better to move that change to patch 3.

Hmm, it would be a secondary change there, too. I was actually meaning to
commit patches 2-5, but if things want moving around I guess I better
wait with doing so?

Hmm, maybe just drop this hunk and let patch 7 handle it?

Ah yes, that seem more logical to me. I take it you don't mean "hunk"
though, but really just this one line change.

Oh yes, of course.


Juergen




 


Rackspace

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