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

Re: [PATCH RFC v2 8/8] evtchn: don't call Xen consumer callback with per-channel lock held


  • To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Tamas K Lengyel <tamas@xxxxxxxxxxxxx>, Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>
  • From: Isaila Alexandru <aisaila@xxxxxxxxxxxxxxx>
  • Date: Tue, 3 Nov 2020 12:17:26 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=bitdefender.com; dmarc=pass action=none header.from=bitdefender.com; dkim=pass header.d=bitdefender.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=jdIG85fWpAj0fad0Dj93K+NaZxGc7Fyc4sDoc28mt6M=; b=OdqFTNVDY1kBRjqUJHSKne7iULzCboy9iAb0t+JU6r3VSYx9tQylOag8WskMT8NUYiz+XoYKhB4HrwFpXcjVIIjqhEq1IqPJTIedMc3KoulJTF5oBY9+1y8NHD5eEgzPy87ynXFU1HWMNZDr1FDfCVLSQ9v7W22s/Gl8iwzdOh8eLEoAH6nSX9Hzp9YsXcQL5CpK8nqx0Iy80cOzvbjIiaRhdaBI2LXeHl4f0k04BVIvI+A2BFJJx1BcPkVBRbF6w37RAA71iyNjPUst5hwMxlMyQj+7DohOqPoKJyvpPMBjzCu/D99AzSKB8KAk17lLjP8+Lxmuc7yC/pUevsR3jg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=SdRfrLQjnCs7vEULZAvq0vfyQuszTRi5bDIR0CPD4TIW2TG+Dft7bxd3xC53yVQcxGs0SniAudIxoBNPZ1QhOWUeoEtzuVuzQ3l4i0n34fFg5zoEFpwP/LQiI0cVqN2tIHOYukeEd+KCJ/YqStLOM5ygavXhAcTl3TO0CpMCPjxCAKyufOKu3bszsiAZhIQL8HLQhKqjfAsvmbR7ocAq5CEV7KPZdjiT7wjhyxdT2cL2ZBWKLxD5dKgFTXDr7mMRSj0PtduU7W6xzaERR/vcY6uklrF0WZ4O3ZyMzMuhdOWWA+VvWmQuDurrb7azj2AxiE8P+qMICy+tbCX22E9cyQ==
  • Authentication-results: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=bitdefender.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Wei Liu <wl@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • Delivery-date: Tue, 03 Nov 2020 10:17:42 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>


Hi Jan and sorry for the late reply,

On 20.10.2020 17:13, Jan Beulich wrote:
While there don't look to be any problems with this right now, the lock
order implications from holding the lock can be very difficult to follow
(and may be easy to violate unknowingly). The present callbacks don't
(and no such callback should) have any need for the lock to be held.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
TODO: vm_event_disable() frees the structures used by respective
       callbacks - need to either use call_rcu() for freeing, or maintain
       a count of in-progress calls, for evtchn_close() to wait to drop
       to zero before dropping the lock / returning.

I would go with the second solution and maintain a count of in-progress calls.

Tamas, Petre how does this sound?

Alex


--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -763,9 +763,18 @@ int evtchn_send(struct domain *ld, unsig
          rport = lchn->u.interdomain.remote_port;
          rchn  = evtchn_from_port(rd, rport);
          if ( consumer_is_xen(rchn) )
-            xen_notification_fn(rchn)(rd->vcpu[rchn->notify_vcpu_id], rport);
-        else
-            evtchn_port_set_pending(rd, rchn->notify_vcpu_id, rchn);
+        {
+            /* Don't keep holding the lock for the call below. */
+            xen_event_channel_notification_t fn = xen_notification_fn(rchn);
+            struct vcpu *rv = rd->vcpu[rchn->notify_vcpu_id];
+
+            rcu_lock_domain(rd);
+            spin_unlock_irqrestore(&lchn->lock, flags);
+            fn(rv, rport);
+            rcu_unlock_domain(rd);
+            return 0;
+        }
+        evtchn_port_set_pending(rd, rchn->notify_vcpu_id, rchn);
          break;
      case ECS_IPI:
          evtchn_port_set_pending(ld, lchn->notify_vcpu_id, lchn);




 


Rackspace

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