[PATCH] smbfs async readpage

[PATCH] smbfs async readpage

[Urban Widmark]

This patch implements async readpage for smbfs. It seems to generate
readahead judging from the network traffic and profile_readahead output
from 2.4.

In case someone cares to read it, the changes to file.c are inspired from
what nfs does in read.c.


A simple test using 'time md5sum /mnt/smb/sp6i386.exe' to an old and slow
test-server over 100Mbps eth gives

sync:   2.309u 0.632s 0:15.55 18.8%
async:  2.275u 0.460s 0:12.41 21.9%

Which is about 20% faster wall clock. A plain cp of the same file has a
smaller gain (lost the numbers, 13 vs 12 seconds or so).

/Urban


diff -urN -X exclude linux-2.5.30-orig/fs/smbfs/file.c linux-2.5.30-smbfs/fs/smbfs/file.c
--- linux-2.5.30-orig/fs/smbfs/file.c	Tue Aug  6 20:34:22 2002
+++ linux-2.5.30-smbfs/fs/smbfs/file.c	Wed Aug  7 00:47:37 2002
@@ -25,6 +25,7 @@
 
 #include "smb_debug.h"
 #include "proto.h"
+#include "request.h"
 
 static int
 smb_fsync(struct file *file, struct dentry * dentry, int datasync)
@@ -96,6 +97,58 @@
 	return result;
 }
 
+static void
+smb_readpage_result(struct smb_request *req)
+{
+	struct page *page = req->rq_page;
+	int result = req->rq_errno;
+
+	VERBOSE("result: %d  page: %p\n", result, page);
+
+	/* FIXME: anyone not using smb_request_ok should probably be using
+	   somthing like smb_valid_packet and smb_verify. */
+
+	if (result >= 0) {
+		result = WVAL(req->rq_header, smb_vwv5);
+		VERBOSE("count: %d\n", result);
+		if (result < PAGE_CACHE_SIZE) {
+			char *buffer = req->rq_pagemap;
+			memset(buffer + result, 0, PAGE_CACHE_SIZE - result);
+		}
+		SetPageUptodate(page);
+	} else {
+		SetPageError(page);
+	}
+
+	flush_dcache_page(page);
+	kunmap(page);
+	unlock_page(page);
+	page_cache_release(page);
+	smb_rput(req);
+}
+
+static int
+smb_readpage_async(struct dentry *dentry, struct page *page)
+{
+	struct smb_sb_info *server = server_from_dentry(dentry);
+	int result;
+	struct smb_request *req;
+
+	VERBOSE("page: %p\n", page);
+	result = -ENOMEM;
+	if (! (req = smb_alloc_request(server, 0)))
+		goto out;
+
+	req->rq_page = page;		/* One page per request to begin with */
+	page_cache_get(page);
+	req->rq_completion = smb_readpage_result;
+	result = server->ops->reada(dentry->d_inode, req);
+
+	/* FIXME: nfs checks the number of read requests for this inode here */
+out:
+	return result;
+}
+
 /*
  * We are called with the page locked and we unlock it when done.
  */
@@ -104,10 +157,20 @@
 {
 	int		error;
 	struct dentry  *dentry = file->f_dentry;
+	struct smb_sb_info *server = server_from_dentry(dentry);
+
+	VERBOSE("\n");
+
+	if (server->ops->reada &&
+	    !PageError(page) && smb_get_rsize(server) > PAGE_CACHE_SIZE) {
+		error = smb_readpage_async(dentry, page);
+		goto out;
+	}
 
 	page_cache_get(page);
 	error = smb_readpage_sync(dentry, page);
 	page_cache_release(page);
+out:
 	return error;
 }
 
diff -urN -X exclude linux-2.5.30-orig/fs/smbfs/proc.c linux-2.5.30-smbfs/fs/smbfs/proc.c
--- linux-2.5.30-orig/fs/smbfs/proc.c	Tue Aug  6 20:34:10 2002
+++ linux-2.5.30-smbfs/fs/smbfs/proc.c	Tue Aug  6 22:31:39 2002
@@ -1252,7 +1252,7 @@
 	req->rq_iov[0].iov_base = req->rq_buffer;
 	req->rq_iov[0].iov_len  = 3;
 
-	req->rq_iov[1].iov_base = req->rq_page;
+	req->rq_iov[1].iov_base = req->rq_pagemap;
 	req->rq_iov[1].iov_len  = req->rq_rsize;
 	req->rq_iovlen = 2;
 
@@ -1280,7 +1280,7 @@
 	DSET(buf, smb_vwv2, offset);
 	WSET(buf, smb_vwv4, 0);
 
-	req->rq_page = data;
+	req->rq_pagemap = data;		/* FIXME: should work on the page */
 	req->rq_rsize = count;
 	req->rq_callback = smb_proc_read_data;
 	req->rq_buffer = rbuf;
@@ -1375,7 +1375,7 @@
 	req->rq_iov[0].iov_base = req->rq_buffer;
 	req->rq_iov[0].iov_len  = data_off;
 
-	req->rq_iov[1].iov_base = req->rq_page;
+	req->rq_iov[1].iov_base = req->rq_pagemap;
 	req->rq_iov[1].iov_len  = req->rq_rsize;
 	req->rq_iovlen = 2;
 
@@ -1408,7 +1408,7 @@
 	DSET(buf, smb_vwv10, (u32)(offset >> 32));      /* high 32 bits */
 	WSET(buf, smb_vwv11, 0);
 
-	req->rq_page = data;
+	req->rq_pagemap = data;
 	req->rq_rsize = count;
 	req->rq_callback = smb_proc_readX_data;
 	req->rq_buffer = pad;
@@ -1429,6 +1429,39 @@
 }
 
 static int
+smb_proc_readaX(struct inode *inode, struct smb_request *req)
+{
+	struct page *page = req->rq_page;
+	loff_t offset = (loff_t)page->index << PAGE_CACHE_SHIFT;
+	unsigned int count = PAGE_SIZE;
+	unsigned char *buf;
+	static char pad[SMB_READX_MAX_PAD];
+
+	smb_setup_header(req, SMBreadX, 12, 0);
+	buf = req->rq_header;
+	WSET(buf, smb_vwv0, 0x00ff);
+	WSET(buf, smb_vwv1, 0);
+	WSET(buf, smb_vwv2, SMB_I(inode)->fileid);
+	DSET(buf, smb_vwv3, (u32)offset);               /* low 32 bits */
+	WSET(buf, smb_vwv5, count);
+	WSET(buf, smb_vwv6, 0);
+	DSET(buf, smb_vwv7, 0);
+	WSET(buf, smb_vwv9, 0);
+	DSET(buf, smb_vwv10, (u32)(offset >> 32));      /* high 32 bits */
+	WSET(buf, smb_vwv11, 0);
+
+	req->rq_pagemap = kmap(page);
+	req->rq_rsize = count;
+	req->rq_callback = smb_proc_readX_data;
+	req->rq_buffer = pad;
+	req->rq_bufsize = SMB_READX_MAX_PAD;
+	req->rq_flags |= SMB_REQ_STATIC | SMB_REQ_NORETRY | SMB_REQ_NORECEIVER;
+	req->rq_resp_wct = 12;
+	req->rq_resp_bcc = -1;
+	return smb_add_request(req);
+}
+
+static int
 smb_proc_writeX(struct inode *inode, loff_t offset, int count, const char *data)
 {
 	struct smb_sb_info *server = server_from_inode(inode);
@@ -2948,6 +2981,7 @@
 static struct smb_ops smb_ops_winNT =
 {
 	read:		smb_proc_readX,
+	reada:		smb_proc_readaX,
 	write:		smb_proc_writeX,
 	readdir:	smb_proc_readdir_long,
 	getattr:	smb_proc_getattr_trans2_all,
diff -urN -X exclude linux-2.5.30-orig/fs/smbfs/request.c linux-2.5.30-smbfs/fs/smbfs/request.c
--- linux-2.5.30-orig/fs/smbfs/request.c	Tue Aug  6 20:34:10 2002
+++ linux-2.5.30-smbfs/fs/smbfs/request.c	Wed Aug  7 00:14:01 2002
@@ -332,6 +332,9 @@
 
 	smbiod_wake_up();
 
+	if (req->rq_flags & SMB_REQ_NORECEIVER)
+		return 0;
+
 	/* FIXME: replace with a timeout-able wake_event_interruptible */
 	timeleft = interruptible_sleep_on_timeout(&req->rq_wait, 30*HZ);
 	if (!timeleft || signal_pending(current)) {
@@ -346,7 +349,7 @@
 		}
 		smb_unlock_server(server);
 	}
-		
+
 	if (!timeleft) {
 		PARANOIA("request [%p, mid=%d] timed out!\n",
 			 req, req->rq_mid);
@@ -391,8 +394,14 @@
 	if (!(req->rq_flags & SMB_REQ_TRANSMITTED))
 		goto out;
 
-	list_del_init(&req->rq_queue);
-	list_add_tail(&req->rq_queue, &server->recvq);
+	if (req->rq_flags & SMB_REQ_NOREPLY) {
+		if (!(req->rq_flags & SMB_REQ_NORECEIVER))
+			wake_up_interruptible(&req->rq_wait);
+		smb_rput(req);
+	} else {
+		list_del_init(&req->rq_queue);
+		list_add_tail(&req->rq_queue, &server->recvq);
+	}
 	result = 1;
 out:
 	return result;
@@ -789,8 +798,11 @@
 	if (!result) {
 		list_del_init(&req->rq_queue);
 		req->rq_flags |= SMB_REQ_RECEIVED;
+		if (req->rq_completion)
+			req->rq_completion(req);
+		if (!(req->rq_flags & SMB_REQ_NORECEIVER))
+			wake_up_interruptible(&req->rq_wait);
 		smb_rput(req);
-		wake_up_interruptible(&req->rq_wait);
 	}
 	return 0;
 }
diff -urN -X exclude linux-2.5.30-orig/fs/smbfs/request.h linux-2.5.30-smbfs/fs/smbfs/request.h
--- linux-2.5.30-orig/fs/smbfs/request.h	Tue Aug  6 20:34:10 2002
+++ linux-2.5.30-smbfs/fs/smbfs/request.h	Tue Aug  6 20:55:42 2002
@@ -19,8 +19,11 @@
 	int rq_bufsize;
 	unsigned char *rq_buffer;
 
+	/* FIXME: union to save space. This is never done with a trans2
+	   request. */
 	/* FIXME: this is not good enough for merging IO requests. */
-	unsigned char *rq_page;
+	struct page *rq_page;
+	unsigned char *rq_pagemap;
 	int rq_rsize;
 
 	int rq_resp_wct;
@@ -37,6 +40,7 @@
 
 	int (*rq_setup_read) (struct smb_request *);
 	void (*rq_callback) (struct smb_request *);
+	void (*rq_completion) (struct smb_request *);
 
 	/* ------ trans2 stuff ------ */
 
@@ -61,9 +65,8 @@
 
 #define SMB_REQ_STATIC		0x0001	/* rq_buffer is static */
 #define SMB_REQ_NORETRY		0x0002	/* request is invalid after retry */
+#define SMB_REQ_NOREPLY		0x0004	/* we don't want the reply (if any) */
+#define SMB_REQ_NORECEIVER	0x0008	/* caller doesn't wait for response */
 
 #define SMB_REQ_TRANSMITTED	0x4000	/* all data has been sent */
 #define SMB_REQ_RECEIVED	0x8000	/* reply received, smbiod is done */
-
-#define xSMB_REQ_NOREPLY	0x0004	/* we don't want the reply (if any) */
-#define xSMB_REQ_NORECEIVER	0x0008	/* caller doesn't wait for response */
diff -urN -X exclude linux-2.5.30-orig/fs/smbfs/smbiod.c linux-2.5.30-smbfs/fs/smbfs/smbiod.c
--- linux-2.5.30-orig/fs/smbfs/smbiod.c	Tue Aug  6 20:34:22 2002
+++ linux-2.5.30-smbfs/fs/smbfs/smbiod.c	Tue Aug  6 21:01:18 2002
@@ -110,15 +110,17 @@
 		req = list_entry(tmp, struct smb_request, rq_queue);
 		req->rq_errno = -EIO;
 		list_del_init(&req->rq_queue);
+		if (!(req->rq_flags & SMB_REQ_NORECEIVER))
+			wake_up_interruptible(&req->rq_wait);
 		smb_rput(req);
-		wake_up_interruptible(&req->rq_wait);
 	}
 	list_for_each_safe(tmp, n, &server->recvq) {
 		req = list_entry(tmp, struct smb_request, rq_queue);
 		req->rq_errno = -EIO;
 		list_del_init(&req->rq_queue);
+		if (!(req->rq_flags & SMB_REQ_NORECEIVER))
+			wake_up_interruptible(&req->rq_wait);
 		smb_rput(req);
-		wake_up_interruptible(&req->rq_wait);
 	}
 }
 
@@ -154,8 +156,9 @@
 			VERBOSE("aborting request %p on xmitq\n", req);
 			req->rq_errno = -EIO;
 			list_del_init(&req->rq_queue);
+			if (!(req->rq_flags & SMB_REQ_NORECEIVER))
+				wake_up_interruptible(&req->rq_wait);
 			smb_rput(req);
-			wake_up_interruptible(&req->rq_wait);
 		}
 	}
 
@@ -180,8 +183,9 @@
 		/* req->rq_rcls = ???; */ /* FIXME: set smb error code too? */
 		req->rq_errno = -EIO;
 		list_del_init(&req->rq_queue);
+		if (!(req->rq_flags & SMB_REQ_NORECEIVER))
+			wake_up_interruptible(&req->rq_wait);
 		smb_rput(req);
-		wake_up_interruptible(&req->rq_wait);
 	}
 
 	smb_close_socket(server);
diff -urN -X exclude linux-2.5.30-orig/include/linux/smb_fs.h linux-2.5.30-smbfs/include/linux/smb_fs.h
--- linux-2.5.30-orig/include/linux/smb_fs.h	Tue Aug  6 20:34:18 2002
+++ linux-2.5.30-smbfs/include/linux/smb_fs.h	Tue Aug  6 22:02:19 2002
@@ -167,10 +167,13 @@
 	int				filled, valid, idx;
 };
 
+struct smb_request;
+
 #define SMB_OPS_NUM_STATIC	5
 struct smb_ops {
 	int (*read)(struct inode *inode, loff_t offset, int count,
 		    char *data);
+	int (*reada) (struct inode *inode, struct smb_request *req);
 	int (*write)(struct inode *inode, loff_t offset, int count, const
 		     char *data);
 	int (*readdir)(struct file *filp, void *dirent, filldir_t filldir,

Re: [PATCH] smbfs async readpage

[Andrew Morton]

Urban Widmark wrote:
> 
> This patch implements async readpage for smbfs. It seems to generate
> readahead judging from the network traffic and profile_readahead output
> from 2.4.
>

You'll find that each smbfs inode has its mapping->backing_dev_info
pointing at the global default_backing_dev_info.  Which is not
tunable.

What you can do (please, I'd be interested in checking that it's
workable) is to create an smbfs_backing_dev_info, and arrange
(in smb_iget) for each new inode's mapping->backing_dev_info
to point at it.

Then implement some way of diddling smbfs_backing_dev_info.ra_pages
and voila, tunable readahead.

Re: [PATCH] smbfs async readpage

[Urban Widmark]

On Tue, 6 Aug 2002, Andrew Morton wrote:

> What you can do (please, I'd be interested in checking that it's
> workable) is to create an smbfs_backing_dev_info, and arrange
> (in smb_iget) for each new inode's mapping->backing_dev_info
> to point at it.

Yes, the highest number of pages in action appears to be (mostly) limited
by what I set ra_pages to. If I set ra_pages to 1 or 2 my printks still
report up to 4 requests issued, but with 10 I never get more than 10.
40 gives something that looks like 40, without actually counting the lines
...


> Then implement some way of diddling smbfs_backing_dev_info.ra_pages
> and voila, tunable readahead.

Other fs' seems to be using the default too. New 2.5 feature or does block
device based filesystems get this value from somewhere else?
(like the block device they live on?)

If being able to tune readahead is important, I could imagine a desire to
tune differently on a per-mount basis. So I'd stick it in the smbfs
superblock and have a readahead=N mount option.

/Urban

Re: [PATCH] smbfs async readpage

[Andrew Morton]

Urban Widmark wrote:
> 
> On Tue, 6 Aug 2002, Andrew Morton wrote:
> 
> > What you can do (please, I'd be interested in checking that it's
> > workable) is to create an smbfs_backing_dev_info, and arrange
> > (in smb_iget) for each new inode's mapping->backing_dev_info
> > to point at it.
> 
> Yes, the highest number of pages in action appears to be (mostly) limited
> by what I set ra_pages to. If I set ra_pages to 1 or 2 my printks still
> report up to 4 requests issued, but with 10 I never get more than 10.
> 40 gives something that looks like 40, without actually counting the lines
> ...

That sounds promising.

> 
> > Then implement some way of diddling smbfs_backing_dev_info.ra_pages
> > and voila, tunable readahead.
> 
> Other fs' seems to be using the default too. New 2.5 feature or does block
> device based filesystems get this value from somewhere else?
> (like the block device they live on?)

Yes, there's a backing_dev_info structure in each block request queue and
a pointer to that is placed in each address_space which is hosted
by that queue.

> If being able to tune readahead is important, I could imagine a desire to
> tune differently on a per-mount basis. So I'd stick it in the smbfs
> superblock and have a readahead=N mount option.

Tunability may be important for preventing bad behaviour rather
than for giving users something good ;)   Does readahead across
a n/w filesystem help much?

Re: [PATCH] smbfs async readpage

[Urban Widmark]

On Wed, 7 Aug 2002, Andrew Morton wrote:

> Tunability may be important for preventing bad behaviour rather
> than for giving users something good ;)   Does readahead across
> a n/w filesystem help much?

My simple md5sum test is now about 25% faster than when I started.
(readahead=8, got 20% with the default)

For a usage pattern: read a page, do something slow with it, read the next
page, ... why wouldn't it help?


For increasing transfer speed for things like cp I hope that merging
several read requests into a single large request will help more. With
sync readpage you never really get more than one request at a time, but
with async you can queue up a bunch. This is how I understand that NFS
does large reads.

I recently noticed readpages and writepages, and using those instead might
be simpler for 2.5 but doesn't help a 2.4 version. Hmm.

/Urban