smbfs Oops with Linux 2.6.3

smbfs Oops with Linux 2.6.3

[Adam Sampson]

Hiya.

I use smbfs on my x86 Linux 2.6.3 machine to mount filesystems from a
(Debian) Samba 3.0.0beta2 server. This has worked fine with both this
kernel and previous ones for the last few months, but I've just had an
Oops message while trying to open a directory with ROX-Filer. The
filesystem in question is automounted (using autofs4), and this would
have been the first operation upon it after being mounted.

My kernel config is at <http://offog.org/stuff/config-2.6.3>. Here's the
ksymoops output:

ksymoops 2.4.9 on i686 2.6.3.  Options used
     -v /src/linux-2.6.3/vmlinux (specified)
     -k /proc/ksyms (default)
     -l /proc/modules (default)
     -o /lib/modules/2.6.3/ (default)
     -m /src/linux-2.6.3/System.map (specified)

Error (regular_file): read_ksyms stat /proc/ksyms failed
No modules in ksyms, skipping objects
No ksyms, skipping lsmod
Unable to handle kernel NULL pointer dereference at virtual address 00000000
00000000
*pde = 00000000
Oops: 0000 [#1]
CPU:    0
EIP:    0060:[<00000000>]    Not tainted
Using defaults from ksymoops -t elf32-i386 -a i386
EFLAGS: 00210246
eax: ee433c40   ebx: 00000002   ecx: c5107840   edx: cf84bfa0
esi: c0165560   edi: c1750080   ebp: cf84bf68   esp: cf84bef8
ds: 007b   es: 007b   ss: 0068
Stack: c01dbbfb c5107840 cf84bfa0 c0165560 cf84bf2c 00000000 00000002 00000004 
       d4ea7ee4 00000000 eecd0000 c40520c0 d0a0c980 00000000 5e50af11 00000000 
       00000000 00000000 eecd0000 00000002 00000000 00000000 00000001 00000004 
Call Trace:
 [<c01dbbfb>] smb_readdir+0x4fb/0x6e0
 [<c0165560>] filldir64+0x0/0x130
 [<c016524a>] vfs_readdir+0x8a/0x90
 [<c0165560>] filldir64+0x0/0x130
 [<c01656fd>] sys_getdents64+0x6d/0xa6
 [<c0165560>] filldir64+0x0/0x130
 [<c010adff>] syscall_call+0x7/0xb
Code:  Bad EIP value.


>>EIP; 00000000 Before first symbol

>>eax; ee433c40 <_end+2dc7cad8/3f846e98>
>>ecx; c5107840 <_end+49506d8/3f846e98>
>>edx; cf84bfa0 <_end+f094e38/3f846e98>
>>esi; c0165560 <filldir64+0/130>
>>edi; c1750080 <_end+f98f18/3f846e98>
>>ebp; cf84bf68 <_end+f094e00/3f846e98>
>>esp; cf84bef8 <_end+f094d90/3f846e98>

Trace; c01dbbfb <smb_readdir+4fb/6e0>
Trace; c0165560 <filldir64+0/130>
Trace; c016524a <vfs_readdir+8a/90>
Trace; c0165560 <filldir64+0/130>
Trace; c01656fd <sys_getdents64+6d/a6>
Trace; c0165560 <filldir64+0/130>
Trace; c010adff <syscall_call+7/b>


1 error issued.  Results may not be reliable.

And here's the raw Oops:

Unable to handle kernel NULL pointer dereference at virtual address 00000000
 printing eip:
00000000
*pde = 00000000
Oops: 0000 [#1]
CPU:    0
EIP:    0060:[<00000000>]    Not tainted
EFLAGS: 00210246
EIP is at 0x0
eax: ee433c40   ebx: 00000002   ecx: c5107840   edx: cf84bfa0
esi: c0165560   edi: c1750080   ebp: cf84bf68   esp: cf84bef8
ds: 007b   es: 007b   ss: 0068
Process ROX-Filer (pid: 19894, threadinfo=cf84a000 task=d0b93320)
Stack: c01dbbfb c5107840 cf84bfa0 c0165560 cf84bf2c 00000000 00000002 00000004 
       d4ea7ee4 00000000 eecd0000 c40520c0 d0a0c980 00000000 5e50af11 00000000 
       00000000 00000000 eecd0000 00000002 00000000 00000000 00000001 00000004 
Call Trace:
 [<c01dbbfb>] smb_readdir+0x4fb/0x6e0
 [<c0165560>] filldir64+0x0/0x130
 [<c016524a>] vfs_readdir+0x8a/0x90
 [<c0165560>] filldir64+0x0/0x130
 [<c01656fd>] sys_getdents64+0x6d/0xa6
 [<c0165560>] filldir64+0x0/0x130
 [<c010adff>] syscall_call+0x7/0xb

Code:  Bad EIP value.

If there's any other information that'd be useful, please let me know.

Thanks,

-- 
Adam Sampson <azz@us-lot.org>                        <http://offog.org/>

Re: smbfs Oops with Linux 2.6.3

[Linus Torvalds]

On Wed, 10 Mar 2004, Adam Sampson wrote:
> 
> I use smbfs on my x86 Linux 2.6.3 machine to mount filesystems from a
> (Debian) Samba 3.0.0beta2 server. This has worked fine with both this
> kernel and previous ones for the last few months, but I've just had an
> Oops message while trying to open a directory with ROX-Filer. The
> filesystem in question is automounted (using autofs4), and this would
> have been the first operation upon it after being mounted.

Hmm.. Looks like an indirect call that jumped through a NULL pointer.

There's a few different indirect calls in "smb_readdir()", so it's a bit 
hard to guess which one. It's at the end of the function, but gcc 
re-orders code so much (and usually wrong, I have to say), that it's hard 
to guess which one it would be.

Three out of four calls are to "readdir()", which is an argument to the 
function and should not really reasonably be NULL unless there is a 
compiler bug or some _major_ stack corruption going on. So I consider 
those to be the less likely causes.

The last one is to "server->ops->readdir()", and it's entirely possible 
that that might be NULL. That's reinforced by the data on the stack 
which would be the arguments to that function:

	c5107840 cf84bfa0 c0165560 cf84bf2c

which makes some amount of sense (arg 2 and 4 are both pointing to the
call-stack, which I think is correct for those arguments if it is that
"->readdir()" call). In contrast, the above would _not_ make sense as the
four first arguments to "filldir" (the third one should be a name length).

So I think you had a NULL pointer for "server->ops->readdir".

As to how something like that could happen, I have absolutely no clue. The 
"smb_install_null_ops()" would seem to cause that, but that's all I can 
say.

Maybe the "smp_ops_null" thing should be filled in with stuff that always
returns EINVAL or something? Rather than actual NULL pointers that will
oops if they are ever used?

(That structure is actually from Andrew's patch from Zwane Mwaikambo 
as a workaround for smb_proc_getattr oops from last summer)

Urban? Andrew? Zwane? I can decode the oopses, but when it comes to smbfs 
I'm a retard.

		Linus

Re: smbfs Oops with Linux 2.6.3

[Urban Widmark]

On Tue, 9 Mar 2004, Linus Torvalds wrote:

> As to how something like that could happen, I have absolutely no clue. The 
> "smb_install_null_ops()" would seem to cause that, but that's all I can 
> say.
> 
> Maybe the "smp_ops_null" thing should be filled in with stuff that always
> returns EINVAL or something? Rather than actual NULL pointers that will
> oops if they are ever used?

The getattr problem was that after the mount smbfs isn't ready to do 
anything until it gets a connection from smbmount. Filling smp_ops_null 
would give the user an error on whatever it is that it does immediately 
after mounting.

smp_ops_null should really make all functions block and wait for the
connection to be created and given to us from smbmount. That would make it
behave more like smbfs in 2.4 does.

I am thinking of something like this for each entry in smp_ops_null.

int whatever_it_is_that_I_am(args)
{
	timeleft = wait_event_interruptible_timeout(...)
	if (!timeleft || signal_pending(current))
		return -EIO;
	if (!server->ops->whatever_it_is_that_I_am)
		return -EIO;
	return server->ops->whatever_it_is_that_I_am(args)
}


I know bugme has bug1671 with a similar oops. If no one else does, I'll
make a patch for Adam and #1671 to test.

I have not been able to reproduce this before, but if it is that readdir 
is used before it is ready then some extra delays in smbmount might help.

/Urban

Re: smbfs Oops with Linux 2.6.3

[Zwane Mwaikambo]

On Wed, 10 Mar 2004, Urban Widmark wrote:

> On Tue, 9 Mar 2004, Linus Torvalds wrote:
>
> > As to how something like that could happen, I have absolutely no clue. The
> > "smb_install_null_ops()" would seem to cause that, but that's all I can
> > say.
> >
> > Maybe the "smp_ops_null" thing should be filled in with stuff that always
> > returns EINVAL or something? Rather than actual NULL pointers that will
> > oops if they are ever used?

I originally didn't fill them all in intentionally, doing so may be best.

> smp_ops_null should really make all functions block and wait for the
> connection to be created and given to us from smbmount. That would make it
> behave more like smbfs in 2.4 does.
>
> I am thinking of something like this for each entry in smp_ops_null.
>
> int whatever_it_is_that_I_am(args)
> {
> 	timeleft = wait_event_interruptible_timeout(...)
> 	if (!timeleft || signal_pending(current))
> 		return -EIO;
> 	if (!server->ops->whatever_it_is_that_I_am)
> 		return -EIO;
> 	return server->ops->whatever_it_is_that_I_am(args)
> }

Thanks Urban, i have posted the following on bugzilla
(http://bugzilla.kernel.org/show_bug.cgi?id=1671) for testing. But,
it appears racy wrt getattr and win9x servers.

Index: linux-2.6.4-rc3/fs/smbfs/proc.c
===================================================================
RCS file: /home/cvsroot/linux-2.6.4-rc3/fs/smbfs/proc.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 proc.c
--- linux-2.6.4-rc3/fs/smbfs/proc.c	10 Mar 2004 01:05:32 -0000	1.1.1.1
+++ linux-2.6.4-rc3/fs/smbfs/proc.c	10 Mar 2004 17:16:22 -0000
@@ -56,6 +56,7 @@ static struct smb_ops smb_ops_os2;
 static struct smb_ops smb_ops_win95;
 static struct smb_ops smb_ops_winNT;
 static struct smb_ops smb_ops_unix;
+static struct smb_ops smb_ops_null;

 static void
 smb_init_dirent(struct smb_sb_info *server, struct smb_fattr *fattr);
@@ -2794,10 +2795,46 @@ out:
 }

 static int
+smb_proc_ops_wait(struct smb_sb_info *server)
+{
+	int result;
+	DECLARE_WAIT_QUEUE_HEAD(wq);
+
+	result = wait_event_interruptible_timeout(wq,
+			server->ops != &smb_ops_null, 5*HZ);
+
+	if (!result || signal_pending(current))
+		return -EIO;
+
+	return 0;
+}
+
+static int
 smb_proc_getattr_null(struct smb_sb_info *server, struct dentry *dir,
-		      struct smb_fattr *attr)
+			  struct smb_fattr *fattr)
 {
-	return -EIO;
+	int result;
+
+	if (smb_proc_ops_wait(server) < 0)
+		return -EIO;
+
+	smb_init_dirent(server, fattr);
+	result = server->ops->getattr(server, dir, fattr);
+	smb_finish_dirent(server, fattr);
+
+	return result;
+}
+
+static int
+smb_proc_readdir_null(struct file *filp, void *dirent, filldir_t filldir,
+		      struct smb_cache_control *ctl)
+{
+	struct smb_sb_info *server = server_from_dentry(filp->f_dentry);
+
+	if (smb_proc_ops_wait(server) < 0)
+		return -EIO;
+
+	return server->ops->readdir(filp, dirent, filldir, ctl);
 }

 int
@@ -3431,6 +3467,7 @@ static struct smb_ops smb_ops_unix =
 /* Place holder until real ops are in place */
 static struct smb_ops smb_ops_null =
 {
+	.readdir	= smb_proc_readdir_null,
 	.getattr	= smb_proc_getattr_null,
 };

Re: smbfs Oops with Linux 2.6.3

[Zwane Mwaikambo]

On Wed, 10 Mar 2004, Zwane Mwaikambo wrote:

> Thanks Urban, i have posted the following on bugzilla
> (http://bugzilla.kernel.org/show_bug.cgi?id=1671) for testing. But,
> it appears racy wrt getattr and win9x servers.

How about the following to synchronize with smb_newconn()

smb_lock_server(server);
smb_unlock_server(server);

I've already uploaded the new patch on Bugzilla, but i also came across a
smb_dir_cache related oops whilst testing, which i'm debugging.

Re: smbfs Oops with Linux 2.6.3

[Urban Widmark]

On Wed, 10 Mar 2004, Zwane Mwaikambo wrote:

> On Wed, 10 Mar 2004, Zwane Mwaikambo wrote:
> 
> > Thanks Urban, i have posted the following on bugzilla
> > (http://bugzilla.kernel.org/show_bug.cgi?id=1671) for testing. But,
> > it appears racy wrt getattr and win9x servers.

The 5 second timeout is probably too short. Some bad configs can use a 
long time to connect, possibly more. 30?


> How about the following to synchronize with smb_newconn()
> 
> smb_lock_server(server);
> smb_unlock_server(server);

Shouldn't "wq" be accessible to both smb_newconn and smb_proc_ops_wait?
I'd put it in the "server" struct and then have smb_newconn() do this 
when it is done:
	wake_up_interruptible_all(&server->ops_wq);

I don't know enough about wait_queue's to understand why it would work
otherwise. The only thing I can think of is that the condition is true
before it actually waits on anything.

Since install_ops isn't the last thing done in smb_newconn perhaps a
different variable should be used to signal that a new connection is
there. I would suggest using "server->state == CONN_VALID" and then move
that assignment to the end of smb_newconn.


I'm guessing read/write/truncate can't be called before smb_newconn since
they all require a file to be opened, and open needs getattr (or?). But
just to be safe how about adding the code below?

static int
smb_proc_ops_bug(void)
{
	BUG();
}

static struct smb_ops smb_ops_null =
{
	.readdir	= smb_proc_readdir_null,
	.getattr	= smb_proc_getattr_null,
	.read		= (void *) smb_proc_ops_bug,
	.write		= (void *) smb_proc_ops_bug,
	.truncate	= (void *) smb_proc_ops_bug,
};

If the void* can be avoided by something clever then that is what I really 
meant :)


> I've already uploaded the new patch on Bugzilla, but i also came across a
> smb_dir_cache related oops whilst testing, which i'm debugging.

If you are in cleanup mode the following changes should probably be made:

server->rcls	replaced by	req->rq_rcls
server->err	replaced by	req->rq_err

and remove the server->{rcls,err} fields.

/Urban

Re: smbfs Oops with Linux 2.6.3

[Zwane Mwaikambo]

On Wed, 10 Mar 2004, Urban Widmark wrote:

> Shouldn't "wq" be accessible to both smb_newconn and smb_proc_ops_wait?
> I'd put it in the "server" struct and then have smb_newconn() do this
> when it is done:
> 	wake_up_interruptible_all(&server->ops_wq);

Oops, yes my code was horribly broken, the previous patch will only avoid
the oops since readdir won't be NULL, but is still fundamentally wrong.

> I don't know enough about wait_queue's to understand why it would work
> otherwise. The only thing I can think of is that the condition is true
> before it actually waits on anything.
>
> Since install_ops isn't the last thing done in smb_newconn perhaps a
> different variable should be used to signal that a new connection is
> there. I would suggest using "server->state == CONN_VALID" and then move
> that assignment to the end of smb_newconn.
>
> If you are in cleanup mode the following changes should probably be made:
>
> server->rcls	replaced by	req->rq_rcls
> server->err	replaced by	req->rq_err

Sure thing, i'll fix it up.

Thanks,
	Zwane

Re: smbfs Oops with Linux 2.6.3

[Zwane Mwaikambo]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 594 bytes --]

Two patches applied in this order;

patch-smbfs-readdir-oops
 fs/smbfs/inode.c          |    1 +
 fs/smbfs/proc.c           |   45 ++++++++++++++++++++++++++++++++++++++++++---
 include/linux/smb_fs_sb.h |    2 +-
 3 files changed, 44 insertions(+), 4 deletions(-)

patch-smbfs-rcls-cleanup
 fs/smbfs/proc.c           |   12 ++++++------
 include/linux/smb_fs_sb.h |    3 ---
 2 files changed, 6 insertions(+), 9 deletions(-)

I avoided adding the BUG() placeholders, since we will PF# when we hit the
NULL pointer and the call trace should give us a decent idea of where it
is.

Thanks,
	Zwane

[-- Attachment #2: Type: TEXT/PLAIN, Size: 3756 bytes --]

Index: linux-2.6.4-rc3/fs/smbfs/inode.c
===================================================================
RCS file: /home/cvsroot/linux-2.6.4-rc3/fs/smbfs/inode.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 inode.c
--- linux-2.6.4-rc3/fs/smbfs/inode.c	10 Mar 2004 01:05:32 -0000	1.1.1.1
+++ linux-2.6.4-rc3/fs/smbfs/inode.c	11 Mar 2004 05:08:40 -0000
@@ -513,6 +513,7 @@ int smb_fill_super(struct super_block *s
 	server->super_block = sb;
 	server->mnt = NULL;
 	server->sock_file = NULL;
+	init_waitqueue_head(&server->conn_wq);
 	init_MUTEX(&server->sem);
 	INIT_LIST_HEAD(&server->entry);
 	INIT_LIST_HEAD(&server->xmitq);
Index: linux-2.6.4-rc3/fs/smbfs/proc.c
===================================================================
RCS file: /home/cvsroot/linux-2.6.4-rc3/fs/smbfs/proc.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 proc.c
--- linux-2.6.4-rc3/fs/smbfs/proc.c	10 Mar 2004 01:05:32 -0000	1.1.1.1
+++ linux-2.6.4-rc3/fs/smbfs/proc.c	11 Mar 2004 05:07:48 -0000
@@ -56,6 +56,7 @@ static struct smb_ops smb_ops_os2;
 static struct smb_ops smb_ops_win95;
 static struct smb_ops smb_ops_winNT;
 static struct smb_ops smb_ops_unix;
+static struct smb_ops smb_ops_null;
 
 static void
 smb_init_dirent(struct smb_sb_info *server, struct smb_fattr *fattr);
@@ -878,7 +879,6 @@ smb_newconn(struct smb_sb_info *server, 
 	server->conn_pid = current->pid;
 	server->opt = *opt;
 	server->generation += 1;
-	server->state = CONN_VALID;
 	error = 0;
 
 	if (server->conn_error) {
@@ -981,6 +981,9 @@ smb_newconn(struct smb_sb_info *server, 
 	smbiod_wake_up();
 	if (server->opt.capabilities & SMB_CAP_UNIX)
 		smb_proc_query_cifsunix(server);
+
+	server->state = CONN_VALID;
+	wake_up_interruptible_all(&server->conn_wq);
 	return error;
 
 out:
@@ -2794,10 +2797,45 @@ out:
 }
 
 static int
+smb_proc_ops_wait(struct smb_sb_info *server)
+{
+	int result;
+
+	result = wait_event_interruptible_timeout(server->conn_wq,
+				server->state == CONN_VALID, 30*HZ);
+
+	if (!result || signal_pending(current))
+		return -EIO;
+
+	return 0;
+}
+
+static int
 smb_proc_getattr_null(struct smb_sb_info *server, struct dentry *dir,
-		      struct smb_fattr *attr)
+			  struct smb_fattr *fattr)
 {
-	return -EIO;
+	int result;
+
+	if (smb_proc_ops_wait(server) < 0)
+		return -EIO;
+
+	smb_init_dirent(server, fattr);
+	result = server->ops->getattr(server, dir, fattr);
+	smb_finish_dirent(server, fattr);
+
+	return result;
+}
+
+static int
+smb_proc_readdir_null(struct file *filp, void *dirent, filldir_t filldir,
+		      struct smb_cache_control *ctl)
+{
+	struct smb_sb_info *server = server_from_dentry(filp->f_dentry);
+
+	if (smb_proc_ops_wait(server) < 0)
+		return -EIO;
+
+	return server->ops->readdir(filp, dirent, filldir, ctl);
 }
 
 int
@@ -3431,6 +3468,7 @@ static struct smb_ops smb_ops_unix =
 /* Place holder until real ops are in place */
 static struct smb_ops smb_ops_null =
 {
+	.readdir	= smb_proc_readdir_null,
 	.getattr	= smb_proc_getattr_null,
 };
 
Index: linux-2.6.4-rc3/include/linux/smb_fs_sb.h
===================================================================
RCS file: /home/cvsroot/linux-2.6.4-rc3/include/linux/smb_fs_sb.h,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 smb_fs_sb.h
--- linux-2.6.4-rc3/include/linux/smb_fs_sb.h	10 Mar 2004 01:05:23 -0000	1.1.1.1
+++ linux-2.6.4-rc3/include/linux/smb_fs_sb.h	11 Mar 2004 05:37:14 -0000
@@ -57,7 +57,7 @@ struct smb_sb_info {
 	unsigned int generation;
 	pid_t conn_pid;
 	struct smb_conn_opt opt;
-
+	wait_queue_head_t conn_wq;
 	struct semaphore sem;
 
         unsigned short     rcls; /* The error codes we received */

[-- Attachment #3: Type: TEXT/PLAIN, Size: 2837 bytes --]

Index: linux-2.6.4-rc3/fs/smbfs/proc.c
===================================================================
RCS file: /home/cvsroot/linux-2.6.4-rc3/fs/smbfs/proc.c,v
retrieving revision 1.2
diff -u -p -B -r1.2 proc.c
--- linux-2.6.4-rc3/fs/smbfs/proc.c	11 Mar 2004 05:26:07 -0000	1.2
+++ linux-2.6.4-rc3/fs/smbfs/proc.c	11 Mar 2004 05:26:31 -0000
@@ -2373,7 +2373,7 @@ smb_proc_readdir_long(struct file *filp,
 		if (req->rq_rcls != 0) {
 			result = smb_errno(req);
 			PARANOIA("name=%s, result=%d, rcls=%d, err=%d\n",
-				 mask, result, server->rcls, server->err);
+				 mask, result, req->rq_rcls, req->rq_err);
 			break;
 		}
 
@@ -2529,7 +2529,7 @@ smb_proc_getattr_ff(struct smb_sb_info *
 	result = smb_add_request(req);
 	if (result < 0)
 		goto out_free;
-	if (server->rcls != 0) {
+	if (req->rq_rcls != 0) {
 		result = smb_errno(req);
 #ifdef SMBFS_PARANOIA
 		if (result != -ENOENT)
@@ -2642,7 +2642,7 @@ smb_proc_getattr_trans2(struct smb_sb_in
 	result = smb_add_request(req);
 	if (result < 0)
 		goto out;
-	if (server->rcls != 0) {
+	if (req->rq_rcls != 0) {
 		VERBOSE("for %s: result=%d, rcls=%d, err=%d\n",
 			&param[6], result, req->rq_rcls, req->rq_err);
 		result = smb_errno(req);
@@ -3255,7 +3255,7 @@ smb_proc_read_link(struct smb_sb_info *s
 	if (result < 0)
 		goto out_free;
 	DEBUG1("for %s: result=%d, rcls=%d, err=%d\n",
-		&param[6], result, server->rcls, server->err);
+		&param[6], result, req->rq_rcls, req->rq_err);
 
 	/* copy data up to the \0 or buffer length */
 	result = len;
@@ -3305,7 +3305,7 @@ smb_proc_symlink(struct smb_sb_info *ser
 		goto out_free;
 
 	DEBUG1("for %s: result=%d, rcls=%d, err=%d\n",
-		&param[6], result, server->rcls, server->err);
+		&param[6], result, req->rq_rcls, req->rq_err);
 	result = 0;
 
 out_free:
@@ -3352,7 +3352,7 @@ smb_proc_link(struct smb_sb_info *server
 		goto out_free;
 
 	DEBUG1("for %s: result=%d, rcls=%d, err=%d\n",
-	       &param[6], result, server->rcls, server->err);
+	       &param[6], result, req->rq_rcls, req->rq_err);
 	result = 0;
 
 out_free:
Index: linux-2.6.4-rc3/include/linux/smb_fs_sb.h
===================================================================
RCS file: /home/cvsroot/linux-2.6.4-rc3/include/linux/smb_fs_sb.h,v
retrieving revision 1.2
diff -u -p -B -r1.2 smb_fs_sb.h
--- linux-2.6.4-rc3/include/linux/smb_fs_sb.h	11 Mar 2004 05:38:11 -0000	1.2
+++ linux-2.6.4-rc3/include/linux/smb_fs_sb.h	11 Mar 2004 05:38:17 -0000
@@ -60,9 +60,6 @@ struct smb_sb_info {
 	wait_queue_head_t conn_wq;
 	struct semaphore sem;
 
-        unsigned short     rcls; /* The error codes we received */
-        unsigned short     err;
-
 	unsigned char      header[SMB_HEADER_LEN + 20*2 + 2];
 	u32                header_len;
 	u32                smb_len;

Re: smbfs Oops with Linux 2.6.3

[Urban Widmark]

On Thu, 11 Mar 2004, Zwane Mwaikambo wrote:

> Two patches applied in this order;
> 
> patch-smbfs-readdir-oops
>  fs/smbfs/inode.c          |    1 +
>  fs/smbfs/proc.c           |   45 ++++++++++++++++++++++++++++++++++++++++++---
>  include/linux/smb_fs_sb.h |    2 +-
>  3 files changed, 44 insertions(+), 4 deletions(-)
> 
> patch-smbfs-rcls-cleanup
>  fs/smbfs/proc.c           |   12 ++++++------
>  include/linux/smb_fs_sb.h |    3 ---
>  2 files changed, 6 insertions(+), 9 deletions(-)
> 
> I avoided adding the BUG() placeholders, since we will PF# when we hit the
> NULL pointer and the call trace should give us a decent idea of where it
> is.

Not quite there yet.

I have added a sleep(10); in send_fs_socket (smbmount) to give me time to 
send other requests to the fs. Here is what I get when I run the mount and 
two 'ls' in parallel:

1. smbmount mounts and goes to sleep
2. ls 1 ends up in smb_proc_ops_wait
3. ls 2 ends up in smb_proc_ops_wait
4. smbmount wakes up and passes the socket to smbfs

5a. ls 1 & 2 are woken and lists files

5b. Everyone is waiting for ls 1 (or 2 I don't know) to timeout.
6b. ls 1 lists nothing because of the timeout
7b. ls 2 lists normally


The difference between a and b is that in the first case I wait a few 
seconds between the two ls' but in the second I run the within one second 
of each other.

Some debug printks give:

a:
smb_proc_getattr_null: 
smb_proc_ops_wait: state: 1
smb_proc_getattr_null: 
smb_proc_ops_wait: state: 1
(here it waits for smbmount to do the ioctl)
smb_proc_ops_wait: result: 26192
smb_proc_ops_wait: result: 23014

b:
smb_proc_readdir_null: 
smb_proc_ops_wait: state: 1
smb_proc_getattr_null: 
smb_proc_ops_wait: state: 1
(here it waits for smbmount to do the ioctl)
smb_proc_ops_wait: result: 0
smb_proc_ops_wait: result: 682


The difference must be that in a the inode data for the root inode is not 
considered current when the second ls runs, but I don't understand why the 
readdir is printed before the getattr.

/Urban

Re: smbfs Oops with Linux 2.6.3

[Zwane Mwaikambo]

On Fri, 12 Mar 2004, Urban Widmark wrote:

> Not quite there yet.
>
> I have added a sleep(10); in send_fs_socket (smbmount) to give me time to
> send other requests to the fs. Here is what I get when I run the mount and
> two 'ls' in parallel:

Thanks for testing it, i'll have another go at it.

> The difference must be that in a the inode data for the root inode is not
> considered current when the second ls runs, but I don't understand why the
> readdir is printed before the getattr.

I don't understand why to expect the getattr before the readdir, perhaps
you can elaborate for me?

Thanks,
	Zwane

Re: smbfs Oops with Linux 2.6.3

[Urban Widmark]

On Fri, 12 Mar 2004, Zwane Mwaikambo wrote:

> > The difference must be that in a the inode data for the root inode is not
> > considered current when the second ls runs, but I don't understand why the
> > readdir is printed before the getattr.
> 
> I don't understand why to expect the getattr before the readdir, perhaps
> you can elaborate for me?

smb_readdir
  smb_revalidate_inode
    smb_refresh_inode
      smb_proc_getattr
        server->ops->getattr
  server->ops->readdir


The first ls should find the inode out-of-date (smb_readdir probably isn't 
the first call, but that doesn't matter) because it is the first user.
The second ls is run shortly after and should find the inode to be 
up-to-date.

I'm not sure it is important at all, it just wasn't what I expected.

/Urban