[OpenAFS-devel] afs_osi_Sleep and afs_osi_Wakeup on Linux

chas williams chas@cmf.nrl.navy.mil
Wed, 05 Jun 2002 17:04:22 -0400


In message <Pine.LNX.3.96L.1020605160615.9946B-100000@scully.trafford.dementia.
org>,Derrick J Brashear writes:
>On Wed, 5 Jun 2002, chas williams wrote:
>so why not add addevent to all the systypes and call it everywhere,
>always. 

i dont know.  why wasnt it done?  the previous patch (that split up
getevent into getevent and addevent) was supposed to fix the problem.
the patch was either incomplete or just not addressing the real problem.

>trading one set of side effects for another isn't really a win, you just
>piss off a different group of users

i dont think its trading side effects.  all the other ports dont expect
the alloc function to drop the global lock.  on most of the other platforms
its a define to the memory allocator.  apparently linux is the only
port to try to address the memory leaks.  not quite the right way in my
opinion.   alloc shouldnt need to keep track of what was allocated so
it can free it later.  (tracking the type of alloc is different, i can
see why that was done).  given some of the code that you see in afs
its seems obvious that the original developers didnt intend for glock
to drop during alloc.

the attached patch 'fixes', in my opinion, in osi_alloc.  objects <=
AFS_SMALLOCSIZ are alloced with kmalloc, other vmalloc.  if kmalloc
fails it doesnt fail over to vmalloc.  TASK_RUNNING is set after
schedule.  it doesnt drop the GLOCK.  i moved the allocator init
later so that it doesnt need to toggle the semaphore just before
going to linux_alloc.


Index: osi_alloc.c
===================================================================
RCS file: /cvs/openafs/src/afs/LINUX/osi_alloc.c,v
retrieving revision 1.13
diff -u -d -b -w -r1.13 osi_alloc.c
--- osi_alloc.c	2002/04/23 03:26:38	1.13
+++ osi_alloc.c	2002/06/05 20:56:17
@@ -24,7 +24,7 @@
 #include "../afs/afs_atomlist.h"
 #include "../afs/afs_lhash.h"
 
-#define MAX_KMALLOC_SIZE (131072-16) /* Max we can alloc in physmem */
+#define MAX_KMALLOC_SIZE AFS_SMALLOCSIZ /* Max we should alloc with kmalloc */
 #define MAX_BUCKET_LEN 30 /* max. no. of entries per buckets we expect to see */
 #define STAT_INTERVAL 8192 /* we collect stats once every STAT_INTERVAL allocs*/
 
@@ -79,14 +79,12 @@
 static void *linux_alloc(unsigned int asize)
 {
     void *new = NULL;
-    int has_afs_glock = ISAFS_GLOCK();
-
-    /* if global lock has been held save this info and unlock it. */
-    if (has_afs_glock)
-        AFS_GUNLOCK();
+    int max_retry = 10;
 
     /*  if we can use kmalloc use it to allocate the required memory. */
-    if (asize <  MAX_KMALLOC_SIZE) {
+    while(!new && max_retry)
+    {
+        if (asize <=  MAX_KMALLOC_SIZE) {
         new = (void *)(unsigned long)kmalloc(asize, 
 #ifdef GFP_NOFS
 					     GFP_NOFS
@@ -96,30 +94,31 @@
 					     );
         if (new) /* piggy back alloc type */
             (unsigned long)new |= KM_TYPE;
-    }
-    if (!new) { /* otherwise use vmalloc  */
-	int max_wait = 10;
-        while (!(new = (void *)vmalloc(asize))) {
-            if (--max_wait <=0) {
-		break;
+        } else {
+            new = (void *)vmalloc(asize);
+	    if (new) /* piggy back alloc type */
+	        (unsigned long)new |= VM_TYPE;
             }
+
+	if (!new) {
+printk("god damn it -- need to retry alloc()...\n");
 #ifdef set_current_state
 	    set_current_state(TASK_INTERRUPTIBLE);
 #else
 	    current->state = TASK_INTERRUPTIBLE;
 #endif
 	    schedule_timeout(HZ);
+#ifdef set_current_state
+            set_current_state(TASK_RUNNING);
+#else
+            current->state = TASK_RUNNING;
+#endif
+            --max_retry;
         }
-	if (new) /* piggy back alloc type */
-	    (unsigned long)new |= VM_TYPE;
     }
     if (new)
 	memset(MEMADDR(new), 0, asize);
 
-    /* if the global lock had been held, lock it again. */
-    if (has_afs_glock)
-        AFS_GLOCK();
-
     return new;
 }
 
@@ -287,21 +286,21 @@
     void *new = NULL;
     struct osi_linux_mem *lmem;
 
+    new = linux_alloc(asize); /* get a chunk of memory of size asize */
+
+    if (!new) {
+	printf("afs_osi_Alloc: Can't vmalloc %d bytes.\n", asize);
+	return new;
+    }
+
     down(&afs_linux_alloc_sem);
 
-    if (allocator_init == 0) { /* allocator hasn't been initialized yet */
+    /* allocator hasn't been initialized yet */
+    if (allocator_init == 0) {
 	if (linux_alloc_init() == 0) {
 	    goto error;
 	 }
 	allocator_init = 1; /* initialization complete */
-    }
-
-    up(&afs_linux_alloc_sem);
-    new = linux_alloc(asize); /* get a chunk of memory of size asize */
-    down(&afs_linux_alloc_sem);
-    if (!new) {
-	printf("afs_osi_Alloc: Can't vmalloc %d bytes.\n", asize);
-	goto error;
     }
     
     /* get an atom to store the pointer to the chunk */