Re: [PATCH 03/10] gdth: remove gdth_{alloc,free}_ioctl

2018-12-06 Thread Johannes Thumshirn
On 06/12/2018 17:50, Johannes Thumshirn wrote:
> Why not calling dma_alloc_coherent() directly instead of using the
> pci_alloc_consistent() wrapper?

Ah should've read the whole series


-- 
Johannes ThumshirnSUSE Labs Filesystems
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [PATCH 03/10] gdth: remove gdth_{alloc,free}_ioctl

2018-12-06 Thread Johannes Thumshirn
On 06/12/2018 16:57, Christoph Hellwig wrote:
> Out of the three callers once insists on the scratch buffer, and the
> others are fine with a new allocation.  Switch those two to juse use
> pci_alloc_consistent directly, and open code the scratch buffer
> allocation in the remaining one.  This avoids a case where we might
> be doing a memory allocation under a spinlock with irqs disabled.

Why not calling dma_alloc_coherent() directly instead of using the
pci_alloc_consistent() wrapper?

Johannes
-- 
Johannes ThumshirnSUSE Labs Filesystems
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


[PATCH 03/10] gdth: remove gdth_{alloc,free}_ioctl

2018-12-06 Thread Christoph Hellwig
Out of the three callers once insists on the scratch buffer, and the
others are fine with a new allocation.  Switch those two to juse use
pci_alloc_consistent directly, and open code the scratch buffer
allocation in the remaining one.  This avoids a case where we might
be doing a memory allocation under a spinlock with irqs disabled.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/gdth.c  |  7 ++--
 drivers/scsi/gdth_proc.c | 71 
 drivers/scsi/gdth_proc.h |  3 --
 3 files changed, 25 insertions(+), 56 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 45e67d4cb3af..7174e7a88da2 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -4232,7 +4232,7 @@ static int ioc_general(void __user *arg, char *cmnd)
gdth_ioctl_general gen;
gdth_ha_str *ha;
char *buf = NULL;
-   u64 paddr;
+   dma_addr_t paddr;
int rval;
 
if (copy_from_user(, arg, sizeof(gdth_ioctl_general)))
@@ -4251,7 +4251,8 @@ static int ioc_general(void __user *arg, char *cmnd)
if (gen.data_len + gen.sense_len == 0)
goto execute;
 
-   buf = gdth_ioctl_alloc(ha, gen.data_len + gen.sense_len, FALSE, );
+buf = pci_alloc_consistent(ha->pdev, gen.data_len + gen.sense_len,
+   );
if (!buf)
return -EFAULT;
 
@@ -4286,7 +4287,7 @@ static int ioc_general(void __user *arg, char *cmnd)
 
rval = 0;
 out_free_buf:
-   gdth_ioctl_free(ha, gen.data_len+gen.sense_len, buf, paddr);
+   pci_free_consistent(ha->pdev, gen.data_len + gen.sense_len, buf, paddr);
return rval;
 }
  
diff --git a/drivers/scsi/gdth_proc.c b/drivers/scsi/gdth_proc.c
index bd5532a80b0e..8e77f8fd8641 100644
--- a/drivers/scsi/gdth_proc.c
+++ b/drivers/scsi/gdth_proc.c
@@ -31,7 +31,6 @@ static int gdth_set_asc_info(struct Scsi_Host *host, char 
*buffer,
 int i, found;
 gdth_cmd_strgdtcmd;
 gdth_cpar_str   *pcpar;
-u64 paddr;
 
 charcmnd[MAX_COMMAND_SIZE];
 memset(cmnd, 0xff, 12);
@@ -113,13 +112,23 @@ static int gdth_set_asc_info(struct Scsi_Host *host, char 
*buffer,
 }
 
 if (wb_mode) {
-if (!gdth_ioctl_alloc(ha, sizeof(gdth_cpar_str), TRUE, ))
-return(-EBUSY);
+   unsigned long flags;
+
+   BUILD_BUG_ON(sizeof(gdth_cpar_str) > GDTH_SCRATCH);
+
+   spin_lock_irqsave(>smp_lock, flags);
+   if (ha->scratch_busy) {
+   spin_unlock_irqrestore(>smp_lock, flags);
+return -EBUSY;
+   }
+   ha->scratch_busy = TRUE;
+   spin_unlock_irqrestore(>smp_lock, flags);
+
 pcpar = (gdth_cpar_str *)ha->pscratch;
 memcpy( pcpar, >cpar, sizeof(gdth_cpar_str) );
 gdtcmd.Service = CACHESERVICE;
 gdtcmd.OpCode = GDT_IOCTL;
-gdtcmd.u.ioctl.p_param = paddr;
+gdtcmd.u.ioctl.p_param = ha->scratch_phys;
 gdtcmd.u.ioctl.param_size = sizeof(gdth_cpar_str);
 gdtcmd.u.ioctl.subfunc = CACHE_CONFIG;
 gdtcmd.u.ioctl.channel = INVALID_CHANNEL;
@@ -127,7 +136,10 @@ static int gdth_set_asc_info(struct Scsi_Host *host, char 
*buffer,
 
 gdth_execute(host, , cmnd, 30, NULL);
 
-gdth_ioctl_free(ha, GDTH_SCRATCH, ha->pscratch, paddr);
+   spin_lock_irqsave(>smp_lock, flags);
+   ha->scratch_busy = FALSE;
+   spin_unlock_irqrestore(>smp_lock, flags);
+
 printk("Done.\n");
 return(orig_length);
 }
@@ -143,7 +155,7 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host 
*host)
 int id, i, j, k, sec, flag;
 int no_mdrv = 0, drv_no, is_mirr;
 u32 cnt;
-u64 paddr;
+dma_addr_t paddr;
 int rc = -ENOMEM;
 
 gdth_cmd_str *gdtcmd;
@@ -232,7 +244,7 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host 
*host)
 seq_puts(m, "\nPhysical Devices:");
 flag = FALSE;
 
-buf = gdth_ioctl_alloc(ha, size, FALSE, );
+buf = pci_alloc_consistent(ha->pdev, size, );
 if (!buf) 
 goto stop_output;
 for (i = 0; i < ha->bus_cnt; ++i) {
@@ -406,7 +418,7 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host 
*host)
 seq_printf(m,
" To Array Drv.:\t%s\n", hrec);
 }   
-
+
 if (!flag)
 seq_puts(m, "\n --\n");
 
@@ -500,7 +512,7 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host 
*host)
 }
 }
 }
-gdth_ioctl_free(ha, size, buf, paddr);
+   pci_free_consistent(ha->pdev, size, buf, paddr);
 
 for (i = 0; i < MAX_HDRIVES; ++i) {
 if (!(ha->hdr[i].present))
@@ -553,47 +565,6 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host 
*host)
 return rc;
 }
 
-static char *gdth_ioctl_alloc(gdth_ha_str *ha, int size, int scratch,
-  u64 *paddr)
-{
-unsigned long flags;
-char *ret_val;
-
-if (size ==