Re: [PATCH 1/1] cciss: procfs updates to display info about many volumes

2008-02-19 Thread Andrew Morton
On Tue, 19 Feb 2008 11:48:18 +0100 Jens Axboe [EMAIL PROTECTED] wrote:

 On Mon, Feb 11 2008, Mike Miller wrote:
  Patch 1 of 1
  
  This patch allows us to display information about all of the logical volumes
  configured on a particular without stepping on memory even when there are
  many volumes (128 or more) configured. This patch replaces the one submitted
  on 20071214. See
  http://groups.google.com/group/linux.kernel/browse_thread/thread/49a50244b19f8855/ba3dc95b23391521?hl=enlnk=gstq=cciss#ba3dc95b23391521
  which has not been merged. That patch displayed information about only the
  first logical volume on each controller and had negative side effects for 
  some
  installers.
  Please consider this for inclusion.
 
 It looks ok, but has some flaws. Try to disable cciss scsi and tape
 support:
 
 In file included from drivers/block/cciss.c:231:
 drivers/block/cciss_scsi.c:1498:38: error: macro parameters must be
 comma-separated
 drivers/block/cciss.c: In function 'cciss_seq_show_header':
 drivers/block/cciss.c:272: error: implicit declaration of function
 'cciss_seq_tape_report'
 drivers/block/cciss.c: In function 'cciss_proc_write':
 drivers/block/cciss.c:393: error: implicit declaration of function
 'cciss_engage_scsi'
 
 You macro definition of cciss_seq_tape_report() is totally busted.
 Either write is as a macro OR as a function.
 
 Fix these up and resubmit, then I'll take it.
 

It also need to be updated to use the non-racy proc_create(),
please, as per Alexey's comments.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/1] cciss: procfs updates to display info about many volumes

2008-02-19 Thread Miller, Mike (OS Dev)


 -Original Message-
 From: Andrew Morton [mailto:[EMAIL PROTECTED]
 Sent: Tuesday, February 19, 2008 6:00 AM
 To: Jens Axboe
 Cc: Miller, Mike (OS Dev); LKML; LKML-scsi
 Subject: Re: [PATCH 1/1] cciss: procfs updates to display
 info about many volumes

 On Tue, 19 Feb 2008 11:48:18 +0100 Jens Axboe
 [EMAIL PROTECTED] wrote:

  On Mon, Feb 11 2008, Mike Miller wrote:
   Patch 1 of 1
  
   This patch allows us to display information about all of
 the logical
   volumes configured on a particular without stepping on
 memory even
   when there are many volumes (128 or more) configured. This patch
   replaces the one submitted on 20071214. See
  
 http://groups.google.com/group/linux.kernel/browse_thread/thread/49a
  
 50244b19f8855/ba3dc95b23391521?hl=enlnk=gstq=cciss#ba3dc95b2339152
   1 which has not been merged. That patch displayed
 information about
   only the first logical volume on each controller and had negative
   side effects for some installers.
   Please consider this for inclusion.
 
  It looks ok, but has some flaws. Try to disable cciss scsi and tape
  support:
 
  In file included from drivers/block/cciss.c:231:
  drivers/block/cciss_scsi.c:1498:38: error: macro parameters must be
  comma-separated
  drivers/block/cciss.c: In function 'cciss_seq_show_header':
  drivers/block/cciss.c:272: error: implicit declaration of function
  'cciss_seq_tape_report'
  drivers/block/cciss.c: In function 'cciss_proc_write':
  drivers/block/cciss.c:393: error: implicit declaration of function
  'cciss_engage_scsi'
 
  You macro definition of cciss_seq_tape_report() is totally busted.
  Either write is as a macro OR as a function.
 
  Fix these up and resubmit, then I'll take it.
 

 It also need to be updated to use the non-racy proc_create(),
 please, as per Alexey's comments.

Thanks all, I'll fix and resubmit.

-- mikem

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


[PATCH 1/1] cciss: procfs updates to display info about many volumes

2008-02-11 Thread Mike Miller
Patch 1 of 1

This patch allows us to display information about all of the logical volumes
configured on a particular without stepping on memory even when there are
many volumes (128 or more) configured. This patch replaces the one submitted
on 20071214. See
http://groups.google.com/group/linux.kernel/browse_thread/thread/49a50244b19f8855/ba3dc95b23391521?hl=enlnk=gstq=cciss#ba3dc95b23391521
which has not been merged. That patch displayed information about only the
first logical volume on each controller and had negative side effects for some
installers.
Please consider this for inclusion.

Signed-off-by: Mike Miller [EMAIL PROTECTED]

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 9715be3..b7cda85 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -33,6 +33,7 @@
 #include linux/blkpg.h
 #include linux/timer.h
 #include linux/proc_fs.h
+#include linux/seq_file.h
 #include linux/init.h
 #include linux/hdreg.h
 #include linux/spinlock.h
@@ -174,8 +175,6 @@ static int sendcmd_withirq(__u8 cmd, int ctlr, void *buff, 
size_t size,
 static void fail_all_cmds(unsigned long ctlr);
 
 #ifdef CONFIG_PROC_FS
-static int cciss_proc_get_info(char *buffer, char **start, off_t offset,
-  int length, int *eof, void *data);
 static void cciss_procinit(int i);
 #else
 static void cciss_procinit(int i)
@@ -240,24 +239,44 @@ static inline CommandList_struct 
*removeQ(CommandList_struct **Qptr,
  */
 #define ENG_GIG 10
 #define ENG_GIG_FACTOR (ENG_GIG/512)
+#define ENGAGE_SCSIengage scsi
 static const char *raid_label[] = { 0, 4, 1(1+0), 5, 5+1, ADG,
UNKNOWN
 };
 
 static struct proc_dir_entry *proc_cciss;
 
-static int cciss_proc_get_info(char *buffer, char **start, off_t offset,
-  int length, int *eof, void *data)
+static void cciss_seq_show_header(struct seq_file *seq)
 {
-   off_t pos = 0;
-   off_t len = 0;
-   int size, i, ctlr;
-   ctlr_info_t *h = (ctlr_info_t *) data;
-   drive_info_struct *drv;
-   unsigned long flags;
-   sector_t vol_sz, vol_sz_frac;
+   ctlr_info_t *h = seq-private;
+
+   seq_printf(seq, %s: HP %s Controller\n
+   Board ID: 0x%08lx\n
+   Firmware Version: %c%c%c%c\n
+   IRQ: %d\n
+   Logical drives: %d\n
+   Current Q depth: %d\n
+   Current # commands on controller: %d\n
+   Max Q depth since init: %d\n
+   Max # commands on controller since init: %d\n
+   Max SG entries since init: %d\n,
+   h-devname,
+   h-product_name,
+   (unsigned long)h-board_id,
+   h-firm_ver[0], h-firm_ver[1], h-firm_ver[2],
+   h-firm_ver[3], (unsigned int)h-intr[SIMPLE_MODE_INT],
+   h-num_luns,
+   h-Qdepth, h-commands_outstanding,
+   h-maxQsinceinit, h-max_outstanding, h-maxSG);
+
+   cciss_seq_tape_report(seq, h-ctlr);
+}
 
-   ctlr = h-ctlr;
+static void *cciss_seq_start(struct seq_file *seq, loff_t *pos)
+{
+   ctlr_info_t *h = seq-private;
+   unsigned ctlr = h-ctlr;
+   unsigned long flags;
 
/* prevent displaying bogus info during configuration
 * or deconfiguration of a logical volume
@@ -265,115 +284,150 @@ static int cciss_proc_get_info(char *buffer, char 
**start, off_t offset,
spin_lock_irqsave(CCISS_LOCK(ctlr), flags);
if (h-busy_configuring) {
spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags);
-   return -EBUSY;
+   return ERR_PTR(-EBUSY);
}
h-busy_configuring = 1;
spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags);
 
-   size = sprintf(buffer, %s: HP %s Controller\n
-  Board ID: 0x%08lx\n
-  Firmware Version: %c%c%c%c\n
-  IRQ: %d\n
-  Logical drives: %d\n
-  Max sectors: %d\n
-  Current Q depth: %d\n
-  Current # commands on controller: %d\n
-  Max Q depth since init: %d\n
-  Max # commands on controller since init: %d\n
-  Max SG entries since init: %d\n\n,
-  h-devname,
-  h-product_name,
-  (unsigned long)h-board_id,
-  h-firm_ver[0], h-firm_ver[1], h-firm_ver[2],
-  h-firm_ver[3], (unsigned int)h-intr[SIMPLE_MODE_INT],
-  h-num_luns,
-  h-cciss_max_sectors,
-  h-Qdepth, h-commands_outstanding,
-  h-maxQsinceinit, h-max_outstanding, h-maxSG);
-
-   pos += size;
-   len += size;
-   cciss_proc_tape_report(ctlr, buffer, pos, len);
-   for (i = 0; i = h-highest_lun; i++) {
-
-