This has been reported a couple of times and is consistently causing some
folks grief, so Urban, would you mind terribly if i send this patch to at
least clear current bug reports. If there is additional stuff you want
ontop of this let me know and i can send a follow up patch.
The bug is that at times we haven't completed setting up the smb_ops so we
have a temporary 'null' ops in place until the connection is completely
up. With this setup it's possible to hit ->readdir() whilst the null
ops are still in place, so we put the process to sleep until the
connection setup is complete and then call the real ->readdir().
This patch addresses the bugzilla report at
http://bugzilla.kernel.org/show_bug.cgi?id=1671
fs/smbfs/inode.c | 1 +
fs/smbfs/proc.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
include/linux/smb_fs_sb.h | 3 ++-
3 files changed, 45 insertions(+), 3 deletions(-)
Signed-off-by: Zwane Mwaikambo <[email protected]>
Index: linux-2.6.7/include/linux/smb_fs_sb.h
===================================================================
RCS file: /home/cvsroot/linux-2.6.7/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.7/include/linux/smb_fs_sb.h 16 Jun 2004 16:49:26 -0000 1.1.1.1
+++ linux-2.6.7/include/linux/smb_fs_sb.h 20 Jun 2004 00:06:45 -0000
@@ -57,7 +57,8 @@ struct smb_sb_info {
unsigned int generation;
pid_t conn_pid;
struct smb_conn_opt opt;
-
+ wait_queue_head_t conn_wq;
+ int conn_complete;
struct semaphore sem;
unsigned short rcls; /* The error codes we received */
Index: linux-2.6.7/fs/smbfs/inode.c
===================================================================
RCS file: /home/cvsroot/linux-2.6.7/fs/smbfs/inode.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 inode.c
--- linux-2.6.7/fs/smbfs/inode.c 16 Jun 2004 16:49:47 -0000 1.1.1.1
+++ linux-2.6.7/fs/smbfs/inode.c 19 Jun 2004 21:19:04 -0000
@@ -521,6 +521,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.7/fs/smbfs/proc.c
===================================================================
RCS file: /home/cvsroot/linux-2.6.7/fs/smbfs/proc.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 proc.c
--- linux-2.6.7/fs/smbfs/proc.c 16 Jun 2004 16:49:47 -0000 1.1.1.1
+++ linux-2.6.7/fs/smbfs/proc.c 20 Jun 2004 00:26:57 -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);
@@ -981,6 +982,9 @@ smb_newconn(struct smb_sb_info *server,
smbiod_wake_up();
if (server->opt.capabilities & SMB_CAP_UNIX)
smb_proc_query_cifsunix(server);
+
+ server->conn_complete++;
+ wake_up_interruptible_all(&server->conn_wq);
return error;
out:
@@ -2794,10 +2798,45 @@ out:
}
static int
+smb_proc_ops_wait(struct smb_sb_info *server)
+{
+ int result;
+
+ result = wait_event_interruptible_timeout(server->conn_wq,
+ server->conn_complete, 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 +3470,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,
};
On Sun, 20 Jun 2004, Zwane Mwaikambo wrote:
> This has been reported a couple of times and is consistently causing some
> folks grief, so Urban, would you mind terribly if i send this patch to at
> least clear current bug reports. If there is additional stuff you want
> ontop of this let me know and i can send a follow up patch.
I should read all my mail before replying. Yes, this is a problem for
people and I was thinking the same thing that we should get this in now
and fix the remaining problem later.
One question:
Why do you need conn_complete? Isn't "server->state == CONN_VALID" good
enough?
And are you still working on fixing the remaining "multiple-ls" problem?
(The one where one ls would work and the other return an error).
Or is that fixed in this version?
/Urban
On Mon, 21 Jun 2004, Urban Widmark wrote:
> On Sun, 20 Jun 2004, Zwane Mwaikambo wrote:
>
> > This has been reported a couple of times and is consistently causing some
> > folks grief, so Urban, would you mind terribly if i send this patch to at
> > least clear current bug reports. If there is additional stuff you want
> > ontop of this let me know and i can send a follow up patch.
>
> I should read all my mail before replying. Yes, this is a problem for
> people and I was thinking the same thing that we should get this in now
> and fix the remaining problem later.
>
> One question:
> Why do you need conn_complete? Isn't "server->state == CONN_VALID" good
> enough?
I couldn't use CONN_VALID to mark when the connection was completely up
because it's used in some branch decisions during connection setup. So
moving it further down in smb_newconn broke things for the testers.
> And are you still working on fixing the remaining "multiple-ls" problem?
> (The one where one ls would work and the other return an error).
> Or is that fixed in this version?
I'll be working on that, i recall that there is also a bugzilla entry for
it.
Thanks,
Zwane