From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751287AbdCQVH6 (ORCPT ); Fri, 17 Mar 2017 17:07:58 -0400 Received: from mail.kernel.org ([198.145.29.136]:50250 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751047AbdCQVH4 (ORCPT ); Fri, 17 Mar 2017 17:07:56 -0400 Date: Fri, 17 Mar 2017 13:55:19 -0700 (PDT) From: Stefano Stabellini X-X-Sender: sstabellini@sstabellini-ThinkPad-X260 To: Konrad Rzeszutek Wilk cc: Juergen Gross , Stefano Stabellini , Latchesar Ionkov , Eric Van Hensbergen , linux-kernel@vger.kernel.org, Stefano Stabellini , v9fs-developer@lists.sourceforge.net, Ron Minnich , xen-devel@lists.xenproject.org, boris.ostrovsky@oracle.com Subject: Re: [Xen-devel] [PATCH v3 4/7] xen/9pfs: connect to the backend In-Reply-To: <20170317151357.GS7915@char.us.oracle.com> Message-ID: References: <1489449019-13343-1-git-send-email-sstabellini@kernel.org> <1489449019-13343-4-git-send-email-sstabellini@kernel.org> <0f602607-df9c-85e1-e7df-eca76b306679@suse.com> <20170317151357.GS7915@char.us.oracle.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 17 Mar 2017, Konrad Rzeszutek Wilk wrote: > On Fri, Mar 17, 2017 at 05:54:47AM +0100, Juergen Gross wrote: > > On 16/03/17 19:03, Stefano Stabellini wrote: > > > On Thu, 16 Mar 2017, Juergen Gross wrote: > > >> On 15/03/17 19:44, Stefano Stabellini wrote: > > >>> On Wed, 15 Mar 2017, Juergen Gross wrote: > > >>>> On 14/03/17 22:22, Stefano Stabellini wrote: > > >>>>> Hi Juergen, > > >>>>> > > >>>>> thank you for the review! > > >>>>> > > >>>>> On Tue, 14 Mar 2017, Juergen Gross wrote: > > >>>>>> On 14/03/17 00:50, Stefano Stabellini wrote: > > >>>>>>> Implement functions to handle the xenbus handshake. Upon connection, > > >>>>>>> allocate the rings according to the protocol specification. > > >>>>>>> > > >>>>>>> Initialize a work_struct and a wait_queue. The work_struct will be used > > >>>>>>> to schedule work upon receiving an event channel notification from the > > >>>>>>> backend. The wait_queue will be used to wait when the ring is full and > > >>>>>>> we need to send a new request. > > >>>>>>> > > >>>>>>> Signed-off-by: Stefano Stabellini > > >>>>>>> CC: boris.ostrovsky@oracle.com > > >>>>>>> CC: jgross@suse.com > > >>>>>>> CC: Eric Van Hensbergen > > >>>>>>> CC: Ron Minnich > > >>>>>>> CC: Latchesar Ionkov > > >>>>>>> CC: v9fs-developer@lists.sourceforge.net > > >>>>>>> --- > > >>>> > > >>>>>> Did you think about using request_threaded_irq() instead of a workqueue? > > >>>>>> For an example see e.g. drivers/scsi/xen-scsifront.c > > >>>>> > > >>>>> I like workqueues :-) It might come down to personal preferences, but I > > >>>>> think workqueues are more flexible and a better fit for this use case. > > >>>>> Not only it is easy to schedule work in a workqueue from the interrupt > > >>>>> handler, but also they can be used for sleeping in the request function > > >>>>> if there is not enough room on the ring. Besides, they can easily be > > >>>>> configured to share a single thread or to have multiple independent > > >>>>> threads. > > >>>> > > >>>> I'm fine with the workqueues as long as you have decided to use them > > >>>> considering the alternatives. :-) > > >>>> > > >>>>>> Can't you use xenbus_read_unsigned() instead of xenbus_read()? > > >>>>> > > >>>>> I can use xenbus_read_unsigned in the other cases below, but not here, > > >>>>> because versions is in the form: "1,3,4" > > >>>> > > >>>> Is this documented somewhere? > > >>>> > > >>>> Hmm, are any of the Xenstore entries documented? Shouldn't this be done > > >>>> in xen_9pfs.h ? > > >>> > > >>> They are documented in docs/misc/9pfs.markdown, under "Xenstore". Given > > >>> that it's all written there, especially the semantics, I didn't repeat > > >>> it in xen_9pfs.h > > >> > > >> Looking at it from the Linux kernel perspective this documentation is > > >> not really highly visible. For me it is okay, but there have been > > >> multiple examples in the past where documentation in the Xen repository > > >> wasn't regarded as being sufficient. > > >> > > >> I recommend moving the documentation regarding the interface into the > > >> header file like for the other pv interfaces. > > > > > > What about adding a link such as: > > > > > > http://xenbits.xen.org/gitweb/?p=xen.git;a=blob_plain;f=docs/misc/9pfs.markdown;hb=HEAD > > Ewww. > > > How about https://xenbits.xen.org/docs/unstable/misc/9pfs.html > > which gets updated daily. Great idea, I'll do that > > > > > > that should be easily accessible, right? For other specifications, such > > > as 9p, only links are provided (see Documentation/filesystems/9p.txt). > > > I am suggesting a link, because then we are sure the specs don't go out > > > of sync. I realize that older PV protocols were described in header > > > files, but that was before Xen Project had a formal process for getting > > > new specifications accepted, and a formal place where to publish them. > > > > Fine with me. Lets see if other maintainers are okay with it, too. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH v3 4/7] xen/9pfs: connect to the backend Date: Fri, 17 Mar 2017 13:55:19 -0700 (PDT) Message-ID: References: <1489449019-13343-1-git-send-email-sstabellini@kernel.org> <1489449019-13343-4-git-send-email-sstabellini@kernel.org> <0f602607-df9c-85e1-e7df-eca76b306679@suse.com> <20170317151357.GS7915@char.us.oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1coyuH-0005FL-IW for xen-devel@lists.xenproject.org; Fri, 17 Mar 2017 20:55:29 +0000 In-Reply-To: <20170317151357.GS7915@char.us.oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Konrad Rzeszutek Wilk Cc: Juergen Gross , Latchesar Ionkov , Stefano Stabellini , xen-devel@lists.xenproject.org, Eric Van Hensbergen , linux-kernel@vger.kernel.org, Stefano Stabellini , Ron Minnich , v9fs-developer@lists.sourceforge.net, boris.ostrovsky@oracle.com List-Id: xen-devel@lists.xenproject.org T24gRnJpLCAxNyBNYXIgMjAxNywgS29ucmFkIFJ6ZXN6dXRlayBXaWxrIHdyb3RlOgo+IE9uIEZy aSwgTWFyIDE3LCAyMDE3IGF0IDA1OjU0OjQ3QU0gKzAxMDAsIEp1ZXJnZW4gR3Jvc3Mgd3JvdGU6 Cj4gPiBPbiAxNi8wMy8xNyAxOTowMywgU3RlZmFubyBTdGFiZWxsaW5pIHdyb3RlOgo+ID4gPiBP biBUaHUsIDE2IE1hciAyMDE3LCBKdWVyZ2VuIEdyb3NzIHdyb3RlOgo+ID4gPj4gT24gMTUvMDMv MTcgMTk6NDQsIFN0ZWZhbm8gU3RhYmVsbGluaSB3cm90ZToKPiA+ID4+PiBPbiBXZWQsIDE1IE1h ciAyMDE3LCBKdWVyZ2VuIEdyb3NzIHdyb3RlOgo+ID4gPj4+PiBPbiAxNC8wMy8xNyAyMjoyMiwg U3RlZmFubyBTdGFiZWxsaW5pIHdyb3RlOgo+ID4gPj4+Pj4gSGkgSnVlcmdlbiwKPiA+ID4+Pj4+ Cj4gPiA+Pj4+PiB0aGFuayB5b3UgZm9yIHRoZSByZXZpZXchCj4gPiA+Pj4+Pgo+ID4gPj4+Pj4g T24gVHVlLCAxNCBNYXIgMjAxNywgSnVlcmdlbiBHcm9zcyB3cm90ZToKPiA+ID4+Pj4+PiBPbiAx NC8wMy8xNyAwMDo1MCwgU3RlZmFubyBTdGFiZWxsaW5pIHdyb3RlOgo+ID4gPj4+Pj4+PiBJbXBs ZW1lbnQgZnVuY3Rpb25zIHRvIGhhbmRsZSB0aGUgeGVuYnVzIGhhbmRzaGFrZS4gVXBvbiBjb25u ZWN0aW9uLAo+ID4gPj4+Pj4+PiBhbGxvY2F0ZSB0aGUgcmluZ3MgYWNjb3JkaW5nIHRvIHRoZSBw cm90b2NvbCBzcGVjaWZpY2F0aW9uLgo+ID4gPj4+Pj4+Pgo+ID4gPj4+Pj4+PiBJbml0aWFsaXpl IGEgd29ya19zdHJ1Y3QgYW5kIGEgd2FpdF9xdWV1ZS4gVGhlIHdvcmtfc3RydWN0IHdpbGwgYmUg dXNlZAo+ID4gPj4+Pj4+PiB0byBzY2hlZHVsZSB3b3JrIHVwb24gcmVjZWl2aW5nIGFuIGV2ZW50 IGNoYW5uZWwgbm90aWZpY2F0aW9uIGZyb20gdGhlCj4gPiA+Pj4+Pj4+IGJhY2tlbmQuIFRoZSB3 YWl0X3F1ZXVlIHdpbGwgYmUgdXNlZCB0byB3YWl0IHdoZW4gdGhlIHJpbmcgaXMgZnVsbCBhbmQK PiA+ID4+Pj4+Pj4gd2UgbmVlZCB0byBzZW5kIGEgbmV3IHJlcXVlc3QuCj4gPiA+Pj4+Pj4+Cj4g PiA+Pj4+Pj4+IFNpZ25lZC1vZmYtYnk6IFN0ZWZhbm8gU3RhYmVsbGluaSA8c3RlZmFub0BhcG9y ZXRvLmNvbT4KPiA+ID4+Pj4+Pj4gQ0M6IGJvcmlzLm9zdHJvdnNreUBvcmFjbGUuY29tCj4gPiA+ Pj4+Pj4+IENDOiBqZ3Jvc3NAc3VzZS5jb20KPiA+ID4+Pj4+Pj4gQ0M6IEVyaWMgVmFuIEhlbnNi ZXJnZW4gPGVyaWN2aEBnbWFpbC5jb20+Cj4gPiA+Pj4+Pj4+IENDOiBSb24gTWlubmljaCA8cm1p bm5pY2hAc2FuZGlhLmdvdj4KPiA+ID4+Pj4+Pj4gQ0M6IExhdGNoZXNhciBJb25rb3YgPGx1Y2hv QGlvbmtvdi5uZXQ+Cj4gPiA+Pj4+Pj4+IENDOiB2OWZzLWRldmVsb3BlckBsaXN0cy5zb3VyY2Vm b3JnZS5uZXQKPiA+ID4+Pj4+Pj4gLS0tCj4gPiA+Pj4+Cj4gPiA+Pj4+Pj4gRGlkIHlvdSB0aGlu ayBhYm91dCB1c2luZyByZXF1ZXN0X3RocmVhZGVkX2lycSgpIGluc3RlYWQgb2YgYSB3b3JrcXVl dWU/Cj4gPiA+Pj4+Pj4gRm9yIGFuIGV4YW1wbGUgc2VlIGUuZy4gZHJpdmVycy9zY3NpL3hlbi1z Y3NpZnJvbnQuYwo+ID4gPj4+Pj4KPiA+ID4+Pj4+IEkgbGlrZSB3b3JrcXVldWVzIDotKSAgSXQg bWlnaHQgY29tZSBkb3duIHRvIHBlcnNvbmFsIHByZWZlcmVuY2VzLCBidXQgSQo+ID4gPj4+Pj4g dGhpbmsgd29ya3F1ZXVlcyBhcmUgbW9yZSBmbGV4aWJsZSBhbmQgYSBiZXR0ZXIgZml0IGZvciB0 aGlzIHVzZSBjYXNlLgo+ID4gPj4+Pj4gTm90IG9ubHkgaXQgaXMgZWFzeSB0byBzY2hlZHVsZSB3 b3JrIGluIGEgd29ya3F1ZXVlIGZyb20gdGhlIGludGVycnVwdAo+ID4gPj4+Pj4gaGFuZGxlciwg YnV0IGFsc28gdGhleSBjYW4gYmUgdXNlZCBmb3Igc2xlZXBpbmcgaW4gdGhlIHJlcXVlc3QgZnVu Y3Rpb24KPiA+ID4+Pj4+IGlmIHRoZXJlIGlzIG5vdCBlbm91Z2ggcm9vbSBvbiB0aGUgcmluZy4g QmVzaWRlcywgdGhleSBjYW4gZWFzaWx5IGJlCj4gPiA+Pj4+PiBjb25maWd1cmVkIHRvIHNoYXJl IGEgc2luZ2xlIHRocmVhZCBvciB0byBoYXZlIG11bHRpcGxlIGluZGVwZW5kZW50Cj4gPiA+Pj4+ PiB0aHJlYWRzLgo+ID4gPj4+Pgo+ID4gPj4+PiBJJ20gZmluZSB3aXRoIHRoZSB3b3JrcXVldWVz IGFzIGxvbmcgYXMgeW91IGhhdmUgZGVjaWRlZCB0byB1c2UgdGhlbQo+ID4gPj4+PiBjb25zaWRl cmluZyB0aGUgYWx0ZXJuYXRpdmVzLiA6LSkKPiA+ID4+Pj4KPiA+ID4+Pj4+PiBDYW4ndCB5b3Ug dXNlIHhlbmJ1c19yZWFkX3Vuc2lnbmVkKCkgaW5zdGVhZCBvZiB4ZW5idXNfcmVhZCgpPwo+ID4g Pj4+Pj4KPiA+ID4+Pj4+IEkgY2FuIHVzZSB4ZW5idXNfcmVhZF91bnNpZ25lZCBpbiB0aGUgb3Ro ZXIgY2FzZXMgYmVsb3csIGJ1dCBub3QgaGVyZSwKPiA+ID4+Pj4+IGJlY2F1c2UgdmVyc2lvbnMg aXMgaW4gdGhlIGZvcm06ICIxLDMsNCIKPiA+ID4+Pj4KPiA+ID4+Pj4gSXMgdGhpcyBkb2N1bWVu dGVkIHNvbWV3aGVyZT8KPiA+ID4+Pj4KPiA+ID4+Pj4gSG1tLCBhcmUgYW55IG9mIHRoZSBYZW5z dG9yZSBlbnRyaWVzIGRvY3VtZW50ZWQ/IFNob3VsZG4ndCB0aGlzIGJlIGRvbmUKPiA+ID4+Pj4g aW4geGVuXzlwZnMuaCA/Cj4gPiA+Pj4gIAo+ID4gPj4+IFRoZXkgYXJlIGRvY3VtZW50ZWQgaW4g ZG9jcy9taXNjLzlwZnMubWFya2Rvd24sIHVuZGVyICJYZW5zdG9yZSIuIEdpdmVuCj4gPiA+Pj4g dGhhdCBpdCdzIGFsbCB3cml0dGVuIHRoZXJlLCBlc3BlY2lhbGx5IHRoZSBzZW1hbnRpY3MsIEkg ZGlkbid0IHJlcGVhdAo+ID4gPj4+IGl0IGluIHhlbl85cGZzLmgKPiA+ID4+Cj4gPiA+PiBMb29r aW5nIGF0IGl0IGZyb20gdGhlIExpbnV4IGtlcm5lbCBwZXJzcGVjdGl2ZSB0aGlzIGRvY3VtZW50 YXRpb24gaXMKPiA+ID4+IG5vdCByZWFsbHkgaGlnaGx5IHZpc2libGUuIEZvciBtZSBpdCBpcyBv a2F5LCBidXQgdGhlcmUgaGF2ZSBiZWVuCj4gPiA+PiBtdWx0aXBsZSBleGFtcGxlcyBpbiB0aGUg cGFzdCB3aGVyZSBkb2N1bWVudGF0aW9uIGluIHRoZSBYZW4gcmVwb3NpdG9yeQo+ID4gPj4gd2Fz bid0IHJlZ2FyZGVkIGFzIGJlaW5nIHN1ZmZpY2llbnQuCj4gPiA+Pgo+ID4gPj4gSSByZWNvbW1l bmQgbW92aW5nIHRoZSBkb2N1bWVudGF0aW9uIHJlZ2FyZGluZyB0aGUgaW50ZXJmYWNlIGludG8g dGhlCj4gPiA+PiBoZWFkZXIgZmlsZSBsaWtlIGZvciB0aGUgb3RoZXIgcHYgaW50ZXJmYWNlcy4K PiA+ID4gCj4gPiA+IFdoYXQgYWJvdXQgYWRkaW5nIGEgbGluayBzdWNoIGFzOgo+ID4gPiAKPiA+ ID4gaHR0cDovL3hlbmJpdHMueGVuLm9yZy9naXR3ZWIvP3A9eGVuLmdpdDthPWJsb2JfcGxhaW47 Zj1kb2NzL21pc2MvOXBmcy5tYXJrZG93bjtoYj1IRUFECj4gCj4gRXd3dy4KPiAKPiAKPiBIb3cg YWJvdXQgaHR0cHM6Ly94ZW5iaXRzLnhlbi5vcmcvZG9jcy91bnN0YWJsZS9taXNjLzlwZnMuaHRt bAo+IAo+IHdoaWNoIGdldHMgdXBkYXRlZCBkYWlseS4KCkdyZWF0IGlkZWEsIEknbGwgZG8gdGhh dAoKCj4gPiA+IAo+ID4gPiB0aGF0IHNob3VsZCBiZSBlYXNpbHkgYWNjZXNzaWJsZSwgcmlnaHQ/ IEZvciBvdGhlciBzcGVjaWZpY2F0aW9ucywgc3VjaAo+ID4gPiBhcyA5cCwgb25seSBsaW5rcyBh cmUgcHJvdmlkZWQgKHNlZSBEb2N1bWVudGF0aW9uL2ZpbGVzeXN0ZW1zLzlwLnR4dCkuCj4gPiA+ IEkgYW0gc3VnZ2VzdGluZyBhIGxpbmssIGJlY2F1c2UgdGhlbiB3ZSBhcmUgc3VyZSB0aGUgc3Bl Y3MgZG9uJ3QgZ28gb3V0Cj4gPiA+IG9mIHN5bmMuIEkgcmVhbGl6ZSB0aGF0IG9sZGVyIFBWIHBy b3RvY29scyB3ZXJlIGRlc2NyaWJlZCBpbiBoZWFkZXIKPiA+ID4gZmlsZXMsIGJ1dCB0aGF0IHdh cyBiZWZvcmUgWGVuIFByb2plY3QgaGFkIGEgZm9ybWFsIHByb2Nlc3MgZm9yIGdldHRpbmcKPiA+ ID4gbmV3IHNwZWNpZmljYXRpb25zIGFjY2VwdGVkLCBhbmQgYSBmb3JtYWwgcGxhY2Ugd2hlcmUg dG8gcHVibGlzaCB0aGVtLgo+ID4gCj4gPiBGaW5lIHdpdGggbWUuIExldHMgc2VlIGlmIG90aGVy IG1haW50YWluZXJzIGFyZSBva2F5IHdpdGggaXQsIHRvby4KCgpfX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fXwpYZW4tZGV2ZWwgbWFpbGluZyBsaXN0Clhlbi1k ZXZlbEBsaXN0cy54ZW4ub3JnCmh0dHBzOi8vbGlzdHMueGVuLm9yZy94ZW4tZGV2ZWwK