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

[Stefano Stabellini]

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