[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-kernel
Subject:    Re: binfmt_misc - a generic 'wrapper'-binary format [PATCH on WWW]
From:       Richard Guenther <zxmpm11 () student ! uni-tuebingen ! de>
Date:       1997-04-30 11:30:08
[Download RAW message or body]

On Tue, 29 Apr 1997, Mike Meissner wrote:

> I also had a couple of random thoughts (bear in mind I am not a linux
> kernel hacker):
> 
> I personally don't tend to like static limits on number of array
> elements (since if you decided that the limit should be foo, I might
> have a good reason to want 2*foo different formats):
> 
>         spin_lock(&misc_binfmts_lock);
>         if (misc_binfmts_last == MAX_BINFMT_MISC_ENTRIES) {
>                 kfree(ek);
>                 spin_unlock(&misc_binfmts_lock);
>                 return -ENOSPC;
>         }
> 
>         misc_binfmts[++misc_binfmts_last]=ek;
> 
>         spin_unlock(&misc_binfmts_lock);
>         return 1;
> 
> Writing it as a linked list would avoid the problem.
Yes, I'll think about this.

> In load_misc_binary, I suspect you may want to build the arguments
> (using copy_string) outside of the spin lock for misc_binfmts_lock,
> particularly on a SMP machine if copy_string blocks awaiting for
> memory.  Whether or not you want to go to separate spin locks for each
> element might be something to think about as the kernel goes away from
> one giant spin lock around the kernel.
I had the problem, that I rely on misc_binfmts[fmt] beeing correct _after_
the copy_strings() call (for the open_namei one). Therefore I need to
hold the lock this long time (I dont like this, too).

> Not knowing the guts of the Linux kernel, I assume that there is
> probably a function to do the strlen code you open code in
> binfmt_misc_sysctl:
> 
>         /* strlen of interpreter */
>         while (!(res=get_user(c, eu.interpreter+len)) && (c!=0))
>                 len++;
I did not found such a function... (problem is, the string is in user-space!)
But I found a cleaner solution to this (and an other) problem: Now I use
different structs for the sysctl and the internal data-management (see
NEW patch on www.anatom.uni-tuebingen.de/~richi/binfmt_misc.html)

Richard.


[prev in list] [next in list] [prev in thread] [next in thread]