Re: [patch 1/13] signal/timer/event fds v8 - anonymous inode source...

[Andrew Morton]

On Fri, 30 Mar 2007 15:44:15 -0700 (PDT)
Davide Libenzi <davidel@xxxxxxxxxxxxxxx> wrote:

> On Fri, 30 Mar 2007, Andrew Morton wrote:
> 
> > > +#include <asm/uaccess.h>
> > > +
> > > +
> > > +
> > 
> > Too many blank lines
> 
> It'd be interesting to know how much is enough. You use one, ppl says it 
> is too dense. You use more, ppl says it's too much.
> There's the one-line rule for inter-function spacing, but what's the 
> include-functions ones? Or the functions-data ones?
> 

less ;)

> 
> > > +static int __init aino_init(void)
> > > +{
> > > +	int error;
> > > +
> > > +	error = register_filesystem(&aino_fs_type);
> > > +	if (error)
> > > +		goto err_exit;
> > > +	aino_mnt = kern_mount(&aino_fs_type);
> > > +	if (IS_ERR(aino_mnt)) {
> > > +		error = PTR_ERR(aino_mnt);
> > > +		goto err_unregister_filesystem;
> > > +	}
> > > +	aino_inode = aino_mkinode();
> > > +	if (IS_ERR(aino_inode)) {
> > > +		error = PTR_ERR(aino_inode);
> > > +		goto err_mntput;
> > > +	}
> > > +
> > > +	return 0;
> > > +
> > > +err_mntput:
> > > +	mntput(aino_mnt);
> > > +err_unregister_filesystem:
> > > +	unregister_filesystem(&aino_fs_type);
> > > +err_exit:
> > > +	printk(KERN_ERR "aino_init() failed (%d)\n", error);
> > 
> > I suspect this is panic time?
> 
> Ok, it was panincing, and someone made me change it. Would you please 
> agree?
> The system can survive w/out, but it'll be a broken system WRT userspace.

I'd say panic.  There's no much point in limping along with an
incorrectly-working kernel, only to have some small number of apps fail
mysteriously later on.

> > 
> > Can we make this optional if CONFIG_EMBEDDED?  You plan on converting epoll
> > to use this facility, but with CONFIG_EPOLL=n, this is all dead code?
> 
> Hmmm, the whole point is that all this stuff works with or without epoll. 
> And epoll need no changes to support this.

I'm suggesting that all known clients of anon_inode be made optional. 
Hence anon_iode can become optional too.

It's a desirable objective, at least.  The default, really.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/