From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751140AbdCQPPv (ORCPT ); Fri, 17 Mar 2017 11:15:51 -0400 Received: from aserp1050.oracle.com ([141.146.126.70]:32419 "EHLO aserp1050.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751100AbdCQPPr (ORCPT ); Fri, 17 Mar 2017 11:15:47 -0400 Date: Fri, 17 Mar 2017 11:13:57 -0400 From: Konrad Rzeszutek Wilk To: Juergen Gross Cc: 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 Message-ID: <20170317151357.GS7915@char.us.oracle.com> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0f602607-df9c-85e1-e7df-eca76b306679@suse.com> User-Agent: Mutt/1.7.1 (2016-10-04) X-Source-IP: aserp1040.oracle.com [141.146.126.69] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. > > > > 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. > > > Juergen > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH v3 4/7] xen/9pfs: connect to the backend Date: Fri, 17 Mar 2017 11:13:57 -0400 Message-ID: <20170317151357.GS7915@char.us.oracle.com> 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> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cota8-0006Bz-RZ for xen-devel@lists.xenproject.org; Fri, 17 Mar 2017 15:14:20 +0000 Content-Disposition: inline In-Reply-To: <0f602607-df9c-85e1-e7df-eca76b306679@suse.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Juergen Gross Cc: 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 T24gRnJpLCBNYXIgMTcsIDIwMTcgYXQgMDU6NTQ6NDdBTSArMDEwMCwgSnVlcmdlbiBHcm9zcyB3 cm90ZToKPiBPbiAxNi8wMy8xNyAxOTowMywgU3RlZmFubyBTdGFiZWxsaW5pIHdyb3RlOgo+ID4g T24gVGh1LCAxNiBNYXIgMjAxNywgSnVlcmdlbiBHcm9zcyB3cm90ZToKPiA+PiBPbiAxNS8wMy8x NyAxOTo0NCwgU3RlZmFubyBTdGFiZWxsaW5pIHdyb3RlOgo+ID4+PiBPbiBXZWQsIDE1IE1hciAy MDE3LCBKdWVyZ2VuIEdyb3NzIHdyb3RlOgo+ID4+Pj4gT24gMTQvMDMvMTcgMjI6MjIsIFN0ZWZh bm8gU3RhYmVsbGluaSB3cm90ZToKPiA+Pj4+PiBIaSBKdWVyZ2VuLAo+ID4+Pj4+Cj4gPj4+Pj4g dGhhbmsgeW91IGZvciB0aGUgcmV2aWV3IQo+ID4+Pj4+Cj4gPj4+Pj4gT24gVHVlLCAxNCBNYXIg MjAxNywgSnVlcmdlbiBHcm9zcyB3cm90ZToKPiA+Pj4+Pj4gT24gMTQvMDMvMTcgMDA6NTAsIFN0 ZWZhbm8gU3RhYmVsbGluaSB3cm90ZToKPiA+Pj4+Pj4+IEltcGxlbWVudCBmdW5jdGlvbnMgdG8g aGFuZGxlIHRoZSB4ZW5idXMgaGFuZHNoYWtlLiBVcG9uIGNvbm5lY3Rpb24sCj4gPj4+Pj4+PiBh bGxvY2F0ZSB0aGUgcmluZ3MgYWNjb3JkaW5nIHRvIHRoZSBwcm90b2NvbCBzcGVjaWZpY2F0aW9u Lgo+ID4+Pj4+Pj4KPiA+Pj4+Pj4+IEluaXRpYWxpemUgYSB3b3JrX3N0cnVjdCBhbmQgYSB3YWl0 X3F1ZXVlLiBUaGUgd29ya19zdHJ1Y3Qgd2lsbCBiZSB1c2VkCj4gPj4+Pj4+PiB0byBzY2hlZHVs ZSB3b3JrIHVwb24gcmVjZWl2aW5nIGFuIGV2ZW50IGNoYW5uZWwgbm90aWZpY2F0aW9uIGZyb20g dGhlCj4gPj4+Pj4+PiBiYWNrZW5kLiBUaGUgd2FpdF9xdWV1ZSB3aWxsIGJlIHVzZWQgdG8gd2Fp dCB3aGVuIHRoZSByaW5nIGlzIGZ1bGwgYW5kCj4gPj4+Pj4+PiB3ZSBuZWVkIHRvIHNlbmQgYSBu ZXcgcmVxdWVzdC4KPiA+Pj4+Pj4+Cj4gPj4+Pj4+PiBTaWduZWQtb2ZmLWJ5OiBTdGVmYW5vIFN0 YWJlbGxpbmkgPHN0ZWZhbm9AYXBvcmV0by5jb20+Cj4gPj4+Pj4+PiBDQzogYm9yaXMub3N0cm92 c2t5QG9yYWNsZS5jb20KPiA+Pj4+Pj4+IENDOiBqZ3Jvc3NAc3VzZS5jb20KPiA+Pj4+Pj4+IEND OiBFcmljIFZhbiBIZW5zYmVyZ2VuIDxlcmljdmhAZ21haWwuY29tPgo+ID4+Pj4+Pj4gQ0M6IFJv biBNaW5uaWNoIDxybWlubmljaEBzYW5kaWEuZ292Pgo+ID4+Pj4+Pj4gQ0M6IExhdGNoZXNhciBJ b25rb3YgPGx1Y2hvQGlvbmtvdi5uZXQ+Cj4gPj4+Pj4+PiBDQzogdjlmcy1kZXZlbG9wZXJAbGlz dHMuc291cmNlZm9yZ2UubmV0Cj4gPj4+Pj4+PiAtLS0KPiA+Pj4+Cj4gPj4+Pj4+IERpZCB5b3Ug dGhpbmsgYWJvdXQgdXNpbmcgcmVxdWVzdF90aHJlYWRlZF9pcnEoKSBpbnN0ZWFkIG9mIGEgd29y a3F1ZXVlPwo+ID4+Pj4+PiBGb3IgYW4gZXhhbXBsZSBzZWUgZS5nLiBkcml2ZXJzL3Njc2kveGVu LXNjc2lmcm9udC5jCj4gPj4+Pj4KPiA+Pj4+PiBJIGxpa2Ugd29ya3F1ZXVlcyA6LSkgIEl0IG1p Z2h0IGNvbWUgZG93biB0byBwZXJzb25hbCBwcmVmZXJlbmNlcywgYnV0IEkKPiA+Pj4+PiB0aGlu ayB3b3JrcXVldWVzIGFyZSBtb3JlIGZsZXhpYmxlIGFuZCBhIGJldHRlciBmaXQgZm9yIHRoaXMg dXNlIGNhc2UuCj4gPj4+Pj4gTm90IG9ubHkgaXQgaXMgZWFzeSB0byBzY2hlZHVsZSB3b3JrIGlu IGEgd29ya3F1ZXVlIGZyb20gdGhlIGludGVycnVwdAo+ID4+Pj4+IGhhbmRsZXIsIGJ1dCBhbHNv IHRoZXkgY2FuIGJlIHVzZWQgZm9yIHNsZWVwaW5nIGluIHRoZSByZXF1ZXN0IGZ1bmN0aW9uCj4g Pj4+Pj4gaWYgdGhlcmUgaXMgbm90IGVub3VnaCByb29tIG9uIHRoZSByaW5nLiBCZXNpZGVzLCB0 aGV5IGNhbiBlYXNpbHkgYmUKPiA+Pj4+PiBjb25maWd1cmVkIHRvIHNoYXJlIGEgc2luZ2xlIHRo cmVhZCBvciB0byBoYXZlIG11bHRpcGxlIGluZGVwZW5kZW50Cj4gPj4+Pj4gdGhyZWFkcy4KPiA+ Pj4+Cj4gPj4+PiBJJ20gZmluZSB3aXRoIHRoZSB3b3JrcXVldWVzIGFzIGxvbmcgYXMgeW91IGhh dmUgZGVjaWRlZCB0byB1c2UgdGhlbQo+ID4+Pj4gY29uc2lkZXJpbmcgdGhlIGFsdGVybmF0aXZl cy4gOi0pCj4gPj4+Pgo+ID4+Pj4+PiBDYW4ndCB5b3UgdXNlIHhlbmJ1c19yZWFkX3Vuc2lnbmVk KCkgaW5zdGVhZCBvZiB4ZW5idXNfcmVhZCgpPwo+ID4+Pj4+Cj4gPj4+Pj4gSSBjYW4gdXNlIHhl bmJ1c19yZWFkX3Vuc2lnbmVkIGluIHRoZSBvdGhlciBjYXNlcyBiZWxvdywgYnV0IG5vdCBoZXJl LAo+ID4+Pj4+IGJlY2F1c2UgdmVyc2lvbnMgaXMgaW4gdGhlIGZvcm06ICIxLDMsNCIKPiA+Pj4+ Cj4gPj4+PiBJcyB0aGlzIGRvY3VtZW50ZWQgc29tZXdoZXJlPwo+ID4+Pj4KPiA+Pj4+IEhtbSwg YXJlIGFueSBvZiB0aGUgWGVuc3RvcmUgZW50cmllcyBkb2N1bWVudGVkPyBTaG91bGRuJ3QgdGhp cyBiZSBkb25lCj4gPj4+PiBpbiB4ZW5fOXBmcy5oID8KPiA+Pj4gIAo+ID4+PiBUaGV5IGFyZSBk b2N1bWVudGVkIGluIGRvY3MvbWlzYy85cGZzLm1hcmtkb3duLCB1bmRlciAiWGVuc3RvcmUiLiBH aXZlbgo+ID4+PiB0aGF0IGl0J3MgYWxsIHdyaXR0ZW4gdGhlcmUsIGVzcGVjaWFsbHkgdGhlIHNl bWFudGljcywgSSBkaWRuJ3QgcmVwZWF0Cj4gPj4+IGl0IGluIHhlbl85cGZzLmgKPiA+Pgo+ID4+ IExvb2tpbmcgYXQgaXQgZnJvbSB0aGUgTGludXgga2VybmVsIHBlcnNwZWN0aXZlIHRoaXMgZG9j dW1lbnRhdGlvbiBpcwo+ID4+IG5vdCByZWFsbHkgaGlnaGx5IHZpc2libGUuIEZvciBtZSBpdCBp cyBva2F5LCBidXQgdGhlcmUgaGF2ZSBiZWVuCj4gPj4gbXVsdGlwbGUgZXhhbXBsZXMgaW4gdGhl IHBhc3Qgd2hlcmUgZG9jdW1lbnRhdGlvbiBpbiB0aGUgWGVuIHJlcG9zaXRvcnkKPiA+PiB3YXNu J3QgcmVnYXJkZWQgYXMgYmVpbmcgc3VmZmljaWVudC4KPiA+Pgo+ID4+IEkgcmVjb21tZW5kIG1v dmluZyB0aGUgZG9jdW1lbnRhdGlvbiByZWdhcmRpbmcgdGhlIGludGVyZmFjZSBpbnRvIHRoZQo+ ID4+IGhlYWRlciBmaWxlIGxpa2UgZm9yIHRoZSBvdGhlciBwdiBpbnRlcmZhY2VzLgo+ID4gCj4g PiBXaGF0IGFib3V0IGFkZGluZyBhIGxpbmsgc3VjaCBhczoKPiA+IAo+ID4gaHR0cDovL3hlbmJp dHMueGVuLm9yZy9naXR3ZWIvP3A9eGVuLmdpdDthPWJsb2JfcGxhaW47Zj1kb2NzL21pc2MvOXBm cy5tYXJrZG93bjtoYj1IRUFECgpFd3d3LgoKCkhvdyBhYm91dCBodHRwczovL3hlbmJpdHMueGVu Lm9yZy9kb2NzL3Vuc3RhYmxlL21pc2MvOXBmcy5odG1sCgp3aGljaCBnZXRzIHVwZGF0ZWQgZGFp bHkuCgo+ID4gCj4gPiB0aGF0IHNob3VsZCBiZSBlYXNpbHkgYWNjZXNzaWJsZSwgcmlnaHQ/IEZv ciBvdGhlciBzcGVjaWZpY2F0aW9ucywgc3VjaAo+ID4gYXMgOXAsIG9ubHkgbGlua3MgYXJlIHBy b3ZpZGVkIChzZWUgRG9jdW1lbnRhdGlvbi9maWxlc3lzdGVtcy85cC50eHQpLgo+ID4gSSBhbSBz dWdnZXN0aW5nIGEgbGluaywgYmVjYXVzZSB0aGVuIHdlIGFyZSBzdXJlIHRoZSBzcGVjcyBkb24n dCBnbyBvdXQKPiA+IG9mIHN5bmMuIEkgcmVhbGl6ZSB0aGF0IG9sZGVyIFBWIHByb3RvY29scyB3 ZXJlIGRlc2NyaWJlZCBpbiBoZWFkZXIKPiA+IGZpbGVzLCBidXQgdGhhdCB3YXMgYmVmb3JlIFhl biBQcm9qZWN0IGhhZCBhIGZvcm1hbCBwcm9jZXNzIGZvciBnZXR0aW5nCj4gPiBuZXcgc3BlY2lm aWNhdGlvbnMgYWNjZXB0ZWQsIGFuZCBhIGZvcm1hbCBwbGFjZSB3aGVyZSB0byBwdWJsaXNoIHRo ZW0uCj4gCj4gRmluZSB3aXRoIG1lLiBMZXRzIHNlZSBpZiBvdGhlciBtYWludGFpbmVycyBhcmUg b2theSB3aXRoIGl0LCB0b28uCj4gCj4gCj4gSnVlcmdlbgo+IAo+IAo+IF9fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCj4gWGVuLWRldmVsIG1haWxpbmcgbGlz dAo+IFhlbi1kZXZlbEBsaXN0cy54ZW4ub3JnCj4gaHR0cHM6Ly9saXN0cy54ZW4ub3JnL3hlbi1k ZXZlbAoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVu LWRldmVsIG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3Rz Lnhlbi5vcmcveGVuLWRldmVsCg==