Re: [PATCH] sysctl: Allow /proc/sys without sys_sysctl

Eric W. Biederman
2006-07-12T14:47:57Z

Andi Kleen <ak@suse.de> writes:

> On Wednesday 12 July 2006 05:13, Eric W. Biederman wrote:
>> Andi Kleen <ak@suse.de> writes:
>> > ebiederm@xmission.com (Eric W. Biederman) writes:
>> >> Since sys_sysctl is deprecated start allow it to be compiled out.
>> >> This should catch any remaining user space code that cares,
>> >
>> > I tried this long ago, but found that glibc uses sysctl in each
>> > program to get the kernel version. It probably handles ENOSYS,
>> > but there might be slowdowns or subtle problems from it not knowing
>> > the kernel version.
>> >
>> > So I think it's ok to remove the big sysctl, but at a very minimal
>> > replacement that just handles (CTL_KERN, KERN_VERSION) is needed.
>>
>> If glibc is looking at kernel.osrelease it might make sense.
>> If glibc is looking at kernel.version which is just the build number
>> and date I can't imagine a correct usage.
>
> It's KERN_VERSION
>
>>From my /bin/ls:
>
> _sysctl({{CTL_KERN, KERN_VERSION}, 2, 0xbfc8e1e0, 30, (nil), 0}) = 0

Yep. I found it.

In glibc (2.3.6, 2.4, and CVS) nptl/sysdeps/unix/sysv/linux/smp.h
>
> /* Test whether the machine has more than one processor. This is not the
> best test but good enough. More complicated tests would require `malloc'
> which is not available at that time. */
> static inline int
> is_smp_system (void)
> {
> static const int sysctl_args[] = { CTL_KERN, KERN_VERSION };
> char buf[512];
> size_t reslen = sizeof (buf);
>
> /* Try reading the number using `sysctl' first. */
> if (__sysctl ((int *) sysctl_args,
> sizeof (sysctl_args) / sizeof (sysctl_args[0]),
> buf, &reslen, NULL, 0) < 0)
> {
> /* This was not successful. Now try reading the /proc filesystem. */
> int fd = open_not_cancel_2 ("/proc/sys/kernel/version", O_RDONLY);
> if (__builtin_expect (fd, 0) == -1
> || (reslen = read_not_cancel (fd, buf, sizeof (buf))) <= 0)
> /* This also didn't work. We give up and say it's a UP machine. */
> buf[0] = '\0';
>
> close_not_cancel_no_status (fd);
> }
>
> return strstr (buf, "SMP") != NULL;
> }

So it will correctly handle that sysctl being compiled out, and
the fallback to using /proc. The code seems to have been
doing that since it was added to glibc in 2000.

It only uses it to see if it should busy wait when taking a mutex.

If the kernel has SMP support is lousy predictor if it makes sense
for glibc to busy wait. Darn optimizations for the contended case.

>> If this usage is still common in glibc we can decide what to do
>> when the warnings pop up.
>
> printk for everything would annoy basically everybody. Not a good idea.

Well everything isn't using pthreads. Why ls uses pthreads
is beyond me.

Anyway it looks like it's time to send of a patch.

Eric