Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1603623

Re: [Xen-devel] [PATCH v3 4/7] xen/9pfs: connect to the backend

Show all headers | View raw

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 <stefano@aporeto.com>
> > >>>>>>> CC: boris.ostrovsky@oracle.com
> > >>>>>>> CC: jgross@suse.com
> > >>>>>>> CC: Eric Van Hensbergen <ericvh@gmail.com>
> > >>>>>>> CC: Ron Minnich <rminnich@sandia.gov>
> > >>>>>>> CC: Latchesar Ionkov <lucho@ionkov.net>
> > >>>>>>> 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.

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: [PATCH v3 4/7] xen/9pfs: connect to the backend Juergen Gross <jgross@suse.com> - 2017-03-14 07:50 +0100
  Re: [PATCH v3 4/7] xen/9pfs: connect to the backend Stefano Stabellini <sstabellini@kernel.org> - 2017-03-14 22:30 +0100
    Re: [PATCH v3 4/7] xen/9pfs: connect to the backend Juergen Gross <jgross@suse.com> - 2017-03-15 06:10 +0100
      Re: [PATCH v3 4/7] xen/9pfs: connect to the backend Stefano Stabellini <sstabellini@kernel.org> - 2017-03-15 19:50 +0100
        Re: [PATCH v3 4/7] xen/9pfs: connect to the backend Juergen Gross <jgross@suse.com> - 2017-03-16 07:00 +0100
          Re: [PATCH v3 4/7] xen/9pfs: connect to the backend Stefano Stabellini <sstabellini@kernel.org> - 2017-03-16 19:10 +0100
            Re: [PATCH v3 4/7] xen/9pfs: connect to the backend Juergen Gross <jgross@suse.com> - 2017-03-17 06:00 +0100
              Re: [Xen-devel] [PATCH v3 4/7] xen/9pfs: connect to the backend Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> - 2017-03-17 16:20 +0100
                Re: [Xen-devel] [PATCH v3 4/7] xen/9pfs: connect to the backend Stefano Stabellini <sstabellini@kernel.org> - 2017-03-17 22:10 +0100

csiph-web