[PATCH 0/6] 2.4.19-rc1 stack reduction patches

[PATCH 0/6] 2.4.19-rc1 stack reduction patches

[Badari Pulavarty]

Hi Marcelo,

I re-worked all the applicable stack reduction patches for 2.4.19-rc1.

I made 6 different patches, one for each area to pick and choose easily.
Here are the stack reductions for each these:

do_execve                    320
number                       100
nfs_lookup                   184
__revalidate_inode           112
rpc_call_sync                144
xprt_sendmsg                 120

If you have any suggestions, more cleanups or re-works, please let me 
know.
 
Thanks,
Badari

[PATCH 1/6] 2.4.19-rc1 do_execve() stack reduction

[Badari Pulavarty]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: execv.patch --]
[-- Type: text/plain, Size: 2749 bytes --]

Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
--- linux-2.4.29-rc1.org/fs/exec.c	2005-01-07 07:39:06.000000000 -0800
+++ linux-2.4.29-rc1/fs/exec.c	2005-01-10 00:28:52.000000000 -0800
@@ -930,7 +930,7 @@ int search_binary_handler(struct linux_b
  */
 int do_execve(char * filename, char ** argv, char ** envp, struct pt_regs * regs)
 {
-	struct linux_binprm bprm;
+	struct linux_binprm *bprm;
 	struct file *file;
 	int retval;
 	int i;
@@ -941,60 +941,69 @@ int do_execve(char * filename, char ** a
 	if (IS_ERR(file))
 		return retval;
 
-	bprm.p = PAGE_SIZE*MAX_ARG_PAGES-sizeof(void *);
-	memset(bprm.page, 0, MAX_ARG_PAGES*sizeof(bprm.page[0])); 
+	bprm = kmalloc(sizeof(struct linux_binprm), GFP_KERNEL);
+	if (!bprm) {
+		return -ENOMEM;
+	}
+
+	bprm->p = PAGE_SIZE*MAX_ARG_PAGES-sizeof(void *);
+	memset(bprm->page, 0, MAX_ARG_PAGES*sizeof(bprm->page[0])); 
 
-	bprm.file = file;
-	bprm.filename = filename;
-	bprm.sh_bang = 0;
-	bprm.loader = 0;
-	bprm.exec = 0;
-	if ((bprm.argc = count(argv, bprm.p / sizeof(void *))) < 0) {
+	bprm->file = file;
+	bprm->filename = filename;
+	bprm->sh_bang = 0;
+	bprm->loader = 0;
+	bprm->exec = 0;
+	if ((bprm->argc = count(argv, bprm->p / sizeof(void *))) < 0) {
 		allow_write_access(file);
 		fput(file);
-		return bprm.argc;
+		retval = bprm->argc;
+		goto free_bprm;
 	}
 
-	if ((bprm.envc = count(envp, bprm.p / sizeof(void *))) < 0) {
+	if ((bprm->envc = count(envp, bprm->p / sizeof(void *))) < 0) {
 		allow_write_access(file);
 		fput(file);
-		return bprm.envc;
+		retval = bprm->envc;
+		goto free_bprm;
 	}
 
-	retval = prepare_binprm(&bprm);
+	retval = prepare_binprm(bprm);
 	if (retval < 0) 
 		goto out; 
 
-	retval = copy_strings_kernel(1, &bprm.filename, &bprm);
+	retval = copy_strings_kernel(1, &bprm->filename, bprm);
 	if (retval < 0) 
 		goto out; 
 
-	bprm.exec = bprm.p;
-	retval = copy_strings(bprm.envc, envp, &bprm);
+	bprm->exec = bprm->p;
+	retval = copy_strings(bprm->envc, envp, bprm);
 	if (retval < 0) 
 		goto out; 
 
-	retval = copy_strings(bprm.argc, argv, &bprm);
+	retval = copy_strings(bprm->argc, argv, bprm);
 	if (retval < 0) 
 		goto out; 
 
-	retval = search_binary_handler(&bprm,regs);
+	retval = search_binary_handler(bprm,regs);
 	if (retval >= 0)
 		/* execve success */
-		return retval;
+		goto free_bprm;
 
 out:
 	/* Something went wrong, return the inode and free the argument pages*/
-	allow_write_access(bprm.file);
-	if (bprm.file)
-		fput(bprm.file);
+	allow_write_access(bprm->file);
+	if (bprm->file)
+		fput(bprm->file);
 
 	for (i = 0 ; i < MAX_ARG_PAGES ; i++) {
-		struct page * page = bprm.page[i];
+		struct page * page = bprm->page[i];
 		if (page)
 			__free_page(page);
 	}
 
+free_bprm:
+	kfree(bprm);
 	return retval;
 }
 

[PATCH 2/6] 2.4.19-rc1 number() stack reduction

[Badari Pulavarty]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: number.patch --]
[-- Type: text/plain, Size: 1020 bytes --]

Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
--- linux-2.4.29-rc1.org/lib/vsprintf.c	2004-02-18 05:36:32.000000000 -0800
+++ linux-2.4.29-rc1/lib/vsprintf.c	2005-01-07 07:56:00.000000000 -0800
@@ -128,12 +128,16 @@ static int skip_atoi(const char **s)
 #define SPECIAL	32		/* 0x */
 #define LARGE	64		/* use 'ABCDEF' instead of 'abcdef' */
 
+ /* Move these off of the stack for number().  This way we reduce the
+  * size of the stack and don't have to copy them every time we are called.
+  */
+const char small_digits[] = "0123456789abcdefghijklmnopqrstuvwxyz";
+const char large_digits[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
+
 static char * number(char * buf, char * end, long long num, int base, int size, int precision, int type)
 {
 	char c,sign,tmp[66];
 	const char *digits;
-	static const char small_digits[] = "0123456789abcdefghijklmnopqrstuvwxyz";
-	static const char large_digits[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
 	int i;
 
 	digits = (type & LARGE) ? large_digits : small_digits;

Re: [PATCH 2/6] 2.4.19-rc1 number() stack reduction

[Kevin P. Fleming]

Badari Pulavarty wrote:

> + /* Move these off of the stack for number().  This way we reduce the
> +  * size of the stack and don't have to copy them every time we are called.
> +  */
> +const char small_digits[] = "0123456789abcdefghijklmnopqrstuvwxyz";
> +const char large_digits[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
> +
>  static char * number(char * buf, char * end, long long num, int base, int size, int precision, int type)
>  {
>  	char c,sign,tmp[66];
>  	const char *digits;
> -	static const char small_digits[] = "0123456789abcdefghijklmnopqrstuvwxyz";
> -	static const char large_digits[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";

Is this actually correct? Since these are declared "static const", they 
are not on the stack anyway, because they have to persist between calls 
to this function and cannot be changed. I'd be very surprised if the 
compiler was copying this data from the static data segment to the stack 
on every entry to this function.

Re: [PATCH 2/6] 2.4.19-rc1 number() stack reduction

[Badari Pulavarty]

Yep. My bad. I got little carried away...

But I remember seeing this on earlier distros..

Thanks,
Badari

Kevin P. Fleming wrote:

> Badari Pulavarty wrote:
> 
>> + /* Move these off of the stack for number().  This way we reduce the
>> +  * size of the stack and don't have to copy them every time we are 
>> called.
>> +  */
>> +const char small_digits[] = "0123456789abcdefghijklmnopqrstuvwxyz";
>> +const char large_digits[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
>> +
>>  static char * number(char * buf, char * end, long long num, int base, 
>> int size, int precision, int type)
>>  {
>>      char c,sign,tmp[66];
>>      const char *digits;
>> -    static const char small_digits[] = 
>> "0123456789abcdefghijklmnopqrstuvwxyz";
>> -    static const char large_digits[] = 
>> "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
> 
> 
> Is this actually correct? Since these are declared "static const", they 
> are not on the stack anyway, because they have to persist between calls 
> to this function and cannot be changed. I'd be very surprised if the 
> compiler was copying this data from the static data segment to the stack 
> on every entry to this function.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 

Re: [PATCH 2/6] 2.4.19-rc1 number() stack reduction

[Badari Pulavarty]

Greg-KH told me that, this patch is useless.  He told me that,
this won't save any stack since  "static constants" won't be
on the stack. I will take his word for it :)

Please ignore this patch.

Thanks,
Badari

Badari Pulavarty wrote:

> 
> ------------------------------------------------------------------------
> 
> Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
> --- linux-2.4.29-rc1.org/lib/vsprintf.c	2004-02-18 05:36:32.000000000 -0800
> +++ linux-2.4.29-rc1/lib/vsprintf.c	2005-01-07 07:56:00.000000000 -0800
> @@ -128,12 +128,16 @@ static int skip_atoi(const char **s)
>  #define SPECIAL	32		/* 0x */
>  #define LARGE	64		/* use 'ABCDEF' instead of 'abcdef' */
>  
> + /* Move these off of the stack for number().  This way we reduce the
> +  * size of the stack and don't have to copy them every time we are called.
> +  */
> +const char small_digits[] = "0123456789abcdefghijklmnopqrstuvwxyz";
> +const char large_digits[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
> +
>  static char * number(char * buf, char * end, long long num, int base, int size, int precision, int type)
>  {
>  	char c,sign,tmp[66];
>  	const char *digits;
> -	static const char small_digits[] = "0123456789abcdefghijklmnopqrstuvwxyz";
> -	static const char large_digits[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
>  	int i;
>  
>  	digits = (type & LARGE) ? large_digits : small_digits;

[PATCH 3/6] 2.4.19-rc1 nfs_lookup stack reduction patch

[Badari Pulavarty]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: nfs_lookup.patch --]
[-- Type: text/plain, Size: 1392 bytes --]

Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
--- linux-2.4.29-rc1.org/fs/nfs/dir.c	2005-01-07 07:39:06.000000000 -0800
+++ linux-2.4.29-rc1/fs/nfs/dir.c	2005-01-10 00:15:56.000000000 -0800
@@ -580,8 +580,8 @@ static struct dentry *nfs_lookup(struct 
 {
 	struct inode *inode;
 	int error;
-	struct nfs_fh fhandle;
-	struct nfs_fattr fattr;
+	struct nfs_fh *fhandle;
+	struct nfs_fattr *fattr;
 
 	dfprintk(VFS, "NFS: lookup(%s/%s)\n",
 		dentry->d_parent->d_name.name, dentry->d_name.name);
@@ -591,15 +591,22 @@ static struct dentry *nfs_lookup(struct 
 		goto out;
 
 	error = -ENOMEM;
+	fhandle = kmalloc(sizeof(struct nfs_fh), GFP_KERNEL);
+	if (!fhandle)
+		goto out;
+	fattr = kmalloc(sizeof(struct nfs_fattr), GFP_KERNEL);
+	if (!fattr)
+		goto free_fhandle;
+
 	dentry->d_op = &nfs_dentry_operations;
 
-	error = NFS_PROTO(dir)->lookup(dir, &dentry->d_name, &fhandle, &fattr);
+	error = NFS_PROTO(dir)->lookup(dir, &dentry->d_name, fhandle, fattr);
 	inode = NULL;
 	if (error == -ENOENT)
 		goto no_entry;
 	if (!error) {
 		error = -EACCES;
-		inode = nfs_fhget(dentry, &fhandle, &fattr);
+		inode = nfs_fhget(dentry, fhandle, fattr);
 		if (inode) {
 	    no_entry:
 			d_add(dentry, inode);
@@ -607,6 +614,9 @@ static struct dentry *nfs_lookup(struct 
 		}
 		nfs_renew_times(dentry);
 	}
+	kfree(fattr);
+free_fhandle:
+	kfree(fhandle);
 out:
 	return ERR_PTR(error);
 }

[PATCH 4/6] 2.4.19-rc1 nfs revalidate_inode() stack reduction patches

[Badari Pulavarty]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: nfs_revalidate_inode.patch --]
[-- Type: text/plain, Size: 1478 bytes --]

Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
--- linux-2.4.29-rc1.org/fs/nfs/inode.c	2004-04-14 06:05:40.000000000 -0700
+++ linux-2.4.29-rc1/fs/nfs/inode.c	2005-01-09 23:14:48.000000000 -0800
@@ -881,11 +881,15 @@ int
 __nfs_revalidate_inode(struct nfs_server *server, struct inode *inode)
 {
 	int		 status = -ESTALE;
-	struct nfs_fattr fattr;
+	struct nfs_fattr *fattr;
 
 	dfprintk(PAGECACHE, "NFS: revalidating (%x/%Ld)\n",
 		inode->i_dev, (long long)NFS_FILEID(inode));
 
+	fattr = kmalloc(sizeof(struct nfs_fattr), GFP_KERNEL);
+	if (!fattr)
+		return -ENOMEM;
+
 	lock_kernel();
 	if (!inode || is_bad_inode(inode))
  		goto out_nowait;
@@ -903,7 +907,7 @@ __nfs_revalidate_inode(struct nfs_server
 	}
 	NFS_FLAGS(inode) |= NFS_INO_REVALIDATING;
 
-	status = NFS_PROTO(inode)->getattr(inode, &fattr);
+	status = NFS_PROTO(inode)->getattr(inode, fattr);
 	if (status) {
 		dfprintk(PAGECACHE, "nfs_revalidate_inode: (%x/%Ld) getattr failed, error=%d\n",
 			 inode->i_dev, (long long)NFS_FILEID(inode), status);
@@ -915,7 +919,7 @@ __nfs_revalidate_inode(struct nfs_server
 		goto out;
 	}
 
-	status = nfs_refresh_inode(inode, &fattr);
+	status = nfs_refresh_inode(inode, fattr);
 	if (status) {
 		dfprintk(PAGECACHE, "nfs_revalidate_inode: (%x/%Ld) refresh failed, error=%d\n",
 			 inode->i_dev, (long long)NFS_FILEID(inode), status);
@@ -930,6 +934,7 @@ out:
 	wake_up(&inode->i_wait);
  out_nowait:
 	unlock_kernel();
+	kfree(fattr);
 	return status;
 }
 

[PATCH 5/6] 2.4.19-rc1 rpc_call_sync() stack reduction patch

[Badari Pulavarty]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: rpc_call_sync.patch --]
[-- Type: text/plain, Size: 992 bytes --]

Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
--- linux-2.4.29-rc1.org/net/sunrpc/clnt.c	2003-11-28 10:26:21.000000000 -0800
+++ linux-2.4.29-rc1/net/sunrpc/clnt.c	2005-01-09 23:08:31.000000000 -0800
@@ -238,7 +238,7 @@ void rpc_clnt_sigunmask(struct rpc_clnt 
  */
 int rpc_call_sync(struct rpc_clnt *clnt, struct rpc_message *msg, int flags)
 {
-	struct rpc_task	my_task, *task = &my_task;
+	struct rpc_task	*task;
 	sigset_t	oldset;
 	int		status;
 
@@ -253,8 +253,11 @@ int rpc_call_sync(struct rpc_clnt *clnt,
 
 	rpc_clnt_sigmask(clnt, &oldset);		
 
-	/* Create/initialize a new RPC task */
-	rpc_init_task(task, clnt, NULL, flags);
+	status = -ENOMEM;
+	task = rpc_new_task(clnt, NULL, flags);
+	if (task == NULL)
+		goto out;
+
 	rpc_call_setup(task, msg, 0);
 
 	/* Set up the call info struct and execute the task */
@@ -265,6 +268,7 @@ int rpc_call_sync(struct rpc_clnt *clnt,
 		rpc_release_task(task);
 	}
 
+out:
 	rpc_clnt_sigunmask(clnt, &oldset);		
 
 	return status;

[PATCH 6/6] 2.4.19-rc1 xprt_sendmsg() stack reduction patch

[Badari Pulavarty]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: xprt_sendmsg.patch --]
[-- Type: text/plain, Size: 2292 bytes --]

Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
--- linux-2.4.29-rc1.org/net/sunrpc/xprt.c	2004-11-17 03:54:22.000000000 -0800
+++ linux-2.4.29-rc1/net/sunrpc/xprt.c	2005-01-10 00:09:12.000000000 -0800
@@ -221,9 +221,9 @@ static inline int
 xprt_sendmsg(struct rpc_xprt *xprt, struct rpc_rqst *req)
 {
 	struct socket	*sock = xprt->sock;
-	struct msghdr	msg;
+	struct msghdr	*msg;
 	struct xdr_buf	*xdr = &req->rq_snd_buf;
-	struct iovec	niv[MAX_IOVEC];
+	struct iovec	*niv;
 	unsigned int	niov, slen, skip;
 	mm_segment_t	oldfs;
 	int		result;
@@ -231,6 +231,15 @@ xprt_sendmsg(struct rpc_xprt *xprt, stru
 	if (!sock)
 		return -ENOTCONN;
 
+	msg = kmalloc(sizeof(struct msghdr), GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+	niv = kmalloc(sizeof(struct iovec)*MAX_IOVEC, GFP_KERNEL);
+	if (!niv) {
+		kfree(msg);
+		return -ENOMEM;
+	}
+
 	xprt_pktdump("packet data:",
 				req->rq_svec->iov_base,
 				req->rq_svec->iov_len);
@@ -248,20 +257,20 @@ xprt_sendmsg(struct rpc_xprt *xprt, stru
 			break;
 		}
 
-		msg.msg_flags   = MSG_DONTWAIT|MSG_NOSIGNAL;
-		msg.msg_iov	= niv;
-		msg.msg_iovlen	= niov;
-		msg.msg_name	= (struct sockaddr *) &xprt->addr;
-		msg.msg_namelen = sizeof(xprt->addr);
-		msg.msg_control = NULL;
-		msg.msg_controllen = 0;
+		msg->msg_flags   = MSG_DONTWAIT|MSG_NOSIGNAL;
+		msg->msg_iov	= niv;
+		msg->msg_iovlen	= niov;
+		msg->msg_name	= (struct sockaddr *) &xprt->addr;
+		msg->msg_namelen = sizeof(xprt->addr);
+		msg->msg_control = NULL;
+		msg->msg_controllen = 0;
 
 		slen_part = 0;
 		for (n = 0; n < niov; n++)
 			slen_part += niv[n].iov_len;
 
 		clear_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
-		result = sock_sendmsg(sock, &msg, slen_part);
+		result = sock_sendmsg(sock, msg, slen_part);
 
 		xdr_kunmap(xdr, skip, niov);
 
@@ -272,9 +281,9 @@ xprt_sendmsg(struct rpc_xprt *xprt, stru
 
 	dprintk("RPC:      xprt_sendmsg(%d) = %d\n", slen, result);
 
-	if (result >= 0)
-		return result;
-
+	if (result >= 0) 
+		goto out_free;
+	
 	switch (result) {
 	case -ECONNREFUSED:
 		/* When the server has died, an ICMP port unreachable message
@@ -292,6 +301,9 @@ xprt_sendmsg(struct rpc_xprt *xprt, stru
 	default:
 		printk(KERN_NOTICE "RPC: sendmsg returned error %d\n", -result);
 	}
+out_free:
+	kfree(niv);
+	kfree(msg);
 	return result;
 }
 

Re: [PATCH 0/6] 2.4.19-rc1 stack reduction patches

[Arjan van de Ven]

On Mon, 2005-01-10 at 09:35 -0800, Badari Pulavarty wrote:
> Hi Marcelo,
> 
> I re-worked all the applicable stack reduction patches for 2.4.19-rc1.


is it really worth doing this sort of thing for 2.4 still? It's a matter
of risk versus gain... not sure this sort of thing is still worth it in
the deep-maintenance 2.4 tree

Re: [PATCH 0/6] 2.4.19-rc1 stack reduction patches

[Marcelo Tosatti]

On Tue, Jan 11, 2005 at 08:39:03AM +0100, Arjan van de Ven wrote:
> On Mon, 2005-01-10 at 09:35 -0800, Badari Pulavarty wrote:
> > Hi Marcelo,
> > 
> > I re-worked all the applicable stack reduction patches for 2.4.19-rc1.
> 
> is it really worth doing this sort of thing for 2.4 still? It's a matter
> of risk versus gain... not sure this sort of thing is still worth it in
> the deep-maintenance 2.4 tree

Well it seems the s390 fellows are seeing stack overflows, which are serious
enough. Have you noticed that?

Moreover this are simple changes, not complex ones.

Re: [PATCH 0/6] 2.4.19-rc1 stack reduction patches

[Arjan van de Ven]

On Tue, 2005-01-11 at 05:49 -0200, Marcelo Tosatti wrote:
> On Tue, Jan 11, 2005 at 08:39:03AM +0100, Arjan van de Ven wrote:
> > On Mon, 2005-01-10 at 09:35 -0800, Badari Pulavarty wrote:
> > > Hi Marcelo,
> > > 
> > > I re-worked all the applicable stack reduction patches for 2.4.19-rc1.
> > 
> > is it really worth doing this sort of thing for 2.4 still? It's a matter
> > of risk versus gain... not sure this sort of thing is still worth it in
> > the deep-maintenance 2.4 tree
> 
> Well it seems the s390 fellows are seeing stack overflows, which are serious
> enough. Have you noticed that?

well.. is anyone using 2.4.2X mainline on s390, or is ibm making their
s390 customers use vendor kernels instead? 
(the people brave enough to not use those kernels might very well be
using 2.6 by now)

Just trying to get a feeling for who if anyone will benefit inclusion of
such patches, because if that is "just about nobody" then they might
well not be worth the risk.

Re: [PATCH 0/6] 2.4.19-rc1 stack reduction patches

[Marcelo Tosatti]

On Tue, Jan 11, 2005 at 12:18:31PM +0100, Arjan van de Ven wrote:
> On Tue, 2005-01-11 at 05:49 -0200, Marcelo Tosatti wrote:
> > On Tue, Jan 11, 2005 at 08:39:03AM +0100, Arjan van de Ven wrote:
> > > On Mon, 2005-01-10 at 09:35 -0800, Badari Pulavarty wrote:
> > > > Hi Marcelo,
> > > > 
> > > > I re-worked all the applicable stack reduction patches for 2.4.19-rc1.
> > > 
> > > is it really worth doing this sort of thing for 2.4 still? It's a matter
> > > of risk versus gain... not sure this sort of thing is still worth it in
> > > the deep-maintenance 2.4 tree
> > 
> > Well it seems the s390 fellows are seeing stack overflows, which are serious
> > enough. Have you noticed that?
> 
> well.. is anyone using 2.4.2X mainline on s390, or is ibm making their
> s390 customers use vendor kernels instead? 
> (the people brave enough to not use those kernels might very well be
> using 2.6 by now)
> 
> Just trying to get a feeling for who if anyone will benefit inclusion of
> such patches, because if that is "just about nobody" then they might
> well not be worth the risk.

I understand your concern and appreciate it.

I dont expect anyone to be using v2.4 mainline on S390 (you need external 
patches to get it to work anyway) either :)

But the stack growth patches are also useful for other architectures I assume, 
its pretty hard to get them wrong (ie you're simple changing stack to 
kmalloc()'ed memory, the code is essentially the same). 

No?