Re: [PATCH 0/2] usb: gadget: User space URBs for FunctionFS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Greg,

After writing this response: 
I want to redo some parts of the patch, so please ignore the current
version.
I feel that what I did is not extensible enough.

I still want to know if you think, if it makes sense at all to publish
any of that.


On Wed, 11 Nov 2020 19:40:54 +0100
Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:

> On Wed, Nov 11, 2020 at 06:07:16PM +0100, Ingo Rohloff wrote:
> ...
> > I now implement two new ioctls for FunctionFS:
> >   FUNCTIONFS_SUBMITBULKURB
> >   FUNCTIONFS_REAPBULKURB
> > which provide simliar functionality.
> > 
> > A similar functionality is already implemented via AIO. But: To use
> > this API, your user space environment needs to know how to use
> > these system calls.  

OK I now understand why I was confused myself:
I was looking into the POSIX AIO functions ("man 7 aio").
At least in the C library which I use, the implementation of
POSIX AIO does not use the native Linux system calls.
What I meant above was: This C library does not know about the native
Linux system calls.

Now because of your comment I learned about "libaio", which uses the
Linux native system calls.

The first problem here: Where do you get this library from ?
At the end I got it from here:
  https://pagure.io/libaio 
  version 0.3.111
I am still not sure if this is the right place, because at least
Debian provides a 0.3.112 version, which seems to use an extra
system call (io_pgetevents); no clue if I should use the Debian
version or not.

Of course you could call the system calls directly (without using
libaio), but that's even worse I think.

> 
> So instead you created a new interface which requires different system
> calls?
> 

I created a new interface which uses the old IOCTL system call,
by extending it.
Of course the C library I use knows how to do an IOCTL system call;
which means I do not need another library (like "libaio") and extra
knowledge about other system calls.

Granted: Adding new IOCTL codes is very similar to adding new
system calls, but at least I understand the interface here.
In contrast: It was not exactly easy to get my hands on "libaio".
The fact that there seem to exist different versions of this library,
which employ different system calls also does not make me feel very
comfortable.

> Doing it in a different way is "interesting", but you are duplicating
> existing functionality here.  What is wrong with the AIO interface
> that we currently have that keeps you from using it, other than it
> being "different" than some other user/kernel interfaces that people
> are familiar with?

The first problem was: I did not know about "libaio" :)

But there is more: The existing AIO interface, does not give you the
amount of control over USB transactions which I want.

Example: I want to send 8192 bytes   USB Device -> USB Host

Because this should resemble one USB Transaction,
I want to add a Zero Length Packet at the end.

How would I do that via the AIO interface ?

Another example of this kind of missing control can be found in
comments in "f_fs.c":

   /*
    * Dear user space developer!
    *
    * TL;DR: To stop getting below error message in your kernel ...

so this tells you, you need to be aware of some USB specific quirks,
when using AIO together with FunctionFS.
I much rather want to use an interface which already is specific for
USB instead of using a generic interface (AIO), which then exhibits 
USB specific quirks.
I want to be able to get as close to the USB layer as possible with 
user space; AIO adds another abstraction layer between user space and
USB, which in turn means I loose some of the control over USB.

There are more FunctionFS AIO quirks, which I don't like at all:
Here is some output of a slightly patched 
  <linux kernel>/tools/usb/ffs-aio-example/simple/device_app
test application using libaio in conjunction with FuncionFS:

// cable connected and host test application started:
  submit: out
  ev=in; ret=8192   // ret := struct ioevent.res
  submit: in
  ev=out; ret=8192
  submit: out
// cable disconnected
  Event DISABLE  

// The submitted AIOs are not cancelled ?

// cable connected again
  Event ENABLE   
  ev=in; ret=-108  // -ESHUTDOWN from  struct ioevent.res
  ev=out; ret=-108
  submit: in
  submit: out

As you can see from above: The AIO completes with an "-ESHUTDOWN"
error, once you *reconnect* the cable (not after the disconnect).
I think this is the wrong point of time.


This also is another example, why I don't like using a generic
interface (AIO) for triggering USB specific operations:
The "ret" output above comes from a "struct ioevent.res" member;
I guess this is completely specific for FuncionFS. 
This "res" member seems to convey length information (how much 
was actually transferred) plus status/error information when it 
is negative.
If I get this correctly normal AIO operations are not supposed to
complete prematurely ? 
"prematurely": I mean transferring less than the specified length.
Maybe I just don't understand the libaio interface well enough.

> > ...
> > Via this mechanism a user space program can keep precise track,
> > which URBs succeeded and which URBs failed.  
> 
> So, it implements AIO in a different way?

Yes and No:
It does not exhibit the behavior described above.
When the cable gets disconnected, the URBs will complete with
a status which tells you the cable was disconnected.

I find it much clearer to have *specific* URB struct members called
"status" and "actual_length", than accessing a *generic* "struct
ioevent.res" member, which only seems to have this particular meaning
for FunctionFS file descriptors.

To implement 
  <linux kernel>/tools/usb/ffs-aio-example/simple/device_app
it seems you also need "eventfd" to be able to use "poll";
so this adds yet another layer of complexity.

Using an IOCTL call, you get direct control over USB transactions.
So you are much closer to the USB layer than if you are using AIO.
This means you can easily extend the functionality to support 
USB/FunctionFS specific things:

> > The final goal here is to be able to directly let user space
> > provide data buffers (via mmap I guess), which are then transferred
> > via USB; but this is the next step.  
> 
> Isn't that kind of what the AIO inteface provides today?  :)

I think my explanation was not clear at all:
What I want to have is a "zero copy" transfer.
As far as I know, "devio.c" already has that for the USB host side:
User space is able to "mmap" access to coherent DMA buffers.
When you then pass the URB to "devio.c" no copying is going on at all;
the "mmap" buffers are directly used as data buffers for the USB host
controller; at least that's what I understood from the "devio.c" code.
(In "devio.c" you need to grep "as->usbm" to find the code.)

The FunctionFS AIO does copy user data to/from kernel buffers.
I want to be able to *completely* get rid of these copy operations.

Why: Because I want to be able to provide >400MB/s to a PC,
when using USB SuperSpeed; each copy operation hurts here.

so long
  Ingo



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux