[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 4/7] xen/9pfs: connect to the backend
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 <stefano@xxxxxxxxxxx> > >>>>>>> CC: boris.ostrovsky@xxxxxxxxxx > >>>>>>> CC: jgross@xxxxxxxx > >>>>>>> CC: Eric Van Hensbergen <ericvh@xxxxxxxxx> > >>>>>>> CC: Ron Minnich <rminnich@xxxxxxxxxx> > >>>>>>> CC: Latchesar Ionkov <lucho@xxxxxxxxxx> > >>>>>>> CC: v9fs-developer@xxxxxxxxxxxxxxxxxxxxx > >>>>>>> --- > >>>> > >>>>>> 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@xxxxxxxxxxxxx > https://lists.xen.org/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |