From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout3.w1.samsung.com ([210.118.77.13]:47670 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751052AbbGTOJl (ORCPT ); Mon, 20 Jul 2015 10:09:41 -0400 MIME-version: 1.0 Content-type: text/plain; charset=utf-8; format=flowed Content-transfer-encoding: 8BIT Message-id: <55AD0121.4070107@samsung.com> Date: Mon, 20 Jul 2015 16:09:37 +0200 From: Krzysztof Opasiak To: Michal Nazarewicz , balbi@ti.com Cc: stable@vger.kernel.org, david.fisher1@synopsys.com, gregkh@linuxfoundation.org, andrzej.p@samsung.com, m.szyprowski@samsung.com, linux-usb@vger.kernel.org Subject: Re: [PATCH v3 4/5] usb: gadget: mass_storage: Use static array for luns References: <1436281065-32407-1-git-send-email-k.opasiak@samsung.com> <1436281065-32407-5-git-send-email-k.opasiak@samsung.com> <55ACCECC.5000609@samsung.com> In-reply-to: Sender: stable-owner@vger.kernel.org List-ID: On 07/20/2015 03:17 PM, Michal Nazarewicz wrote: >> On 07/08/2015 04:00 PM, Michal Nazarewicz wrote: >>> On Tue, Jul 07 2015, Krzysztof Opasiak wrote: >>>> This patch replace dynamicly allocated luns array with static one. >>>> This simplifies the code of mass storage function and modules. >>>> >>>> It also fix issue with reporting wrong number of LUNs in GET_MAX_LUN >>>> request. According to MS spec we should return the max index of valid >>>> LUN, not the number of luns - 1. >>> >>> This is no longer true, is it? Why my fix this bug has been solved, no? >>> As such, this should not go to stable. Or am I missing something? > > On Mon, Jul 20 2015, Krzysztof Opasiak wrote: >> Unfortunately it's still true. Your fix solved this bug partially. Now >> we report nluns - 1 instead of FSG_LUN_MAX but in case of not contiguous >> luns it is not enough. >> >> Let's consider mass storage function with luns 0 1 3 5. nluns == 4 so in >> GET_MAX_LUN we will return 3 (nluns - 1). > > I don’t believe that’s accurate. See loop in my patch: > > + 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 > + common->nluns = nluns; > + pr_info("Number of LUNs=%u\n", common->nluns); > + } > > It iterates over all luns and if lun is non-NULL it sets nluns to index > of that lun plus one. So if luns[0], lun[1], lun[2], lun[3] and lun[5] > are set, common->nluns = 6. > Yeah I see it now. What I wrote was a problem before your patch but now it seams to be ok, just nluns name is very misleading. Summing up, just like you wrote this patch should not go to stable. -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics