USB: f_fs: Use Functionfs with kernel AIO

USB: f_fs: Use Functionfs with kernel AIO

[Shang Shi]

Hi,

This is Shang Shi from Google. As we are examining the Andorid adb
daemon implementation in AOSP, we found that there's a concern on
functionfs that an io_submit syscall on IN/OUT endpoint after endpoint
becomes disabled could end up blocking [1]. AOSP kills a thread to
avoid it blocking on io_submit, which is not an elegant way to exist.

Is it true that io_submit syscall could end up blocking after endpoint
is disabled? Would opening  endpoint file with O_NONBLOCK resolve the
issue?

From the ffs-aio-example "aio_simple.c", it seems that as long as we
read and process control events before any read/write (with
io_submit), we are not worried about any race condition between
endpoint becoming disabled and io_submit. Is this true?

Really appreciate it for taking time helping us on these questions!

Thanks!
Shang Shi

Reference [1]:
https://cs.android.com/android/platform/superproject/+/master:packages/modules/adb/daemon/usb.cpp;drc=bbfaaac52f1321d3b9b67e89ab23303510cd2593;l=247

Re: USB: f_fs: Use Functionfs with kernel AIO

[John Keeping]

On Wed, Dec 01, 2021 at 09:04:27AM -0800, Shang Shi wrote:
> This is Shang Shi from Google. As we are examining the Andorid adb
> daemon implementation in AOSP, we found that there's a concern on
> functionfs that an io_submit syscall on IN/OUT endpoint after endpoint
> becomes disabled could end up blocking [1]. AOSP kills a thread to
> avoid it blocking on io_submit, which is not an elegant way to exist.
> 
> Is it true that io_submit syscall could end up blocking after endpoint
> is disabled? Would opening  endpoint file with O_NONBLOCK resolve the
> issue?

Looking at ffs_epfile_io() it seems that it can block if the endpoint is
disabled, and indeed there may be a race there between the disabled
notification and the next submit arriving.

There was a recent proposed patch [1] which looks like it inadvertently
addresses this race, but there are other issues with that patch so it's
not a fix for this.

From the code, indeed O_NONBLOCK does avoid any risk of blocking, but it
means you can't queue up I/O requests before the gadget is activated,
although maybe that's not a problem in practice.

[1] https://lore.kernel.org/linux-usb/20211201100205.25448-1-quic_wcheng@quicinc.com/

> From the ffs-aio-example "aio_simple.c", it seems that as long as we
> read and process control events before any read/write (with
> io_submit), we are not worried about any race condition between
> endpoint becoming disabled and io_submit. Is this true?

I don't think this holds, there will always be a windows where the
UDC can change state between reading the ep0 events and attempting I/O
on the other endpoint files.


Regards,
John