On Mon, Jan 22, 2024 at 06:51:38PM +0800, yuan linyu wrote: > When write UDC to empty and unbind gadget driver from gadget device, it is > possible that there are many queue failures for mass storage function. > > The root cause is mass storage main thread alaways try to queue request to > receive a command from host if running flag is on, on platform like dwc3, > if pull down called, it will not queue request again and return > -ESHUTDOWN, but it not affect running flag of mass storage function. > > Check return code from mass storage function and clear running flag if it > is -ESHUTDOWN, also indicate start in/out transfer failure to break loops. > > Signed-off-by: yuan linyu <yuanlinyu@xxxxxxxxxxx> > --- > v1: follow Alan suggestion, only limit change in f_mass_storage > RFC: https://lore.kernel.org/linux-usb/5f4c9d8b6e0e4e73a5b3b1540a500b6a@xxxxxxxxxxx/T/#t This is basically okay. I have a suggestion for improvement, see below. > drivers/usb/gadget/function/f_mass_storage.c | 28 +++++++++++++++++--- > 1 file changed, 24 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c > index 722a3ab2b337..d5174e066078 100644 > --- a/drivers/usb/gadget/function/f_mass_storage.c > +++ b/drivers/usb/gadget/function/f_mass_storage.c > @@ -545,21 +545,41 @@ static int start_transfer(struct fsg_dev *fsg, struct usb_ep *ep, > > static bool start_in_transfer(struct fsg_common *common, struct fsg_buffhd *bh) > { > + int rc; > + > if (!fsg_is_set(common)) > return false; > bh->state = BUF_STATE_SENDING; > - if (start_transfer(common->fsg, common->fsg->bulk_in, bh->inreq)) > - bh->state = BUF_STATE_EMPTY; > + rc = start_transfer(common->fsg, common->fsg->bulk_in, bh->inreq); > + if (!rc) > + return true; > + > + bh->state = BUF_STATE_EMPTY; > + if (rc == -ESHUTDOWN) { > + common->running = 0; > + return false; > + } > + > return true; > } This could be written more cleanly as: rc = start_transfer(common->fsg, common->fsg->bulk_in, bh->inreq); if (rc) { bh->state = BUF_STATE_EMPTY; if (rc == -ESHUTDOWN) { common->running = 0; return false; } } return true; And the same goes for start_out_transfer(). Have you tested this? Does it do what you want? Alan Stern
