Hi, Michal Nazarewicz <mina86@xxxxxxxxxx> writes: > On Tue, May 17 2016, Changbin Du wrote: >>> There appears to be no kfifo support for iov_iter though, so I just went >>> with a simple buffer. >>> >>> I haven’t looked at the patch too carefully so this is an RFC rather >>> than an actual patch at this point. It does compile at least. >>> >>> Regardless, the more I thin about it, the more I’m under the impression >>> that the whole rounding up in f_fs was a mistake. And the more I’m >>> leaning towards ignoring the excess data set by the host. >>> >>> ---------- >8 ---------------------------------------------------------- >>> Subject: usb: gadget: f_fs: buffer data from ‘oversized’ OUT requests >>> >>> f_fs rounds up read(2) requests to a multiple of a max packet size >>> which means that host may provide more data than user has space for. >>> So far, the excess data has been silently ignored. >>> >>> This introduces a buffer for a tail of such requests so that they are >>> returned on next read instead of being ignored. >>> >> >> Congratulations finally reach an agreement, > > To be honest, if it was up to me, I would rip request size rounding up > out of the code. we've been through this before. This needs to be done at the gadget layer. Gadget driver can over-allocate ahead of time if gadget->quirk_ep_out_aligned_size is true, then we avoid memcpy() at the UDC driver level. >> thanks Alan Stern and Michal. >> Here just have a comment - the buffered data need be dropped when the >> epfile is closed, because it means the session is terminated. > > I blame that on sleep deprivation. Another issue is what to do when > endpoint is disabled. Should the buffer be cleared as soon as the > endpoint is disabled? Or maybe when the endpoint is enabled again? Or > maybe it should never be cleared? > > If the buffer is cleared when endpoint is disabled, we again silently > drop data. On the other hand, if we don’t do that, read() on the > endpoint will may succeed even if the configuration is disabled which > may be surprising for users. tough decision... but seems like clearing the buffer as soon as ep is disabled is the way to go. -- balbi
Attachment:
signature.asc
Description: PGP signature
