Re: [PATCH 34/36] usb: gadget: f_tcm: Free tags earlier

Thinh Nguyen
2022-07-07T10:12:00Z

On 7/7/2022, Dmitriy Bogdanov wrote:
> Hi Thinh,
>
>> Free up sbitmap index immediately once the command is completed rather
>> than waiting for kref to call freeing tag. This keeps the sbitmap queue
>> free quicker.
>>
> That is completely wrong. Sbitmap index is a place of usbg_cmd* in
> a pre-allocated memory pool. It must not be freed until usbg_cmd*
> is not used.

As noted in the commit message and as you can see in the code change, I
only free it when the command is completed or no longer in used.

BR,
Thinh

> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> ---
> drivers/usb/gadget/function/f_tcm.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index c13aa61d82aa..fa09999adda7 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -570,6 +570,7 @@ static void uasp_status_data_cmpl(struct usb_ep *ep, struct usb_request *req)
> struct usbg_cmd *cmd = req->context;
> struct uas_stream *stream = cmd->stream;
> struct f_uas *fu = cmd->fu;
> + struct se_session *se_sess = cmd->se_cmd.se_sess;
> int ret;
>
> if (req->status == -ESHUTDOWN)
> @@ -603,6 +604,8 @@ static void uasp_status_data_cmpl(struct usb_ep *ep, struct usb_request *req)
> break;
>
> case UASP_QUEUE_COMMAND:
> +
> + target_free_tag(se_sess, &cmd->se_cmd);
> transport_generic_free_cmd(&cmd->se_cmd, 0);
> usb_ep_queue(fu->ep_cmd, cmd->req, GFP_ATOMIC);
>
> @@ -614,6 +617,7 @@ static void uasp_status_data_cmpl(struct usb_ep *ep, struct usb_request *req)
> return;
>
> cleanup:
> + target_free_tag(se_sess, &cmd->se_cmd);
> transport_generic_free_cmd(&cmd->se_cmd, 0);
> }
>
> @@ -941,6 +945,7 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req)
> struct se_cmd *se_cmd = &cmd->se_cmd;
>
> if (req->status == -ESHUTDOWN) {
> + target_free_tag(se_cmd->se_sess, se_cmd);
> transport_generic_free_cmd(&cmd->se_cmd, 0);
> return;
> }
> @@ -963,6 +968,7 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req)
> cleanup:
> /* Command was aborted */
> if (cmd->state == UASP_QUEUE_COMMAND) {
> + target_free_tag(se_cmd->se_sess, se_cmd);
> transport_generic_free_cmd(se_cmd, 0);
> return;
> }