[PATCH] Re: scsi_scan.c complaints...

On Fri, Dec 20, 2002 at 08:29:23PM -0500, Doug Ledford wrote:
> And I was right.  One little q = NULL; is all that was missing.  Anyway,
> here's a printout of what startup looks like with this patch in place
> under 2.5.52.  This should make you happy Justin ;-)

Okay, I looked at the patches that are in mainline and they look pretty
cool to me.  When looking over the code (in preparation of implementing
Justin's suggestion to get rid of the highmem_io flag) I found quite
a bit small stuff to make the code in that area a lot cleaner:

- new helper scsi_calculate_bounce_limit to calculate the bounce
  limit for a scsi host, remove a copy of that code ni st.c
- scsi_initialize_queue gets replace with scsi_alloc_queue, this
  one now takes only a struct Scsi_Host and returns the request queue,
  it's paired with a small scsi_free_queue helper.

Diffstat:

 hosts.h     |    3 -
 scsi.c      |   43 ----------------------
 scsi.h      |    1
 scsi_scan.c |  113 ++++++++++++++++++++++++++++++++++--------------------------
 scsi_syms.c |    5 ++
 st.c        |   16 +-------
 6 files changed, 73 insertions(+), 108 deletions(-)

diff --git a/drivers/scsi/hosts.h b/drivers/scsi/hosts.h
index 8b2dd70f6bb31df3e98790638a07dbb6e0189568..5b53fcbd91a947b7a52b4c8d849d36b84c12b37f 100644 (file)
--- a/drivers/scsi/hosts.h
+++ b/drivers/scsi/hosts.h
@@ -554,9 +554,6 @@ struct Scsi_Device_Template
     struct device_driver scsi_driverfs_driver;
 };
 
-void  scsi_initialize_queue(Scsi_Device *, struct Scsi_Host *);
-
-
 /*
  * Highlevel driver registration/unregistration.
  */

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 83a84dbd5b80cd75f2f5aa5da123f15298cac10a..d5a6094c9a2053dbc0b22c0b0cac28ab8b9f3dce 100644 (file)
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -147,49 +147,6 @@ LIST_HEAD(scsi_dev_info_list);
 extern void scsi_times_out(Scsi_Cmnd * SCpnt);
 void scsi_build_commandblocks(Scsi_Device * SDpnt);
 
-/*
- * Function:    scsi_initialize_queue()
- *
- * Purpose:     Sets up the block queue for a device.
- *
- * Arguments:   SDpnt   - device for which we need a handler function.
- *
- * Returns:     Nothing
- *
- * Lock status: No locking assumed or required.
- */
-void  scsi_initialize_queue(Scsi_Device * SDpnt, struct Scsi_Host * SHpnt)
-{
-       request_queue_t *q = SDpnt->request_queue;
-
-       /*
-        * tell block layer about assigned host_lock for this host
-        */
-       blk_init_queue(q, scsi_request_fn, SHpnt->host_lock);
-
-       /* Hardware imposed limit. */
-       blk_queue_max_hw_segments(q, SHpnt->sg_tablesize);
-
-       /*
-        * scsi_alloc_sgtable max
-        */
-       blk_queue_max_phys_segments(q, MAX_PHYS_SEGMENTS);
-
-       if(!SHpnt->max_sectors)
-               /* driver imposes no hard sector transfer limit.
-                * start at machine infinity initially */
-               SHpnt->max_sectors = SCSI_DEFAULT_MAX_SECTORS;
-
-       /* FIXME: we should also adjust this limit later on
-        * after we know what the device capabilities are */
-       blk_queue_max_sectors(q, SHpnt->max_sectors);
-
-       if (!SHpnt->use_clustering)
-               clear_bit(QUEUE_FLAG_CLUSTER, &q->queue_flags);
-
-        blk_queue_prep_rq(q, scsi_prep_fn);
-}
-
 #ifdef MODULE
 MODULE_PARM(scsi_logging_level, "i");
 MODULE_PARM_DESC(scsi_logging_level, "SCSI logging level; should be zero or nonzero");

diff --git a/drivers/scsi/scsi.h b/drivers/scsi/scsi.h
index 8f6496f67cd1c3ace4f900e576968118f8010f35..64051a69c1317026fbad76ded2c40e2c81d11e60 100644 (file)
--- a/drivers/scsi/scsi.h
+++ b/drivers/scsi/scsi.h
@@ -511,6 +511,7 @@ static inline void scsi_proc_host_rm(struct Scsi_Host *);
  */
 extern int scsi_add_single_device(uint, uint, uint, uint);
 extern int scsi_remove_single_device(uint, uint, uint, uint);
+extern u64 scsi_calculate_bounce_limit(struct Scsi_Host *);
 
 /*
  * Prototypes for functions in constants.c

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 4a02c735dbf2964162e39b6a370aa6995f836b42..ac07ad6a954f792e23d46ca1cf5959460e87103a 100644 (file)
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -28,7 +28,6 @@
 #include <linux/config.h>
 #include <linux/module.h>
 #include <linux/init.h>
-
 #include <linux/blk.h>
 
 #include "scsi.h"
@@ -365,34 +364,60 @@ static void print_inquiry(unsigned char *inq_result)
                printk("\n");
 }
 
-/**
- * scsi_initialize_merge_fn() -ƣinitialize merge function for a host
- * @sd:                host descriptor
- */
-static void scsi_initialize_merge_fn(struct scsi_device *sd)
+u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
 {
-       request_queue_t *q = sd->request_queue;
-       struct Scsi_Host *sh = sd->host;
-       struct device *dev = scsi_get_device(sh);
-       u64 bounce_limit;
-
-       if (sh->highmem_io) {
-               if (dev && dev->dma_mask && PCI_DMA_BUS_IS_PHYS) {
-                       bounce_limit = *dev->dma_mask;
-               } else {
-                       /*
-                        * Platforms with virtual-DMA translation
-                        * hardware have no practical limit.
-                        */
-                       bounce_limit = BLK_BOUNCE_ANY;
-               }
-       } else if (sh->unchecked_isa_dma) {
-               bounce_limit = BLK_BOUNCE_ISA;
-       } else {
-               bounce_limit = BLK_BOUNCE_HIGH;
+       if (shost->highmem_io) {
+               struct device *host_dev = scsi_get_device(shost);
+
+               if (PCI_DMA_BUS_IS_PHYS && host_dev && host_dev->dma_mask)
+                       return *host_dev->dma_mask;
+
+               /*
+                * Platforms with virtual-DMA translation
+                * hardware have no practical limit.
+                */
+               return BLK_BOUNCE_ANY;
+       } else if (shost->unchecked_isa_dma)
+               return BLK_BOUNCE_ISA;
+
+       return BLK_BOUNCE_HIGH;
+}
+
+static request_queue_t *scsi_alloc_queue(struct Scsi_Host *shost)
+{
+       request_queue_t *q;
+
+       q = kmalloc(sizeof(*q), GFP_ATOMIC);
+       if (!q)
+               return NULL;
+       memset(q, 0, sizeof(*q));
+
+       if (!shost->max_sectors) {
+               /*
+                * Driver imposes no hard sector transfer limit.
+                * start at machine infinity initially.
+                */
+               shost->max_sectors = SCSI_DEFAULT_MAX_SECTORS;
        }
 
-       blk_queue_bounce_limit(q, bounce_limit);
+       blk_init_queue(q, scsi_request_fn, shost->host_lock);
+       blk_queue_prep_rq(q, scsi_prep_fn);
+
+       blk_queue_max_hw_segments(q, shost->sg_tablesize);
+       blk_queue_max_phys_segments(q, MAX_PHYS_SEGMENTS);
+       blk_queue_max_sectors(q, shost->max_sectors);
+       blk_queue_bounce_limit(q, scsi_calculate_bounce_limit(shost));
+
+       if (!shost->use_clustering)
+               clear_bit(QUEUE_FLAG_CLUSTER, &q->queue_flags);
+
+       return q;
+}
+
+static void scsi_free_queue(request_queue_t *q)
+{
+       blk_cleanup_queue(q);
+       kfree(q);
 }
 
 /**
@@ -435,19 +460,15 @@ static struct scsi_device *scsi_alloc_sdev(struct Scsi_Host *shost,
                 */
                sdev->borken = 1;
 
-               if(!q || *q == NULL) {
-                       sdev->request_queue = kmalloc(sizeof(struct request_queue), GFP_ATOMIC);
-                       if(sdev->request_queue == NULL) {
+               if (!q || *q == NULL) {
+                       sdev->request_queue = scsi_alloc_queue(shost);
+                       if (!sdev->request_queue)
                                goto out_bail;
-                       }
-                       memset(sdev->request_queue, 0,
-                                       sizeof(struct request_queue));
-                       scsi_initialize_queue(sdev, shost);
-                       scsi_initialize_merge_fn(sdev);
                } else {
                        sdev->request_queue = *q;
                        *q = NULL;
                }
+
                sdev->request_queue->queuedata = sdev;
                scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun);
                scsi_build_commandblocks(sdev);
@@ -488,13 +509,12 @@ static struct scsi_device *scsi_alloc_sdev(struct Scsi_Host *shost,
        }
 out_bail:
        printk(ALLOC_FAILURE_MSG, __FUNCTION__);
-       if(q && sdev->request_queue) {
+       if (q && sdev->request_queue) {
                *q = sdev->request_queue;
                sdev->request_queue = NULL;
-       } else if(sdev->request_queue) {
-               blk_cleanup_queue(sdev->request_queue);
-               kfree(sdev->request_queue);
-       }
+       } else if (sdev->request_queue)
+               scsi_free_queue(sdev->request_queue);
+
        scsi_release_commandblocks(sdev);
        kfree(sdev);
        return NULL;
@@ -513,14 +533,12 @@ static void scsi_free_sdev(struct scsi_device *sdev)
        list_del(&sdev->siblings);
        list_del(&sdev->same_target_siblings);
 
-       if(sdev->request_queue != NULL) {
-               blk_cleanup_queue(sdev->request_queue);
-               kfree(sdev->request_queue);
-       }
+       if (sdev->request_queue)
+               scsi_free_queue(sdev->request_queue);
        scsi_release_commandblocks(sdev);
        if (sdev->host->hostt->slave_destroy)
                sdev->host->hostt->slave_destroy(sdev);
-       if (sdev->inquiry != NULL)
+       if (sdev->inquiry)
                kfree(sdev->inquiry);
        kfree(sdev);
 }
@@ -1946,10 +1964,9 @@ void scsi_scan_host(struct Scsi_Host *shost)
                        scsi_scan_target(shost, &q, channel, order_id);
                }
        }
-       if(q) {
-               blk_cleanup_queue(q);
-               kfree(q);
-       }
+
+       if (q)
+               scsi_free_queue(q);
 }
 
 void scsi_forget_host(struct Scsi_Host *shost)

diff --git a/drivers/scsi/scsi_syms.c b/drivers/scsi/scsi_syms.c
index 85de9b673e3329a8f76aaaefab43874db8e5587d..c3f938e2a99c7e1578c1a1dfbda561081a7f6747 100644 (file)
--- a/drivers/scsi/scsi_syms.c
+++ b/drivers/scsi/scsi_syms.c
@@ -97,6 +97,11 @@ EXPORT_SYMBOL(scsi_host_hn_get);
 EXPORT_SYMBOL(scsi_host_put);
 EXPORT_SYMBOL(scsi_device_types);
 
+/*
+ * This is for st to find the bounce limit
+ */
+EXPORT_SYMBOL(scsi_calculate_bounce_limit);
+
 /*
  * Externalize timers so that HBAs can safely start/restart commands.
  */

diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 8dc808d1e7274ff862d0ba5fcdbf539fb33cd0b7..b463de9d1367cb5c651cc3c846842bafe7cafc61 100644 (file)
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -3764,21 +3764,9 @@ static int st_attach(Scsi_Device * SDp)
        tpnt->nbr_partitions = 0;
        tpnt->timeout = ST_TIMEOUT;
        tpnt->long_timeout = ST_LONG_TIMEOUT;
-
        tpnt->try_dio = try_direct_io && !SDp->host->unchecked_isa_dma;
-       bounce_limit = BLK_BOUNCE_HIGH; /* Borrowed from scsi_merge.c */
-       if (SDp->host->highmem_io) {
-               struct device *dev = scsi_get_device(SDp->host);
-               if (!PCI_DMA_BUS_IS_PHYS)
-                       /* Platforms with virtual-DMA translation
-                        * hardware have no practical limit.
-                        */
-                       bounce_limit = BLK_BOUNCE_ANY;
-               else if (dev && dev->dma_mask)
-                       bounce_limit = *dev->dma_mask;
-       } else if (SDp->host->unchecked_isa_dma)
-               bounce_limit = BLK_BOUNCE_ISA;
-       bounce_limit >>= PAGE_SHIFT;
+
+       bounce_limit = scsi_calculate_bounce_limit(SDp->host) >> PAGE_SHIFT;
        if (bounce_limit > ULONG_MAX)
                bounce_limit = ULONG_MAX;
        tpnt->max_pfn = bounce_limit;