Re: Configfs usb mass_storage device always reports 8 LUNs ?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On 07/02/2015 05:25 PM, Michal Nazarewicz wrote:
>> Since I think we agreed that luns should just be turned into an array
>> with static length the above won’t be too far from that goal.

On Thu, Jul 02 2015, Krzysztof Opasiak wrote:
> True, just started working on it.

I just got this to compile but haven’t tested it past that.  Feel free
to grab it or whatever:


>From 7fd36d82c7d190224961cbdd8b3ee365fe5f5d18 Mon Sep 17 00:00:00 2001
From: Michal Nazarewicz <mina86@xxxxxxxxxx>
Date: Thu, 2 Jul 2015 19:14:59 +0200
Subject: [PATCH] usb: f_mass_storage: convert luns pointer into an array

Simplify the code by turning fsg_commen::luns into an array of pointers
with a static size rather than having it a pointer to dynamically
allocated array.  For legacy gadgets this will waste some memory, but
in configfs-based gadgets the array is allocated with maximum size
anyway.

Signed-off-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
---
 drivers/usb/gadget/function/f_mass_storage.c | 52 ++++++----------------------
 drivers/usb/gadget/function/f_mass_storage.h |  2 --
 drivers/usb/gadget/legacy/acm_ms.c           |  6 ++--
 drivers/usb/gadget/legacy/mass_storage.c     |  6 ++--
 drivers/usb/gadget/legacy/multi.c            |  6 ++--
 5 files changed, 17 insertions(+), 55 deletions(-)

diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index 15c3071..e34c6b2 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -281,7 +281,7 @@ struct fsg_common {
 
 	unsigned int		nluns;
 	unsigned int		lun;
-	struct fsg_lun		**luns;
+	struct fsg_lun		*luns[FSG_MAX_LUNS];
 	struct fsg_lun		*curlun;
 
 	unsigned int		bulk_out_maxpacket;
@@ -2751,11 +2751,11 @@ void fsg_common_remove_lun(struct fsg_lun *lun, bool sysfs)
 }
 EXPORT_SYMBOL_GPL(fsg_common_remove_lun);
 
-static void _fsg_common_remove_luns(struct fsg_common *common, int n)
+void fsg_common_remove_luns(struct fsg_common *common)
 {
 	int i;
 
-	for (i = 0; i < n; ++i)
+	for (i = 0; i < FSG_MAX_LUNS; ++i)
 		if (common->luns[i]) {
 			fsg_common_remove_lun(common->luns[i], common->sysfs);
 			common->luns[i] = NULL;
@@ -2763,37 +2763,16 @@ static void _fsg_common_remove_luns(struct fsg_common *common, int n)
 }
 EXPORT_SYMBOL_GPL(fsg_common_remove_luns);
 
-void fsg_common_remove_luns(struct fsg_common *common)
-{
-	_fsg_common_remove_luns(common, common->nluns);
-}
-
-void fsg_common_free_luns(struct fsg_common *common)
-{
-	fsg_common_remove_luns(common);
-	kfree(common->luns);
-	common->luns = NULL;
-}
-EXPORT_SYMBOL_GPL(fsg_common_free_luns);
-
 int fsg_common_set_nluns(struct fsg_common *common, int nluns)
 {
-	struct fsg_lun **curlun;
-
 	/* Find out how many LUNs there should be */
 	if (nluns < 1 || nluns > FSG_MAX_LUNS) {
 		pr_err("invalid number of LUNs: %u\n", nluns);
 		return -EINVAL;
 	}
 
-	curlun = kcalloc(FSG_MAX_LUNS, sizeof(*curlun), GFP_KERNEL);
-	if (unlikely(!curlun))
-		return -ENOMEM;
-
-	if (common->luns)
-		fsg_common_free_luns(common);
+	fsg_common_remove_luns(common);
 
-	common->luns = curlun;
 	common->nluns = nluns;
 
 	return 0;
@@ -2880,7 +2859,7 @@ int fsg_common_create_lun(struct fsg_common *common, struct fsg_lun_config *cfg,
 	char *pathbuf, *p;
 	int rc = -ENOMEM;
 
-	if (!common->nluns || !common->luns)
+	if (!common->nluns)
 		return -ENODEV;
 
 	if (common->luns[id])
@@ -2976,7 +2955,7 @@ int fsg_common_create_luns(struct fsg_common *common, struct fsg_config *cfg)
 	return 0;
 
 fail:
-	_fsg_common_remove_luns(common, i);
+	fsg_common_remove_luns(common);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(fsg_common_create_luns);
@@ -3020,6 +2999,7 @@ EXPORT_SYMBOL_GPL(fsg_common_run_thread);
 static void fsg_common_release(struct kref *ref)
 {
 	struct fsg_common *common = container_of(ref, struct fsg_common, ref);
+	unsigned i;
 
 	/* If the thread isn't already dead, tell it to exit now */
 	if (common->state != FSG_STATE_TERMINATED) {
@@ -3027,22 +3007,14 @@ static void fsg_common_release(struct kref *ref)
 		wait_for_completion(&common->thread_notifier);
 	}
 
-	if (likely(common->luns)) {
-		struct fsg_lun **lun_it = common->luns;
-		unsigned i = common->nluns;
-
-		/* In error recovery common->nluns may be zero. */
-		for (; i; --i, ++lun_it) {
-			struct fsg_lun *lun = *lun_it;
-			if (!lun)
-				continue;
+	for (i = 0; i < FSG_MAX_LUNS; ++i) {
+		struct fsg_lun *lun = common->luns[i];
+		if (lun) {
 			fsg_lun_close(lun);
 			if (common->sysfs)
 				device_unregister(&lun->dev);
 			kfree(lun);
 		}
-
-		kfree(common->luns);
 	}
 
 	_fsg_common_free_buffers(common->buffhds, common->fsg_num_buffers);
@@ -3516,7 +3488,7 @@ static struct usb_function_instance *fsg_alloc_inst(void)
 	rc = fsg_common_set_num_buffers(opts->common,
 					CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS);
 	if (rc)
-		goto release_luns;
+		goto release_opts;
 
 	pr_info(FSG_DRIVER_DESC ", version: " FSG_DRIVER_VERSION "\n");
 
@@ -3534,8 +3506,6 @@ static struct usb_function_instance *fsg_alloc_inst(void)
 
 	return &opts->func_inst;
 
-release_luns:
-	kfree(opts->common->luns);
 release_opts:
 	kfree(opts);
 	return ERR_PTR(rc);
diff --git a/drivers/usb/gadget/function/f_mass_storage.h b/drivers/usb/gadget/function/f_mass_storage.h
index b4866fc..55098eb 100644
--- a/drivers/usb/gadget/function/f_mass_storage.h
+++ b/drivers/usb/gadget/function/f_mass_storage.h
@@ -141,8 +141,6 @@ void fsg_common_remove_lun(struct fsg_lun *lun, bool sysfs);
 
 void fsg_common_remove_luns(struct fsg_common *common);
 
-void fsg_common_free_luns(struct fsg_common *common);
-
 int fsg_common_set_nluns(struct fsg_common *common, int nluns);
 
 void fsg_common_set_ops(struct fsg_common *common,
diff --git a/drivers/usb/gadget/legacy/acm_ms.c b/drivers/usb/gadget/legacy/acm_ms.c
index 1194b09..ebc67b4 100644
--- a/drivers/usb/gadget/legacy/acm_ms.c
+++ b/drivers/usb/gadget/legacy/acm_ms.c
@@ -206,12 +206,12 @@ static int acm_ms_bind(struct usb_composite_dev *cdev)
 
 	status = fsg_common_set_cdev(opts->common, cdev, config.can_stall);
 	if (status)
-		goto fail_set_cdev;
+		goto fail_set_nluns;
 
 	fsg_common_set_sysfs(opts->common, true);
 	status = fsg_common_create_luns(opts->common, &config);
 	if (status)
-		goto fail_set_cdev;
+		goto fail_set_nluns;
 
 	fsg_common_set_inquiry_string(opts->common, config.vendor_name,
 				      config.product_name);
@@ -238,8 +238,6 @@ static int acm_ms_bind(struct usb_composite_dev *cdev)
 	/* error recovery */
 fail_string_ids:
 	fsg_common_remove_luns(opts->common);
-fail_set_cdev:
-	fsg_common_free_luns(opts->common);
 fail_set_nluns:
 	fsg_common_free_buffers(opts->common);
 fail:
diff --git a/drivers/usb/gadget/legacy/mass_storage.c b/drivers/usb/gadget/legacy/mass_storage.c
index e7bfb08..fd7ad4b 100644
--- a/drivers/usb/gadget/legacy/mass_storage.c
+++ b/drivers/usb/gadget/legacy/mass_storage.c
@@ -199,12 +199,12 @@ static int msg_bind(struct usb_composite_dev *cdev)
 
 	status = fsg_common_set_cdev(opts->common, cdev, config.can_stall);
 	if (status)
-		goto fail_set_cdev;
+		goto fail_set_nluns;
 
 	fsg_common_set_sysfs(opts->common, true);
 	status = fsg_common_create_luns(opts->common, &config);
 	if (status)
-		goto fail_set_cdev;
+		goto fail_set_nluns;
 
 	fsg_common_set_inquiry_string(opts->common, config.vendor_name,
 				      config.product_name);
@@ -226,8 +226,6 @@ static int msg_bind(struct usb_composite_dev *cdev)
 
 fail_string_ids:
 	fsg_common_remove_luns(opts->common);
-fail_set_cdev:
-	fsg_common_free_luns(opts->common);
 fail_set_nluns:
 	fsg_common_free_buffers(opts->common);
 fail:
diff --git a/drivers/usb/gadget/legacy/multi.c b/drivers/usb/gadget/legacy/multi.c
index b21b51f..a17d819 100644
--- a/drivers/usb/gadget/legacy/multi.c
+++ b/drivers/usb/gadget/legacy/multi.c
@@ -413,12 +413,12 @@ static int __ref multi_bind(struct usb_composite_dev *cdev)
 
 	status = fsg_common_set_cdev(fsg_opts->common, cdev, config.can_stall);
 	if (status)
-		goto fail_set_cdev;
+		goto fail_set_nluns;
 
 	fsg_common_set_sysfs(fsg_opts->common, true);
 	status = fsg_common_create_luns(fsg_opts->common, &config);
 	if (status)
-		goto fail_set_cdev;
+		goto fail_set_nluns;
 
 	fsg_common_set_inquiry_string(fsg_opts->common, config.vendor_name,
 				      config.product_name);
@@ -447,8 +447,6 @@ static int __ref multi_bind(struct usb_composite_dev *cdev)
 	/* error recovery */
 fail_string_ids:
 	fsg_common_remove_luns(fsg_opts->common);
-fail_set_cdev:
-	fsg_common_free_luns(fsg_opts->common);
 fail_set_nluns:
 	fsg_common_free_buffers(fsg_opts->common);
 fail2:
-- 
2.4.3.573.g4eafbef
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux