> 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