Currently, f_midi_free uses snd_card_free, which will wait until the user has released the sound card before returning. However, if the user doesn't release the sound card, then f_midi_free can block for an arbitrary amount of time, which also blocks any gadget operations on that thread.
Instead, we can use snd_card_free_when_closed which returns before all handles are released. Since f_midi can be accessed through rmidi if usb_put_function is called before release_card_device, add refcounting to f_midi_free and have rawmidi's private free call it. The f_midi memory is only kfreed when usb_put_function and release_card_device have both been called. Also make use of refcnt in freeing f_midi_opts. The opts are only freed after the config items are released and all associated f_midi functions are freed. Signed-off-by: Jerry Zhang <[email protected]> --- I see that this patch is already in Greg's tree. Can it still be updated, or should I submit these changes as a new patch? Changelog since V1: - Added f_midi_opts refcounting - Fixed compile error drivers/usb/gadget/function/f_midi.c | 46 ++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c index a5719f271bf0..93464b42770c 100644 --- a/drivers/usb/gadget/function/f_midi.c +++ b/drivers/usb/gadget/function/f_midi.c @@ -98,6 +98,7 @@ struct f_midi { DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *); spinlock_t transmit_lock; unsigned int in_last_port; + unsigned char free_ref; struct gmidi_in_port in_ports_array[/* in_ports */]; }; @@ -108,6 +109,8 @@ static inline struct f_midi *func_to_midi(struct usb_function *f) } static void f_midi_transmit(struct f_midi *midi); +static void f_midi_rmidi_free(struct snd_rawmidi *rmidi); +static void f_midi_free_inst(struct usb_function_instance *f); DECLARE_UAC_AC_HEADER_DESCRIPTOR(1); DECLARE_USB_MIDI_OUT_JACK_DESCRIPTOR(1); @@ -818,6 +821,8 @@ static int f_midi_register_card(struct f_midi *midi) SNDRV_RAWMIDI_INFO_INPUT | SNDRV_RAWMIDI_INFO_DUPLEX; rmidi->private_data = midi; + rmidi->private_free = f_midi_rmidi_free; + midi->free_ref++; /* * Yes, rawmidi OUTPUT = USB IN, and rawmidi INPUT = USB OUT. @@ -1062,7 +1067,7 @@ static ssize_t f_midi_opts_##name##_store(struct config_item *item, \ u32 num; \ \ mutex_lock(&opts->lock); \ - if (opts->refcnt) { \ + if (opts->refcnt > 1) { \ ret = -EBUSY; \ goto end; \ } \ @@ -1117,7 +1122,7 @@ static ssize_t f_midi_opts_id_store(struct config_item *item, char *c; mutex_lock(&opts->lock); - if (opts->refcnt) { + if (opts->refcnt > 1) { ret = -EBUSY; goto end; } @@ -1158,13 +1163,21 @@ static struct config_item_type midi_func_type = { static void f_midi_free_inst(struct usb_function_instance *f) { struct f_midi_opts *opts; + bool free = false; opts = container_of(f, struct f_midi_opts, func_inst); - if (opts->id_allocated) - kfree(opts->id); + mutex_lock(&opts->lock); + if (!--opts->refcnt) { + free = true; + } + mutex_unlock(&opts->lock); - kfree(opts); + if (free) { + if (opts->id_allocated) + kfree(opts->id); + kfree(opts); + } } static struct usb_function_instance *f_midi_alloc_inst(void) @@ -1183,6 +1196,7 @@ static struct usb_function_instance *f_midi_alloc_inst(void) opts->qlen = 32; opts->in_ports = 1; opts->out_ports = 1; + opts->refcnt = 1; config_group_init_type_name(&opts->func_inst.group, "", &midi_func_type); @@ -1194,15 +1208,26 @@ static void f_midi_free(struct usb_function *f) { struct f_midi *midi; struct f_midi_opts *opts; + bool free = false; midi = func_to_midi(f); opts = container_of(f->fi, struct f_midi_opts, func_inst); - kfree(midi->id); mutex_lock(&opts->lock); - kfifo_free(&midi->in_req_fifo); - kfree(midi); - --opts->refcnt; + if (!--midi->free_ref) { + kfree(midi->id); + kfifo_free(&midi->in_req_fifo); + kfree(midi); + free = true; + } mutex_unlock(&opts->lock); + + if (free) + f_midi_free_inst(&opts->func_inst); +} + +static void f_midi_rmidi_free(struct snd_rawmidi *rmidi) +{ + f_midi_free(rmidi->private_data); } static void f_midi_unbind(struct usb_configuration *c, struct usb_function *f) @@ -1219,7 +1244,7 @@ static void f_midi_unbind(struct usb_configuration *c, struct usb_function *f) card = midi->card; midi->card = NULL; if (card) - snd_card_free(card); + snd_card_free_when_closed(card); usb_free_all_descriptors(f); } @@ -1263,6 +1288,7 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi) midi->buflen = opts->buflen; midi->qlen = opts->qlen; midi->in_last_port = 0; + midi->free_ref = 1; status = kfifo_alloc(&midi->in_req_fifo, midi->qlen, GFP_KERNEL); if (status) -- 2.14.1.342.g6490525c54-goog -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
