PAG support only

Due to the slight unpopularity of the AFS multiplexor, here's a patch with

include/asm-i386/posix_types.h +1 -0

@@ -13,6 +13,7 @@
 typedef unsigned short	__kernel_nlink_t;
 typedef long		__kernel_off_t;
 typedef int		__kernel_pid_t;
+typedef int		__kernel_pag_t;
 typedef unsigned short	__kernel_ipc_pid_t;
 typedef unsigned short	__kernel_uid_t;
 typedef unsigned short	__kernel_gid_t;

+} /* end vfs_pag_put() */

kernel/cred.c line 22

Quoted 2 lines

kernel/cred.c +2 -0

+static kmem_cache_t *vfs_token_cache;
+static kmem_cache_t *vfs_pag_cache;

How many of those will be around for a typical AFS client? I have the vague feeling the slabs are overkill..

Quoted 5 lines

kernel/cred.c line 22

Quoted 2 lines

kernel/cred.c +2 -0

+static kmem_cache_t *vfs_token_cache;
+static kmem_cache_t *vfs_pag_cache;

How many of those will be around for a typical AFS client? I have the vague feeling the slabs are overkill..

What's a "typical client"?

Per pag on most clients will only have a single token, but number of pags will depend totally on the nature of the machine.

I also have machines that have a few hundred pags around that don't have any tokens in them, but could at any time.

-- Nathan

Please split this into 2 lines as per Documentation/CodingStyle. Even better make vfs_token_put handle a NULL argument.

Random whitespace change

- and even a wrong one..

kernel/cred.c line 22

Quoted 2 lines

kernel/cred.c +2 -0

+static kmem_cache_t *vfs_token_cache;
+static kmem_cache_t *vfs_pag_cache;

How many of those will be around for a typical AFS client? I have the vague feeling the slabs are overkill..

kernel/cred.c line 83

Quoted 5 lines

kernel/cred.c +5 -0

+	if (pag>0) {
+		/* join existing PAG */
+		if (tsk->vfspag->pag &&
+		    tsk->vfspag->pag==pag)
+			return pag;

Please try to get your code in conformance with Documentation/CodingStyle.

Quoted 1 line

- and even a wrong one..

I disagree, but no matter.

Quoted 5 lines

kernel/cred.c line 22

Quoted 2 lines

kernel/cred.c +2 -0

+static kmem_cache_t *vfs_token_cache;
+static kmem_cache_t *vfs_pag_cache;

How many of those will be around for a typical AFS client? I have the vague feeling the slabs are overkill..

And then there's the people who said I shouldn't use kmalloc but should create a slab instead...

Quoted 8 lines

kernel/cred.c line 83

Quoted 6 lines

kernel/cred.c +5 -0

+	if (pag>0) {
+		/* join existing PAG */
+		if (tsk->vfspag->pag &&
+		    tsk->vfspag->pag==pag)
+			return pag;

Please try to get your code in conformance with Documentation/CodingStyle.

You are suggesting what changes exactly?

Hi Linus,

Due to the slight unpopularity of the AFS multiplexor, here's a patch with only the PAG support.

David

diff -uNr linux-2.5.69/include/linux/cred.h linux-2.5.69-cred/include/linux/cred.h

include/linux/cred.h +15 -0

@@ -0,0 +1,79 @@
+#ifndef _LINUX_CRED_H
+#define _LINUX_CRED_H
+
+#ifdef __KERNEL__
+
+#include <linux/param.h>
+#include <linux/types.h>
+#include <linux/list.h>
+#include <linux/rbtree.h>
+#include <asm/atomic.h>
+
+/*
+ * VFS session authentication token cache
+ *
+ * This is used to store the data required for extra levels of filesystem

diff -uNr linux-2.5.69/kernel/cred.c linux-2.5.69-cred/kernel/cred.c

kernel/cred.c +71 -0

@@ -0,0 +1,367 @@
+/* cred.c: authentication credentials management
+ *
+ * Copyright (C) 2003 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells ([email protected])
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/sched.h>
+#include <linux/string.h>
+#include <linux/sched.h>
+#include <linux/cred.h>
+
+static kmem_cache_t *vfs_token_cache;
+static kmem_cache_t *vfs_pag_cache;
+
+static struct rb_root	vfs_pag_tree;
+static spinlock_t	vfs_pag_lock = SPIN_LOCK_UNLOCKED;
+static pag_t		vfs_pag_next = 1;
+
+static void vfs_pag_init_once(void *_vfspag, kmem_cache_t *cachep, unsigned long flags)
+{
+	struct vfs_pag *vfspag = _vfspag;
+
+	if ((flags & (SLAB_CTOR_VERIFY|SLAB_CTOR_CONSTRUCTOR)) == SLAB_CTOR_CONSTRUCTOR) {
+		memset(vfspag,0,sizeof(*vfspag));
+		INIT_LIST_HEAD(&vfspag->tokens);
+		rwlock_init(&vfspag->lock);
+	}
+}
+
+static void vfs_token_init_once(void *_vtoken, kmem_cache_t *cachep, unsigned long flags)
+{
+	struct vfs_token *vtoken = _vtoken;
+
+	if ((flags & (SLAB_CTOR_VERIFY|SLAB_CTOR_CONSTRUCTOR)) == SLAB_CTOR_CONSTRUCTOR) {
+		memset(vtoken,0,sizeof(*vtoken));
+		INIT_LIST_HEAD(&vtoken->link);
+	}
+}
+
+void __init credentials_init(void)
+{
+	vfs_pag_cache = kmem_cache_create("vfs_pag",
+					      sizeof(struct vfs_pag),
+					      0,
+					      SLAB_HWCACHE_ALIGN,
+					      vfs_pag_init_once, NULL);
+	if (!vfs_pag_cache)
+		panic("Cannot create vfs pag SLAB cache");
+
+	vfs_token_cache = kmem_cache_create("vfs_token",
+					    sizeof(struct vfs_token),
+					    0,
+					    SLAB_HWCACHE_ALIGN,
+					    vfs_token_init_once, NULL);
+	if (!vfs_token_cache)
+		panic("Cannot create vfs token SLAB cache");
+}
+
+/*****************************************************************************/
+/*
+ * join an existing PAG (+ve), run without PAG (0), or create and join new PAG (-1)
+ * - returns ID of PAG joined or 0 if now running without a PAG

kernel/cred.c +47 -0

+		    tsk->vfspag->pag==pag)
+			return pag;
+
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+
+		spin_lock(&vfs_pag_lock);
+
+		parent = NULL;
+		p = &vfs_pag_tree.rb_node;
+
+		while (*p) {
+			parent = *p;
+			vfspag = rb_entry(parent,struct vfs_pag,node);
+
+			if (pag < vfspag->pag)
+				p = &(*p)->rb_left;
+			else if (pag > vfspag->pag)
+				p = &(*p)->rb_right;
+			else
+				goto pag_found;
+		}
+
+		spin_unlock(&vfs_pag_lock);
+		return -ENOENT;
+
+	pag_found:
+		xvfspag = xchg(&current->vfspag,vfs_pag_get(vfspag));
+		spin_unlock(&vfs_pag_lock);
+
+		if (xvfspag) vfs_pag_put(xvfspag);
+		return pag;
+	}
+	else if (pag==0) {
+		/* run without a PAG */
+		xvfspag = xchg(&current->vfspag,NULL);
+
+		if (xvfspag) vfs_pag_put(xvfspag);
+		return 0;
+	}
+	else if (pag!=-1) {
+		     return -EINVAL;
+	}
+
+	/* start new PAG */
+	vfspag = kmem_cache_alloc(vfs_pag_cache,SLAB_KERNEL);
+	if (!vfspag)

kernel/cred.c +14 -0

+
+	parent = NULL;
+	p = &vfs_pag_tree.rb_node;
+
+	while (*p) {
+		parent = *p;
+		xvfspag = rb_entry(parent,struct vfs_pag,node);
+
+		if (vfspag->pag < xvfspag->pag)
+			p = &(*p)->rb_left;
+		else if (vfspag->pag > xvfspag->pag)
+			p = &(*p)->rb_right;
+		else
+			goto pag_exists;

kernel/cred.c +15 -0

+struct vfs_token *vfs_pag_find_token(const char *fsname,
+				     unsigned short klen,
+				     const void *key)
+{
+	struct vfs_token *vtoken;
+	struct vfs_pag *vfspag = current->vfspag;
+
+	if (!vfspag) return NULL;
+
+	read_lock(&vfspag->lock);
+
+	list_for_each_entry(vtoken,&vfspag->tokens,link) {
+		if (vtoken->d_off-vtoken->k_off == klen &&
+		    strcmp(vtoken->blob,fsname)==0 &&
+		    memcmp(vtoken->blob+vtoken->k_off,key,klen)==0)

kernel/cred.c +51 -0

+
+EXPORT_SYMBOL(vfs_pag_find_token);
+
+/*****************************************************************************/
+/*
+ * withdraw a token from a pag list
+ */
+void vfs_pag_withdraw_token(struct vfs_token *vtoken)
+{
+	struct vfs_pag *vfspag = current->vfspag;
+
+	if (!vfspag)
+		return;
+
+	write_lock(&vfspag->lock);
+	list_del_init(&vtoken->link);
+	write_unlock(&vfspag->lock);
+
+	vfs_token_put(vtoken);
+
+} /* end vfs_pag_withdraw_token() */
+
+EXPORT_SYMBOL(vfs_pag_withdraw_token);
+
+/*****************************************************************************/
+/*
+ * withdraw all tokens for the named filesystem from the current PAG
+ */
+void vfs_unpag(const char *fsname)
+{
+	struct list_head *_n, *_p;
+	struct vfs_token *vtoken;
+	struct vfs_pag *vfspag = current->vfspag;
+
+	if (!vfspag)
+		return;
+
+	write_lock(&vfspag->lock);
+
+	list_for_each_safe(_p,_n,&vfspag->tokens) {
+		vtoken = list_entry(_p,struct vfs_token,link);
+
+		if (strcmp(fsname,vtoken->blob)==0) {
+			list_del_init(&vtoken->link);
+			vfs_token_put(vtoken);
+		}
+	}
+
+	write_unlock(&vfspag->lock);
+
+} /* end vfs_unpag() */

Quoted 1 line

+ * VFS session authentication token cache

...

If you know the type of the data, why keep it all in one binary blob, instead of a struct? cache effects?

kernel/cred.c line 81

Quoted 1 line

kernel/cred.c +1 -0

+		if (tsk->vfspag->pag &&

Isn't the check here meant to be against tsk->vfspag? If not, there should be.

If vfs_pag_next wraps around, you will get two pag's with the value '1'.

Nothing in this patch appears to be using it. You aren't taking a reference to the token here, what's protecting it from disappearing after the return and before the caller gets a chance to do something with it?

Quoted 3 lines · Muli Ben-Yehuda

Quoted 1 line

+ * VFS session authentication token cache

...

Quoted 2 lines

If you know the type of the data, why keep it all in one binary blob, instead of a struct? cache effects?

No. We _don't_ know the type of the data. A filesystem entrusts us with a token to keep in the PAG on its behalf. However, since this is meant to be a generic mechanism, it's entirely dependent on the fs as to what's in the blob.

Quoted 4 lines

Nothing in this patch appears to be using it. You aren't taking a reference to the token here, what's protecting it from disappearing after the return and before the caller gets a chance to do something with it?

Thanks. Fixed.

Quoted 5 lines

No. We _don't_ know the type of the data. A filesystem entrusts us with a token to keep in the PAG on its behalf. However, since this is meant to be a generic mechanism, it's entirely dependent on the fs as to what's in the blob.

But you do know the type of the data in the blob... it's char* fsname, const void* key and const void* data, according to your code.

struct fsblob { 
       const char* fsname; 
       const void* key; 
       size_t keysize; 
       const void* data; 
       size_t datasize; 
}; 

Your method loses on additional complexity, and wins on speed.

However, unless it's really, really speed sensitive code, I would go for KISS any day.