public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH2.5] ips 3/4: use pci_dma APIs and remove GFP abuse
@ 2003-08-15 12:43 David Jeffery
  2003-08-15 21:09 ` Jeff Garzik
  0 siblings, 1 reply; 4+ messages in thread
From: David Jeffery @ 2003-08-15 12:43 UTC (permalink / raw)
  To: linux-scsi@vger.kernel.org

[-- Attachment #1: Type: text/plain, Size: 235 bytes --]

This patch removes several places where we were using kmalloc flag
tricks to always get memory <4GB instead of using the proper
dma_alloc_consistent() API.  It also no longer #errors on when compiled
for x86-64 kernels.

David Jeffery

[-- Attachment #2: test3.patch3 --]
[-- Type: text/x-patch, Size: 14483 bytes --]

diff -urN linux-2.6.0-test3_p2/drivers/scsi/ips.c linux-2.6.0-test3_p3/drivers/scsi/ips.c
--- linux-2.6.0-test3_p2/drivers/scsi/ips.c	2003-08-11 13:53:24.000000000 -0400
+++ linux-2.6.0-test3_p3/drivers/scsi/ips.c	2003-08-11 13:54:05.000000000 -0400
@@ -198,8 +198,8 @@
 #define IPS_VERSION_HIGH        "5.99"
 #define IPS_VERSION_LOW         ".01-BETA"
 
-#if !defined(__i386__) && !defined(__ia64__)
-#error "This driver has only been tested on the x86/ia64 platforms"
+#if !defined(__i386__) && !defined(__ia64__) && !defined(__x86_64__)
+#error "This driver has only been tested on the x86/ia64/x86_64 platforms"
 #endif
 
 #if LINUX_VERSION_CODE <= KERNEL_VERSION(2,5,0)
@@ -250,6 +250,7 @@
 static int ips_ioctlsize = IPS_IOCTL_SIZE;	/* Size of the ioctl buffer        */
 static int ips_cd_boot;		/* Booting from Manager CD         */
 static char *ips_FlashData = NULL;	/* CD Boot - Flash Data Buffer      */
+static dma_addr_t ips_flashbusaddr;
 static long ips_FlashDataInUse;	/* CD Boot - Flash Data In Use Flag */
 static uint32_t MaxLiteCmds = 32;	/* Max Active Cmds for a Lite Adapter */
 static Scsi_Host_Template ips_driver_template = {
@@ -589,17 +590,6 @@
 		ips_setup(ips);
 #endif
 
-	/* If Booting from the Manager CD, Allocate a large Flash        */
-	/* Buffer ( so we won't need to allocate one for each adapter ). */
-	if (ips_cd_boot) {
-		ips_FlashData = (char *) __get_free_pages(IPS_INIT_GFP, 7);
-		if (ips_FlashData == NULL) {
-			/* The validity of this pointer is checked in ips_make_passthru() before it is used */
-			printk(KERN_WARNING
-			       "ERROR: Can't Allocate Large Buffer for Flashing\n");
-		}
-	}
-
 	for (i = 0; i < ips_num_controllers; i++) {
 		if (ips_register_scsi(i))
 			ips_free(ips_ha[i]);
@@ -1630,21 +1620,20 @@
 ips_alloc_passthru_buffer(ips_ha_t * ha, int length)
 {
 	void *bigger_buf;
-	int count;
-	int order;
+	dma_addr_t dma_busaddr;
 
-	if (ha->ioctl_data && length <= (PAGE_SIZE << ha->ioctl_order))
+	if (ha->ioctl_data && length <= ha->ioctl_len)
 		return 0;
 	/* there is no buffer or it's not big enough, allocate a new one */
-	for (count = PAGE_SIZE, order = 0;
-	     count < length; order++, count <<= 1) ;
-	bigger_buf = (void *) __get_free_pages(IPS_ATOMIC_GFP, order);
+	bigger_buf = pci_alloc_consistent(ha->pcidev, length, &dma_busaddr);
 	if (bigger_buf) {
 		/* free the old memory */
-		free_pages((unsigned long) ha->ioctl_data, ha->ioctl_order);
+		pci_free_consistent(ha->pcidev, ha->ioctl_len, ha->ioctl_data,
+				    ha->ioctl_busaddr);
 		/* use the new memory */
 		ha->ioctl_data = (char *) bigger_buf;
-		ha->ioctl_order = order;
+		ha->ioctl_len = length;
+		ha->ioctl_busaddr = dma_busaddr;
 	} else {
 		return -1;
 	}
@@ -1761,7 +1750,7 @@
 static int
 ips_flash_copperhead(ips_ha_t * ha, ips_passthru_t * pt, ips_scb_t * scb)
 {
-	int datasize, count;
+	int datasize;
 
 	/* Trombone is the only copperhead that can do packet flash, but only
 	 * for firmware. No one said it had to make sence. */
@@ -1781,24 +1770,28 @@
 		pt->BasicStatus = 0;
 		return ips_flash_bios(ha, pt, scb);
 	} else if (pt->CoppCP.cmd.flashfw.packet_num == 0) {
-		if (ips_FlashData && !test_and_set_bit(0, &ips_FlashDataInUse)) {
+		if (ips_FlashData && !test_and_set_bit(0, &ips_FlashDataInUse)){
 			ha->flash_data = ips_FlashData;
-			ha->flash_order = 7;
+			ha->flash_busaddr = ips_flashbusaddr;
+			ha->flash_len = PAGE_SIZE << 7;
 			ha->flash_datasize = 0;
 		} else if (!ha->flash_data) {
 			datasize = pt->CoppCP.cmd.flashfw.total_packets *
 			    pt->CoppCP.cmd.flashfw.count;
-			for (count = PAGE_SIZE, ha->flash_order = 0;
-			     count < datasize; ha->flash_order++, count <<= 1) ;
-			ha->flash_data =
-			    (char *) __get_free_pages(IPS_ATOMIC_GFP,
-						      ha->flash_order);
+			ha->flash_data = pci_alloc_consistent(ha->pcidev,
+					                      datasize,
+							      &ha->flash_busaddr);
+			if (!ha->flash_data){
+				printk(KERN_WARNING "Unable to allocate a flash buffer\n");
+				return IPS_FAILURE;
+			}
 			ha->flash_datasize = 0;
+			ha->flash_len = datasize;
 		} else
 			return IPS_FAILURE;
 	} else {
 		if (pt->CoppCP.cmd.flashfw.count + ha->flash_datasize >
-		    (PAGE_SIZE << ha->flash_order)) {
+		    ha->flash_len) {
 			ips_free_flash_copperhead(ha);
 			IPS_PRINTK(KERN_WARNING, ha->pcidev,
 				   "failed size sanity check\n");
@@ -1987,7 +1980,8 @@
 	if (ha->flash_data == ips_FlashData)
 		test_and_clear_bit(0, &ips_FlashDataInUse);
 	else if (ha->flash_data)
-		free_pages((unsigned long) ha->flash_data, ha->flash_order);
+		pci_free_consistent(ha->pcidev, ha->flash_len, ha->flash_data,
+				    ha->flash_busaddr);
 	ha->flash_data = NULL;
 }
 
@@ -2040,12 +2034,7 @@
 
 	if (pt->CmdBSize) {
 		scb->data_len = pt->CmdBSize;
-		scb->data_busaddr = pci_map_single(ha->pcidev,
-						   ha->ioctl_data +
-						   sizeof (ips_passthru_t),
-						   pt->CmdBSize,
-						   IPS_DMA_DIR(scb));
-		scb->flags |= IPS_SCB_MAP_SINGLE;
+		scb->data_busaddr = ha->ioctl_busaddr + sizeof (ips_passthru_t);
 	} else {
 		scb->data_busaddr = 0L;
 	}
@@ -2469,9 +2458,7 @@
 	} else {
 		/* Morpheus Family - Send Command to the card */
 
-		buffer = kmalloc(0x1000, IPS_ATOMIC_GFP);
-		if (!buffer)
-			return;
+		buffer = ha->ioctl_data;
 
 		memset(buffer, 0, 0x1000);
 
@@ -2490,11 +2477,7 @@
 		scb->cmd.flashfw.total_packets = 1;
 		scb->cmd.flashfw.packet_num = 0;
 		scb->data_len = 0x1000;
-		scb->data_busaddr =
-		    pci_map_single(ha->pcidev, buffer, scb->data_len,
-				   IPS_DMA_DIR(scb));
-		scb->cmd.flashfw.buffer_addr = scb->data_busaddr;
-		scb->flags |= IPS_SCB_MAP_SINGLE;
+		scb->cmd.flashfw.buffer_addr = ha->ioctl_busaddr;
 
 		/* issue the command */
 		if (((ret =
@@ -2503,7 +2486,6 @@
 		    || (ret == IPS_SUCCESS_IMM)
 		    || ((scb->basic_status & IPS_GSC_STATUS_MASK) > 1)) {
 			/* Error occurred */
-			kfree(buffer);
 
 			return;
 		}
@@ -2513,11 +2495,8 @@
 			minor = buffer[0x1fe + 0xC0];	/* Offset 0x1fe after the header (0xc0) */
 			subminor = buffer[0x1fd + 0xC0];	/* Offset 0x1fd after the header (0xc0) */
 		} else {
-			kfree(buffer);
 			return;
 		}
-
-		kfree(buffer);
 	}
 
 	ha->bios_version[0] = hexDigits[(major & 0xF0) >> 4];
@@ -3984,11 +3963,7 @@
 			scb->cmd.basic_io.segment_4G = 0;
 			scb->cmd.basic_io.enhanced_sg = 0;
 			scb->data_len = sizeof (*ha->enq);
-			scb->data_busaddr = pci_map_single(ha->pcidev, ha->enq,
-							   scb->data_len,
-							   IPS_DMA_DIR(scb));
-			scb->cmd.basic_io.sg_addr = scb->data_busaddr;
-			scb->flags |= IPS_SCB_MAP_SINGLE;
+			scb->cmd.basic_io.sg_addr = ha->enq_busaddr;
 			ret = IPS_SUCCESS;
 			break;
 
@@ -4550,7 +4525,8 @@
 
 	if (ha) {
 		if (ha->enq) {
-			kfree(ha->enq);
+			pci_free_consistent(ha->pcidev, sizeof(IPS_ENQ),
+					    ha->enq, ha->enq_busaddr);
 			ha->enq = NULL;
 		}
 
@@ -4578,11 +4554,11 @@
 		}
 
 		if (ha->ioctl_data) {
-			free_pages((unsigned long) ha->ioctl_data,
-				   ha->ioctl_order);
+			pci_free_consistent(ha->pcidev, ha->ioctl_len,
+					    ha->ioctl_data, ha->ioctl_busaddr);
 			ha->ioctl_data = NULL;
 			ha->ioctl_datasize = 0;
-			ha->ioctl_order = 0;
+			ha->ioctl_len = 0;
 		}
 		ips_deallocatescbs(ha, ha->max_cmds);
 
@@ -5920,10 +5896,7 @@
 	scb->cmd.basic_io.sector_count = 0;
 	scb->cmd.basic_io.log_drv = 0;
 	scb->data_len = sizeof (*ha->enq);
-	scb->data_busaddr = pci_map_single(ha->pcidev, ha->enq, scb->data_len,
-					   IPS_DMA_DIR(scb));
-	scb->cmd.basic_io.sg_addr = scb->data_busaddr;
-	scb->flags |= IPS_SCB_MAP_SINGLE;
+	scb->cmd.basic_io.sg_addr = ha->enq_busaddr;
 
 	/* send command */
 	if (((ret =
@@ -5966,10 +5939,7 @@
 	scb->cmd.basic_io.sector_count = 0;
 	scb->cmd.basic_io.log_drv = 0;
 	scb->data_len = sizeof (*ha->subsys);
-	scb->data_busaddr = pci_map_single(ha->pcidev, ha->subsys,
-					   scb->data_len, IPS_DMA_DIR(scb));
-	scb->cmd.basic_io.sg_addr = scb->data_busaddr;
-	scb->flags |= IPS_SCB_MAP_SINGLE;
+	scb->cmd.basic_io.sg_addr = ha->ioctl_busaddr;
 
 	/* send command */
 	if (((ret =
@@ -5978,6 +5948,7 @@
 	    || ((scb->basic_status & IPS_GSC_STATUS_MASK) > 1))
 		return (0);
 
+	memcpy(ha->subsys, ha->ioctl_data, sizeof(*ha->subsys));
 	return (1);
 }
 
@@ -6013,10 +5984,7 @@
 	scb->cmd.basic_io.op_code = IPS_CMD_READ_CONF;
 	scb->cmd.basic_io.command_id = IPS_COMMAND_ID(ha, scb);
 	scb->data_len = sizeof (*ha->conf);
-	scb->data_busaddr = pci_map_single(ha->pcidev, ha->conf,
-					   scb->data_len, IPS_DMA_DIR(scb));
-	scb->cmd.basic_io.sg_addr = scb->data_busaddr;
-	scb->flags |= IPS_SCB_MAP_SINGLE;
+	scb->cmd.basic_io.sg_addr = ha->ioctl_busaddr;
 
 	/* send command */
 	if (((ret =
@@ -6037,7 +6005,8 @@
 
 		return (0);
 	}
-
+	
+	memcpy(ha->conf, ha->ioctl_data, sizeof(*ha->conf));
 	return (1);
 }
 
@@ -6072,11 +6041,10 @@
 	scb->cmd.nvram.reserved = 0;
 	scb->cmd.nvram.reserved2 = 0;
 	scb->data_len = sizeof (*ha->nvram);
-	scb->data_busaddr = pci_map_single(ha->pcidev, ha->nvram,
-					   scb->data_len, IPS_DMA_DIR(scb));
-	scb->cmd.nvram.buffer_addr = scb->data_busaddr;
-	scb->flags |= IPS_SCB_MAP_SINGLE;
-
+	scb->cmd.nvram.buffer_addr = ha->ioctl_busaddr;
+	if (write)
+		memcpy(ha->ioctl_data, ha->nvram, sizeof(*ha->nvram));
+	
 	/* issue the command */
 	if (((ret =
 	      ips_send_wait(ha, scb, ips_cmd_timeout, intr)) == IPS_FAILURE)
@@ -6087,7 +6055,8 @@
 
 		return (0);
 	}
-
+	if (!write)
+		memcpy(ha->nvram, ha->ioctl_data, sizeof(*ha->nvram));
 	return (1);
 }
 
@@ -7242,7 +7211,6 @@
 	uint16_t subdevice_id;
 	int j;
 	int index;
-	uint32_t count;
 	dma_addr_t dma_address;
 	char *ioremap_ptr;
 	char *mem_ptr;
@@ -7367,9 +7335,13 @@
 			return ips_abort_init(ha, index);
 		}
 	}
+	if(ips_cd_boot && !ips_FlashData){
+		ips_FlashData = pci_alloc_consistent(pci_dev, PAGE_SIZE << 7,
+						     &ips_flashbusaddr);
+	}
 
-	ha->enq = kmalloc(sizeof (IPS_ENQ), IPS_INIT_GFP);
-
+	ha->enq = pci_alloc_consistent(pci_dev, sizeof (IPS_ENQ),
+				       &ha->enq_busaddr);
 	if (!ha->enq) {
 		IPS_PRINTK(KERN_WARNING, pci_dev,
 			   "Unable to allocate host inquiry structure\n");
@@ -7386,7 +7358,7 @@
 	ha->adapt->hw_status_start = dma_address;
 	ha->dummy = (void *) (ha->adapt + 1);
 
-	ha->conf = kmalloc(sizeof (IPS_CONF), IPS_INIT_GFP);
+	ha->conf = kmalloc(sizeof (IPS_CONF), GFP_KERNEL);
 
 	if (!ha->conf) {
 		IPS_PRINTK(KERN_WARNING, pci_dev,
@@ -7394,7 +7366,7 @@
 		return ips_abort_init(ha, index);
 	}
 
-	ha->nvram = kmalloc(sizeof (IPS_NVRAM_P5), IPS_INIT_GFP);
+	ha->nvram = kmalloc(sizeof (IPS_NVRAM_P5), GFP_KERNEL);
 
 	if (!ha->nvram) {
 		IPS_PRINTK(KERN_WARNING, pci_dev,
@@ -7402,7 +7374,7 @@
 		return ips_abort_init(ha, index);
 	}
 
-	ha->subsys = kmalloc(sizeof (IPS_SUBSYS), IPS_INIT_GFP);
+	ha->subsys = kmalloc(sizeof (IPS_SUBSYS), GFP_KERNEL);
 
 	if (!ha->subsys) {
 		IPS_PRINTK(KERN_WARNING, pci_dev,
@@ -7410,19 +7382,18 @@
 		return ips_abort_init(ha, index);
 	}
 
-	for (count = PAGE_SIZE, ha->ioctl_order = 0;
-	     count < ips_ioctlsize; ha->ioctl_order++, count <<= 1) ;
-
-	ha->ioctl_data =
-	    (char *) __get_free_pages(IPS_INIT_GFP, ha->ioctl_order);
-	ha->ioctl_datasize = count;
-
+	/* the ioctl buffer is now used during adapter initialization, so its
+	 * successful allocation is now required */
+	if (ips_ioctlsize < PAGE_SIZE)
+		ips_ioctlsize = PAGE_SIZE;
+
+	ha->ioctl_data = pci_alloc_consistent(pci_dev, ips_ioctlsize,
+					      &ha->ioctl_busaddr);
+	ha->ioctl_len = ips_ioctlsize;
 	if (!ha->ioctl_data) {
 		IPS_PRINTK(KERN_WARNING, pci_dev,
 			   "Unable to allocate IOCTL data\n");
-		ha->ioctl_data = NULL;
-		ha->ioctl_order = 0;
-		ha->ioctl_datasize = 0;
+		return ips_abort_init(ha, index);
 	}
 
 	/*
diff -urN linux-2.6.0-test3_p2/drivers/scsi/ips.h linux-2.6.0-test3_p3/drivers/scsi/ips.h
--- linux-2.6.0-test3_p2/drivers/scsi/ips.h	2003-08-11 12:43:05.000000000 -0400
+++ linux-2.6.0-test3_p3/drivers/scsi/ips.h	2003-08-11 13:54:05.000000000 -0400
@@ -128,22 +128,13 @@
       #define min(x,y) ((x) < (y) ? x : y)
    #endif
 
+   #define pci_dma_hi32(a)         ((a >> 16) >> 16)
    #define pci_dma_lo32(a)         (a & 0xffffffff)
 
    #if (BITS_PER_LONG > 32) || (defined CONFIG_HIGHMEM64G && defined IPS_HIGHIO)
       #define IPS_ENABLE_DMA64        (1)
-      #define pci_dma_hi32(a)         (a >> 32)
    #else
       #define IPS_ENABLE_DMA64        (0)
-      #define pci_dma_hi32(a)         (0)
-   #endif
-
-   #if defined(__ia64__)
-      #define IPS_ATOMIC_GFP	(GFP_DMA | GFP_ATOMIC)
-      #define IPS_INIT_GFP	GFP_DMA
-   #else
-      #define IPS_ATOMIC_GFP    GFP_ATOMIC
-      #define IPS_INIT_GFP	GFP_KERNEL
    #endif
 
    /*
@@ -1110,7 +1101,8 @@
    uint16_t           device_id;          /* PCI device ID              */
    uint8_t            slot_num;           /* PCI Slot Number            */
    uint16_t           subdevice_id;       /* Subsystem device ID        */
-   uint8_t            ioctl_order;        /* Number of pages in ioctl   */
+   int                ioctl_len;          /* size of ioctl buffer       */
+   dma_addr_t         ioctl_busaddr;      /* dma address of ioctl buffer*/
    uint8_t            bios_version[8];    /* BIOS Revision              */
    uint32_t           mem_addr;           /* Memory mapped address      */
    uint32_t           io_len;             /* Size of IO Address         */
@@ -1120,8 +1112,10 @@
    ips_hw_func_t      func;               /* hw function pointers       */
    struct pci_dev    *pcidev;             /* PCI device handle          */
    char              *flash_data;         /* Save Area for flash data   */
-   u8                 flash_order;        /* Save Area for flash size order */
+   int                flash_len;          /* length of flash buffer     */
    u32                flash_datasize;     /* Save Area for flash data size */
+   dma_addr_t         flash_busaddr;      /* dma address of flash buffer*/
+   dma_addr_t         enq_busaddr;        /* dma address of enq struct  */
    uint8_t            requires_esl;       /* Requires an EraseStripeLock */
 } ips_ha_t;
 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* RE: [PATCH2.5] ips 3/4: use pci_dma APIs and remove GFP abuse
@ 2003-08-18 16:09 Jeffery, David
  2003-08-18 23:52 ` Jeff Garzik
  0 siblings, 1 reply; 4+ messages in thread
From: Jeffery, David @ 2003-08-18 16:09 UTC (permalink / raw)
  To: 'Jeff Garzik'; +Cc: linux-scsi


> -----Original Message-----
> From: Jeff Garzik
> Sent: Friday, August 15, 2003 5:10 PM
> To: Jeffery, David
> Cc: linux-scsi
> Subject: Re: [PATCH2.5] ips 3/4: use pci_dma APIs and remove GFP abuse
> 
> 
> David Jeffery wrote:
> > -#if !defined(__i386__) && !defined(__ia64__)
> > -#error "This driver has only been tested on the x86/ia64 platforms"
> > +#if !defined(__i386__) && !defined(__ia64__) && 
> !defined(__x86_64__)
> > +#error "This driver has only been tested on the 
> x86/ia64/x86_64 platforms"
> >  #endif
> 
> This should be removed completely.
> 
> Under Linux, you limit the architectures that build your driver via 
> Kconfig (or in 2.4, Config.in) dependencies.
> 
> Further, Linux actively encourages that all PCI platforms at least 
> attempt to build PCI drivers -- even if the company does not 
> support the 
> driver on that platform.  This leads to an increase in driver 
> quality, 
> as it increases the opportunities others have to point out bugs, and 
> offer fixes.

I need to fix up the Kconfig file.  How about I also downgrade that to
a #warning like Andi suggested earlier?
 
> 
> >  #if LINUX_VERSION_CODE <= KERNEL_VERSION(2,5,0)
> > @@ -250,6 +250,7 @@
> >  static int ips_ioctlsize = IPS_IOCTL_SIZE;	/* Size of the 
> ioctl buffer        */
> >  static int ips_cd_boot;		/* Booting from Manager 
> CD         */
> >  static char *ips_FlashData = NULL;	/* CD Boot - Flash Data 
> Buffer      */
> > +static dma_addr_t ips_flashbusaddr;
> >  static long ips_FlashDataInUse;	/* CD Boot - Flash Data 
> In Use Flag */
> >  static uint32_t MaxLiteCmds = 32;	/* Max Active Cmds for 
> a Lite Adapter */
> >  static Scsi_Host_Template ips_driver_template = {
> > @@ -589,17 +590,6 @@
> >  		ips_setup(ips);
> >  #endif
> >  
> > -	/* If Booting from the Manager CD, Allocate a large 
> Flash        */
> > -	/* Buffer ( so we won't need to allocate one for each 
> adapter ). */
> > -	if (ips_cd_boot) {
> > -		ips_FlashData = (char *) 
> __get_free_pages(IPS_INIT_GFP, 7);
> > -		if (ips_FlashData == NULL) {
> > -			/* The validity of this pointer is 
> checked in ips_make_passthru() before it is used */
> > -			printk(KERN_WARNING
> > -			       "ERROR: Can't Allocate Large 
> Buffer for Flashing\n");
> > -		}
> > -	}
> > -
> >  	for (i = 0; i < ips_num_controllers; i++) {
> >  		if (ips_register_scsi(i))
> >  			ips_free(ips_ha[i]);
> > @@ -1630,21 +1620,20 @@
> >  ips_alloc_passthru_buffer(ips_ha_t * ha, int length)
> >  {
> >  	void *bigger_buf;
> > -	int count;
> > -	int order;
> > +	dma_addr_t dma_busaddr;
> >  
> > -	if (ha->ioctl_data && length <= (PAGE_SIZE << ha->ioctl_order))
> > +	if (ha->ioctl_data && length <= ha->ioctl_len)
> >  		return 0;
> >  	/* there is no buffer or it's not big enough, allocate 
> a new one */
> > -	for (count = PAGE_SIZE, order = 0;
> > -	     count < length; order++, count <<= 1) ;
> > -	bigger_buf = (void *) __get_free_pages(IPS_ATOMIC_GFP, order);
> > +	bigger_buf = pci_alloc_consistent(ha->pcidev, length, 
> &dma_busaddr);
> 
> It's not wrong... but it's a bit nasty to be calling 
> pci_alloc_consistent from the interrupt handler.  Typically, 
> if you can, 
> it would be better to have buffers waiting for you, and you 
> simply map 
> them here, using pci_map_single or pci_map_page or whatever.

Normally, that is what happens.  A new, larger buffer will
only be allocated in a few very rare cases.  No user should
ever hit those allocations when using the ServeRAID tools.
Only a few special commands like flashing firmware should
ever trigger reallocation.

> Also, as a tangent, I was thinking that it might be 
> beneficial to move 
> ips_next() to a tasklet.  Change all callers to call 
> schedule_tasklet() 
> instead.  That should allow more streamlined usage, IMO...

Reworking ips_next is on my large list of Things To Do if I
Had Time.

> 
> 
> > @@ -7394,7 +7366,7 @@
> >  		return ips_abort_init(ha, index);
> >  	}
> >  
> > -	ha->nvram = kmalloc(sizeof (IPS_NVRAM_P5), IPS_INIT_GFP);
> > +	ha->nvram = kmalloc(sizeof (IPS_NVRAM_P5), GFP_KERNEL);
> >  
> >  	if (!ha->nvram) {
> >  		IPS_PRINTK(KERN_WARNING, pci_dev,
> > @@ -7402,7 +7374,7 @@
> >  		return ips_abort_init(ha, index);
> >  	}
> >  
> > -	ha->subsys = kmalloc(sizeof (IPS_SUBSYS), IPS_INIT_GFP);
> > +	ha->subsys = kmalloc(sizeof (IPS_SUBSYS), GFP_KERNEL);
> 
> Looks good.  Now if we could convince you to change structs to style 
> "struct foo" instead of SHOUTING STYLE, it would be even cooler :)

The code still has plenty of warts and I wish I had more time to work
on them. (Notice a theme about time? :)

David Jeffery

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2003-08-18 23:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-08-15 12:43 [PATCH2.5] ips 3/4: use pci_dma APIs and remove GFP abuse David Jeffery
2003-08-15 21:09 ` Jeff Garzik
  -- strict thread matches above, loose matches on Subject: below --
2003-08-18 16:09 Jeffery, David
2003-08-18 23:52 ` Jeff Garzik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox