linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH][RFT] smbfs bugfixes for 2.4.4
@ 2001-05-02 21:59 Urban Widmark
  2001-05-06  0:14 ` Xuan Baldauf
  0 siblings, 1 reply; 11+ messages in thread
From: Urban Widmark @ 2001-05-02 21:59 UTC (permalink / raw)
  To: linux-kernel; +Cc: Xuan Baldauf

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


Hello all

This patch have been building up for a while, without reaching some
undefined level of readiness. I would like some feedback from other smbfs
users before submitting this for 2.4.4-something. Particularly from people
mounting win9x shares.

* win9x will sometimes not give back the right filesize, this can cause
  problems when cp'ing a file over an existing one. When truncating the
  file to 0 bytes the server keeps reporting the old size and much
  confusion arises.
  (reported by Xuan Baldauf, could you verify that this fixes it? If it
   does it's a much better fix than the workaround you got before.)

* The timeout for a reconnect was only 5 seconds, which may not be enough.
  To make it worse, if there is a timeout the mount will be dead.
  umount time.

* Allow smbmount to supply a new connection even if the retry attempt has
  timed-out. "dead" mounts can then be resurrected ... this allows you to
  replace a smbmount that have crashed, or it would if smbmount supported
  that.

* Locking bug in smb_newconn, it modifies the "server" struct without
  having the server lock.

* Fix theoretical memory leak and the missing smb malloc debug functions.

* fsync now causes a SMBflush to tell the server that we want the data to
  hit the disc. (as suggested by Jochen Dolze)

* Don't zero out the superblock flags to allow readonly mounts to be
  readonly. If someone knows why sb->s_flags = 0; is a good idea I'd
  like to know. (fix&testing by René Scharfe)

Patch vs 2.4.4, if someone prefers 2.4.4-acX it should work (even if the
patch fails on changes to Configure.help).

/Urban

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

diff -urN -X exclude linux-2.4.4-orig/Documentation/Configure.help linux-2.4.4-smbfs/Documentation/Configure.help
--- linux-2.4.4-orig/Documentation/Configure.help	Wed May  2 20:01:07 2001
+++ linux-2.4.4-smbfs/Documentation/Configure.help	Wed May  2 20:44:43 2001
@@ -12081,10 +12081,7 @@
   The nls settings can be changed at mount time, if your smbmount
   supports that, using the codepage and iocharset parameters.
 
-  Currently no smbmount distributed with samba supports this, it is
-  assumed future versions will. In the meantime you can get an
-  unofficial patch for samba 2.0.7 from:
-  http://www.hojdpunkten.ac.se/054/samba/index.html
+  smbmount from samba 2.2.0 or later supports this.
 
 nls support setting
 CONFIG_SMB_NLS_REMOTE
@@ -12096,10 +12093,7 @@
   The nls settings can be changed at mount time, if your smbmount
   supports that, using the codepage and iocharset parameters.
 
-  Currently no smbmount distributed with samba supports this, it is
-  assumed future versions will. In the meantime you can get an
-  unofficial patch for samba 2.0.7 from:
-  http://www.hojdpunkten.ac.se/054/samba/index.html
+  smbmount from samba 2.2.0 or later supports this.
 
 Coda file system support (advanced network fs)
 CONFIG_CODA_FS
diff -urN -X exclude linux-2.4.4-orig/MAINTAINERS linux-2.4.4-smbfs/MAINTAINERS
--- linux-2.4.4-orig/MAINTAINERS	Wed May  2 20:01:07 2001
+++ linux-2.4.4-smbfs/MAINTAINERS	Wed May  2 20:42:24 2001
@@ -1171,7 +1171,7 @@
 
 SMB FILESYSTEM
 P:	Urban Widmark
-M:	urban@svenskatest.se
+M:	urban@teststation.com
 W:	http://samba.org/
 L:	samba@samba.org
 S:	Maintained
diff -urN -X exclude linux-2.4.4-orig/fs/smbfs/ChangeLog linux-2.4.4-smbfs/fs/smbfs/ChangeLog
--- linux-2.4.4-orig/fs/smbfs/ChangeLog	Sat Mar 31 19:11:53 2001
+++ linux-2.4.4-smbfs/fs/smbfs/ChangeLog	Wed May  2 23:48:56 2001
@@ -1,5 +1,18 @@
 ChangeLog for smbfs.
 
+2001-04-25 René Scharfe <l.s.r@web.de>
+
+	* inode.c: Don't clear s_flags and allow ro mounts
+
+2001-04-21 Urban Widmark <urban@teststation.com>
+
+	* dir.c, proc.c: replace tests on conn_pid with tests on state to
+	  fix smbmount reconnect on smb_retry timeout and up the timeout to 30s.
+	* proc.c: smb_newconn must have the server locked while updating it.
+	* inode.c, proc.c: need flush after truncate on some servers (win9x)
+	* file.c: add call to send SMBflush on fsync
+	  (as suggested by Jochen Dolze <dolze@epcnet.de>)
+
 2001-03-06 Urban Widmark <urban@teststation.com>
 
 	* cache.c: d_add on hashed dentries corrupts d_hash list and
diff -urN -X exclude linux-2.4.4-orig/fs/smbfs/dir.c linux-2.4.4-smbfs/fs/smbfs/dir.c
--- linux-2.4.4-orig/fs/smbfs/dir.c	Thu Feb 22 20:52:03 2001
+++ linux-2.4.4-smbfs/fs/smbfs/dir.c	Wed May  2 20:42:24 2001
@@ -210,10 +210,6 @@
 	return result;
 }
 
-/*
- * Note: in order to allow the smbmount process to open the
- * mount point, we don't revalidate if conn_pid is NULL.
- */
 static int
 smb_dir_open(struct inode *dir, struct file *file)
 {
@@ -230,14 +226,18 @@
 	 */
 	lock_kernel();
 	server = server_from_dentry(dentry);
-	if (server->opt.protocol < SMB_PROTOCOL_LANMAN2)
-	{
+	if (server->opt.protocol < SMB_PROTOCOL_LANMAN2) {
 		unsigned long age = jiffies - dir->u.smbfs_i.oldmtime;
 		if (age > 2*HZ)
 			smb_invalid_dir_cache(dir);
 	}
 
-	if (server->conn_pid)
+	/*
+	 * Note: in order to allow the smbmount process to open the
+	 * mount point, we only revalidate if the connection is valid or
+	 * if the process is trying to access something other than the root.
+	 */
+	if (server->state == CONN_VALID || !IS_ROOT(dentry))
 		error = smb_revalidate_inode(dentry);
 	unlock_kernel();
 	return error;
diff -urN -X exclude linux-2.4.4-orig/fs/smbfs/file.c linux-2.4.4-smbfs/fs/smbfs/file.c
--- linux-2.4.4-orig/fs/smbfs/file.c	Thu Feb 22 20:52:03 2001
+++ linux-2.4.4-smbfs/fs/smbfs/file.c	Wed May  2 21:55:58 2001
@@ -28,8 +28,23 @@
 static int
 smb_fsync(struct file *file, struct dentry * dentry, int datasync)
 {
+	int result;
+	struct smb_sb_info *server = server_from_dentry(dentry);
+
 	VERBOSE("sync file %s/%s\n", DENTRY_PATH(dentry));
-	return 0;
+
+	/*
+	 * The VFS will writepage() all dirty pages for us, but we
+	 * should send a SMBflush to the server, letting it know that
+	 * we want things synchronized with actual storage.
+	 *
+	 * Note: this function requires all pages to have been written already
+	 *       (should be ok with writepage_sync)
+	 */
+	smb_lock_server(server);
+	result = smb_proc_flush(server, dentry->d_inode->u.smbfs_i.fileid);
+	smb_unlock_server(server);
+	return result;
 }
 
 /*
@@ -205,6 +220,20 @@
 
 	VERBOSE("file %s/%s, count=%lu@%lu\n", DENTRY_PATH(dentry),
 		(unsigned long) count, (unsigned long) *ppos);
+
+	/*
+	 * The localwrite flag is needed vs some servers that do not
+	 * immediately return the correct filesize. But it also prevents us
+	 * from ever seeing changes made on the serverside. Clearing the flag
+	 * on read helps one case and breaks the other.
+	 * (broken are: win98se, possibly not win95-original?)
+	 *
+	 * Jochen Dolze <dolze@epcnet.de> has the details. I hope oplocks
+	 * are the solution. -- puw
+	 */
+#if 0
+	dentry->d_inode->u.smbfs_i.flags &= ~SMB_F_LOCALWRITE;
+#endif
 
 	status = smb_revalidate_inode(dentry);
 	if (status)
diff -urN -X exclude linux-2.4.4-orig/fs/smbfs/inode.c linux-2.4.4-smbfs/fs/smbfs/inode.c
--- linux-2.4.4-orig/fs/smbfs/inode.c	Wed May  2 20:01:37 2001
+++ linux-2.4.4-smbfs/fs/smbfs/inode.c	Wed May  2 22:30:52 2001
@@ -121,8 +121,10 @@
 	inode->i_blksize= fattr->f_blksize;
 	inode->i_blocks = fattr->f_blocks;
 	/*
-	 * Don't change the size and mtime/atime fields
-	 * if we're writing to the file.
+	 * Don't change the size and mtime/atime fields if we're
+	 * writing to the file. We need this as we may have local
+	 * unwritten changes that we don't want replaced just because
+	 * someone revalidated.
 	 */
 	if (!(inode->u.smbfs_i.flags & SMB_F_LOCALWRITE)) {
 		inode->i_size  = fattr->f_size;
@@ -234,9 +236,10 @@
 	last_sz   = inode->i_size;
 	error = smb_refresh_inode(dentry);
 	if (error || inode->i_mtime != last_time || inode->i_size != last_sz) {
-		VERBOSE("%s/%s changed, old=%ld, new=%ld\n",
+		VERBOSE("%s/%s changed, old=%ld, new=%ld, oz=%ld, nz=%ld\n",
 			DENTRY_PATH(dentry),
-			(long) last_time, (long) inode->i_mtime);
+			(long) last_time, (long) inode->i_mtime,
+			(long) last_sz, (long) inode->i_size);
 
 		if (!S_ISDIR(inode->i_mode))
 			invalidate_inode_pages(inode);
@@ -355,8 +358,8 @@
 	if (server->conn_pid)
 	       kill_proc(server->conn_pid, SIGTERM, 1);
 
-	kfree(server->mnt);
-	kfree(sb->u.smbfs_sb.temp_buf);
+	smb_kfree(server->mnt);
+	smb_kfree(sb->u.smbfs_sb.temp_buf);
 	if (server->packet)
 		smb_vfree(server->packet);
 
@@ -390,7 +393,6 @@
 	sb->s_blocksize = 1024;	/* Eh...  Is this correct? */
 	sb->s_blocksize_bits = 10;
 	sb->s_magic = SMB_SUPER_MAGIC;
-	sb->s_flags = 0;
 	sb->s_op = &smb_sops;
 
 	sb->u.smbfs_sb.mnt = NULL;
@@ -400,13 +402,13 @@
 	sb->u.smbfs_sb.conn_pid = 0;
 	sb->u.smbfs_sb.state = CONN_INVALID; /* no connection yet */
 	sb->u.smbfs_sb.generation = 0;
-	sb->u.smbfs_sb.packet_size = smb_round_length(SMB_INITIAL_PACKET_SIZE);	
+	sb->u.smbfs_sb.packet_size = smb_round_length(SMB_INITIAL_PACKET_SIZE);
 	sb->u.smbfs_sb.packet = smb_vmalloc(sb->u.smbfs_sb.packet_size);
 	if (!sb->u.smbfs_sb.packet)
 		goto out_no_mem;
 
 	/* Allocate the global temp buffer */
-	sb->u.smbfs_sb.temp_buf = kmalloc(2*SMB_MAXPATHLEN + 20, GFP_KERNEL);
+	sb->u.smbfs_sb.temp_buf = smb_kmalloc(2*SMB_MAXPATHLEN+20, GFP_KERNEL);
 	if (!sb->u.smbfs_sb.temp_buf)
 		goto out_no_temp;
 
@@ -417,7 +419,7 @@
 
 	/* Allocate the mount data structure */
 	/* FIXME: merge this with the other malloc and get a whole page? */
-	mnt = kmalloc(sizeof(struct smb_mount_data_kernel), GFP_KERNEL);
+	mnt = smb_kmalloc(sizeof(struct smb_mount_data_kernel), GFP_KERNEL);
 	if (!mnt)
 		goto out_no_mount;
 	sb->u.smbfs_sb.mnt = mnt;
@@ -482,9 +484,9 @@
 out_no_root:
 	iput(root_inode);
 out_bad_option:
-	kfree(sb->u.smbfs_sb.mnt);
+	smb_kfree(sb->u.smbfs_sb.mnt);
 out_no_mount:
-	kfree(sb->u.smbfs_sb.temp_buf);
+	smb_kfree(sb->u.smbfs_sb.temp_buf);
 out_no_temp:
 	smb_vfree(sb->u.smbfs_sb.packet);
 out_no_mem:
diff -urN -X exclude linux-2.4.4-orig/fs/smbfs/ioctl.c linux-2.4.4-smbfs/fs/smbfs/ioctl.c
--- linux-2.4.4-orig/fs/smbfs/ioctl.c	Wed May  2 20:01:37 2001
+++ linux-2.4.4-smbfs/fs/smbfs/ioctl.c	Wed May  2 20:42:24 2001
@@ -23,7 +23,7 @@
 smb_ioctl(struct inode *inode, struct file *filp,
 	  unsigned int cmd, unsigned long arg)
 {
-	struct smb_sb_info *server = SMB_SERVER(inode);
+	struct smb_sb_info *server = server_from_inode(inode);
 	struct smb_conn_opt opt;
 	int result = -EINVAL;
 
@@ -37,7 +37,7 @@
 		break;
 
 	case SMB_IOC_NEWCONN:
-		/* require an argument == the mount data, else it is EINVAL */
+		/* require an argument == smb_conn_opt, else it is EINVAL */
 		if (!arg)
 			break;
 
@@ -45,7 +45,8 @@
 		if (!copy_from_user(&opt, (void *)arg, sizeof(opt)))
 			result = smb_newconn(server, &opt);
 		break;
-	default:;
+	default:
+		break;
 	}
 
 	return result;
diff -urN -X exclude linux-2.4.4-orig/fs/smbfs/proc.c linux-2.4.4-smbfs/fs/smbfs/proc.c
--- linux-2.4.4-orig/fs/smbfs/proc.c	Wed May  2 20:01:37 2001
+++ linux-2.4.4-smbfs/fs/smbfs/proc.c	Wed May  2 23:32:41 2001
@@ -54,18 +54,6 @@
 		    struct smb_fattr *fattr);
 
 
-static inline void
-smb_lock_server(struct smb_sb_info *server)
-{
-	down(&(server->sem));
-}
-
-static inline void
-smb_unlock_server(struct smb_sb_info *server)
-{
-	up(&(server->sem));
-}
-
 
 static void
 str_upper(char *name, int len)
@@ -661,29 +649,31 @@
 	pid_t pid = server->conn_pid;
 	int error, result = 0;
 
-	if (server->state != CONN_INVALID)
+	if (server->state == CONN_VALID || server->state == CONN_RETRYING)
 		goto out;
 
 	smb_close_socket(server);
 
 	if (pid == 0) {
+		/* FIXME: this is fatal */
 		printk(KERN_ERR "smb_retry: no connection process\n");
 		server->state = CONN_RETRIED;
 		goto out;
 	}
 
 	/*
-	 * Clear the pid to enable the ioctl.
+	 * Change state so that only one retry per server will be started.
 	 */
-	server->conn_pid = 0;
+	server->state = CONN_RETRYING;
 
 	/*
 	 * Note: use the "priv" flag, as a user process may need to reconnect.
 	 */
 	error = kill_proc(pid, SIGUSR1, 1);
 	if (error) {
+		/* FIXME: this is fatal */
 		printk(KERN_ERR "smb_retry: signal failed, error=%d\n", error);
-		goto out_restore;
+		goto out;
 	}
 	VERBOSE("signalled pid %d, waiting for new connection\n", pid);
 
@@ -691,7 +681,9 @@
 	 * Wait for the new connection.
 	 */
 #ifdef SMB_RETRY_INTR
-	interruptible_sleep_on_timeout(&server->wait,  5*HZ);
+	smb_unlock_server(server);
+	interruptible_sleep_on_timeout(&server->wait,  30*HZ);
+	smb_lock_server(server);
 	if (signal_pending(current))
 		printk(KERN_INFO "smb_retry: caught signal\n");
 #else
@@ -702,8 +694,13 @@
 	 *
 	 * smbmount should be able to reconnect later, but it can't because
 	 * it will get an -EIO on attempts to open the mountpoint!
+	 *
+	 * FIXME: go back to the interruptable version now that smbmount
+	 * can avoid -EIO on the mountpoint when reconnecting?
 	 */
-	sleep_on_timeout(&server->wait, 5*HZ);
+	smb_unlock_server(server);
+	sleep_on_timeout(&server->wait, 30*HZ);
+	smb_lock_server(server);
 #endif
 
 	/*
@@ -715,15 +712,11 @@
 		PARANOIA("successful, new pid=%d, generation=%d\n",
 			 server->conn_pid, server->generation);
 		result = 1;
+	} else if (server->state == CONN_RETRYING) {
+		/* allow further attempts later */
+		server->state = CONN_RETRIED;
 	}
 
-	/*
-	 * Restore the original pid if we didn't get a new one.
-	 */
-out_restore:
-	if (!server->conn_pid)
-		server->conn_pid = pid;
-
 out:
 	return result;
 }
@@ -742,19 +735,16 @@
 	s->err = 0;
 
 	/* Make sure we have a connection */
-	if (s->state != CONN_VALID)
-	{
+	if (s->state != CONN_VALID) {
 		if (!smb_retry(s))
 			goto out;
 	}
 
-	if (smb_request(s) < 0)
-	{
+	if (smb_request(s) < 0) {
 		DEBUG1("smb_request failed\n");
 		goto out;
 	}
-	if (smb_valid_packet(s->packet) != 0)
-	{
+	if (smb_valid_packet(s->packet) != 0) {
 		PARANOIA("invalid packet!\n");
 		goto out;
 	}
@@ -764,8 +754,7 @@
 	 * is squashing some error codes, but I don't think this is
 	 * correct: after a server error the packet won't be valid.
 	 */
-	if (s->rcls != 0)
-	{
+	if (s->rcls != 0) {
 		result = -smb_errno(s);
 		if (!result)
 			printk(KERN_DEBUG "smb_request_ok: rcls=%d, err=%d mapped to 0\n",
@@ -785,10 +774,6 @@
 /*
  * This implements the NEWCONN ioctl. It installs the server pid,
  * sets server->state to CONN_VALID, and wakes up the waiting process.
- *
- * Note that this must be called with the server locked, except for
- * the first call made after mounting the volume. The server pid
- * will be set to zero to indicate that smbfs is awaiting a connection.
  */
 int
 smb_newconn(struct smb_sb_info *server, struct smb_conn_opt *opt)
@@ -798,11 +783,13 @@
 
 	VERBOSE("fd=%d, pid=%d\n", opt->fd, current->pid);
 
+	smb_lock_server(server);
+
 	/*
-	 * Make sure we don't already have a pid ...
+	 * Make sure we don't already have a valid connection ...
 	 */
 	error = -EINVAL;
-	if (server->conn_pid)
+	if (server->state == CONN_VALID)
 		goto out;
 
 	error = -EACCES;
@@ -851,6 +838,8 @@
 		int len = smb_round_length(server->opt.max_xmit);
 		char *buf = smb_vmalloc(len);
 		if (buf) {
+			if (server->packet)
+				smb_vfree(server->packet);
 			server->packet = buf;
 			server->packet_size = len;
 		} else {
@@ -863,6 +852,8 @@
 	}
 
 out:
+	smb_unlock_server(server);
+
 #ifdef SMB_RETRY_INTR
 	wake_up_interruptible(&server->wait);
 #else
@@ -1016,7 +1007,7 @@
 	}
 
 	if (!smb_is_open(inode)) {
-		struct smb_sb_info *server = SMB_SERVER(inode);
+		struct smb_sb_info *server = server_from_inode(inode);
 		smb_lock_server(server);
 		result = 0;
 		if (!smb_is_open(inode))
@@ -1084,6 +1075,8 @@
 		 * two seconds ... round the times to avoid needless
 		 * cache invalidations!
 		 */
+		/* FIXME: could this be the cause of the problems with tar
+		   randomly reporting files as changed? */
 		if (ino->i_mtime & 1)
 			ino->i_mtime--;
 		if (ino->i_atime & 1)
@@ -1119,9 +1112,8 @@
 {
 	int result = 0;
 
-	if (smb_is_open(ino))
-	{
-		struct smb_sb_info *server = SMB_SERVER(ino);
+	if (smb_is_open(ino)) {
+		struct smb_sb_info *server = server_from_inode(ino);
 		smb_lock_server(server);
 		result = smb_proc_close_inode(server, ino);
 		smb_unlock_server(server);
@@ -1423,6 +1415,17 @@
 	return result;
 }
 
+/*
+ * Called with the server locked
+ */
+int
+smb_proc_flush(struct smb_sb_info *server, __u16 fileid)
+{
+	smb_setup_header(server, SMBflush, 1, 0);
+	WSET(server->packet, smb_vwv0, fileid);
+	return smb_request_ok(server, SMBflush, 0, 0);
+}
+
 int
 smb_proc_trunc(struct smb_sb_info *server, __u16 fid, __u32 length)
 {
@@ -1431,7 +1434,7 @@
 
 	smb_lock_server(server);
 
-      retry:
+retry:
 	p = smb_setup_header(server, SMBwrite, 5, 3);
 	WSET(server->packet, smb_vwv0, fid);
 	WSET(server->packet, smb_vwv1, 0);
@@ -1445,7 +1448,16 @@
 			goto retry;
 		goto out;
 	}
+
+	/*
+	 * win9x doesn't appear to update the size immediately.
+	 * It will return the old file size after the truncate,
+	 * confusing smbfs.
+	 * NT and Samba return the new value immediately.
+	 */
 	result = 0;
+	if (server->mnt->flags & SMB_MOUNT_WIN95)
+		result = smb_proc_flush(server, fid);
 out:
 	smb_unlock_server(server);
 	return result;
diff -urN -X exclude linux-2.4.4-orig/fs/smbfs/sock.c linux-2.4.4-smbfs/fs/smbfs/sock.c
--- linux-2.4.4-orig/fs/smbfs/sock.c	Thu Feb 22 20:52:03 2001
+++ linux-2.4.4-smbfs/fs/smbfs/sock.c	Wed May  2 20:42:24 2001
@@ -150,14 +150,14 @@
 	DEBUG1("found=%d, count=%d, result=%d\n", found, count, result);
 	if (found)
 		found_data(job->sk);
-	kfree(ptr);
+	smb_kfree(ptr);
 }
 
 static void
 smb_data_ready(struct sock *sk, int len)
 {
 	struct data_callback* job;
-	job = kmalloc(sizeof(struct data_callback),GFP_ATOMIC);
+	job = smb_kmalloc(sizeof(struct data_callback),GFP_ATOMIC);
 	if(job == 0) {
 		printk("smb_data_ready: lost SESSION KEEPALIVE due to OOM.\n");
 		found_data(sk);
diff -urN -X exclude linux-2.4.4-orig/include/linux/smb.h linux-2.4.4-smbfs/include/linux/smb.h
--- linux-2.4.4-orig/include/linux/smb.h	Thu Feb 22 21:34:21 2001
+++ linux-2.4.4-smbfs/include/linux/smb.h	Wed May  2 22:00:19 2001
@@ -94,21 +94,14 @@
 };
 
 enum smb_conn_state {
-        CONN_VALID,             /* everything's fine */
-        CONN_INVALID,           /* Something went wrong, but did not
-                                   try to reconnect yet. */
-        CONN_RETRIED            /* Tried a reconnection, but was refused */
+	CONN_VALID,		/* everything's fine */
+	CONN_INVALID,		/* Something went wrong, but did not
+				   try to reconnect yet. */
+	CONN_RETRIED,		/* Tried a reconnection, but was refused */
+	CONN_RETRYING		/* Currently trying to reconnect */
 };
 
-/*
- * The readdir cache size controls how many directory entries are cached.
- */
-#define SMB_READDIR_CACHE_SIZE        64
-
 #define SMB_SUPER_MAGIC               0x517B
-
-#define SMB_SERVER(inode)    (&(inode->i_sb->u.smbfs_sb))
-#define SMB_INOP(inode)      (&(inode->u.smbfs_i))
 
 #define SMB_HEADER_LEN   37     /* includes everything up to, but not
                                  * including smb_bcc */
diff -urN -X exclude linux-2.4.4-orig/include/linux/smb_fs.h linux-2.4.4-smbfs/include/linux/smb_fs.h
--- linux-2.4.4-orig/include/linux/smb_fs.h	Thu Feb 22 21:44:20 2001
+++ linux-2.4.4-smbfs/include/linux/smb_fs.h	Wed May  2 22:09:58 2001
@@ -48,8 +48,11 @@
 
 #ifdef DEBUG_SMB_MALLOC
 
+#include <linux/slab.h>
+
 extern int smb_malloced;
 extern int smb_current_vmalloced;
+extern int smb_current_kmalloced;
 
 static inline void *
 smb_vmalloc(unsigned int size)
@@ -66,12 +69,27 @@
         vfree(obj);
 }
 
+static inline void *
+smb_kmalloc(size_t size, int flags)
+{
+	smb_malloced += 1;
+	smb_current_kmalloced += 1;
+	return kmalloc(size, flags);
+}
+
+static inline void
+smb_kfree(void *obj)
+{
+	smb_current_kmalloced -= 1;
+	kfree(obj);
+}
+
 #else /* DEBUG_SMB_MALLOC */
 
-#define smb_kmalloc(s,p) kmalloc(s,p)
-#define smb_kfree_s(o,s) kfree(o)
-#define smb_vmalloc(s)   vmalloc(s)
-#define smb_vfree(o)     vfree(o)
+#define smb_kmalloc(s,p)	kmalloc(s,p)
+#define smb_kfree(o)		kfree(o)
+#define smb_vmalloc(s)		vmalloc(s)
+#define smb_vfree(o)		vfree(o)
 
 #endif /* DEBUG_SMB_MALLOC */
 
@@ -139,7 +157,7 @@
 static inline int
 smb_is_open(struct inode *i)
 {
-	return (i->u.smbfs_i.open == SMB_SERVER(i)->generation);
+	return (i->u.smbfs_i.open == server_from_inode(i)->generation);
 }
 
 
@@ -194,6 +212,7 @@
 int smb_proc_dskattr(struct super_block *, struct statfs *);
 int smb_proc_disconnect(struct smb_sb_info *);
 int smb_proc_trunc(struct smb_sb_info *, __u16, __u32);
+int smb_proc_flush(struct smb_sb_info *, __u16);
 void smb_init_root_dirent(struct smb_sb_info *, struct smb_fattr *);
 
 /* linux/fs/smbfs/sock.c */
diff -urN -X exclude linux-2.4.4-orig/include/linux/smb_fs_sb.h linux-2.4.4-smbfs/include/linux/smb_fs_sb.h
--- linux-2.4.4-orig/include/linux/smb_fs_sb.h	Thu Feb 22 21:34:21 2001
+++ linux-2.4.4-smbfs/include/linux/smb_fs_sb.h	Wed May  2 22:00:19 2001
@@ -58,6 +58,19 @@
 		       struct nls_table *, struct nls_table *);
 };
 
+
+static inline void
+smb_lock_server(struct smb_sb_info *server)
+{
+	down(&(server->sem));
+}
+
+static inline void
+smb_unlock_server(struct smb_sb_info *server)
+{
+	up(&(server->sem));
+}
+
 #endif /* __KERNEL__ */
 
 #endif
Binary files linux-2.4.4-orig/patch-2.4.4-ac3.gz and linux-2.4.4-smbfs/patch-2.4.4-ac3.gz differ

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH][RFT] smbfs bugfixes for 2.4.4
  2001-05-02 21:59 [PATCH][RFT] smbfs bugfixes for 2.4.4 Urban Widmark
@ 2001-05-06  0:14 ` Xuan Baldauf
  2001-05-06 10:16   ` Urban Widmark
  2001-05-07 17:44   ` Linus Torvalds
  0 siblings, 2 replies; 11+ messages in thread
From: Xuan Baldauf @ 2001-05-06  0:14 UTC (permalink / raw)
  To: Urban Widmark; +Cc: linux-kernel, Xuan Baldauf



Urban Widmark wrote:

> Hello all
>
> This patch have been building up for a while, without reaching some
> undefined level of readiness. I would like some feedback from other smbfs
> users before submitting this for 2.4.4-something. Particularly from people
> mounting win9x shares.
>
> * win9x will sometimes not give back the right filesize, this can cause
>   problems when cp'ing a file over an existing one. When truncating the
>   file to 0 bytes the server keeps reporting the old size and much
>   confusion arises.
>   (reported by Xuan Baldauf, could you verify that this fixes it? If it
>    does it's a much better fix than the workaround you got before.)

Hello Urban,

it does not fix|work around the bug completely:

1. windows: Create a file, e.g. with 741 bytes.
2. linux: "ls -la" will show you the file with the correct size (741)
3. linux: read the file into your smbfs cache (e.g. "less file")
4. windows: add some contents to the file, e.g. so that it is now 896 bytes
long
5. linux: "ls -la" will show you the file with the correct size (896)
6. linux: read the file (e.g. "less file")

What you should see, on the linux box, are the new contents of the file. What
you will see are the old contents of the file plus a lot "^@^@^@^@^@^@^@"
(which mean null bytes) at the end of the old contents.

The file sizes used here should be arbitrary, but this concrete case just
happened now. They do not need to have to cross a 512-byte-block boundary or
so.

Also, "ls" is correct every time, only the content is incorrect.

So maybe, if reader and writer are the same smbfs, the bug might be fixed, but
if the writer is another machine than the reader, it is not.

Xuân.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH][RFT] smbfs bugfixes for 2.4.4
  2001-05-06  0:14 ` Xuan Baldauf
@ 2001-05-06 10:16   ` Urban Widmark
  2001-05-07 17:44   ` Linus Torvalds
  1 sibling, 0 replies; 11+ messages in thread
From: Urban Widmark @ 2001-05-06 10:16 UTC (permalink / raw)
  To: Xuan Baldauf; +Cc: linux-kernel

On Sun, 6 May 2001, Xuan Baldauf wrote:

> it does not fix|work around the bug completely:
> 
> 1. windows: Create a file, e.g. with 741 bytes.
> 2. linux: "ls -la" will show you the file with the correct size (741)
> 3. linux: read the file into your smbfs cache (e.g. "less file")
> 4. windows: add some contents to the file, e.g. so that it is now 896 bytes
> long
> 5. linux: "ls -la" will show you the file with the correct size (896)
> 6. linux: read the file (e.g. "less file")

Ah, but now you are talking about a different bug.

Your original testcase only contained changes from the smbfs client (the
abc/xyz test). For me that is solved by this patch and I wanted you to
check if it did in your environment as well.

I have one other report of something being broken with changes made on the
server side only.

There is also yet another problem if you change a file from smbfs and from
the server. The smbfs side will remember the wrong filesize. This may be a
fix for that:
http://www.hojdpunkten.ac.se/054/samba/smbfs-2.4.4-truncate+retry-3.patch
								(-3, not -2)

/Urban


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH][RFT] smbfs bugfixes for 2.4.4
  2001-05-06  0:14 ` Xuan Baldauf
  2001-05-06 10:16   ` Urban Widmark
@ 2001-05-07 17:44   ` Linus Torvalds
  2001-05-08 20:43     ` Urban Widmark
  1 sibling, 1 reply; 11+ messages in thread
From: Linus Torvalds @ 2001-05-07 17:44 UTC (permalink / raw)
  To: linux-kernel

In article <3AF4974C.D5D85498@baldauf.org>,
Xuan Baldauf  <xuan--lkml@baldauf.org> wrote:
>
>it does not fix|work around the bug completely:
>
>1. windows: Create a file, e.g. with 741 bytes.
>2. linux: "ls -la" will show you the file with the correct size (741)
>3. linux: read the file into your smbfs cache (e.g. "less file")
>4. windows: add some contents to the file, e.g. so that it is now 896 bytes
>long
>5. linux: "ls -la" will show you the file with the correct size (896)
>6. linux: read the file (e.g. "less file")
>
>What you should see, on the linux box, are the new contents of the file. What
>you will see are the old contents of the file plus a lot "^@^@^@^@^@^@^@"
>(which mean null bytes) at the end of the old contents.

This is a different problem. Apparently the Linux client does not
invalidate its caches sufficiently often. The smb client should at least
do a "invalidate_inode_pages(inode);" when it notices that the file size
has changed.

It has code to do that in smb_revalidate_inode(), but it may be that
something else refreshes the inode size _without_ doing the proper
invalidation checks. Or maybe Urban broke that logic by mistake while
fixing the other one ;)

		Linus

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH][RFT] smbfs bugfixes for 2.4.4
  2001-05-07 17:44   ` Linus Torvalds
@ 2001-05-08 20:43     ` Urban Widmark
  2001-05-08 22:02       ` James H. Puttick
  2001-05-21 11:00       ` Xuan Baldauf
  0 siblings, 2 replies; 11+ messages in thread
From: Urban Widmark @ 2001-05-08 20:43 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, Xuan Baldauf, James H. Puttick

On 7 May 2001, Linus Torvalds wrote:

> It has code to do that in smb_revalidate_inode(), but it may be that
> something else refreshes the inode size _without_ doing the proper
> invalidation checks. Or maybe Urban broke that logic by mistake while
> fixing the other one ;)

No, I broke it when copying the ncpfs dircache code.

That code will reuse an old inode if it already exists (and thus also any
pages attached to it), which is what I wanted and should be fine except
that it needs to invalidate_inode_pages() if something changed.


Xuan and James, you have both seen this bug with smbfs not properly
handling changes made on the server. Could you please test this patch vs
2.4.4 and let me know if it helps or not.

http://www.hojdpunkten.ac.se/054/samba/smbfs-2.4.4-truncate+retry-4.patch
(Apply with 'patch -p1' in the linux/ source dir)

/Urban


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH][RFT] smbfs bugfixes for 2.4.4
  2001-05-08 20:43     ` Urban Widmark
@ 2001-05-08 22:02       ` James H. Puttick
  2001-05-21 11:00       ` Xuan Baldauf
  1 sibling, 0 replies; 11+ messages in thread
From: James H. Puttick @ 2001-05-08 22:02 UTC (permalink / raw)
  To: Urban Widmark; +Cc: Linus Torvalds, linux-kernel, Xuan Baldauf

> No, I broke it when copying the ncpfs dircache code.
> 
> That code will reuse an old inode if it already exists (and thus also
> any pages attached to it), which is what I wanted and should be fine
> except that it needs to invalidate_inode_pages() if something changed.
> 
> Xuan and James, you have both seen this bug with smbfs not properly
> handling changes made on the server. Could you please test this patch
> vs 2.4.4 and let me know if it helps or not.
> 
> http://www.hojdpunkten.ac.se/054/samba/smbfs-2.4.4-truncate+retry-4.patch

Urban:

I am actually using a 2.4.3 kernel, rather than 2.4.4. However, I 
manually applied the patches to my 2.4.3 kernel, and did some tests - 
it appears to work now!

I probably won't be using Samba heavily until next week, but I will let 
you know if I see any evidence that the problem is not fixed.

Thank you very much for the fix.

-- James


----------------------------------------
James H. Puttick

Kerr Vayne Systems Ltd.
1 Valleywood Drive, Unit 5A
Markham, Ontario L3R 5L9
Canada

+1 905 475 6161  office
+1 905 479 9833  fax

mailto:james.puttick@kvs.com
----------------------------------------

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH][RFT] smbfs bugfixes for 2.4.4
  2001-05-08 20:43     ` Urban Widmark
  2001-05-08 22:02       ` James H. Puttick
@ 2001-05-21 11:00       ` Xuan Baldauf
  2001-05-21 18:03         ` Urban Widmark
  2001-05-22 22:25         ` Urban Widmark
  1 sibling, 2 replies; 11+ messages in thread
From: Xuan Baldauf @ 2001-05-21 11:00 UTC (permalink / raw)
  To: Urban Widmark
  Cc: Linus Torvalds, linux-kernel, Xuan Baldauf, James H. Puttick



Urban Widmark wrote:

> On 7 May 2001, Linus Torvalds wrote:
>
> > It has code to do that in smb_revalidate_inode(), but it may be that
> > something else refreshes the inode size _without_ doing the proper
> > invalidation checks. Or maybe Urban broke that logic by mistake while
> > fixing the other one ;)
>
> No, I broke it when copying the ncpfs dircache code.
>
> That code will reuse an old inode if it already exists (and thus also any
> pages attached to it), which is what I wanted and should be fine except
> that it needs to invalidate_inode_pages() if something changed.
>
> Xuan and James, you have both seen this bug with smbfs not properly
> handling changes made on the server. Could you please test this patch vs
> 2.4.4 and let me know if it helps or not.
>
> http://www.hojdpunkten.ac.se/054/samba/smbfs-2.4.4-truncate+retry-4.patch
> (Apply with 'patch -p1' in the linux/ source dir)
>
> /Urban

Hello Urban,

I've been playing around a while with that patch and so far could not find any
problems anymore. But I've noticed some other annoying behaviour, which might
be caused by trying to work around the initially reported bug where the
win98se server does not update something (the new file contents after writing
to a file or the file size?) when something is written by the client: Every
little SMBwrite is followed by an SMBflush, which is translated by win98se
into a "commit" of the file, which seems to be an fsync(2) equivalent.

That is annoying, because it heavily slows down bulk transfers of small
writes, like automatically unzipping a new mozilla build from the linux box to
the windows box. Every write of say 100 bytes is implemented as

send write req
recv write ack
send flush req
sync to disk (on the windows machine)
recv flush ack

This creates heaviest disk load (on the windows machine) for minimal
throughput. Is the SMBflush needed anymore, after you seem to have found
another, better workaround?

Xuân.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH][RFT] smbfs bugfixes for 2.4.4
  2001-05-21 11:00       ` Xuan Baldauf
@ 2001-05-21 18:03         ` Urban Widmark
  2001-05-22 22:25         ` Urban Widmark
  1 sibling, 0 replies; 11+ messages in thread
From: Urban Widmark @ 2001-05-21 18:03 UTC (permalink / raw)
  To: Xuan Baldauf; +Cc: linux-kernel, James H. Puttick

On Mon, 21 May 2001, Xuan Baldauf wrote:

> Hello Urban,
> 
> I've been playing around a while with that patch and so far could not find any
> problems anymore. But I've noticed some other annoying behaviour, which might

Good.

> be caused by trying to work around the initially reported bug where the
> win98se server does not update something (the new file contents after writing
> to a file or the file size?) when something is written by the client: Every
> little SMBwrite is followed by an SMBflush, which is translated by win98se
> into a "commit" of the file, which seems to be an fsync(2) equivalent.

Yes, I know.

> 
> That is annoying, because it heavily slows down bulk transfers of small
> writes, like automatically unzipping a new mozilla build from the linux box to
> the windows box. Every write of say 100 bytes is implemented as
> 
> send write req
> recv write ack
> send flush req
> sync to disk (on the windows machine)
> recv flush ack
>
> This creates heaviest disk load (on the windows machine) for minimal
> throughput. Is the SMBflush needed anymore, after you seem to have found
> another, better workaround?

SMBflush is the better variant of the workaround I sent you first, as
flush will always work but setting that flag doesn't ("correctness" over
speed, or something like that).

But does it really do that? It will only flush when the page you wrote to
is sent to the fs for writing. The page cache doesn't do that on each
write (I assume, should check) so then a 10 byte write, followed by
another 10 doesn't give 2 flushes but only one when that page is written.

It's still very much a slowdown with win9x servers.


I'll see if I can come up with something better than flush. What I need is
a way to tell the server that the file did change so that it will start
giving back the correct size.

I'm guessing that the win9x smb server caches some values but doesn't
understand that writing changes that. Possibly an assumption that the
client "knows" the correct filesize (but that breaks if someone else
changes the file).

People shouldn't be using win9x as "servers" anyway ... :)

/Urban


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH][RFT] smbfs bugfixes for 2.4.4
  2001-05-21 11:00       ` Xuan Baldauf
  2001-05-21 18:03         ` Urban Widmark
@ 2001-05-22 22:25         ` Urban Widmark
  2001-05-22 22:57           ` Xuan Baldauf
  1 sibling, 1 reply; 11+ messages in thread
From: Urban Widmark @ 2001-05-22 22:25 UTC (permalink / raw)
  To: Xuan Baldauf; +Cc: linux-kernel, James H. Puttick

On Mon, 21 May 2001, Xuan Baldauf wrote:

> That is annoying, because it heavily slows down bulk transfers of small
> writes, like automatically unzipping a new mozilla build from the linux box to
> the windows box. Every write of say 100 bytes is implemented as
> 
> send write req
> recv write ack
> send flush req
> sync to disk (on the windows machine)
> recv flush ack

The only other way I have found so far to get it to return the right file
size is to do a "seek-to-end". That still means an extra SMB but it avoids
the very painful "sync to disk".

Fortunately the seek is only necessary when refreshing inode info, on a
"win95" server, on a file that is open and that we have written to.

This should be significantly better, but still works with my testcases.
patch vs 2.4.5-pre4, please test.

/Urban


diff -urN -X exclude linux-2.4.5-pre4-orig/fs/smbfs/Makefile linux-2.4.5-pre4-smbfs/fs/smbfs/Makefile
--- linux-2.4.5-pre4-orig/fs/smbfs/Makefile	Thu Feb 22 20:52:03 2001
+++ linux-2.4.5-pre4-smbfs/fs/smbfs/Makefile	Wed May 23 00:19:08 2001
@@ -16,6 +16,7 @@
 # SMBFS_PARANOIA should normally be enabled.
 
 EXTRA_CFLAGS += -DSMBFS_PARANOIA
+EXTRA_CFLAGS += -DSMB_WIN95_LOCALWRITE_FIX
 #EXTRA_CFLAGS += -DSMBFS_DEBUG
 #EXTRA_CFLAGS += -DSMBFS_DEBUG_VERBOSE
 #EXTRA_CFLAGS += -DDEBUG_SMB_MALLOC
diff -urN -X exclude linux-2.4.5-pre4-orig/fs/smbfs/file.c linux-2.4.5-pre4-smbfs/fs/smbfs/file.c
--- linux-2.4.5-pre4-orig/fs/smbfs/file.c	Sun May 20 15:20:17 2001
+++ linux-2.4.5-pre4-smbfs/fs/smbfs/file.c	Tue May 22 23:53:43 2001
@@ -151,6 +151,7 @@
 		 * Update the inode now rather than waiting for a refresh.
 		 */
 		inode->i_mtime = inode->i_atime = CURRENT_TIME;
+		inode->u.smbfs_i.flags |= SMB_F_LOCALWRITE;
 		if (offset > inode->i_size)
 			inode->i_size = offset;
 	} while (count);
diff -urN -X exclude linux-2.4.5-pre4-orig/fs/smbfs/inode.c linux-2.4.5-pre4-smbfs/fs/smbfs/inode.c
--- linux-2.4.5-pre4-orig/fs/smbfs/inode.c	Sun May 20 15:20:17 2001
+++ linux-2.4.5-pre4-smbfs/fs/smbfs/inode.c	Tue May 22 21:02:29 2001
@@ -141,8 +141,8 @@
 	inode->u.smbfs_i.oldmtime = jiffies;
 
 	if (inode->i_mtime != last_time || inode->i_size != last_sz) {
-		VERBOSE("%s/%s changed, old=%ld, new=%ld, oz=%ld, nz=%ld\n",
-			DENTRY_PATH(dentry),
+		VERBOSE("%ld changed, old=%ld, new=%ld, oz=%ld, nz=%ld\n",
+			inode->i_ino,
 			(long) last_time, (long) inode->i_mtime,
 			(long) last_sz, (long) inode->i_size);
 
diff -urN -X exclude linux-2.4.5-pre4-orig/fs/smbfs/proc.c linux-2.4.5-pre4-smbfs/fs/smbfs/proc.c
--- linux-2.4.5-pre4-orig/fs/smbfs/proc.c	Sun May 20 15:20:17 2001
+++ linux-2.4.5-pre4-smbfs/fs/smbfs/proc.c	Wed May 23 00:19:19 2001
@@ -919,6 +919,31 @@
 }
 
 /*
+ * Called with the server locked
+ */
+static int
+smb_proc_seek(struct smb_sb_info *server, __u16 fileid,
+	      __u16 mode, off_t offset)
+{
+	int result;
+
+	smb_setup_header(server, SMBlseek, 4, 0);
+	WSET(server->packet, smb_vwv0, fileid);
+	WSET(server->packet, smb_vwv1, mode);
+	DSET(server->packet, smb_vwv2, offset);
+
+	result = smb_request_ok(server, SMBlseek, 2, 0);
+	if (result < 0) {
+		result = 0;
+		goto out;
+	}
+
+	result = DVAL(server->packet, smb_vwv0);
+out:
+	return result;
+}
+
+/*
  * We're called with the server locked, and we leave it that way.
  */
 static int
@@ -1210,10 +1235,12 @@
 	if (result >= 0)
 		result = WVAL(server->packet, smb_vwv0);
 
+#ifndef SMB_WIN95_LOCALWRITE_FIX
 	/* flush to disk, to trigger win9x to update its filesize */
 	/* FIXME: this will be rather costly, won't it? */
 	if (server->mnt->flags & SMB_MOUNT_WIN95)
 		smb_proc_flush(server, fileid);
+#endif
 
 	smb_unlock_server(server);
 	return result;
@@ -2246,6 +2273,7 @@
 		    struct smb_fattr *fattr)
 {
 	int result;
+	struct inode *inode = dir->d_inode;
 
 	smb_init_dirent(server, fattr);
 
@@ -2262,6 +2290,22 @@
 		else
 			result = smb_proc_getattr_trans2(server, dir, fattr);
 	}
+
+#ifdef SMB_WIN95_LOCALWRITE_FIX
+	/*
+	 * None of the getattr versions here can make win95 return the right
+	 * filesize if there are changes made to it. A seek-to-end does return
+	 * the right size, but we only need to do that on files we have written.
+	 */
+	if (server->mnt->flags & SMB_MOUNT_WIN95 &&
+	    inode &&
+	    inode->u.smbfs_i.flags & SMB_F_LOCALWRITE &&
+	    smb_is_open(inode))
+	{
+		__u16 fileid = inode->u.smbfs_i.fileid;
+		fattr->f_size = smb_proc_seek(server, fileid, 2, 0);
+	}
+#endif
 
 	smb_finish_dirent(server, fattr);
 	return result;
diff -urN -X exclude linux-2.4.5-pre4-orig/include/linux/smb_fs.h linux-2.4.5-pre4-smbfs/include/linux/smb_fs.h
--- linux-2.4.5-pre4-orig/include/linux/smb_fs.h	Sun May 20 15:37:59 2001
+++ linux-2.4.5-pre4-smbfs/include/linux/smb_fs.h	Tue May 22 21:47:05 2001
@@ -93,6 +93,11 @@
 
 #endif /* DEBUG_SMB_MALLOC */
 
+/*
+ * Flags for the in-memory inode
+ */
+#define SMB_F_LOCALWRITE	0x02	/* file modified locally */
+
 
 /* NT1 protocol capability bits */
 #define SMB_CAP_RAW_MODE         0x0001
diff -urN -X exclude linux-2.4.5-pre4-orig/include/linux/smb_fs_i.h linux-2.4.5-pre4-smbfs/include/linux/smb_fs_i.h
--- linux-2.4.5-pre4-orig/include/linux/smb_fs_i.h	Sun May 20 15:23:22 2001
+++ linux-2.4.5-pre4-smbfs/include/linux/smb_fs_i.h	Tue May 22 20:58:03 2001
@@ -26,6 +26,7 @@
 	__u16 attr;		/* Attribute fields, DOS value */
 
 	__u16 access;		/* Access mode */
+	__u16 flags;
 	unsigned long oldmtime;	/* last time refreshed */
 	unsigned long closed;	/* timestamp when closed */
 	unsigned openers;	/* number of fileid users */


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH][RFT] smbfs bugfixes for 2.4.4
  2001-05-22 22:25         ` Urban Widmark
@ 2001-05-22 22:57           ` Xuan Baldauf
  2001-05-22 23:12             ` Urban Widmark
  0 siblings, 1 reply; 11+ messages in thread
From: Xuan Baldauf @ 2001-05-22 22:57 UTC (permalink / raw)
  To: Urban Widmark; +Cc: linux-kernel, James H. Puttick



Urban Widmark wrote:

> On Mon, 21 May 2001, Xuan Baldauf wrote:
>
> > That is annoying, because it heavily slows down bulk transfers of small
> > writes, like automatically unzipping a new mozilla build from the linux box to
> > the windows box. Every write of say 100 bytes is implemented as
> >
> > send write req
> > recv write ack
> > send flush req
> > sync to disk (on the windows machine)
> > recv flush ack
>
> The only other way I have found so far to get it to return the right file
> size is to do a "seek-to-end". That still means an extra SMB but it avoids
> the very painful "sync to disk".
>
> Fortunately the seek is only necessary when refreshing inode info, on a
> "win95" server, on a file that is open and that we have written to.

Maybe it is also a workaround for the problem where changes on the windows side are not reflected?

>
>
> This should be significantly better, but still works with my testcases.
> patch vs 2.4.5-pre4, please test.
>
> /Urban
>

[...patch...]

Is it possible to resend the patch in mime format or publish it somewhere accessible by an URL? Netscape Messenger
creates spaces everywhere where tabs should be :-(

Xuân.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH][RFT] smbfs bugfixes for 2.4.4
  2001-05-22 22:57           ` Xuan Baldauf
@ 2001-05-22 23:12             ` Urban Widmark
  0 siblings, 0 replies; 11+ messages in thread
From: Urban Widmark @ 2001-05-22 23:12 UTC (permalink / raw)
  To: Xuan Baldauf; +Cc: linux-kernel, James H. Puttick

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

On Wed, 23 May 2001, Xuan Baldauf wrote:

> Urban Widmark wrote:
> 
> > The only other way I have found so far to get it to return the right file
> > size is to do a "seek-to-end". That still means an extra SMB but it avoids
> > the very painful "sync to disk".
> >
> > Fortunately the seek is only necessary when refreshing inode info, on a
> > "win95" server, on a file that is open and that we have written to.
> 
> Maybe it is also a workaround for the problem where changes on the
> windows side are not reflected?

Yes, that was the point of removing the LOCALWRITE flag in the first
place. As far as I can tell this should still fix the file cache/sync
problems that have been found.


> Is it possible to resend the patch in mime format or publish it somewhere accessible by an URL? Netscape Messenger
> creates spaces everywhere where tabs should be :-(

Certainly.

/Urban

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

diff -urN -X exclude linux-2.4.5-pre4-orig/fs/smbfs/Makefile linux-2.4.5-pre4-smbfs/fs/smbfs/Makefile
--- linux-2.4.5-pre4-orig/fs/smbfs/Makefile	Thu Feb 22 20:52:03 2001
+++ linux-2.4.5-pre4-smbfs/fs/smbfs/Makefile	Wed May 23 00:19:08 2001
@@ -16,6 +16,7 @@
 # SMBFS_PARANOIA should normally be enabled.
 
 EXTRA_CFLAGS += -DSMBFS_PARANOIA
+EXTRA_CFLAGS += -DSMB_WIN95_LOCALWRITE_FIX
 #EXTRA_CFLAGS += -DSMBFS_DEBUG
 #EXTRA_CFLAGS += -DSMBFS_DEBUG_VERBOSE
 #EXTRA_CFLAGS += -DDEBUG_SMB_MALLOC
diff -urN -X exclude linux-2.4.5-pre4-orig/fs/smbfs/file.c linux-2.4.5-pre4-smbfs/fs/smbfs/file.c
--- linux-2.4.5-pre4-orig/fs/smbfs/file.c	Sun May 20 15:20:17 2001
+++ linux-2.4.5-pre4-smbfs/fs/smbfs/file.c	Tue May 22 23:53:43 2001
@@ -151,6 +151,7 @@
 		 * Update the inode now rather than waiting for a refresh.
 		 */
 		inode->i_mtime = inode->i_atime = CURRENT_TIME;
+		inode->u.smbfs_i.flags |= SMB_F_LOCALWRITE;
 		if (offset > inode->i_size)
 			inode->i_size = offset;
 	} while (count);
diff -urN -X exclude linux-2.4.5-pre4-orig/fs/smbfs/inode.c linux-2.4.5-pre4-smbfs/fs/smbfs/inode.c
--- linux-2.4.5-pre4-orig/fs/smbfs/inode.c	Sun May 20 15:20:17 2001
+++ linux-2.4.5-pre4-smbfs/fs/smbfs/inode.c	Tue May 22 21:02:29 2001
@@ -141,8 +141,8 @@
 	inode->u.smbfs_i.oldmtime = jiffies;
 
 	if (inode->i_mtime != last_time || inode->i_size != last_sz) {
-		VERBOSE("%s/%s changed, old=%ld, new=%ld, oz=%ld, nz=%ld\n",
-			DENTRY_PATH(dentry),
+		VERBOSE("%ld changed, old=%ld, new=%ld, oz=%ld, nz=%ld\n",
+			inode->i_ino,
 			(long) last_time, (long) inode->i_mtime,
 			(long) last_sz, (long) inode->i_size);
 
diff -urN -X exclude linux-2.4.5-pre4-orig/fs/smbfs/proc.c linux-2.4.5-pre4-smbfs/fs/smbfs/proc.c
--- linux-2.4.5-pre4-orig/fs/smbfs/proc.c	Sun May 20 15:20:17 2001
+++ linux-2.4.5-pre4-smbfs/fs/smbfs/proc.c	Wed May 23 00:19:19 2001
@@ -919,6 +919,31 @@
 }
 
 /*
+ * Called with the server locked
+ */
+static int
+smb_proc_seek(struct smb_sb_info *server, __u16 fileid,
+	      __u16 mode, off_t offset)
+{
+	int result;
+
+	smb_setup_header(server, SMBlseek, 4, 0);
+	WSET(server->packet, smb_vwv0, fileid);
+	WSET(server->packet, smb_vwv1, mode);
+	DSET(server->packet, smb_vwv2, offset);
+
+	result = smb_request_ok(server, SMBlseek, 2, 0);
+	if (result < 0) {
+		result = 0;
+		goto out;
+	}
+
+	result = DVAL(server->packet, smb_vwv0);
+out:
+	return result;
+}
+
+/*
  * We're called with the server locked, and we leave it that way.
  */
 static int
@@ -1210,10 +1235,12 @@
 	if (result >= 0)
 		result = WVAL(server->packet, smb_vwv0);
 
+#ifndef SMB_WIN95_LOCALWRITE_FIX
 	/* flush to disk, to trigger win9x to update its filesize */
 	/* FIXME: this will be rather costly, won't it? */
 	if (server->mnt->flags & SMB_MOUNT_WIN95)
 		smb_proc_flush(server, fileid);
+#endif
 
 	smb_unlock_server(server);
 	return result;
@@ -2246,6 +2273,7 @@
 		    struct smb_fattr *fattr)
 {
 	int result;
+	struct inode *inode = dir->d_inode;
 
 	smb_init_dirent(server, fattr);
 
@@ -2262,6 +2290,22 @@
 		else
 			result = smb_proc_getattr_trans2(server, dir, fattr);
 	}
+
+#ifdef SMB_WIN95_LOCALWRITE_FIX
+	/*
+	 * None of the getattr versions here can make win95 return the right
+	 * filesize if there are changes made to it. A seek-to-end does return
+	 * the right size, but we only need to do that on files we have written.
+	 */
+	if (server->mnt->flags & SMB_MOUNT_WIN95 &&
+	    inode &&
+	    inode->u.smbfs_i.flags & SMB_F_LOCALWRITE &&
+	    smb_is_open(inode))
+	{
+		__u16 fileid = inode->u.smbfs_i.fileid;
+		fattr->f_size = smb_proc_seek(server, fileid, 2, 0);
+	}
+#endif
 
 	smb_finish_dirent(server, fattr);
 	return result;
diff -urN -X exclude linux-2.4.5-pre4-orig/include/linux/smb_fs.h linux-2.4.5-pre4-smbfs/include/linux/smb_fs.h
--- linux-2.4.5-pre4-orig/include/linux/smb_fs.h	Sun May 20 15:37:59 2001
+++ linux-2.4.5-pre4-smbfs/include/linux/smb_fs.h	Tue May 22 21:47:05 2001
@@ -93,6 +93,11 @@
 
 #endif /* DEBUG_SMB_MALLOC */
 
+/*
+ * Flags for the in-memory inode
+ */
+#define SMB_F_LOCALWRITE	0x02	/* file modified locally */
+
 
 /* NT1 protocol capability bits */
 #define SMB_CAP_RAW_MODE         0x0001
diff -urN -X exclude linux-2.4.5-pre4-orig/include/linux/smb_fs_i.h linux-2.4.5-pre4-smbfs/include/linux/smb_fs_i.h
--- linux-2.4.5-pre4-orig/include/linux/smb_fs_i.h	Sun May 20 15:23:22 2001
+++ linux-2.4.5-pre4-smbfs/include/linux/smb_fs_i.h	Tue May 22 20:58:03 2001
@@ -26,6 +26,7 @@
 	__u16 attr;		/* Attribute fields, DOS value */
 
 	__u16 access;		/* Access mode */
+	__u16 flags;
 	unsigned long oldmtime;	/* last time refreshed */
 	unsigned long closed;	/* timestamp when closed */
 	unsigned openers;	/* number of fileid users */

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2001-05-22 23:13 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2001-05-02 21:59 [PATCH][RFT] smbfs bugfixes for 2.4.4 Urban Widmark
2001-05-06  0:14 ` Xuan Baldauf
2001-05-06 10:16   ` Urban Widmark
2001-05-07 17:44   ` Linus Torvalds
2001-05-08 20:43     ` Urban Widmark
2001-05-08 22:02       ` James H. Puttick
2001-05-21 11:00       ` Xuan Baldauf
2001-05-21 18:03         ` Urban Widmark
2001-05-22 22:25         ` Urban Widmark
2001-05-22 22:57           ` Xuan Baldauf
2001-05-22 23:12             ` Urban Widmark

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).