[patch 14/16] qla2xxx: use memory_read_from_buffer()

[patch 14/16] qla2xxx: use memory_read_from_buffer()

[akpm]

From: Akinobu Mita <akinobu.mita@gmail.com>

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Andrew Vasquez <linux-driver@qlogic.com>
Cc: James E.J. Bottomley <James.Bottomley@HansenPartnership.com>
Tested-by: Andrew Vasquez <andrew.vasquez@qlogic.com>
Cc: Seokmann Ju <seokmann.ju@qlogic.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/scsi/qla2xxx/qla_attr.c |   45 ++++++------------------------
 1 file changed, 10 insertions(+), 35 deletions(-)

diff -puN drivers/scsi/qla2xxx/qla_attr.c~qla2xxx-use-memory_read_from_buffer drivers/scsi/qla2xxx/qla_attr.c
--- a/drivers/scsi/qla2xxx/qla_attr.c~qla2xxx-use-memory_read_from_buffer
+++ a/drivers/scsi/qla2xxx/qla_attr.c
@@ -8,6 +8,7 @@
 
 #include <linux/kthread.h>
 #include <linux/vmalloc.h>
+#include <linux/fs.h>
 
 static int qla24xx_vport_disable(struct fc_vport *, bool);
 
@@ -20,18 +21,12 @@ qla2x00_sysfs_read_fw_dump(struct kobjec
 {
 	struct scsi_qla_host *ha = shost_priv(dev_to_shost(container_of(kobj,
 	    struct device, kobj)));
-	char *rbuf = (char *)ha->fw_dump;
 
 	if (ha->fw_dump_reading == 0)
 		return 0;
-	if (off > ha->fw_dump_len)
-                return 0;
-	if (off + count > ha->fw_dump_len)
-		count = ha->fw_dump_len - off;
 
-	memcpy(buf, &rbuf[off], count);
-
-	return (count);
+	return memory_read_from_buffer(buf, count, &off, ha->fw_dump,
+					ha->fw_dump_len);
 }
 
 static ssize_t
@@ -94,20 +89,13 @@ qla2x00_sysfs_read_nvram(struct kobject 
 {
 	struct scsi_qla_host *ha = shost_priv(dev_to_shost(container_of(kobj,
 	    struct device, kobj)));
-	int		size = ha->nvram_size;
-	char		*nvram_cache = ha->nvram;
 
-	if (!capable(CAP_SYS_ADMIN) || off > size || count == 0)
+	if (!capable(CAP_SYS_ADMIN))
 		return 0;
-	if (off + count > size) {
-		size -= off;
-		count = size;
-	}
 
 	/* Read NVRAM data from cache. */
-	memcpy(buf, &nvram_cache[off], count);
-
-	return count;
+	return memory_read_from_buffer(buf, count, &off, ha->nvram,
+					ha->nvram_size);
 }
 
 static ssize_t
@@ -175,14 +163,9 @@ qla2x00_sysfs_read_optrom(struct kobject
 
 	if (ha->optrom_state != QLA_SREADING)
 		return 0;
-	if (off > ha->optrom_region_size)
-		return 0;
-	if (off + count > ha->optrom_region_size)
-		count = ha->optrom_region_size - off;
 
-	memcpy(buf, &ha->optrom_buffer[off], count);
-
-	return count;
+	return memory_read_from_buffer(buf, count, &off, ha->optrom_buffer,
+					ha->optrom_region_size);
 }
 
 static ssize_t
@@ -374,20 +357,12 @@ qla2x00_sysfs_read_vpd(struct kobject *k
 {
 	struct scsi_qla_host *ha = shost_priv(dev_to_shost(container_of(kobj,
 	    struct device, kobj)));
-	int           size = ha->vpd_size;
-	char          *vpd_cache = ha->vpd;
 
-	if (!capable(CAP_SYS_ADMIN) || off > size || count == 0)
+	if (!capable(CAP_SYS_ADMIN))
 		return 0;
-	if (off + count > size) {
-		size -= off;
-		count = size;
-	}
 
 	/* Read NVRAM data from cache. */
-	memcpy(buf, &vpd_cache[off], count);
-
-	return count;
+	return memory_read_from_buffer(buf, count, &off, ha->vpd, ha->vpd_size);
 }
 
 static ssize_t
_

Re: [patch 14/16] qla2xxx: use memory_read_from_buffer()

[James Bottomley]

On Thu, 2008-07-03 at 23:47 -0700, akpm@linux-foundation.org wrote:
> From: Akinobu Mita <akinobu.mita@gmail.com>
> 
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Andrew Vasquez <linux-driver@qlogic.com>
> Cc: James E.J. Bottomley <James.Bottomley@HansenPartnership.com>
> Tested-by: Andrew Vasquez <andrew.vasquez@qlogic.com>
> Cc: Seokmann Ju <seokmann.ju@qlogic.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  drivers/scsi/qla2xxx/qla_attr.c |   45 ++++++------------------------
>  1 file changed, 10 insertions(+), 35 deletions(-)
> 
> diff -puN drivers/scsi/qla2xxx/qla_attr.c~qla2xxx-use-memory_read_from_buffer drivers/scsi/qla2xxx/qla_attr.c
> --- a/drivers/scsi/qla2xxx/qla_attr.c~qla2xxx-use-memory_read_from_buffer
> +++ a/drivers/scsi/qla2xxx/qla_attr.c
> @@ -8,6 +8,7 @@
>  
>  #include <linux/kthread.h>
>  #include <linux/vmalloc.h>
> +#include <linux/fs.h>

For such a micro optimisation, this is a pretty big price to pay.

Inclusion of linux/fs.h in a low level driver was always a danger signal
usually it meant the driver was trying to access files or something at
the very least it was a warning of a potential layering violation.  Now
you're trying to make it standard practice ... I really don't like that.
Surely a function that does memory to memory copies belongs either in
string.h with the rest of our memory copies ... or in another header
that would be a usual include for the potential users.

James

[PATCH] move memory_read_from_buffer() from fs.h to string.h

[Akinobu Mita]

On Fri, Jul 04, 2008 at 09:42:35AM -0500, James Bottomley wrote:
> For such a micro optimisation, this is a pretty big price to pay.
> 
> Inclusion of linux/fs.h in a low level driver was always a danger signal
> usually it meant the driver was trying to access files or something at
> the very least it was a warning of a potential layering violation.  Now
> you're trying to make it standard practice ... I really don't like that.
> Surely a function that does memory to memory copies belongs either in
> string.h with the rest of our memory copies ... or in another header
> that would be a usual include for the potential users.

OK, I'll move it to linux/string.h

From: Akinobu Mita <akinobu.mita@gmail.com>
Subject: [PATCH] move memory_read_from_buffer() from fs.h to string.h

James Bottomley warns that inclusion of linux/fs.h in a low level driver
was always a danger signal. This patch moves memory_read_from_buffer()
from fs.h to string.h and fixes includes in existing memory_read_from_buffer()
users.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: James Bottomley <James.Bottomley@hansenpartnership.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Bob Moore <robert.moore@intel.com>
Cc: Thomas Renninger <trenn@suse.de>
Cc: Len Brown <lenb@kernel.org>
---
 drivers/acpi/system.c       |    1 +
 drivers/zorro/zorro-sysfs.c |    1 -
 include/linux/fs.h          |    2 --
 include/linux/string.h      |    3 +++
 4 files changed, 4 insertions(+), 3 deletions(-)

Index: 2.6-mm/include/linux/fs.h
===================================================================
--- 2.6-mm.orig/include/linux/fs.h
+++ 2.6-mm/include/linux/fs.h
@@ -2026,8 +2026,6 @@ extern void simple_release_fs(struct vfs
 
 extern ssize_t simple_read_from_buffer(void __user *to, size_t count,
 			loff_t *ppos, const void *from, size_t available);
-extern ssize_t memory_read_from_buffer(void *to, size_t count, loff_t *ppos,
-			const void *from, size_t available);
 
 #ifdef CONFIG_MIGRATION
 extern int buffer_migrate_page(struct address_space *,
Index: 2.6-mm/include/linux/string.h
===================================================================
--- 2.6-mm.orig/include/linux/string.h
+++ 2.6-mm/include/linux/string.h
@@ -111,5 +111,8 @@ extern void argv_free(char **argv);
 
 extern bool sysfs_streq(const char *s1, const char *s2);
 
+extern ssize_t memory_read_from_buffer(void *to, size_t count, loff_t *ppos,
+			const void *from, size_t available);
+
 #endif
 #endif /* _LINUX_STRING_H_ */
Index: 2.6-mm/drivers/acpi/system.c
===================================================================
--- 2.6-mm.orig/drivers/acpi/system.c
+++ 2.6-mm/drivers/acpi/system.c
@@ -26,6 +26,7 @@
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #include <linux/init.h>
+#include <linux/string.h>
 #include <asm/uaccess.h>
 
 #include <acpi/acpi_drivers.h>
Index: 2.6-mm/drivers/zorro/zorro-sysfs.c
===================================================================
--- 2.6-mm.orig/drivers/zorro/zorro-sysfs.c
+++ 2.6-mm/drivers/zorro/zorro-sysfs.c
@@ -15,7 +15,6 @@
 #include <linux/zorro.h>
 #include <linux/stat.h>
 #include <linux/string.h>
-#include <linux/fs.h>
 
 #include "zorro.h"