| Message ID | 20221124170430.3998755-1-john@metanate.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b566d38857fcb6777f25b674b90a831eec0817a2 |
| Headers | show |
| Series | usb: gadget: f_fs: use io_data->status consistently | expand |
Reviewed-by: Linyu Yuan <quic_linyyuan@quicinc.com> On 11/25/2022 1:04 AM, John Keeping wrote: > Commit fb1f16d74e26 ("usb: gadget: f_fs: change ep->status safe in > ffs_epfile_io()") added a new ffs_io_data::status field to fix lifetime > issues in synchronous requests. > > While there are no similar lifetime issues for asynchronous requests > (the separate ep member in ffs_io_data avoids them) using the status > field means the USB request can be freed earlier and that there is more > consistency between the synchronous and asynchronous I/O paths. > > Cc: Linyu Yuan <quic_linyyuan@quicinc.com> > Signed-off-by: John Keeping <john@metanate.com> > --- > drivers/usb/gadget/function/f_fs.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c > index 73dc10a77cde..1221f0d1b1a9 100644 > --- a/drivers/usb/gadget/function/f_fs.c > +++ b/drivers/usb/gadget/function/f_fs.c > @@ -825,8 +825,7 @@ static void ffs_user_copy_worker(struct work_struct *work) > { > struct ffs_io_data *io_data = container_of(work, struct ffs_io_data, > work); > - int ret = io_data->req->status ? io_data->req->status : > - io_data->req->actual; > + int ret = io_data->status; > bool kiocb_has_eventfd = io_data->kiocb->ki_flags & IOCB_EVENTFD; > > if (io_data->read && ret > 0) { > @@ -840,8 +839,6 @@ static void ffs_user_copy_worker(struct work_struct *work) > if (io_data->ffs->ffs_eventfd && !kiocb_has_eventfd) > eventfd_signal(io_data->ffs->ffs_eventfd, 1); > > - usb_ep_free_request(io_data->ep, io_data->req); > - > if (io_data->read) > kfree(io_data->to_free); > ffs_free_buffer(io_data); > @@ -856,6 +853,9 @@ static void ffs_epfile_async_io_complete(struct usb_ep *_ep, > > ENTER(); > > + io_data->status = req->status ? req->status : req->actual; > + usb_ep_free_request(_ep, req); > + > INIT_WORK(&io_data->work, ffs_user_copy_worker); > queue_work(ffs->io_completion_wq, &io_data->work); > }
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 73dc10a77cde..1221f0d1b1a9 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -825,8 +825,7 @@ static void ffs_user_copy_worker(struct work_struct *work) { struct ffs_io_data *io_data = container_of(work, struct ffs_io_data, work); - int ret = io_data->req->status ? io_data->req->status : - io_data->req->actual; + int ret = io_data->status; bool kiocb_has_eventfd = io_data->kiocb->ki_flags & IOCB_EVENTFD; if (io_data->read && ret > 0) { @@ -840,8 +839,6 @@ static void ffs_user_copy_worker(struct work_struct *work) if (io_data->ffs->ffs_eventfd && !kiocb_has_eventfd) eventfd_signal(io_data->ffs->ffs_eventfd, 1); - usb_ep_free_request(io_data->ep, io_data->req); - if (io_data->read) kfree(io_data->to_free); ffs_free_buffer(io_data); @@ -856,6 +853,9 @@ static void ffs_epfile_async_io_complete(struct usb_ep *_ep, ENTER(); + io_data->status = req->status ? req->status : req->actual; + usb_ep_free_request(_ep, req); + INIT_WORK(&io_data->work, ffs_user_copy_worker); queue_work(ffs->io_completion_wq, &io_data->work); }
Commit fb1f16d74e26 ("usb: gadget: f_fs: change ep->status safe in ffs_epfile_io()") added a new ffs_io_data::status field to fix lifetime issues in synchronous requests. While there are no similar lifetime issues for asynchronous requests (the separate ep member in ffs_io_data avoids them) using the status field means the USB request can be freed earlier and that there is more consistency between the synchronous and asynchronous I/O paths. Cc: Linyu Yuan <quic_linyyuan@quicinc.com> Signed-off-by: John Keeping <john@metanate.com> --- drivers/usb/gadget/function/f_fs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)