From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0C97BC43603 for ; Mon, 9 Dec 2019 13:55:43 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D6C172068E for ; Mon, 9 Dec 2019 13:55:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D6C172068E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1ieJVT-0004um-ON; Mon, 09 Dec 2019 13:55:23 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1ieJVT-0004uh-8I for xen-devel@lists.xenproject.org; Mon, 09 Dec 2019 13:55:23 +0000 X-Inumbo-ID: 8a537fc0-1a8b-11ea-87d8-12813bfff9fa Received: from mx1.suse.de (unknown [195.135.220.15]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 8a537fc0-1a8b-11ea-87d8-12813bfff9fa; Mon, 09 Dec 2019 13:55:21 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id B3B57AC3F; Mon, 9 Dec 2019 13:55:20 +0000 (UTC) To: Paul Durrant , linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org References: <20191205140123.3817-1-pdurrant@amazon.com> <20191205140123.3817-4-pdurrant@amazon.com> From: =?UTF-8?B?SsO8cmdlbiBHcm/Dnw==?= Message-ID: <8a42e7a2-e1aa-69ff-32a4-f43cc5df10d9@suse.com> Date: Mon, 9 Dec 2019 14:55:19 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.1 MIME-Version: 1.0 In-Reply-To: <20191205140123.3817-4-pdurrant@amazon.com> Content-Language: en-US Subject: Re: [Xen-devel] [PATCH 3/4] xen/interface: don't discard pending work in FRONT/BACK_RING_ATTACH X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Boris Ostrovsky , Stefano Stabellini Content-Transfer-Encoding: base64 Content-Type: text/plain; charset="utf-8"; Format="flowed" Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" T24gMDUuMTIuMTkgMTU6MDEsIFBhdWwgRHVycmFudCB3cm90ZToKPiBDdXJyZW50bHkgdGhlc2Ug bWFjcm9zIHdpbGwgc2tpcCBvdmVyIGFueSByZXF1ZXN0cy9yZXNwb25zZXMgdGhhdCBhcmUKPiBh ZGRlZCB0byB0aGUgc2hhcmVkIHJpbmcgd2hpbHN0IGl0IGlzIGRldGFjaGVkLiBUaGlzLCBpbiBn ZW5lcmFsLCBpcyBub3QKPiBhIGRlc2lyYWJsZSBzZW1hbnRpYyBzaW5jZSBtb3N0IGZyb250ZW5k IGltcGxlbWVudGF0aW9ucyB3aWxsIGV2ZW50dWFsbHkKPiBibG9jayB3YWl0aW5nIGZvciBhIHJl c3BvbnNlIHdoaWNoIHdvdWxkIGVpdGhlciBuZXZlciBhcHBlYXIgb3IgbmV2ZXIgYmUKPiBwcm9j ZXNzZWQuCj4gCj4gTk9URTogVGhlc2UgbWFjcm9zIGFyZSBjdXJyZW50bHkgdW51c2VkLiBCQUNL X1JJTkdfQVRUQUNIKCksIGhvd2V2ZXIsIHdpbGwKPiAgICAgICAgYmUgdXNlZCBpbiBhIHN1YnNl cXVlbnQgcGF0Y2guCj4gCj4gU2lnbmVkLW9mZi1ieTogUGF1bCBEdXJyYW50IDxwZHVycmFudEBh bWF6b24uY29tPgo+IC0tLQo+IENjOiBCb3JpcyBPc3Ryb3Zza3kgPGJvcmlzLm9zdHJvdnNreUBv cmFjbGUuY29tPgo+IENjOiBKdWVyZ2VuIEdyb3NzIDxqZ3Jvc3NAc3VzZS5jb20+Cj4gQ2M6IFN0 ZWZhbm8gU3RhYmVsbGluaSA8c3N0YWJlbGxpbmlAa2VybmVsLm9yZz4KPiAtLS0KPiAgIGluY2x1 ZGUveGVuL2ludGVyZmFjZS9pby9yaW5nLmggfCA0ICsrLS0KPiAgIDEgZmlsZSBjaGFuZ2VkLCAy IGluc2VydGlvbnMoKyksIDIgZGVsZXRpb25zKC0pCj4gCj4gZGlmZiAtLWdpdCBhL2luY2x1ZGUv eGVuL2ludGVyZmFjZS9pby9yaW5nLmggYi9pbmNsdWRlL3hlbi9pbnRlcmZhY2UvaW8vcmluZy5o Cj4gaW5kZXggM2Y0MDUwMWZjNjBiLi40MDVhZGZlZDg3ZTYgMTAwNjQ0Cj4gLS0tIGEvaW5jbHVk ZS94ZW4vaW50ZXJmYWNlL2lvL3JpbmcuaAo+ICsrKyBiL2luY2x1ZGUveGVuL2ludGVyZmFjZS9p by9yaW5nLmgKPiBAQCAtMTQzLDE0ICsxNDMsMTQgQEAgc3RydWN0IF9fbmFtZSMjX2JhY2tfcmlu ZyB7CQkJCQkJXAo+ICAgI2RlZmluZSBGUk9OVF9SSU5HX0FUVEFDSChfciwgX3MsIF9fc2l6ZSkg ZG8gewkJCQlcCj4gICAgICAgKF9yKS0+c3JpbmcgPSAoX3MpOwkJCQkJCQlcCj4gICAgICAgKF9y KS0+cmVxX3Byb2RfcHZ0ID0gKF9zKS0+cmVxX3Byb2Q7CQkJCVwKPiAtICAgIChfciktPnJzcF9j b25zID0gKF9zKS0+cnNwX3Byb2Q7CQkJCQlcCj4gKyAgICAoX3IpLT5yc3BfY29ucyA9IChfcykt PnJlcV9wcm9kOwkJCQkJXAo+ICAgICAgIChfciktPm5yX2VudHMgPSBfX1JJTkdfU0laRShfcywg X19zaXplKTsJCQkJXAo+ICAgfSB3aGlsZSAoMCkKPiAgIAo+ICAgI2RlZmluZSBCQUNLX1JJTkdf QVRUQUNIKF9yLCBfcywgX19zaXplKSBkbyB7CQkJCVwKPiAgICAgICAoX3IpLT5zcmluZyA9IChf cyk7CQkJCQkJCVwKPiAgICAgICAoX3IpLT5yc3BfcHJvZF9wdnQgPSAoX3MpLT5yc3BfcHJvZDsJ CQkJXAo+IC0gICAgKF9yKS0+cmVxX2NvbnMgPSAoX3MpLT5yZXFfcHJvZDsJCQkJCVwKPiArICAg IChfciktPnJlcV9jb25zID0gKF9zKS0+cnNwX3Byb2Q7CQkJCQlcCj4gICAgICAgKF9yKS0+bnJf ZW50cyA9IF9fUklOR19TSVpFKF9zLCBfX3NpemUpOwkJCQlcCj4gICB9IHdoaWxlICgwKQoKTGV0 cyBsb29rIGF0IGFsbCBwb3NzaWJsZSBzY2VuYXJpb3Mgd2hlcmUgQkFDS19SSU5HX0FUVEFDSCgp Cm1pZ2h0IGhhcHBlbjoKCkluaXRpYWxseSAoYWZ0ZXIgW0ZST05UfEJBQ0tdX1JJTkdfSU5JVCgp LCBsZWF2aW5nIF9wdnQgYXdheSk6CnJlcV9wcm9kPTAsIHJzcF9jb25zPTAsIHJzcF9wcm9kPTAs IHJlcV9jb25zPTAKVXNpbmcgQkFDS19SSU5HX0FUVEFDSCgpIGlzIGZpbmUgKG5vIGNoYW5nZSkK ClJlcXVlc3QgcXVldWVkOgpyZXFfcHJvZD0xLCByc3BfY29ucz0wLCByc3BfcHJvZD0wLCByZXFf Y29ucz0wClVzaW5nIEJBQ0tfUklOR19BVFRBQ0goKSBpcyBmaW5lIChubyBjaGFuZ2UpCgphbmQg dGFrZW4gYnkgYmFja2VuZDoKcmVxX3Byb2Q9MSwgcnNwX2NvbnM9MCwgcnNwX3Byb2Q9MCwgcmVx X2NvbnM9MQpVc2luZyBCQUNLX1JJTkdfQVRUQUNIKCkgaXMgcmVzZXR0aW5nIHJlcV9jb25zIHRv IDAsIHdpbGwgcmVzdWx0CmluIHJlZG9pbmcgcmVxdWVzdCAoZm9yIGJsayB0aGlzIGlzIGZpbmUs IG90aGVyIGRldmljZXMgbGlrZSBTQ1NJCnRhcGVzIHdpbGwgaGF2ZSBpc3N1ZXMgd2l0aCB0aGF0 KS4gT25lIHBvc3NpYmxlIHNvbHV0aW9uIHdvdWxkIGJlCnRvIGVuc3VyZSBhbGwgdGFrZW4gcmVx dWVzdHMgYXJlIGVpdGhlciBzdG9wcGVkIG9yIHRoZSByZXNwb25zZQppcyBxdWV1ZWQgYWxyZWFk eS4KClJlc3BvbnNlIHF1ZXVlZDoKcmVxX3Byb2Q9MSwgcnNwX2NvbnM9MCwgcnNwX3Byb2Q9MSwg cmVxX2NvbnM9MQpVc2luZyBCQUNLX1JJTkdfQVRUQUNIKCkgaXMgZmluZSAobm8gY2hhbmdlKQoK UmVzcG9uc2UgdGFrZW46CnJlcV9wcm9kPTEsIHJzcF9jb25zPTEsIHJzcF9wcm9kPTEsIHJlcV9j b25zPTEKVXNpbmcgQkFDS19SSU5HX0FUVEFDSCgpIGlzIGZpbmUgKG5vIGNoYW5nZSkKCkluIGdl bmVyYWwgSSBiZWxpZXZlIHRoZSBbRlJPTlR8QkFDS11fUklOR19BVFRBQ0goKSBtYWNyb3MgYXJl IG5vdApmaW5lIHRvIGJlIHVzZWQgaW4gdGhlIGN1cnJlbnQgc3RhdGUsIGFzIHRoZSAqX3B2dCBm aWVsZHMgbm9ybWFsbHkgbm90CmFjY2Vzc2libGUgYnkgdGhlIG90aGVyIGVuZCBhcmUgaW5pdGlh bGl6ZWQgdXNpbmcgdGhlIChwb3NzaWJseQp1bnRydXN0ZWQpIHZhbHVlcyBmcm9tIHRoZSBzaGFy ZWQgcmluZy4gVGhlcmUgbmVlZHMgYXQgbGVhc3QgdG8gYmUgYQp0ZXN0IGZvciB0aGUgdmFsdWVz IHRvIGJlIHNhbmUsIGFuZCB5b3VyIGNoYW5nZSBzaG91bGQgbm90IHJlc3VsdCBpbiB0aGUKc2Ft ZSB2YWx1ZSB0byBiZSByZWFkIHR3aWNlLCBhcyBpdCBjb3VsZCBoYXZlIGNoYW5nZWQgaW4gYmV0 d2Vlbi4KCkFzIHRoaXMgaXMgYW4gZXJyb3Igd2hpY2ggY2FuIGhhcHBlbiBpbiBvdGhlciBPUydz LCB0b28sIEknZCByZWNvbW1lbmQKdG8gYWRkIHRoZSBhZGFwdGVkIG1hY3JvcyAocGx1cyBhIGNv bW1lbnQgcmVnYXJkaW5nIHRoZSBwb3NzaWJsZQpwcm9ibGVtIG5vdGVkIGFib3ZlIGZvciBzcGVj aWFsIGRldmljZXMgbGlrZSB0YXBlcykgdG8gdGhlIFhlbiB2YXJpYW50Cm9mIHJpbmcuaC4KCgpK dWVyZ2VuCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpY ZW4tZGV2ZWwgbWFpbGluZyBsaXN0Clhlbi1kZXZlbEBsaXN0cy54ZW5wcm9qZWN0Lm9yZwpodHRw czovL2xpc3RzLnhlbnByb2plY3Qub3JnL21haWxtYW4vbGlzdGluZm8veGVuLWRldmVs From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B1011C43603 for ; Mon, 9 Dec 2019 13:55:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 897022068E for ; Mon, 9 Dec 2019 13:55:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727643AbfLINzX (ORCPT ); Mon, 9 Dec 2019 08:55:23 -0500 Received: from mx2.suse.de ([195.135.220.15]:59704 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727268AbfLINzX (ORCPT ); Mon, 9 Dec 2019 08:55:23 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id B3B57AC3F; Mon, 9 Dec 2019 13:55:20 +0000 (UTC) Subject: Re: [PATCH 3/4] xen/interface: don't discard pending work in FRONT/BACK_RING_ATTACH To: Paul Durrant , linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org Cc: Boris Ostrovsky , Stefano Stabellini References: <20191205140123.3817-1-pdurrant@amazon.com> <20191205140123.3817-4-pdurrant@amazon.com> From: =?UTF-8?B?SsO8cmdlbiBHcm/Dnw==?= Message-ID: <8a42e7a2-e1aa-69ff-32a4-f43cc5df10d9@suse.com> Date: Mon, 9 Dec 2019 14:55:19 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.1 MIME-Version: 1.0 In-Reply-To: <20191205140123.3817-4-pdurrant@amazon.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05.12.19 15:01, Paul Durrant wrote: > Currently these macros will skip over any requests/responses that are > added to the shared ring whilst it is detached. This, in general, is not > a desirable semantic since most frontend implementations will eventually > block waiting for a response which would either never appear or never be > processed. > > NOTE: These macros are currently unused. BACK_RING_ATTACH(), however, will > be used in a subsequent patch. > > Signed-off-by: Paul Durrant > --- > Cc: Boris Ostrovsky > Cc: Juergen Gross > Cc: Stefano Stabellini > --- > include/xen/interface/io/ring.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/xen/interface/io/ring.h b/include/xen/interface/io/ring.h > index 3f40501fc60b..405adfed87e6 100644 > --- a/include/xen/interface/io/ring.h > +++ b/include/xen/interface/io/ring.h > @@ -143,14 +143,14 @@ struct __name##_back_ring { \ > #define FRONT_RING_ATTACH(_r, _s, __size) do { \ > (_r)->sring = (_s); \ > (_r)->req_prod_pvt = (_s)->req_prod; \ > - (_r)->rsp_cons = (_s)->rsp_prod; \ > + (_r)->rsp_cons = (_s)->req_prod; \ > (_r)->nr_ents = __RING_SIZE(_s, __size); \ > } while (0) > > #define BACK_RING_ATTACH(_r, _s, __size) do { \ > (_r)->sring = (_s); \ > (_r)->rsp_prod_pvt = (_s)->rsp_prod; \ > - (_r)->req_cons = (_s)->req_prod; \ > + (_r)->req_cons = (_s)->rsp_prod; \ > (_r)->nr_ents = __RING_SIZE(_s, __size); \ > } while (0) Lets look at all possible scenarios where BACK_RING_ATTACH() might happen: Initially (after [FRONT|BACK]_RING_INIT(), leaving _pvt away): req_prod=0, rsp_cons=0, rsp_prod=0, req_cons=0 Using BACK_RING_ATTACH() is fine (no change) Request queued: req_prod=1, rsp_cons=0, rsp_prod=0, req_cons=0 Using BACK_RING_ATTACH() is fine (no change) and taken by backend: req_prod=1, rsp_cons=0, rsp_prod=0, req_cons=1 Using BACK_RING_ATTACH() is resetting req_cons to 0, will result in redoing request (for blk this is fine, other devices like SCSI tapes will have issues with that). One possible solution would be to ensure all taken requests are either stopped or the response is queued already. Response queued: req_prod=1, rsp_cons=0, rsp_prod=1, req_cons=1 Using BACK_RING_ATTACH() is fine (no change) Response taken: req_prod=1, rsp_cons=1, rsp_prod=1, req_cons=1 Using BACK_RING_ATTACH() is fine (no change) In general I believe the [FRONT|BACK]_RING_ATTACH() macros are not fine to be used in the current state, as the *_pvt fields normally not accessible by the other end are initialized using the (possibly untrusted) values from the shared ring. There needs at least to be a test for the values to be sane, and your change should not result in the same value to be read twice, as it could have changed in between. As this is an error which can happen in other OS's, too, I'd recommend to add the adapted macros (plus a comment regarding the possible problem noted above for special devices like tapes) to the Xen variant of ring.h. Juergen