scsi proc_info called unconditionally

scsi proc_info called unconditionally

[Olaf Hering]

Why is ->proc_info() called when the function pointer is NULL?

(none):/# mount proc
(none):/# cat /proc/scsi/
53c94        device_info  scsi
(none):/# cat /proc/scsi/53c94/0
proc_scsi_read: 00000000 called
Oops: kernel access of bad area, sig: 11 [#1]
NIP: 00000000 LR: C00EC6C8 SP: C03BBE90 REGS: c03bbde0 TRAP: 0401    Not tainted
MSR: 40009030 EE: 1 PR: 0 FP: 0 ME: 1 IR/DR: 11
TASK = c027b7e0[16] 'cat' Last syscall: 3
GPR00: C00EC6A0 C03BBE90 C027B7E0 C4705400 C43B5000 C03BBEC8 00000000 00000C00
GPR08: 00000000 00000000 C01C6E8F C01B0000 0000000D
Call trace:
 [c007a254] proc_file_read+0xb8/0x2cc
 [c004d97c] vfs_read+0xdc/0x128
 [c004dbe8] sys_read+0x40/0x74
 [c000780c] ret_from_syscall+0x0/0x4c
Segmentation fault


-- 
USB is for mice, FireWire is for men!

sUse lINUX ag, nÜRNBERG

Re: scsi proc_info called unconditionally

[Christoph Hellwig]

On Sat, Aug 16, 2003 at 10:44:09AM +0200, Olaf Hering wrote:
> 
> Why is ->proc_info() called when the function pointer is NULL?

Looks like the check for it's presence got lost in

	[PATCH] Correct removal of procfs host enteries [1/2]

Here's a trivial patch to get it back:


--- 1.32/drivers/scsi/scsi_proc.c	Thu Jul 31 10:31:51 2003
+++ edited/drivers/scsi/scsi_proc.c	Sat Aug 16 10:31:37 2003
@@ -81,6 +81,9 @@
 
 void scsi_proc_hostdir_add(struct scsi_host_template *sht)
 {
+	if (!sht->proc_info)
+		return;
+
 	down(&global_host_template_sem);
 	if (!sht->present++) {
 		sht->proc_dir = proc_mkdir(sht->proc_name, proc_scsi);
@@ -96,6 +99,9 @@
 
 void scsi_proc_hostdir_rm(struct scsi_host_template *sht)
 {
+	if (!sht->proc_info)
+		return;
+
 	down(&global_host_template_sem);
 	if (!--sht->present && sht->proc_dir) {
 		remove_proc_entry(sht->proc_name, proc_scsi);

Re: scsi proc_info called unconditionally

[Alan Cox]

On Sad, 2003-08-16 at 09:44, Olaf Hering wrote:
> Why is ->proc_info() called when the function pointer is NULL?
> 
> (none):/# mount proc

Because proc_info is mandatory ?

Re: scsi proc_info called unconditionally

[Olaf Hering]

 On Sat, Aug 16, Alan Cox wrote:

> On Sad, 2003-08-16 at 09:44, Olaf Hering wrote:
> > Why is ->proc_info() called when the function pointer is NULL?
> > 
> > (none):/# mount proc
> 
> Because proc_info is mandatory ?

Paul, do you want to fill in some content in that proc file?


--- linuxppc-2.5/drivers/scsi/mac53c94.c	2003-08-04 01:59:22.000000000 +0200
+++ linux-2.6.0-test3-lxppc25/drivers/scsi/mac53c94.c	2003-08-17 10:00:06.000000000 +0200
@@ -614,8 +614,15 @@ data_goes_out(Scsi_Cmnd *cmd)
 	}
 }
 
+int mac53c94_proc_info(struct Scsi_Host *host, char *buffer, char **start, off_t offset,
+		  int length, int inout)
+{
+	return 0;
+}
+
 static Scsi_Host_Template driver_template = {
 	.proc_name	= "53c94",
+	.proc_info	= mac53c94_proc_info,
 	.name		= "53C94",
 	.detect		= mac53c94_detect,
 	.release	= mac53c94_release,

-- 
USB is for mice, FireWire is for men!

sUse lINUX ag, nÜRNBERG

Re: scsi proc_info called unconditionally

[Christoph Hellwig]

On Sun, Aug 17, 2003 at 10:09:01AM +0200, Olaf Hering wrote:
> Paul, do you want to fill in some content in that proc file?

No!  proc_info is deprecated in 2.6 and you should not add a new
implementation.  If you want to expose information to userland
use sysfs.

Re: scsi proc_info called unconditionally

[Alan Cox]

On Sul, 2003-08-17 at 10:39, Christoph Hellwig wrote:
> No!  proc_info is deprecated in 2.6 and you should not add a new
> implementation.  If you want to expose information to userland
> use sysfs.

It probably should give the driver name and version. Christoph is
right in the longer term but in the real world people still expect
/proc/scsi/* to contain at least that info.

Possibly for 2.7.x /proc/scsi/* should become an fs or entirely sysfs

Re: scsi proc_info called unconditionally

[Christoph Hellwig]

On Sun, Aug 17, 2003 at 10:56:17AM +0100, Alan Cox wrote:
> It probably should give the driver name and version. Christoph is
> right in the longer term but in the real world people still expect
> /proc/scsi/* to contain at least that info.

/proc/scsi/* is not standardized so even if you add a name no
userspace tool can sanely parse it without knowing all drivers.
sysfs OTOH can do this sanely.

So, no it does not make sense to add anything here for a 2.6 driver.

> Possibly for 2.7.x /proc/scsi/* should become an fs or entirely sysfs

/proc/scsi/ will be optional and marked deprecated with with one of my
next patches and go away in 2.9.