RE: [PATCH] usb: gadget: don't create new string_container if already exist

[Neil Zhang]

> -----Original Message-----
> From: Andrzej Pietrasiewicz [mailto:andrzej.p@xxxxxxxxxxx]
> Sent: 2014年10月28日 21:10
> To: Neil Zhang; linux-usb@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Cc: balbi@xxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH] usb: gadget: don't create new string_container if already
> exist
> 
> Hi,
> 
> W dniu 28.10.2014 o 12:33, Neil Zhang pisze:
> > Don't create new usb_gadget_string_container if the current strings
> > are already exist in the usb_composite_dev.
> > Otherwise the ids_tab will overflow soon if we bind / unbind usb
> > functions frequently like android does.
> 
> The problem you are describing does not exist in mainline kernel, where
> functions are always unbound as part of the whole gadget's unbind - regardless
> of whether it is a legacy gadget or configfs-composed gadget. When the whole
> gadget is unbound, composite_dev_cleanup() is called which zeroes cdev-
> >next_string_id and frees all gadget strings containers.
> 

Yes, you are right that the current mainline won't suffer this issue.
But it will be needed if we want to implement similar features like android do.
It will only remove config rather than whole gadget driver.


> >
> > Signed-off-by: Neil Zhang <zhangwm@xxxxxxxxxxx>
> > ---
> >   drivers/usb/gadget/composite.c |    6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/usb/gadget/composite.c
> > b/drivers/usb/gadget/composite.c index a8c18df..6fe3c6b 100644
> > --- a/drivers/usb/gadget/composite.c
> > +++ b/drivers/usb/gadget/composite.c
> > @@ -1183,6 +1183,12 @@ struct usb_string *usb_gstrings_attach(struct
> usb_composite_dev *cdev,
> >   	if (!n_gstrings)
> >   		return ERR_PTR(-EINVAL);
> >
> > +	list_for_each_entry(uc, &cdev->gstrings, list) {
> > +		n_gs = get_containers_gs(uc);
> > +		if (!strcmp(n_gs[0]->strings[0].s, sp[0]->strings[0].s))
> 
> To me it looks like it is a big assumption that if the first string matches,
> the rest are the same, too. Isn't it?
> 
> Anyway, this solution looks more like pushing the moment when cdev-
> >next_string_id becomes 254 to a later time rather than preventing such a
> situation.
> 
> If usb_gstrings_attach() happens at function bind time, perhaps there should
> be some usb_gstrings_detach() called at function unbind?
> 
> AP


Best Regards,
Neil Zhang