public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: James.Bottomley@steeleye.com, linux-scsi@vger.kernel.org
Subject: Re: scsi_scan.c complaints...
Date: Sat, 21 Dec 2002 18:42:25 +0000	[thread overview]
Message-ID: <20021221184225.A3075@infradead.org> (raw)

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(-)


--- 1.46/drivers/scsi/hosts.h	Thu Nov 28 14:36:44 2002
+++ edited/drivers/scsi/hosts.h	Sat Dec 21 16:47:36 2002
@@ -554,9 +554,6 @@
     struct device_driver scsi_driverfs_driver;
 };
 
-void  scsi_initialize_queue(Scsi_Device *, struct Scsi_Host *);
-
-
 /*
  * Highlevel driver registration/unregistration.
  */
--- 1.81/drivers/scsi/scsi.c	Fri Dec 20 20:59:59 2002
+++ edited/drivers/scsi/scsi.c	Sat Dec 21 16:34:24 2002
@@ -147,49 +147,6 @@
 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");
--- 1.53/drivers/scsi/scsi.h	Fri Dec 20 20:59:59 2002
+++ edited/drivers/scsi/scsi.h	Sat Dec 21 16:54:22 2002
@@ -511,6 +511,7 @@
  */
 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
--- 1.50/drivers/scsi/scsi_scan.c	Fri Dec 20 21:00:00 2002
+++ edited/drivers/scsi/scsi_scan.c	Sat Dec 21 17:44:03 2002
@@ -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 @@
 		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 @@
 		 */
 		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 @@
 	}
 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 @@
 	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 @@
 			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)
--- 1.23/drivers/scsi/scsi_syms.c	Thu Nov 28 09:09:53 2002
+++ edited/drivers/scsi/scsi_syms.c	Sat Dec 21 17:52:49 2002
@@ -98,6 +98,11 @@
 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.
  */
 extern void scsi_add_timer(Scsi_Cmnd *, int, void ((*) (Scsi_Cmnd *)));
--- 1.51/drivers/scsi/st.c	Mon Dec 16 10:55:37 2002
+++ edited/drivers/scsi/st.c	Sat Dec 21 16:53:05 2002
@@ -3764,21 +3764,9 @@
 	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;




-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

             reply	other threads:[~2002-12-21 18:42 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-12-21 18:42 Christoph Hellwig [this message]
  -- strict thread matches above, loose matches on Subject: below --
2002-12-14 21:55 Aic7xxx v6.2.22 and Aic79xx v1.3.0Alpha2 Released Gérard Roudier
2002-12-14 23:29 ` Justin T. Gibbs
2002-12-19 18:56   ` scsi_scan.c complaints Doug Ledford
2002-12-21  1:29     ` Doug Ledford

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20021221184225.A3075@infradead.org \
    --to=hch@infradead.org \
    --cc=James.Bottomley@steeleye.com \
    --cc=linux-scsi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox