[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] ioreq_broadcast(): accept partial broadcast success
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: "Per Bilse (3P)" <Per.Bilse@xxxxxxxxxx>
- Date: Mon, 28 Nov 2022 10:16:56 +0000
- Accept-language: en-US
- 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=XI9KoTAeUO1ke64jAv3W7cYMqIp6oRzu/NVY/wNK59c=; b=FRLlxzIB/f6E/7YHxy03CjMN4ZpE4YUxVFcvKusOgGbqlm8CeT4vCmYUzPsVgAcnCGwEXr6KrJ9wRz9tLNJC0rYyVdoLMy4+Ic58HTzpO5kWS70pgXgjqqti7tNxfST5cdZlD5K42a13Aj7st0+v9aSS7u2v9u9b4dyiH2YHOI9fpaSvXPehDQMLffr4nDPFKmu/Az62uJRl1P3gyN4b9gls3xtencZOc6y8/uI8fWhPqe6unELUpKEfF2WFaTqKDbbANPVhdw3MUbwE/ExaqOwCKt38s/OtKRYA3fTXE9UOLJlb+stcT0BYJJK9PcZPtA5TPHY7Q83pVXfz7/iYVg==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=oe5OdqmJvWf8EMgRkZBJabEvIESwMpYWoznJYTOQJ8T9texW89Vp8Foyuo+IRm94iLZh/Q6eXn6h1zSvyRxbE3/IcsjRWy4WGt7/2poyYUFuGIvyubNAM4DsvV/gkWeEeBkt7Dz4A2718te23tXgkq2/n+ugZEzA8VZA1WLbkAMr0XsMNGRUkxCRUrj9j0IeEeisXjxHcZlYF7QFLTb9SfQYYr1EfiJ61l5lSNAwd1qaSQRUutx+2aqyg15/GoyOkpR9MlxzREvSt4tlPwBV/V5Al/0UwekjtrgYTCSKRouUFQTjwpUapOVqpVCsIxNsmi6fT6Kolyo2LtWzumoT4g==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Paul Durrant <paul@xxxxxxx>, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Julien Grall <julien@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Mon, 28 Nov 2022 10:17:30 +0000
- Ironport-data: A9a23:RiKYzKD/OmpxaBVW/xPiw5YqxClBgxIJ4kV8jS/XYbTApGx01DAGy zRKUWqOOqzeZzT2edklbYjl8B8EscKDx9ViQQY4rX1jcSlH+JHPbTi7wuUcHAvJd5GeExg3h yk6QoOdRCzhZiaE/n9BCpC48T8nk/nNHuCnYAL9EngZbRd+Tys8gg5Ulec8g4p56fC0GArIs t7pyyHlEAbNNwVcbyRFtcpvlDs15K6o4WpC7gRnDRx2lAS2e0c9Xcp3yZ6ZdxMUcqEMdsamS uDKyq2O/2+x13/B3fv8z94X2mVTKlLjFVDmZkh+AsBOsTAbzsAG6Y4pNeJ0VKtio27hc+ada jl6ncfYpQ8BZsUgkQmGOvVSO3kW0aZuoNcrLZUj2CA6IoKvn3bEmp1T4E8K0YIw9+lJM1xUz +YjLRNKX1O7hNy08OnrY7w57igjBJGD0II3nFhFlW2cJ9B2BJfJTuPN+MNS2yo2ioZWB/HCa sEFaD1pKhPdfxlIPVRRA5U79AuqriCnL3sE9xTK+exrvAA/zyQouFTpGOLYYJqgRMFOk26Tp 37c/nS/CRYfXDCa4WrYqCj92LeS9c/9cNsdDq/hqtFnvHiS+0xJVC0wZGG6iubs3yZSXPoac ST44BEGr6I/6UiqRdnVRACjrTiPuRt0c8JZDukS+AyLjK3O7G6xGWwsXjNHLts8u6ceRyEu1 1KPt8PkA3poqrL9YWKQ8PKYoC2/PQARLHQefmkUQA0d+d7hrYovyBXVQb5e/LWdi9T0HXT6x WCMpS1m27EL15ZXiOO84EzNhC+qqt7RVAkp6w7LX2WjqARkeIqiYI/u4l/ehRpdELukopC6l CBss6CjAComV/lhSATlrD0xIYyU
- Ironport-hdrordr: A9a23:yz5846/FjwxXE4kBVq9uk+Hpdr1zdoMgy1knxilNoENuH/Bwxv rFoB1E73TJYW4qKQodcdDpAtjifZtFnaQFq7X5To3SJjUO31HYYb2KjLGSiAEIfheTygcz79 YGT0ETMrzN5B1B/L7HCWqDYpodKbu8gcaVbI7lph8DIz2CKZsQljuRYTzrcHGeMTM2YabRY6 Dsg/avyQDBRV0nKuCAQlUVVenKoNPG0LrgfB49HhYirCWekD+y77b+Mh6AmjMTSSlGz7sO+X XM11WR3NTsj9iLjjvnk0PD5ZVfn9XsjvNFGcy3k8AQbhn8lwqyY4xlerua+BQ4uvum5loGmM TF5z0gI8NwwXXMeXzdm2qj5yDQlBIVr1Pyw16RhnXu5ebjQighNsZHjYVFNjPE9ksJprhHoe J29lPck6ASIQLLnSz76dSNfQptjFCIrX0rlvNWp2BDULEZdKRaoeUkjQBo+dY7bWDHAbIcYa 1T5fLnlbFrmJShHjbkV1xUsZmRt7IIb067qwY5y5SoOnNt7Q1EJgMjtbAidzE7hdMAotB/lp v5G7Utm7dUQsAMa6VhQO8HXMusE2TIBQnBKWSIPD3cZdc60l/22urKCY8OlZaXUY1NyIF3lI XKUVteu2J3c0XyCdeW1JkO9hzWWm2yUTnk18kbvvFCy/XBbauuNTfGREElksOmrflaCsrHW+ yrMJYTB/P4N2PhFYtAwgW7UZhPLnsVVtETp78AKhuzi9OOLpevuv3Qcf7VKraoGTE4WnnnCn 9GRzT3LNUo1DHfZpY5ummiZ5rAQD2OwXsrKtmlwwE68vl9CqRc9g4IlF+++saHbTVfr61eRj oMHI/a
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Thread-index: AQHZAxKKQ5gC8MSrQkutQViuEWIcyQ==
- Thread-topic: [PATCH] ioreq_broadcast(): accept partial broadcast success
On 28/11/2022 08:21, Jan Beulich wrote:
> On 26.11.2022 23:19, Julien Grall wrote:
>>
>> The commit message is quite vague, so it is hard to know what you are
>> trying to solve exactly. AFAIU, there are two reasons for
>> ioreq_broadcast to fails:
>> 1) The IOREQ server didn't register the bufioreq
>> 2) The IOREQ buffer page is full
>>
>> While I would agree that the error message is not necessary for 1) (the
>> IOREQ server doesn't care about the event), I would disagree for 2)
>> because it would indicate something went horribly wrong in the IOREQ
>> server and there is a chance your domain may misbehave afterwards.
>
> In addition I think ignoring failure (and, as said by Julien, only because
> of no bufioreq being registered) is (kind of implicitly) valid only for
> buffered requests. Hence I'm unconvinced of the need of a new boolean
> function parameter. Instead I think we need a new IOREQ_STATUS_... value
> representing the "not registered" case. And that could then be used by
> ioreq_broadcast() to skip incrementing of "failed".
Hi guys, and thank you very much for the feedback. As I'm sure you've
guessed I'm a newbie in Xen terms, so apologies for not getting things
quite right.
Varstored dropped support for buffered ioreqs, hence the persistent
error message(s), and the proposed fix was derived from discussion in
Citrix's hypervisor team. The 'partial' parameter could arguably be
considered a case of (undesirable) special case handling, but
ioreq_broadcast() is called from only two places in the code, so this
seemed to be the lightest and simplest solution. I'll have to defer to
more knowledgeable team members for further thoughts on this.
Best,
-- Per
|