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. Also change the nluns to max_luns which is better name for value stored in that place as we no longer need to store size of luns array. Reported-by: David Fisher <david.fisher1@xxxxxxxxxxxx> Signed-off-by: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx> --- drivers/usb/gadget/function/f_mass_storage.c | 125 ++++++++++---------------- drivers/usb/gadget/function/f_mass_storage.h | 4 - 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, 48 insertions(+), 99 deletions(-) diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index ad0f69b..befe251 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -279,9 +279,9 @@ struct fsg_common { int cmnd_size; u8 cmnd[MAX_COMMAND_SIZE]; - unsigned int nluns; + int max_lun; unsigned int lun; - struct fsg_lun **luns; + struct fsg_lun *luns[FSG_MAX_LUNS]; struct fsg_lun *curlun; unsigned int bulk_out_maxpacket; @@ -533,7 +533,7 @@ static int fsg_setup(struct usb_function *f, w_length != 1) return -EDOM; VDBG(fsg, "get max LUN\n"); - *(u8 *)req->buf = fsg->common->nluns - 1; + *(u8 *)req->buf = fsg->common->max_lun; /* Respond with data/status */ req->length = min((u16)1, w_length); @@ -2131,7 +2131,7 @@ static int received_cbw(struct fsg_dev *fsg, struct fsg_buffhd *bh) } /* Is the CBW meaningful? */ - if (cbw->Lun >= FSG_MAX_LUNS || cbw->Flags & ~US_BULK_FLAG_IN || + if (cbw->Lun > common->max_lun || cbw->Flags & ~US_BULK_FLAG_IN || cbw->Length <= 0 || cbw->Length > MAX_COMMAND_SIZE) { DBG(fsg, "non-meaningful CBW: lun = %u, flags = 0x%x, " "cmdlen %u\n", @@ -2159,7 +2159,7 @@ static int received_cbw(struct fsg_dev *fsg, struct fsg_buffhd *bh) if (common->data_size == 0) common->data_dir = DATA_DIR_NONE; common->lun = cbw->Lun; - if (common->lun < common->nluns) + if (common->lun < ARRAY_SIZE(common->luns)) common->curlun = common->luns[common->lun]; else common->curlun = NULL; @@ -2307,7 +2307,7 @@ reset: } common->running = 1; - for (i = 0; i < common->nluns; ++i) + for (i = 0; i <= common->max_lun; ++i) if (common->luns[i]) common->luns[i]->unit_attention_data = SS_RESET_OCCURRED; @@ -2409,7 +2409,7 @@ static void handle_exception(struct fsg_common *common) if (old_state == FSG_STATE_ABORT_BULK_OUT) common->state = FSG_STATE_STATUS_PHASE; else { - for (i = 0; i < common->nluns; ++i) { + for (i = 0; i <= common->max_lun; ++i) { curlun = common->luns[i]; if (!curlun) continue; @@ -2453,7 +2453,7 @@ static void handle_exception(struct fsg_common *common) * a waste of time. Ditto for the INTERFACE_CHANGE and * CONFIG_CHANGE cases. */ - /* for (i = 0; i < common->nluns; ++i) */ + /* for (i = 0; i <= common->max_lun; ++i) */ /* if (common->luns[i]) */ /* common->luns[i]->unit_attention_data = */ /* SS_RESET_OCCURRED; */ @@ -2552,12 +2552,11 @@ static int fsg_main_thread(void *common_) if (!common->ops || !common->ops->thread_exits || common->ops->thread_exits(common) < 0) { - struct fsg_lun **curlun_it = common->luns; - unsigned i = common->nluns; + int i = 0; down_write(&common->filesem); - for (; i--; ++curlun_it) { - struct fsg_lun *curlun = *curlun_it; + for (; i <= common->max_lun; ++i) { + struct fsg_lun *curlun = common->luns[i]; if (!curlun || !fsg_lun_is_open(curlun)) continue; @@ -2676,6 +2675,8 @@ static struct fsg_common *fsg_common_setup(struct fsg_common *common) init_completion(&common->thread_notifier); init_waitqueue_head(&common->fsg_wait); common->state = FSG_STATE_TERMINATED; + memset(common->luns, 0, sizeof(common->luns)); + common->max_lun = -1; return common; } @@ -2751,6 +2752,12 @@ void fsg_common_remove_lun(struct fsg_lun *lun, bool sysfs) } EXPORT_SYMBOL_GPL(fsg_common_remove_lun); +static void _fsg_common_reduce_max_lun(struct fsg_common *common) +{ + while (common->max_lun >= 0 && common->luns[common->max_lun] == NULL) + common->max_lun--; +} + static void _fsg_common_remove_luns(struct fsg_common *common, int n) { int i; @@ -2760,48 +2767,16 @@ static void _fsg_common_remove_luns(struct fsg_common *common, int n) fsg_common_remove_lun(common->luns[i], common->sysfs); common->luns[i] = NULL; } + + _fsg_common_reduce_max_lun(common); } void fsg_common_remove_luns(struct fsg_common *common) { - _fsg_common_remove_luns(common, common->nluns); + _fsg_common_remove_luns(common, common->max_lun); } EXPORT_SYMBOL_GPL(fsg_common_remove_luns); -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(nluns, sizeof(*curlun), GFP_KERNEL); - if (unlikely(!curlun)) - return -ENOMEM; - - if (common->luns) - fsg_common_free_luns(common); - - common->luns = curlun; - common->nluns = nluns; - - pr_info("Number of LUNs=%d\n", common->nluns); - - return 0; -} -EXPORT_SYMBOL_GPL(fsg_common_set_nluns); - void fsg_common_set_ops(struct fsg_common *common, const struct fsg_operations *ops) { @@ -2882,8 +2857,8 @@ 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) - return -ENODEV; + if (id >= ARRAY_SIZE(common->luns)) + return -EINVAL; if (common->luns[id]) return -EBUSY; @@ -2924,6 +2899,8 @@ int fsg_common_create_lun(struct fsg_common *common, struct fsg_lun_config *cfg, } common->luns[id] = lun; + if (common->max_lun < (int)id) + common->max_lun = id; if (cfg->filename) { rc = fsg_lun_open(lun, cfg->filename); @@ -2954,7 +2931,7 @@ error_lun: if (common->sysfs) device_unregister(&lun->dev); fsg_lun_close(lun); - common->luns[id] = NULL; + _fsg_common_reduce_max_lun(common); error_sysfs: kfree(lun); return rc; @@ -2966,14 +2943,14 @@ int fsg_common_create_luns(struct fsg_common *common, struct fsg_config *cfg) char buf[8]; /* enough for 100000000 different numbers, decimal */ int i, rc; - for (i = 0; i < common->nluns; ++i) { + for (i = 0; i < cfg->nluns; ++i) { snprintf(buf, sizeof(buf), "lun%d", i); rc = fsg_common_create_lun(common, &cfg->luns[i], i, buf, NULL); if (rc) goto fail; } - pr_info("Number of LUNs=%d\n", common->nluns); + pr_info("Number of LUNs=%d\n", cfg->nluns); return 0; @@ -3022,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 int i; /* If the thread isn't already dead, tell it to exit now */ if (common->state != FSG_STATE_TERMINATED) { @@ -3029,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; - fsg_lun_close(lun); - if (common->sysfs) - device_unregister(&lun->dev); - kfree(lun); - } - - kfree(common->luns); + for (i = 0; i < ARRAY_SIZE(common->luns); ++i) { + struct fsg_lun *lun = common->luns[i]; + if (!lun) + continue; + fsg_lun_close(lun); + if (common->sysfs) + device_unregister(&lun->dev); + kfree(lun); } _fsg_common_free_buffers(common->buffhds, common->fsg_num_buffers); @@ -3058,6 +3028,7 @@ static void fsg_common_release(struct kref *ref) static int fsg_bind(struct usb_configuration *c, struct usb_function *f) { struct fsg_dev *fsg = fsg_from_func(f); + struct fsg_common *common = fsg->common; struct usb_gadget *gadget = c->cdev->gadget; int i; struct usb_ep *ep; @@ -3065,6 +3036,12 @@ static int fsg_bind(struct usb_configuration *c, struct usb_function *f) int ret; struct fsg_opts *opts; + /* Don't allow to bind if we don't have at least one LUN */ + if (common->max_lun < 0) { + pr_err("There should be at least one LUN.\n"); + return -EINVAL; + } + opts = fsg_opts_from_func_inst(f->fi); if (!opts->no_configfs) { ret = fsg_common_set_cdev(fsg->common, c->cdev, @@ -3305,11 +3282,9 @@ static struct config_group *fsg_lun_make(struct config_group *group, return ERR_PTR(ret); fsg_opts = to_fsg_opts(&group->cg_item); - if (num >= FSG_MAX_LUNS) - return ERR_PTR(-ERANGE); mutex_lock(&fsg_opts->lock); - if (fsg_opts->refcnt || fsg_opts->common->luns[num]) { + if (fsg_opts->refcnt) { ret = -EBUSY; goto out; } @@ -3359,6 +3334,7 @@ static void fsg_lun_drop(struct config_group *group, struct config_item *item) fsg_common_remove_lun(lun_opts->lun, fsg_opts->common->sysfs); fsg_opts->common->luns[lun_opts->lun_id] = NULL; + _fsg_common_reduce_max_lun(fsg_opts->common); lun_opts->lun_id = 0; mutex_unlock(&fsg_opts->lock); @@ -3511,14 +3487,11 @@ static struct usb_function_instance *fsg_alloc_inst(void) rc = PTR_ERR(opts->common); goto release_opts; } - rc = fsg_common_set_nluns(opts->common, FSG_MAX_LUNS); - if (rc) - goto release_opts; 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"); @@ -3541,8 +3514,6 @@ static struct usb_function_instance *fsg_alloc_inst(void) release_buffers: fsg_common_free_buffers(opts->common); -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..764807c 100644 --- a/drivers/usb/gadget/function/f_mass_storage.h +++ b/drivers/usb/gadget/function/f_mass_storage.h @@ -141,10 +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, const struct fsg_operations *ops); diff --git a/drivers/usb/gadget/legacy/acm_ms.c b/drivers/usb/gadget/legacy/acm_ms.c index 1194b09..3c8e309 100644 --- a/drivers/usb/gadget/legacy/acm_ms.c +++ b/drivers/usb/gadget/legacy/acm_ms.c @@ -200,10 +200,6 @@ static int acm_ms_bind(struct usb_composite_dev *cdev) if (status) goto fail; - status = fsg_common_set_nluns(opts->common, config.nluns); - if (status) - goto fail_set_nluns; - status = fsg_common_set_cdev(opts->common, cdev, config.can_stall); if (status) goto fail_set_cdev; @@ -239,8 +235,6 @@ static int acm_ms_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: usb_put_function_instance(fi_msg); diff --git a/drivers/usb/gadget/legacy/mass_storage.c b/drivers/usb/gadget/legacy/mass_storage.c index e7bfb08..8e2be7f 100644 --- a/drivers/usb/gadget/legacy/mass_storage.c +++ b/drivers/usb/gadget/legacy/mass_storage.c @@ -191,10 +191,6 @@ static int msg_bind(struct usb_composite_dev *cdev) if (status) goto fail; - status = fsg_common_set_nluns(opts->common, config.nluns); - if (status) - goto fail_set_nluns; - fsg_common_set_ops(opts->common, &ops); status = fsg_common_set_cdev(opts->common, cdev, config.can_stall); @@ -227,8 +223,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: usb_put_function_instance(fi_msg); diff --git a/drivers/usb/gadget/legacy/multi.c b/drivers/usb/gadget/legacy/multi.c index b21b51f..fb683e3 100644 --- a/drivers/usb/gadget/legacy/multi.c +++ b/drivers/usb/gadget/legacy/multi.c @@ -407,10 +407,6 @@ static int __ref multi_bind(struct usb_composite_dev *cdev) if (status) goto fail2; - status = fsg_common_set_nluns(fsg_opts->common, config.nluns); - if (status) - goto fail_set_nluns; - status = fsg_common_set_cdev(fsg_opts->common, cdev, config.can_stall); if (status) goto fail_set_cdev; @@ -448,8 +444,6 @@ static int __ref multi_bind(struct usb_composite_dev *cdev) 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: usb_put_function_instance(fi_msg); -- 1.7.9.5 -- 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