[RFC PATCH] usb: gadget: dfu: Wrap fs operation in workqueue
Jules Maselbas
jmaselbas at kalray.eu
Wed Jan 27 11:49:37 EST 2021
File system operation shouldn't be executed in a poller. Use
a workqueue to delay filesystem operation to command context.
This is an RFC, extra work must be done to properly handle error
cases and dfu cleanup.
Signed-off-by: Jules Maselbas <jmaselbas at kalray.eu>
---
drivers/usb/gadget/dfu.c | 321 ++++++++++++++++++++++++++-------------
1 file changed, 216 insertions(+), 105 deletions(-)
diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c
index 9d6a9d252..75abd1576 100644
--- a/drivers/usb/gadget/dfu.c
+++ b/drivers/usb/gadget/dfu.c
@@ -54,6 +54,7 @@
#include <fs.h>
#include <ioctl.h>
#include <linux/mtd/mtd-abi.h>
+#include <work.h>
#define USB_DT_DFU 0x21
@@ -153,6 +154,7 @@ struct f_dfu {
u8 dfu_state;
u8 dfu_status;
struct usb_request *dnreq;
+ struct work_queue wq;
};
static inline struct f_dfu *func_to_dfu(struct usb_function *f)
@@ -173,6 +175,178 @@ static struct usb_gadget_strings *dfu_strings[] = {
};
static void dn_complete(struct usb_ep *ep, struct usb_request *req);
+static void up_complete(struct usb_ep *ep, struct usb_request *req);
+static void dfu_cleanup(struct f_dfu *dfu);
+
+struct dfu_work {
+ struct work_struct work;
+ struct f_dfu *dfu;
+ void (*task)(struct dfu_work *dw);
+ size_t len;
+ uint8_t *rbuf;
+ uint8_t wbuf[CONFIG_USBD_DFU_XFER_SIZE];
+};
+
+static void dfu_do_work(struct work_struct *w)
+{
+ struct dfu_work *dw = container_of(w, struct dfu_work, work);
+
+ /* TODO: find a better way to skip tasks when the dfu gadget
+ * has encounter an error and dfu_cleanup has been called */
+ if (dw->task && dw->dfu->dfu_status == DFU_STATUS_OK)
+ dw->task(dw);
+
+ free(dw);
+}
+
+static void dfu_work_cancel(struct work_struct *w)
+{
+ struct dfu_work *dw = container_of(w, struct dfu_work, work);
+
+ free(dw);
+}
+
+static void dfu_do_write(struct dfu_work *dw)
+{
+ struct f_dfu *dfu = dw->dfu;
+ size_t size, wlen = dw->len;
+ int ret;
+
+ debug("do write\n");
+
+ if (prog_erase && (dfu_written + wlen) > dfu_erased) {
+ size = roundup(wlen, dfu_mtdinfo.erasesize);
+ ret = erase(dfufd, size, dfu_erased);
+ dfu_erased += size;
+ if (ret && ret != -ENOSYS) {
+ perror("erase");
+ dfu->dfu_status = DFU_STATUS_errERASE;
+ dfu_cleanup(dfu);
+ return;
+ }
+ }
+
+ dfu_written += wlen;
+ ret = write(dfufd, dw->wbuf, wlen);
+ if (ret < (int)wlen) {
+ perror("write");
+ dfu->dfu_status = DFU_STATUS_errWRITE;
+ dfu_cleanup(dfu);
+ }
+}
+
+static void dfu_do_read(struct dfu_work *dw)
+{
+ struct f_dfu *dfu = dw->dfu;
+ struct usb_composite_dev *cdev = dfu->func.config->cdev;
+ size_t size, rlen = dw->len;
+
+ debug("do read\n");
+
+ size = read(dfufd, dfu->dnreq->buf, rlen);
+ dfu->dnreq->length = size;
+ if (size < (int)rlen) {
+ perror("read");
+ dfu_cleanup(dfu);
+ dfu->dfu_state = DFU_STATE_dfuIDLE;
+ }
+
+ dfu->dnreq->complete = up_complete;
+ usb_ep_queue(cdev->gadget->ep0, dfu->dnreq);
+}
+
+static void dfu_do_open_dnload(struct dfu_work *dw)
+{
+ struct f_dfu *dfu = dw->dfu;
+ int ret;
+
+ debug("do open dnload\n");
+
+ if (dfu_file_entry->flags & FILE_LIST_FLAG_SAFE) {
+ dfufd = open(DFU_TEMPFILE, O_WRONLY | O_CREAT);
+ } else {
+ unsigned flags = O_WRONLY;
+
+ if (dfu_file_entry->flags & FILE_LIST_FLAG_CREATE)
+ flags |= O_CREAT | O_TRUNC;
+
+ dfufd = open(dfu_file_entry->filename, flags);
+ }
+
+ if (dfufd < 0) {
+ perror("open");
+ dfu->dfu_status = DFU_STATUS_errFILE;
+ goto out;
+ }
+
+ if (!(dfu_file_entry->flags & FILE_LIST_FLAG_SAFE)) {
+ ret = ioctl(dfufd, MEMGETINFO, &dfu_mtdinfo);
+ if (!ret) /* file is on a mtd device */
+ prog_erase = 1;
+ }
+
+ return;
+out:
+ dfu->dfu_state = DFU_STATE_dfuERROR;
+ dfu_cleanup(dfu);
+}
+
+static void dfu_do_open_upload(struct dfu_work *dw)
+{
+ struct f_dfu *dfu = dw->dfu;
+
+ debug("do open upload\n");
+
+ dfufd = open(dfu_file_entry->filename, O_RDONLY);
+ if (dfufd < 0) {
+ perror("open");
+ dfu->dfu_status = DFU_STATUS_errFILE;
+ dfu->dfu_state = DFU_STATE_dfuERROR;
+ dfu_cleanup(dfu);
+ }
+}
+
+static void dfu_do_copy(struct dfu_work *dw)
+{
+ struct f_dfu *dfu = dw->dfu;
+ unsigned flags = O_WRONLY;
+ int ret, fd;
+
+ debug("do copy\n");
+
+ if (dfu_file_entry->flags & FILE_LIST_FLAG_CREATE)
+ flags |= O_CREAT | O_TRUNC;
+
+ fd = open(dfu_file_entry->filename, flags);
+ if (fd < 0) {
+ perror("open");
+ dfu->dfu_status = DFU_STATUS_errERASE;
+ goto out;
+ }
+
+ ret = erase(fd, ERASE_SIZE_ALL, 0);
+ close(fd);
+ if (ret && ret != -ENOSYS) {
+ perror("erase");
+ dfu->dfu_status = DFU_STATUS_errERASE;
+ goto out;
+ }
+
+ ret = copy_file(DFU_TEMPFILE, dfu_file_entry->filename, 0);
+ if (ret) {
+ dfu->dfu_status = DFU_STATUS_errWRITE;
+ printf("copy file failed\n");
+ goto out;
+ }
+
+ dfu->dfu_state = DFU_STATE_dfuIDLE;
+ dfu_cleanup(dfu);
+
+ return;
+out:
+ dfu->dfu_state = DFU_STATE_dfuERROR;
+ dfu_cleanup(dfu);
+}
static int
dfu_bind(struct usb_configuration *c, struct usb_function *f)
@@ -223,6 +397,10 @@ dfu_bind(struct usb_configuration *c, struct usb_function *f)
goto out;
}
+ dfu->wq.fn = dfu_do_work;
+ dfu->wq.cancel = dfu_work_cancel;
+ wq_register(&dfu->wq);
+
/* allocate instance-specific interface IDs, and patch descriptors */
status = usb_interface_id(c, f);
if (status < 0)
@@ -278,6 +456,8 @@ dfu_unbind(struct usb_configuration *c, struct usb_function *f)
dfu_file_entry = NULL;
dfudetach = 0;
+ wq_unregister(&dfu->wq);
+
usb_free_all_descriptors(f);
dma_free(dfu->dnreq->buf);
@@ -327,6 +507,9 @@ static void dfu_cleanup(struct f_dfu *dfu)
dfu_erased = 0;
prog_erase = 0;
+ /* TODO: Right now, close and stat operation can be called
+ * in a poller, in dfu_abort and dfu_disable. */
+
if (dfufd > 0) {
close(dfufd);
dfufd = -EINVAL;
@@ -339,28 +522,15 @@ static void dfu_cleanup(struct f_dfu *dfu)
static void dn_complete(struct usb_ep *ep, struct usb_request *req)
{
struct f_dfu *dfu = req->context;
- loff_t size;
- int ret;
+ struct dfu_work *dw;
- if (prog_erase && (dfu_written + req->length) > dfu_erased) {
- size = roundup(req->length, dfu_mtdinfo.erasesize);
- ret = erase(dfufd, size, dfu_erased);
- dfu_erased += size;
- if (ret && ret != -ENOSYS) {
- perror("erase");
- dfu->dfu_status = DFU_STATUS_errERASE;
- dfu_cleanup(dfu);
- return;
- }
- }
+ dw = xzalloc(sizeof(*dw));
+ dw->dfu = dfu;
+ dw->task = dfu_do_write;
+ dw->len = min_t(unsigned int, req->length, CONFIG_USBD_DFU_XFER_SIZE);
+ memcpy(dw->wbuf, req->buf, dw->len);
- dfu_written += req->length;
- ret = write(dfufd, req->buf, req->length);
- if (ret < (int)req->length) {
- perror("write");
- dfu->dfu_status = DFU_STATUS_errWRITE;
- dfu_cleanup(dfu);
- }
+ wq_queue_work(&dfu->wq, &dw->work);
}
static int handle_dnload(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
@@ -370,12 +540,7 @@ static int handle_dnload(struct usb_function *f, const struct usb_ctrlrequest *c
u16 w_length = le16_to_cpu(ctrl->wLength);
if (w_length == 0) {
- if (dfu_file_entry->flags & FILE_LIST_FLAG_SAFE) {
- dfu->dfu_state = DFU_STATE_dfuMANIFEST;
- } else {
- dfu->dfu_state = DFU_STATE_dfuIDLE;
- dfu_cleanup(dfu);
- }
+ dfu->dfu_state = DFU_STATE_dfuMANIFEST_SYNC;
return 0;
}
@@ -389,48 +554,18 @@ static int handle_dnload(struct usb_function *f, const struct usb_ctrlrequest *c
static int handle_manifest(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
{
struct f_dfu *dfu = func_to_dfu(f);
- int ret;
+ struct dfu_work *dw;
dfu->dfu_state = DFU_STATE_dfuIDLE;
if (dfu_file_entry->flags & FILE_LIST_FLAG_SAFE) {
- int fd;
- unsigned flags = O_WRONLY;
-
- if (dfu_file_entry->flags & FILE_LIST_FLAG_CREATE)
- flags |= O_CREAT | O_TRUNC;
-
- fd = open(dfu_file_entry->filename, flags);
- if (fd < 0) {
- perror("open");
- dfu->dfu_status = DFU_STATUS_errERASE;
- ret = -EINVAL;
- goto out;
- }
-
- ret = erase(fd, ERASE_SIZE_ALL, 0);
- close(fd);
- if (ret && ret != -ENOSYS) {
- dfu->dfu_status = DFU_STATUS_errERASE;
- perror("erase");
- goto out;
- }
-
- ret = copy_file(DFU_TEMPFILE, dfu_file_entry->filename, 0);
- if (ret) {
- printf("copy file failed\n");
- ret = -EINVAL;
- goto out;
- }
+ dw = xzalloc(sizeof(*dw));
+ dw->dfu = dfu;
+ dw->task = dfu_do_copy;
+ wq_queue_work(&dfu->wq, &dw->work);
}
return 0;
-
-out:
- dfu->dfu_status = DFU_STATUS_errWRITE;
- dfu->dfu_state = DFU_STATE_dfuERROR;
- dfu_cleanup(dfu);
- return ret;
}
static void up_complete(struct usb_ep *ep, struct usb_request *req)
@@ -440,20 +575,17 @@ static void up_complete(struct usb_ep *ep, struct usb_request *req)
static int handle_upload(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
{
struct f_dfu *dfu = func_to_dfu(f);
- struct usb_composite_dev *cdev = f->config->cdev;
+ struct dfu_work *dw;
u16 w_length = le16_to_cpu(ctrl->wLength);
- int len;
-
- len = read(dfufd, dfu->dnreq->buf, w_length);
-
- dfu->dnreq->length = len;
- if (len < w_length) {
- dfu_cleanup(dfu);
- dfu->dfu_state = DFU_STATE_dfuIDLE;
- }
- dfu->dnreq->complete = up_complete;
- usb_ep_queue(cdev->gadget->ep0, dfu->dnreq);
+ /* RFC: I didn't found a better way to queue the usb response other
+ * than making dfu_do_read call usb_ep_queue after reading from file */
+ dw = xzalloc(sizeof(*dw));
+ dw->dfu = dfu;
+ dw->task = dfu_do_read;
+ dw->len = w_length;
+ dw->rbuf = dfu->dnreq->buf;
+ wq_queue_work(&dfu->wq, &dw->work);
return 0;
}
@@ -474,7 +606,7 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
int value = -EOPNOTSUPP;
int w_length = le16_to_cpu(ctrl->wLength);
int w_value = le16_to_cpu(ctrl->wValue);
- int ret;
+ struct dfu_work *dw;
if (ctrl->bRequestType == USB_DIR_IN && ctrl->bRequest == USB_REQ_GET_DESCRIPTOR
&& (w_value >> 8) == 0x21) {
@@ -501,28 +633,10 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
goto out;
}
debug("dfu: starting download to %s\n", dfu_file_entry->filename);
- if (dfu_file_entry->flags & FILE_LIST_FLAG_SAFE) {
- dfufd = open(DFU_TEMPFILE, O_WRONLY | O_CREAT);
- } else {
- unsigned flags = O_WRONLY;
-
- if (dfu_file_entry->flags & FILE_LIST_FLAG_CREATE)
- flags |= O_CREAT | O_TRUNC;
-
- dfufd = open(dfu_file_entry->filename, flags);
- }
-
- if (dfufd < 0) {
- dfu->dfu_state = DFU_STATE_dfuERROR;
- perror("open");
- goto out;
- }
-
- if (!(dfu_file_entry->flags & FILE_LIST_FLAG_SAFE)) {
- ret = ioctl(dfufd, MEMGETINFO, &dfu_mtdinfo);
- if (!ret) /* file is on a mtd device */
- prog_erase = 1;
- }
+ dw = xzalloc(sizeof(*dw));
+ dw->dfu = dfu;
+ dw->task = dfu_do_open_dnload;
+ wq_queue_work(&dfu->wq, &dw->work);
value = handle_dnload(f, ctrl);
dfu->dfu_state = DFU_STATE_dfuDNLOAD_IDLE;
@@ -534,12 +648,12 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
dfu->dfu_state = DFU_STATE_dfuERROR;
goto out;
}
- dfufd = open(dfu_file_entry->filename, O_RDONLY);
- if (dfufd < 0) {
- dfu->dfu_state = DFU_STATE_dfuERROR;
- perror("open");
- goto out;
- }
+
+ dw = xzalloc(sizeof(*dw));
+ dw->dfu = dfu;
+ dw->task = dfu_do_open_upload;
+ wq_queue_work(&dfu->wq, &dw->work);
+
handle_upload(f, ctrl);
return 0;
case USB_REQ_DFU_ABORT:
@@ -648,9 +762,6 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
break;
case DFU_STATE_dfuMANIFEST:
value = handle_manifest(f, ctrl);
- if (dfu->dfu_state != DFU_STATE_dfuIDLE) {
- return 0;
- }
switch (ctrl->bRequest) {
case USB_REQ_DFU_GETSTATUS:
value = dfu_status(f, ctrl);
--
2.17.1
More information about the barebox
mailing list