Re: CVS commit: src/sys
On Tue Apr 13 2010 at 13:26:55 -0700, Jason Thorpe wrote: > > On Apr 12, 2010, at 3:15 PM, Antti Kantee wrote: > > > Module Name:src > > Committed By: pooka > > Date: Mon Apr 12 22:15:32 UTC 2010 > > > > Modified Files: > > src/sys/conf: files > > src/sys/kern: kern_lwp.c > > src/sys/sys: lwp.h > > Added Files: > > src/sys/kern: subr_lwp_specificdata.c > > > > Log Message: > > Separate lwp specificdata data structure management from lwp cpu/vm > > management. > > > > No functional change. > > > > (specificdata routines went from kern_lwp.c to subr_lwp_specificdata.c) > > I find this sort of re-factoring somewhat irritating. Can't you just #ifndef > _RUMP_BUILD or some such so that we don't have to constantly split related > functions into separate files? I also find it irritating and wish they had been done right from the start. But this, of course, is a completely absurd wish, since nobody can be expected to reliably predict what will happen in the future. As the commit message clearly indicates, the routines are not related all axes considered. I'm sure when people realized things like MI and MD code weren't related and had to split, there was similar irritation, but aren't we now glad ifdefs were *not* used. Now, I could just bulldoze through the kernel and organize it how I think it should be organized and because of too much intrusiveness not commit that or any dependant work, or, for the other extreme, lay in ifdef after ifdef and end up with complete spaghetti. However, I find the approach of the multidimensional minmax for developer irritation, working code and final resulting mess the most practical approach to attempting something new.
Re: CVS commit: src/sys
On Apr 12, 2010, at 3:15 PM, Antti Kantee wrote: > Module Name: src > Committed By: pooka > Date: Mon Apr 12 22:15:32 UTC 2010 > > Modified Files: > src/sys/conf: files > src/sys/kern: kern_lwp.c > src/sys/sys: lwp.h > Added Files: > src/sys/kern: subr_lwp_specificdata.c > > Log Message: > Separate lwp specificdata data structure management from lwp cpu/vm > management. > > No functional change. > > (specificdata routines went from kern_lwp.c to subr_lwp_specificdata.c) I find this sort of re-factoring somewhat irritating. Can't you just #ifndef _RUMP_BUILD or some such so that we don't have to constantly split related functions into separate files? > > > To generate a diff of this commit: > cvs rdiff -u -r1.985 -r1.986 src/sys/conf/files > cvs rdiff -u -r1.143 -r1.144 src/sys/kern/kern_lwp.c > cvs rdiff -u -r0 -r1.1 src/sys/kern/subr_lwp_specificdata.c > cvs rdiff -u -r1.129 -r1.130 src/sys/sys/lwp.h > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. -- thorpej
Re: CVS commit: src/sys/miscfs/specfs
On Tue, Apr 13, 2010 at 10:47 PM, Antti Kantee wrote: > On Tue Apr 13 2010 at 13:25:24 +, Andrew Doran wrote: >> So the result of our teeth-pulling so far is: >> >> 1. You are concerned about namespace conflicts. I am too. >> 2. I would be happy to see these managed through documentation and >> a straightforward approval process for adding modules. > > There is an additional pitfall with this. We don't control all modules > and cannot be sure potential 3rd parties use the same processes. > >> 3. You suggest that it would be better for the computer to manage it. > > Maybe not manage it, but at least detect it and fail gracefully. > >> Can you suggest an alternative mechanism that will (a) allow us to >> autoload things that are not anointed device drivers and (b) satisfy >> your concerns? > > devfs ;) > > IOW, Given that we don't know where we most likely are in 6 months, > I'm not too keen on trying to spend energy to solve this right now ... > >> As an example of something that wants autoloading both as a file >> system and a driver, see ZFS. > > ... if zfs is the only use case, since IIUC it's broken anyway. > > Meanwhile, if there is something else which is catastrophically broken > due to lack of holy christening, we can revisit this and see if just > flipping the switch and maybe writing a simple audit script to verify > no collisions is the path of least wailing (this would be close to "2"). I like module names to become longer and unambiguous by making them match the relative paths in src/sys, like "sys/ufs/ffs/ffs.kmod". I don't think of any reason why module (driver, filesystem, ...) names need to be very short. Masao
Re: CVS commit: src/sys/miscfs/specfs
On Tue Apr 13 2010 at 13:25:24 +, Andrew Doran wrote: > So the result of our teeth-pulling so far is: > > 1. You are concerned about namespace conflicts. I am too. > 2. I would be happy to see these managed through documentation and >a straightforward approval process for adding modules. There is an additional pitfall with this. We don't control all modules and cannot be sure potential 3rd parties use the same processes. > 3. You suggest that it would be better for the computer to manage it. Maybe not manage it, but at least detect it and fail gracefully. > Can you suggest an alternative mechanism that will (a) allow us to > autoload things that are not anointed device drivers and (b) satisfy > your concerns? devfs ;) IOW, Given that we don't know where we most likely are in 6 months, I'm not too keen on trying to spend energy to solve this right now ... > As an example of something that wants autoloading both as a file > system and a driver, see ZFS. ... if zfs is the only use case, since IIUC it's broken anyway. Meanwhile, if there is something else which is catastrophically broken due to lack of holy christening, we can revisit this and see if just flipping the switch and maybe writing a simple audit script to verify no collisions is the path of least wailing (this would be close to "2").
Re: CVS commit: src/sys/miscfs/specfs
On Tue, Apr 13, 2010 at 03:40:24PM +0300, Antti Kantee wrote: > On Tue Apr 13 2010 at 12:32:38 +, Andrew Doran wrote: > > So the kernel of the problem is namespace collisions, correct? > > Mostly. Though I still think it's not expected that opening a /dev > node will load e.g. an exec package or secmodel even if that's what some > programmer wants. > > > Would you agree that's it's the kernel programmers responsibility > > to avoid conflicts? > > > > If so how about sprinkling a little process in order to make it harder to > > screw up? > > I'd say computers do conflict detection better. So the result of our teeth-pulling so far is: 1. You are concerned about namespace conflicts. I am too. 2. I would be happy to see these managed through documentation and a straightforward approval process for adding modules. 3. You suggest that it would be better for the computer to manage it. Can you suggest an alternative mechanism that will (a) allow us to autoload things that are not anointed device drivers and (b) satisfy your concerns? As an example of something that wants autoloading both as a file system and a driver, see ZFS.
Re: CVS commit: src/sys/miscfs/specfs
On Tue Apr 13 2010 at 12:32:38 +, Andrew Doran wrote: > So the kernel of the problem is namespace collisions, correct? Mostly. Though I still think it's not expected that opening a /dev node will load e.g. an exec package or secmodel even if that's what some programmer wants. > Would you agree that's it's the kernel programmers responsibility > to avoid conflicts? > > If so how about sprinkling a little process in order to make it harder to > screw up? I'd say computers do conflict detection better.
Re: CVS commit: src/sys/miscfs/specfs
On Tue, Apr 13, 2010 at 03:27:11PM +0300, Antti Kantee wrote: > On Tue Apr 13 2010 at 12:18:38 +, Andrew Doran wrote: > > On Tue, Apr 13, 2010 at 10:47:51AM +0300, Antti Kantee wrote: > > > On Tue Apr 13 2010 at 01:15:56 +, Adam Hoka wrote: > > > > Module Name:src > > > > Committed By: ahoka > > > > Date: Tue Apr 13 01:15:56 UTC 2010 > > > > > > > > Modified Files: > > > > src/sys/miscfs/specfs: spec_vnops.c > > > > > > > > Log Message: > > > > Autoload modules with any class. > > > > > > > > This fixes autoloading of pf, zfs and possibly others. > > > > > > What is wrong with making drivers MODULE_CLASS_DRIVER? I think we > > > should limit autoload aiming for safety, not splatter it all over. > > > > > > Eventually everything will of course be fixed by, *tadaa*, devfs. > > > Meanwhile, please back this out and if you don't think calling a driver > > > a driver is the right thing to do, have a discussion. > > > > What's the problem with autoloading any class? Is it just a cosmetic thing? > > I already mentioned my opinion above: "aim for safety". Since there > is currently no common place to manage the different names we have, > I don't want to discover some day that e.g. some secmodel name matches > a devsw name. So the kernel of the problem is namespace collisions, correct? Would you agree that's it's the kernel programmers responsibility to avoid conflicts? If so how about sprinkling a little process in order to make it harder to screw up?
Re: CVS commit: src/sys/miscfs/specfs
On Tue Apr 13 2010 at 12:18:38 +, Andrew Doran wrote: > On Tue, Apr 13, 2010 at 10:47:51AM +0300, Antti Kantee wrote: > > On Tue Apr 13 2010 at 01:15:56 +, Adam Hoka wrote: > > > Module Name: src > > > Committed By: ahoka > > > Date: Tue Apr 13 01:15:56 UTC 2010 > > > > > > Modified Files: > > > src/sys/miscfs/specfs: spec_vnops.c > > > > > > Log Message: > > > Autoload modules with any class. > > > > > > This fixes autoloading of pf, zfs and possibly others. > > > > What is wrong with making drivers MODULE_CLASS_DRIVER? I think we > > should limit autoload aiming for safety, not splatter it all over. > > > > Eventually everything will of course be fixed by, *tadaa*, devfs. > > Meanwhile, please back this out and if you don't think calling a driver > > a driver is the right thing to do, have a discussion. > > What's the problem with autoloading any class? Is it just a cosmetic thing? I already mentioned my opinion above: "aim for safety". Since there is currently no common place to manage the different names we have, I don't want to discover some day that e.g. some secmodel name matches a devsw name.
Re: CVS commit: src/sys/dist/pf/net
On Tue, Apr 13, 2010 at 02:54:23PM +0300, Antti Kantee wrote:
> On Tue Apr 13 2010 at 01:02:44 +, Adam Hoka wrote:
> > Module Name:src
> > Committed By: ahoka
> > Date: Tue Apr 13 01:02:43 UTC 2010
> >
> > Modified Files:
> > src/sys/dist/pf/net: pf_ioctl.c
> >
> > Log Message:
> > Do not auto unload pf if it's enabled.
>
> Well what happens if you non-auto unload it when it's enabled? Just let
> unload fail if it's busy.
There are a few issues with this:
- Unload should be "atomic". If it fails for some reason, we must not
urinate in the user's corkflakes. So on that basis the EBUSY check
belongs in the CMD_UNLOAD block.
- At the point this is called, only module_lock is held. So there
is no locking in at least the autounload case noted. Presumably the
same is true for unload? That could cause crashes and is therefore
unacceptable. (A strong choice of words - I am not trying to frustrate
or rile anyone. As we say in Dublin the truth does be bitter. :-)
- ENOTTY implies "I don't understand what you just asked", zero would
be a better return code as in "yes that's fine, whatever". :-)
This all needs to be written down somewhere, I'll have a look.
> > #ifdef _KERNEL_OPT
> > #include "opt_inet.h"
> > @@ -3367,6 +3367,13 @@
> > pfdetach();
> > pflogdetach();
> > return devsw_detach(NULL, &pf_cdevsw);
> > + case MODULE_CMD_AUTOUNLOAD:
> > + /* Do not auto unload pf if it's enabled. */
> > + if (pf_status.running) {
> > + return EBUSY;
> > + } else {
> > + return ENOTTY;
> > + }
> > default:
> > return ENOTTY;
> > }
Re: CVS commit: src/sys/miscfs/specfs
On Tue, Apr 13, 2010 at 10:47:51AM +0300, Antti Kantee wrote: > On Tue Apr 13 2010 at 01:15:56 +, Adam Hoka wrote: > > Module Name:src > > Committed By: ahoka > > Date: Tue Apr 13 01:15:56 UTC 2010 > > > > Modified Files: > > src/sys/miscfs/specfs: spec_vnops.c > > > > Log Message: > > Autoload modules with any class. > > > > This fixes autoloading of pf, zfs and possibly others. > > What is wrong with making drivers MODULE_CLASS_DRIVER? I think we > should limit autoload aiming for safety, not splatter it all over. > > Eventually everything will of course be fixed by, *tadaa*, devfs. > Meanwhile, please back this out and if you don't think calling a driver > a driver is the right thing to do, have a discussion. What's the problem with autoloading any class? Is it just a cosmetic thing? One doesn't have to be an anointed driver to have an entry in /dev. (It implies that the modules and devsw entries inhabit the same namespace.)
Re: CVS commit: src/sys/modules/pf
On Mon Apr 12 2010 at 14:05:38 +, Adam Hoka wrote: > Module Name: src > Committed By: ahoka > Date: Mon Apr 12 14:05:38 UTC 2010 > > Added Files: > src/sys/modules/pf: Makefile bpfilter.h pf.h pflog.h pfsync.h > > Log Message: > Add new type kernel module for pf (includes pflog, but not pfsync). bpfilter.h is no longer used, so there is no reason to add it.
Re: CVS commit: src/sys/dist/pf/net
On Tue Apr 13 2010 at 01:02:44 +, Adam Hoka wrote:
> Module Name: src
> Committed By: ahoka
> Date: Tue Apr 13 01:02:43 UTC 2010
>
> Modified Files:
> src/sys/dist/pf/net: pf_ioctl.c
>
> Log Message:
> Do not auto unload pf if it's enabled.
Well what happens if you non-auto unload it when it's enabled? Just let
unload fail if it's busy.
> #ifdef _KERNEL_OPT
> #include "opt_inet.h"
> @@ -3367,6 +3367,13 @@
> pfdetach();
> pflogdetach();
> return devsw_detach(NULL, &pf_cdevsw);
> + case MODULE_CMD_AUTOUNLOAD:
> + /* Do not auto unload pf if it's enabled. */
> + if (pf_status.running) {
> + return EBUSY;
> + } else {
> + return ENOTTY;
> + }
> default:
> return ENOTTY;
> }
>
Re: CVS commit: src
On Mon Apr 12 2010 at 19:59:57 +0200, Adam Hoka wrote: > On Sat, 10 Apr 2010 16:58:53 +1000 > matthew green wrote: > > > > >On Fri Apr 09 2010 at 13:49:12 +, Adam Hoka wrote: > >> Module Name: src > >> Committed By: ahoka > >> Date: Fri Apr 9 13:49:12 UTC 2010 > >> > >> Modified Files: > >>src/distrib/sets/lists/modules: md.amd64 md.i386 > >>src/sys/modules: Makefile > >> > >> Log Message: > >> Connect the pad(4) kernel module to the build on i386 and amd64. > > > >Is pad i386/amd64 specific code? > > > > no. it should work on any audio-capable system. > > I could enable it on every platform and see if it breaks somewhere. > (Altough it shouldnt, I just kept the status quo with having it as x86 only) Please do. Module build is not a kernel config file. Stuff is missing from most GENERICs since there is no easy way to properly add MI things to our 10 zillion different MD GENERICs.
Re: CVS commit: src/sys/miscfs/specfs
On Tue Apr 13 2010 at 01:15:56 +, Adam Hoka wrote: > Module Name: src > Committed By: ahoka > Date: Tue Apr 13 01:15:56 UTC 2010 > > Modified Files: > src/sys/miscfs/specfs: spec_vnops.c > > Log Message: > Autoload modules with any class. > > This fixes autoloading of pf, zfs and possibly others. What is wrong with making drivers MODULE_CLASS_DRIVER? I think we should limit autoload aiming for safety, not splatter it all over. Eventually everything will of course be fixed by, *tadaa*, devfs. Meanwhile, please back this out and if you don't think calling a driver a driver is the right thing to do, have a discussion.
