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

Re: Ping: [PATCH v5 0/6] evtchn: (not so) recent XSAs follow-on


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Fri, 14 May 2021 17:29:26 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.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=YRA4CqEJCMYhykLDIMrQjSRpyhy5yWsDo08vRAuyV4w=; b=BZMmItqawl8UdwEB0vY4ByjHjhjh7fyLEkQFDDg3VunRHfTHcDQ7pYrwyztURmhtHSW62aqH6w0PKykWnhs1mY1hDpVrKSZlFWVNNgS9/7NQ7HwYZ1FBviQRkX7AAWmJXSinNmQk6UjSpcKlTDxGu/GRJLmQL6c6LhqG4Zb2y6MgvL/PBFg2RiWHwjHOlIHwyNr143YEyBR74xjfGRrbdt+OfkAQqgzIxK7gFdtz17lwVUaE/rOIytPi9mNSBbI1QSHgeUJe+fNwajAhaUg90TorX5xTzSj3XweLYk20OCad6ZYQqb1TtohNBOAgfD/q1kzA23cJHwdYGYzy1al/Vw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=GLz3LoK+wgIXGCHTA2kFBxdPo+qx5c45Qk5bw4AsdvBlHO2yp1yRxRpnswApneHTzyjwRGF3MqUrnX48qSerrAyhY7oaE77svuSbGOgWqAQYkO8RartMnTEQNtKApjc/TWFxOe+lv2dbbned4F3rIHHrOmjy4KfigqzgHoBXkIzx5gZprr6gvkTUJuU5CcmcfNIAooUIForx1W91ykHZQ89W/NMBybAFUmEmWWPorSosyzjJUJ6BhfBsgM5jXJZJ+V09ZwQ0lokedY7OlMIxz64IfE0liachHev9NlLusBJdYLhcA/X7q0BahXYv+F5ZH/uStrn105guzl5Bim6VpQ==
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Julien Grall <julien@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 14 May 2021 15:29:47 +0000
  • Ironport-hdrordr: A9a23:QQt3n6PwlZPx6cBcT8P155DYdb4zR+YMi2TDiHoedfUFSKOlfp 6V8MjztSWVtN4QMEtQ/+xoHJPwPE80lKQFm7X5WI3CYOCIghrMEGgP1/qH/9SkIVyDygc/79 YRT0EdMqyJMbESt6+Ti2PUYrVQoqj0zEnrv5ak854Ed3AaV0gK1XYBNu/0KDwQeOEQbqBJaq Z0q/A36wZJFh8sH4uGL0hAe9KGi8zAlZrgbxJDLQUg8hOygTSh76O/OwSE3z8FOgk/gYsKwC zgqUjU96+ju/a0xlv3zGnI9albn9Pn159qGNGMsM4IMT/h4zzYJbiJY4fy/gzdndvfrWrDyL L30lMd1oVImj3sl1iO0FjQM1KK6kdo15eKomXo8kcKoqTCNXkH4oR69MRkmraw0TtXgDhG6t M+44uujeseMfrxplWJ2zH2bWAcqqOVmwtprQdBtQ0TbWMhAIUh5LD3q3klb6voWhiKsbwaLA ==
  • Ironport-sdr: h7FvfSEZxs5R2AYAtOsE+/Hf8DtokJBGi9Mkq1ObRdDQb11zLHDy62wYJq25bo3bPGh9CnBrqC SSfWX9m7/8113ICTj/OXXBugF2K8ZH4mef+VEaYUCGlzY+94wk3QX+EAyZBQv5Akb77RndTfqj yckfnePIQHZpbNlFfOghz71DjlbrdtvNUuQOOWMWiQbD6w2x5LzuR8R5pYPnCmuZhCYtJNgnei 3/bVq6Qw2r8nEUgnhxwEn6cdA3Qc67qO8EszSr95pruQdBlSfTm6xkEm7SmYXqLRZdWsl1JCSh 2u0=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Thu, Apr 22, 2021 at 10:53:05AM +0200, Jan Beulich wrote:
> On 21.04.2021 17:56, Julien Grall wrote:
> > 
> > 
> > On 21/04/2021 16:23, Jan Beulich wrote:
> >> On 27.01.2021 09:13, Jan Beulich wrote:
> >>> These are grouped into a series largely because of their origin,
> >>> not so much because there are (heavy) dependencies among them.
> >>> The main change from v4 is the dropping of the two patches trying
> >>> to do away with the double event lock acquires in interdomain
> >>> channel handling. See also the individual patches.
> >>>
> >>> 1: use per-channel lock where possible
> >>> 2: convert domain event lock to an r/w one
> >>> 3: slightly defer lock acquire where possible
> >>> 4: add helper for port_is_valid() + evtchn_from_port()
> >>> 5: type adjustments
> >>> 6: drop acquiring of per-channel lock from send_guest_{global,vcpu}_virq()
> >>
> >> Only patch 4 here has got an ack so far - may I ask for clear feedback
> >> as to at least some of these being acceptable (I can see the last one
> >> being controversial, and if this was the only one left I probably
> >> wouldn't even ping, despite thinking that it helps reduce unecessary
> >> overhead).
> > 
> > I left feedback for the series one the previous version (see [1]). It 
> > would have been nice is if it was mentionned somewhere as this is still 
> > unresolved.
> 
> I will admit I forgot about the controversy on patch 1. I did, however,
> reply to your concerns. What didn't happen is the feedback from others
> that you did ask for.
> 
> And of course there are 4 more patches here (one of them having an ack,
> yes) which could do with feedback. I'm certainly willing, where possible,
> to further re-order the series such that controversial changes are at its
> end.

I think it would easier to figure out whether the changes are correct
if we had some kind of documentation about what/how the per-domain
event_lock and the per-event locks are supposed to be used. I don't
seem to be able to find any comments regarding how they are to be
used.

Regarding the changes itself in patch 1 (which I think has caused part
of the controversy here), I'm unsure they are worth it because the
functions modified all seem to be non-performance critical:
evtchn_status, domain_dump_evtchn_info, flask_get_peer_sid.

So I would say that unless we have clear rules written down for what
the per-domain event_lock protects, I would be hesitant to change any
of the logic, specially for critical paths.

Thanks, Roger.



 


Rackspace

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