On Mon, Jun 22 2015, Krzysztof Opasiak wrote:
> On 06/20/2015 12:18 AM, Michal Nazarewicz wrote:
>> On Fri, Jun 19 2015, Felipe Balbi wrote:
>>> the fact is that this needs to be configurable from configfs. If user
>>> sets up a single lun, then this should be as you said, however, if 2
>>> luns are configured, I still want to have those 2 working.
>>>
>>> From a user perspective, configfs should support N luns just fine as
>>> long as we have enough memory available.
>>
>> Yes, sorry, you’re right. I didn’t fallow closely migration to configfs
>> based configuration and didn’t have the full picture.
>
> Well, not exactly. MS spec says that there should be no more that 16 LUNs.
Yeah. fsg_lun_make limits LUN to < FSG_MAX_LUNS which is eight so we’re
fine.
>> After taking another look, I think the best solution is to have a loop
>> in fsg_alloc, something like the following (beware that this has not
>> been tested in any way):
>>
>>
>>>From 19ec2796009f8650c7db4358f7e16931ba96e4ee Mon Sep 17 00:00:00 2001
>> From: Michal Nazarewicz <mina86@xxxxxxxxxx>
>> Date: Fri, 19 Jun 2015 23:56:34 +0200
>> Subject: [PATCH] usb: f_mass_storage: limit number of reported LUNs
>>
>> Mass storage function created via configfs always reports eight LUNs
>> to the hosts even if only one LUN has been configured. Adjust the
>> number when the USB function is allocated based on LUNs that user
>> has created.
>>
>> Signed-off-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
>> ---
>> drivers/usb/gadget/function/f_mass_storage.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
>> index 3cc109f..8e26a12 100644
>> --- a/drivers/usb/gadget/function/f_mass_storage.c
>> +++ b/drivers/usb/gadget/function/f_mass_storage.c
>> @@ -3563,14 +3563,28 @@ static struct usb_function *fsg_alloc(struct usb_function_instance *fi)
>> struct fsg_opts *opts = fsg_opts_from_func_inst(fi);
>> struct fsg_common *common = opts->common;
>> struct fsg_dev *fsg;
>> + unsigned nluns, i;
>>
>> fsg = kzalloc(sizeof(*fsg), GFP_KERNEL);
>> if (unlikely(!fsg))
>> return ERR_PTR(-ENOMEM);
>>
>> mutex_lock(&opts->lock);
>> + if (!opts->refcnt) {
>> + for (nluns = i = 0; i < FSG_MAX_LUNS; ++i)
>> + if (common->luns[i])
>> + nluns = i + 1;
>> + if (!nluns) {
>> + pr_warn("No LUNS defined, continuing anyway\n");
>> + } else if (nluns != common->nluns) {
>> + pr_info("Adjusting number of LUNs=%u (was %u)\n",
>> + nluns, common->nluns);
>> + common->nluns = nluns;
>> + }
>> + }
>> opts->refcnt++;
>> mutex_unlock(&opts->lock);
>> +
>> fsg->function.name = FSG_DRIVER_DESC;
>> fsg->function.bind = fsg_bind;
>> fsg->function.unbind = fsg_unbind;
>>
>
> Looks simple but we will still get comment "number of LUNs=8". Moreover
> it may cause some trouble if we set nluns == 0,
Which is why the code leaves common->nluns == 8 if nluns == 0.
> because in create lun we check whether nluns > 0. In addition if we
> use for example fsg_common_set_nluns(1) or anything other what <
> FSG_MAX_LUNS this loop will read past allocated buffer.
common->nluns is set to FSG_MAX_LUNS in fsg_alloc_inst (which is called
prior to fsg_alloc) so this is not an issue.
> I have prepared a series which fix several smaller issues and also this one:
>
> http://article.gmane.org/gmane.linux.usb.general/127209
>
>
> Best regards,
>
> --
> Krzysztof Opasiak
> Samsung R&D Institute Poland
> Samsung Electronics
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +--<mpn@xxxxxxxxxx>--<xmpp:mina86@xxxxxxxxxx>--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
