Start using sysctl paths in the core kernel code

This includes the tables themselves and the call to the register_sysctl_table(). Since this call is done from the __init call, I hope this is OK to keep the #ifdef inside the function, rather than making proper helpers outside it.

Signed-off-by Pavel Emelyanov <[email protected]>

---

fs/dquot.c +2 -0

index 50e7c2a..efee14d 100644
@@ -1821,6 +1821,7 @@ struct quotactl_ops vfs_quotactl_ops = {
 	.set_dqblk	= vfs_set_dqblk
 };
 
+#ifdef CONFIG_SYSCTL
 static ctl_table fs_dqstats_table[] = {
 	{
 		.ctl_name	= FS_DQ_LOOKUPS,

@@ -1918,6 +1919,7 @@ static ctl_table sys_table[] = {
 	},
 	{ .ctl_name = 0 },
 };
+#endif
 
 static int __init dquot_init(void)
 {

Quoted 20 lines

fs/dquot.c +4 -0

+#ifdef CONFIG_SYSCTL
 static ctl_table fs_dqstats_table[] = {
 	{
 		.ctl_name	= FS_DQ_LOOKUPS,

@@ -1918,6 +1919,7 @@ static ctl_table sys_table[] = {
 	},
 	{ .ctl_name = 0 },
 };
+#endif
 
 static int __init dquot_init(void)
 {

@@ -1926,7 +1928,9 @@ static int __init dquot_init(void)
 
 	printk(KERN_NOTICE "VFS: Disk quotas %s\n", __DQUOT_VERSION__);
 
+#ifdef CONFIG_SYSCTL
 	register_sysctl_table(sys_table);
+#endif
 

fs/dquot.c static int __init dquot_init(void) · line 1936

Quoted 2 lines

fs/dquot.c +0 -0

 	dquot_cachep = kmem_cache_create("dquot",
 			sizeof(struct dquot), sizeof(unsigned long) * 4,

We should avoid the ifdefs around the register_sysctl_table() call.

At present the !CONFIG_SYSCTL implementation of register_sysctl_table() is a non-inlined NULL-returning stub. All we have to do is to inline that stub then these ifdefs can go away.

The same applies to register_sysctl_paths().

Quoted 37 lines · On Fri, 30 Nov 2007 16:02:50 +0300 Pavel Emelyanov <[email protected]> wrote:

This includes the tables themselves and the call to the register_sysctl_table(). Since this call is done from the __init call, I hope this is OK to keep the #ifdef inside the function, rather than making proper helpers outside it.

Signed-off-by Pavel Emelyanov <[email protected]>

---

fs/dquot.c +4 -0

index 50e7c2a..efee14d 100644
@@ -1821,6 +1821,7 @@ struct quotactl_ops vfs_quotactl_ops = {
 	.set_dqblk	= vfs_set_dqblk
 };
 
+#ifdef CONFIG_SYSCTL
 static ctl_table fs_dqstats_table[] = {
 	{
 		.ctl_name	= FS_DQ_LOOKUPS,

@@ -1918,6 +1919,7 @@ static ctl_table sys_table[] = {
 	},
 	{ .ctl_name = 0 },
 };
+#endif
 
 static int __init dquot_init(void)
 {

@@ -1926,7 +1928,9 @@ static int __init dquot_init(void)
 
 	printk(KERN_NOTICE "VFS: Disk quotas %s\n", __DQUOT_VERSION__);
 
+#ifdef CONFIG_SYSCTL
 	register_sysctl_table(sys_table);
+#endif
 

fs/dquot.c static int __init dquot_init(void) · line 1936

Quoted 2 lines

fs/dquot.c +0 -0

 	dquot_cachep = kmem_cache_create("dquot",
 			sizeof(struct dquot), sizeof(unsigned long) * 4,

We should avoid the ifdefs around the register_sysctl_table() call.

At present the !CONFIG_SYSCTL implementation of register_sysctl_table() is a non-inlined NULL-returning stub. All we have to do is to inline that stub then these ifdefs can go away.

The same applies to register_sysctl_paths().

If that's all agreeable then there isn't a lot of point in me merging these seven patches.

btw, administrivia detail: please don't put the patch's subsystem identifier in []. IOW, this:

Subject: [PATCH 1/7][QUOTA] Move sysctl management code under ifdef CONFIG_SYSCTL

should have been

Subject: [PATCH 1/7] quota: move sysctl management code under ifdef CONFIG_SYSCTL

for reasons described in section 2 of
http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt.

Thanks.

Quoted 9 lines

fs/dquot.c static int __init dquot_init(void) · line 1936

Quoted 6 lines

fs/dquot.c +2 -0

+#ifdef CONFIG_SYSCTL
 	register_sysctl_table(sys_table);
+#endif
 
 	dquot_cachep = kmem_cache_create("dquot",
 			sizeof(struct dquot), sizeof(unsigned long) * 4,

We should avoid the ifdefs around the register_sysctl_table() call.

Quoted 3 lines

At present the !CONFIG_SYSCTL implementation of register_sysctl_table() is a non-inlined NULL-returning stub. All we have to do is to inline that stub then these ifdefs can go away.

What if some code checks for the return value to be not-NULL? In case CONFIG_SYSCTL=n this code will always think, that the registration failed.

Quoted 8 lines · On Tue, 04 Dec 2007 11:58:30 +0300 Pavel Emelyanov <[email protected]> wrote:

Quoted 8 lines

fs/dquot.c static int __init dquot_init(void) · line 1936

Quoted 6 lines

fs/dquot.c +2 -0

+#ifdef CONFIG_SYSCTL
 	register_sysctl_table(sys_table);
+#endif
 
 	dquot_cachep = kmem_cache_create("dquot",
 			sizeof(struct dquot), sizeof(unsigned long) * 4,

We should avoid the ifdefs around the register_sysctl_table() call.

Quoted 5 lines

Quoted 3 lines

At present the !CONFIG_SYSCTL implementation of register_sysctl_table() is a non-inlined NULL-returning stub. All we have to do is to inline that stub then these ifdefs can go away.

What if some code checks for the return value to be not-NULL? In case CONFIG_SYSCTL=n this code will always think, that the registration failed.

The stub function should return success?

Quoted 10 lines · Andrew Morton

Quoted 8 lines · On Tue, 04 Dec 2007 11:58:30 +0300 Pavel Emelyanov <[email protected]> wrote:

Quoted 8 lines

fs/dquot.c static int __init dquot_init(void) · line 1936

Quoted 6 lines

fs/dquot.c +2 -0

+#ifdef CONFIG_SYSCTL
 	register_sysctl_table(sys_table);
+#endif
 
 	dquot_cachep = kmem_cache_create("dquot",
 			sizeof(struct dquot), sizeof(unsigned long) * 4,

We should avoid the ifdefs around the register_sysctl_table() call.

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

At present the !CONFIG_SYSCTL implementation of register_sysctl_table() is a non-inlined NULL-returning stub. All we have to do is to inline that stub then these ifdefs can go away.

What if some code checks for the return value to be not-NULL? In case CONFIG_SYSCTL=n this code will always think, that the registration failed.

The stub function should return success?

Well, I think yes. If some functionality is turned off, then the caller should think that everything is going fine (or he should explicitly removes the call to it with some other ifdef).

At least this is true for stubs that return the error code, not the pointer. E.g. copy_semundo() always returns success if SYSVIPC is off, or namespaces cloning routines act in a similar way.

Thus I though, that routines, that return pointers should better report that everything is OK (somehow) to reduce the number of "helpers" in the outer code. No?

Quoted 7 lines · On Tue, 04 Dec 2007 12:31:37 +0300 Pavel Emelyanov <[email protected]> wrote:

Quoted 6 lines · Andrew Morton

Quoted 4 lines · On Tue, 04 Dec 2007 11:58:30 +0300 Pavel Emelyanov <[email protected]> wrote:

Quoted 4 lines

Quoted 4 lines

fs/dquot.c +2 -0

+#ifdef CONFIG_SYSCTL
 	register_sysctl_table(sys_table);
+#endif

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 2 lines

dquot_cachep = kmem_cache_create("dquot",
sizeof(struct dquot), sizeof(unsigned long) * 4,

We should avoid the ifdefs around the register_sysctl_table() call.

At present the !CONFIG_SYSCTL implementation of register_sysctl_table() is a non-inlined NULL-returning stub. All we have to do is to inline that

stub

Quoted 14 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

then these ifdefs can go away.

What if some code checks for the return value to be not-NULL? In case CONFIG_SYSCTL=n this code will always think, that the registration failed.

The stub function should return success?

Well, I think yes. If some functionality is turned off, then the caller should think that everything is going fine (or he should explicitly removes the call to it with some other ifdef).

At least this is true for stubs that return the error code, not the pointer. E.g. copy_semundo() always returns success if SYSVIPC is off, or namespaces cloning routines act in a similar way.

Quoted 4 lines

Thus I though, that routines, that return pointers should better report that everything is OK (somehow) to reduce the number of "helpers" in the outer code. No?

Dunno. Returning NULL should be OK. If anyone is dereferenceing that pointer with CONFIG_SYSCTL=n then they might need some attention?

Quoted 35 lines · Andrew Morton

Quoted 7 lines · On Tue, 04 Dec 2007 12:31:37 +0300 Pavel Emelyanov <[email protected]> wrote:

Quoted 6 lines · Andrew Morton

Quoted 4 lines · On Tue, 04 Dec 2007 11:58:30 +0300 Pavel Emelyanov <[email protected]> wrote:

Quoted 4 lines

Quoted 4 lines

(quoted diff) +2 -0

+#ifdef CONFIG_SYSCTL
 	register_sysctl_table(sys_table);
+#endif

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 2 lines

dquot_cachep = kmem_cache_create("dquot",
sizeof(struct dquot), sizeof(unsigned long) * 4,

We should avoid the ifdefs around the register_sysctl_table() call.

At present the !CONFIG_SYSCTL implementation of register_sysctl_table() is a non-inlined NULL-returning stub. All we have to do is to inline that

stub

Quoted 18 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

then these ifdefs can go away.

What if some code checks for the return value to be not-NULL? In case CONFIG_SYSCTL=n this code will always think, that the registration failed.

The stub function should return success?

Well, I think yes. If some functionality is turned off, then the caller should think that everything is going fine (or he should explicitly removes the call to it with some other ifdef).

At least this is true for stubs that return the error code, not the pointer. E.g. copy_semundo() always returns success if SYSVIPC is off, or namespaces cloning routines act in a similar way.

Thus I though, that routines, that return pointers should better report that everything is OK (somehow) to reduce the number of "helpers" in the outer code. No?

Quoted 2 lines

Dunno. Returning NULL should be OK. If anyone is dereferenceing that pointer with CONFIG_SYSCTL=n then they might need some attention?

We do have some current code in the network stack that fails miserably when register_sysctl_table returns NULL, and there are explicit checks for that.

Grr.

I had forgotten about that.

I expect the right answer is to simply have code ignore the fact that register_sysctl_xxxx returns NULL, and not error on it.

The alternative is to get fancy and have everyone check the return code and make the return type an IS_ERR thing. That seems a lot more trouble then it is worth.

We can probably define it as register_sysctl_xxxx always returns a token that must be passed to unregister_sysctl, and no errors will be reported except to dmesg. That at sounds simple sane and supportable from where we are now.

Eric

Quoted 3 lines

We do have some current code in the network stack that fails miserably when register_sysctl_table returns NULL, and there are explicit checks for that.

So that code would be failing today with CONFIG_SYSCTL=n? Unless the failing code is itself under #ifdef CONFIG_SYSCTL, in which case we don't need to change anything?

[snip]

Quoted 7 lines

Quoted 3 lines

We do have some current code in the network stack that fails miserably when register_sysctl_table returns NULL, and there are explicit checks for that.

So that code would be failing today with CONFIG_SYSCTL=n? Unless the failing code is itself under #ifdef CONFIG_SYSCTL, in which case we don't need to change anything?

Exactly! If the code checks for the return value it won't work with CONFIG_SYSCTL=n, if it dies not - it may happily use the sysctl stub and avoid extra ifdefs.

But this difference looks clumsy :(