From patchwork Tue May 5 13:39:07 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Akinobu Mita X-Patchwork-Id: 6338101 Return-Path: X-Original-To: patchwork-linux-scsi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 4DB40BEEE1 for ; Tue, 5 May 2015 13:39:16 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 56BC72022D for ; Tue, 5 May 2015 13:39:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 23097201C8 for ; Tue, 5 May 2015 13:39:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2992477AbbEENjM (ORCPT ); Tue, 5 May 2015 09:39:12 -0400 Received: from mail-lb0-f171.google.com ([209.85.217.171]:35095 "EHLO mail-lb0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751840AbbEENjJ (ORCPT ); Tue, 5 May 2015 09:39:09 -0400 Received: by lbbuc2 with SMTP id uc2so128450388lbb.2; Tue, 05 May 2015 06:39:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=bhGx1Sk6rDCOj57NU6ItNWOpE8rVppDS2iCI2Adn+GQ=; b=foqvo9EohTelvGBEbZuSXDyVLk6W7iZdzpRwpl/zLATZsimZE7Bye9EVvJsmf/4hdO QkPXniuyX/hDDo0ZB69GTfT8BFP2wLolGXmkTtxlGyl782yYrJJszgctvZW4xfOo8PAw J1QnpY0A0LTUhN2SILgPefzg6C9W7oWOhS4boiNxp2H1TgFGisL3/g7fUA7VZfLMk3fv mJL+u6JAEe82VSZfwZIK1PwoLJuOJKK6f4cgZsYtmbG3LafvpLAWvFMAr0I6N44tjcOK 2lDZIastKqCcNkm+lUICxI0HheESheTB4+FVZ6WD5FZvrnqtLIw5b0S/4X9evBkxhFDF WybA== MIME-Version: 1.0 X-Received: by 10.152.23.105 with SMTP id l9mr23946227laf.26.1430833148055; Tue, 05 May 2015 06:39:08 -0700 (PDT) Received: by 10.152.106.3 with HTTP; Tue, 5 May 2015 06:39:07 -0700 (PDT) In-Reply-To: <1430775664.2177.36.camel@HansenPartnership.com> References: <1430775664.2177.36.camel@HansenPartnership.com> Date: Tue, 5 May 2015 22:39:07 +0900 Message-ID: Subject: Re: [PATCH v6 0/4] scsi: ufs & ums-* & esp_scsi: fix module reference counting From: Akinobu Mita To: James Bottomley Cc: Alan Stern , "linux-scsi@vger.kernel.org" , Vinayak Holikatti , Dolev Raviv , Sujit Reddy Thumma , Subhash Jadavani , Christoph Hellwig , Matthew Dharm , Greg Kroah-Hartman , "David S. Miller" , Hannes Reinecke , linux-usb@vger.kernel.org, usb-storage@lists.one-eyed-alien.net Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, T_RP_MATCHES_RCVD, T_TVD_MIME_EPI,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 2015-05-05 6:41 GMT+09:00 James Bottomley : > On Mon, 2015-05-04 at 16:09 -0400, Alan Stern wrote: >> On Mon, 4 May 2015, James Bottomley wrote: >> >> > However, it does also strike me that these three drivers have problems >> > because they're using the wrong initialisation pattern: the template is >> > supposed to be in the bus connector for compound drivers not in the >> > core. >> >> Why is it supposed to be done that way? Isn't that less efficient? It >> means you have to have a separate copy of the template for each bus >> connector driver, instead of letting them all share a common template >> in the core. > > Well, no it doesn't. The way 53c700 implements it is that there is a > common template in the core. The drivers just initialise their variant > fields (for 53c700 it's name, proc_name and this_id) and the core fills > in the rest. Admittedly wd33c93 doesn't quite get this right, that's > why I cited 53c700. I have converted usb-storage and ums-alauda in the attached patch. Each ums-* driver needs to expand module_usb_driver() macro as we need to initialize their scsi host template in module_init(). Alan, how do you feel the change like this? From 4c6aa0b56c153afe331a98969a857f16d99f96b6 Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Tue, 5 May 2015 22:30:04 +0900 Subject: [PATCH] usb: storage: fix module reference for scsi host --- drivers/usb/storage/alauda.c | 21 +++++++++++++++++++-- drivers/usb/storage/scsiglue.c | 12 +++++++++++- drivers/usb/storage/scsiglue.h | 3 ++- drivers/usb/storage/usb.c | 25 +++++++++++++++++++++---- drivers/usb/storage/usb.h | 3 ++- 5 files changed, 55 insertions(+), 9 deletions(-) diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c index 4b55ab6..0baa484 100644 --- a/drivers/usb/storage/alauda.c +++ b/drivers/usb/storage/alauda.c @@ -42,6 +42,7 @@ #include "transport.h" #include "protocol.h" #include "debug.h" +#include "scsiglue.h" MODULE_DESCRIPTION("Driver for Alauda-based card readers"); MODULE_AUTHOR("Daniel Drake "); @@ -1232,6 +1233,8 @@ static int alauda_transport(struct scsi_cmnd *srb, struct us_data *us) return USB_STOR_TRANSPORT_FAILED; } +static struct scsi_host_template alauda_host_template; + static int alauda_probe(struct usb_interface *intf, const struct usb_device_id *id) { @@ -1239,7 +1242,8 @@ static int alauda_probe(struct usb_interface *intf, int result; result = usb_stor_probe1(&us, intf, id, - (id - alauda_usb_ids) + alauda_unusual_dev_list); + (id - alauda_usb_ids) + alauda_unusual_dev_list, + &alauda_host_template); if (result) return result; @@ -1266,4 +1270,17 @@ static struct usb_driver alauda_driver = { .no_dynamic_id = 1, }; -module_usb_driver(alauda_driver); +static int __init alauda_driver_init(void) +{ + usb_stor_host_template_init(&alauda_host_template, "ums-alauda", + THIS_MODULE); + + return usb_register(&alauda_driver); +} +module_init(alauda_driver_init); + +static void __exit alauda_driver_exit(void) +{ + usb_deregister(&alauda_driver); +} +module_exit(alauda_driver_exit); diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c index 0e400f3..05b8fe2 100644 --- a/drivers/usb/storage/scsiglue.c +++ b/drivers/usb/storage/scsiglue.c @@ -540,7 +540,7 @@ static struct device_attribute *sysfs_device_attr_list[] = { * this defines our host template, with which we'll allocate hosts */ -struct scsi_host_template usb_stor_host_template = { +static struct scsi_host_template usb_stor_host_template = { /* basic userland interface stuff */ .name = "usb-storage", .proc_name = "usb-storage", @@ -592,6 +592,16 @@ struct scsi_host_template usb_stor_host_template = { .module = THIS_MODULE }; +void usb_stor_host_template_init(struct scsi_host_template *sht, + const char *name, struct module *owner) +{ + memcpy(sht, &usb_stor_host_template, sizeof(*sht)); + sht->name = name; + sht->proc_name = name; + sht->module = owner; +} +EXPORT_SYMBOL_GPL(usb_stor_host_template_init); + /* To Report "Illegal Request: Invalid Field in CDB */ unsigned char usb_stor_sense_invalidCDB[18] = { [0] = 0x70, /* current error */ diff --git a/drivers/usb/storage/scsiglue.h b/drivers/usb/storage/scsiglue.h index ffa1cca..1909601 100644 --- a/drivers/usb/storage/scsiglue.h +++ b/drivers/usb/storage/scsiglue.h @@ -43,6 +43,7 @@ extern void usb_stor_report_device_reset(struct us_data *us); extern void usb_stor_report_bus_reset(struct us_data *us); extern unsigned char usb_stor_sense_invalidCDB[18]; -extern struct scsi_host_template usb_stor_host_template; +extern void usb_stor_host_template_init(struct scsi_host_template *sht, + const char *name, struct module *owner); #endif diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c index 6c10c88..0f76899 100644 --- a/drivers/usb/storage/usb.c +++ b/drivers/usb/storage/usb.c @@ -924,7 +924,8 @@ static unsigned int usb_stor_sg_tablesize(struct usb_interface *intf) int usb_stor_probe1(struct us_data **pus, struct usb_interface *intf, const struct usb_device_id *id, - struct us_unusual_dev *unusual_dev) + struct us_unusual_dev *unusual_dev, + struct scsi_host_template *sht) { struct Scsi_Host *host; struct us_data *us; @@ -936,7 +937,7 @@ int usb_stor_probe1(struct us_data **pus, * Ask the SCSI layer to allocate a host structure, with extra * space at the end for our private us_data structure. */ - host = scsi_host_alloc(&usb_stor_host_template, sizeof(*us)); + host = scsi_host_alloc(sht, sizeof(*us)); if (!host) { dev_warn(&intf->dev, "Unable to allocate the scsi host\n"); return -ENOMEM; @@ -1073,6 +1074,8 @@ void usb_stor_disconnect(struct usb_interface *intf) } EXPORT_SYMBOL_GPL(usb_stor_disconnect); +static struct scsi_host_template usb_stor_host_template; + /* The main probe routine for standard devices */ static int storage_probe(struct usb_interface *intf, const struct usb_device_id *id) @@ -1113,7 +1116,8 @@ static int storage_probe(struct usb_interface *intf, id->idVendor, id->idProduct); } - result = usb_stor_probe1(&us, intf, id, unusual_dev); + result = usb_stor_probe1(&us, intf, id, unusual_dev, + &usb_stor_host_template); if (result) return result; @@ -1137,4 +1141,17 @@ static struct usb_driver usb_storage_driver = { .soft_unbind = 1, }; -module_usb_driver(usb_storage_driver); +static int __init usb_storage_driver_init(void) +{ + usb_stor_host_template_init(&usb_stor_host_template, "usb-storage", + THIS_MODULE); + + return usb_register(&usb_storage_driver); +} +module_init(usb_storage_driver_init); + +static void __exit usb_storage_driver_exit(void) +{ + usb_deregister(&usb_storage_driver); +} +module_exit(usb_storage_driver_exit); diff --git a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h index 307e339..a00ec4f 100644 --- a/drivers/usb/storage/usb.h +++ b/drivers/usb/storage/usb.h @@ -197,7 +197,8 @@ extern int usb_stor_post_reset(struct usb_interface *iface); extern int usb_stor_probe1(struct us_data **pus, struct usb_interface *intf, const struct usb_device_id *id, - struct us_unusual_dev *unusual_dev); + struct us_unusual_dev *unusual_dev, + struct scsi_host_template *sht); extern int usb_stor_probe2(struct us_data *us); extern void usb_stor_disconnect(struct usb_interface *intf); -- 1.9.1