Re: bitops.h ifdef __KERNEL__ cleanup.

Re: bitops.h ifdef __KERNEL__ cleanup.

[Russell King]

On Thu, Jul 19, 2001 at 12:54:43PM +0000, Petr Vandrovec wrote:
> Please do not do this. At least ncpfs checks for usability of these
> ops from its configure script, and if they are not available/usable, 
> it reverts to pthread mutex based implementation, which is slower 
> dozen of times. Same applies for atomic_* functions.

Both of the above mentioned functions can only be guaranteed to act
as per their atomic description if used from kernel space on some
architectures.

I've hit this problem many times, and its not going away, because "it
works on x86".

In fact, the places I came across when it was causing me problems were
places that were just using it as a "oh, someone else has coded a function
to set a bit in the kernel, we'll use that instead of coding it in portable
C" type thing - the application was single threaded, and was altering a
private internal data structure.

Sloppy.

> I think that you should complain to userspace authors who do not
> check for bitops existence and not force other to distrbute 8+ versions
> of bitops.h with their application, together with infrastructure for
> selecting correct version...

I totally disagree here.  We already say "user space should not include
kernel headers".  Why should bitops.h be any different?  Why should atomic.h
be any different?  They contain architecture specific code, yes, which
may not work in user space.

Oh, and thanks for pointing out ncpfs breaks - I hope the authors will
fix up their sloppy coding before Davids patch makes it into the kernel.
;)

--
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: bitops.h ifdef __KERNEL__ cleanup.

[Petr Vandrovec]

On 18 Jul 01 at 23:54, David Woodhouse wrote:

> Not all architectures put clear_bit et al in asm/bitops.h in a form which 
> is usable from userspace. Yet because it happens to work on a PeeCee, 
> people do it anyway. 

Please do not do this. At least ncpfs checks for usability of these
ops from its configure script, and if they are not available/usable, 
it reverts to pthread mutex based implementation, which is slower 
dozen of times. Same applies for atomic_* functions.

I think that you should complain to userspace authors who do not
check for bitops existence and not force other to distrbute 8+ versions
of bitops.h with their application, together with infrastructure for
selecting correct version...

Just my 0.02c.
                                            Petr Vandrovec
                                            vandrove@vc.cvut.cz

Re: bitops.h ifdef __KERNEL__ cleanup.

[Eric W. Biederman]

Jeff Garzik <jgarzik@mandrakesoft.com> writes:

> > I think the idea with <asm/bitops.h> is that they are protected by
> > #ifdef __KERNEL__ if they are kernel-only; however, if they work in
> > user space then there is no #ifdef and autoconf can detect their
> > presence.
> 
> Any amount of sharing between userspace and kernel -adds- constraints to
> kernel code, and leads to namespace pollution on both ends by careless
> (or busy!) developers.
> 
> Let's remove restrictions and constraints from kernel code, not add to
> them...

Sounds reasonable.  Do you think you can get them to remove
/usr/include/linux, and, /usr/include/asm in the next release of Mandrake?

Eric

Re: bitops.h ifdef __KERNEL__ cleanup.

[Jeff Garzik]

"H. Peter Anvin" wrote:
> Followup to:  <11472.995579612@redhat.com>
> By author:    David Woodhouse <dwmw2@infradead.org>
> In newsgroup: linux.dev.kernel
> >
> > It has been stated many times that kernel headers should not be used in
> > apps. Renaming or moving them should not be necessary - and people would
> > probably only start to use them again anyway. We'd see autoconf checks to
> > find whether it's linux/private.h or xunil/private.h :)
> >
> > In the absence of any expectation that userspace developers will ever obey
> > the simple and oft-repeated rule that you don't use kernel headers from
> > userspace, the #ifdef __KERNEL__ approach would seem to be the best on
> > offer.

> Note that the rule is at least in part theoretical; even glibc include
> kernel headers or -derivatives.

Derivatives are fine and IMHO irrelevant to the issue of __KERNEL__: 
you can always do something like gcc's fixincludes.

uClibc and dietlibc do not include any kernel headers at all.  And at
least one glibc developer spoke up in a previous thread and agreed that
it is not necessary to include kernel headers in glibc.

As important as glibc is, it's breaking a rule, which is a bug, that can
be fixed.


> I think the idea with <asm/bitops.h> is that they are protected by
> #ifdef __KERNEL__ if they are kernel-only; however, if they work in
> user space then there is no #ifdef and autoconf can detect their
> presence.

Any amount of sharing between userspace and kernel -adds- constraints to
kernel code, and leads to namespace pollution on both ends by careless
(or busy!) developers.

Let's remove restrictions and constraints from kernel code, not add to
them...

-- 
Jeff Garzik      | "I wouldn't be so judgemental
Building 1024    |  if you weren't such a sick freak."
MandrakeSoft     |             -- goats.com

Re: bitops.h ifdef __KERNEL__ cleanup.

[H. Peter Anvin]

Followup to:  <11472.995579612@redhat.com>
By author:    David Woodhouse <dwmw2@infradead.org>
In newsgroup: linux.dev.kernel
> 
> It has been stated many times that kernel headers should not be used in
> apps. Renaming or moving them should not be necessary - and people would
> probably only start to use them again anyway. We'd see autoconf checks to 
> find whether it's linux/private.h or xunil/private.h :)
> 
> In the absence of any expectation that userspace developers will ever obey
> the simple and oft-repeated rule that you don't use kernel headers from
> userspace, the #ifdef __KERNEL__ approach would seem to be the best on
> offer.
> 

Note that the rule is at least in part theoretical; even glibc include
kernel headers or -derivatives.

I think the idea with <asm/bitops.h> is that they are protected by
#ifdef __KERNEL__ if they are kernel-only; however, if they work in
user space then there is no #ifdef and autoconf can detect their
presence.

	-hpa
-- 
<hpa@transmeta.com> at work, <hpa@zytor.com> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt

Re: bitops.h ifdef __KERNEL__ cleanup.

[David Woodhouse]

VANDROVE@vc.cvut.cz said:
>  If you do not want kernel headers to be used in apps, just move them
> from asm and linux into msa and xunil. Then you can simple remove all
> #ifdef __KERNEL__ from them...

It has been stated many times that kernel headers should not be used in
apps. Renaming or moving them should not be necessary - and people would
probably only start to use them again anyway. We'd see autoconf checks to 
find whether it's linux/private.h or xunil/private.h :)

In the absence of any expectation that userspace developers will ever obey
the simple and oft-repeated rule that you don't use kernel headers from
userspace, the #ifdef __KERNEL__ approach would seem to be the best on
offer.

> P.S.: Part of ncpfs's configure.ac. I do not think that it is that
> hard...

I'm not very familiar with autoconf, but doesn't the snippet you pasted just 
check that the program compiles and links? It won't notice if you build a 
binary with privileged instructions in, or one which just fails to provide 
the correct semantics when the routine is used in a environment for which 
it was not designed?

Where is this used in ncpfs that it makes _such_ a difference?

--
dwmw2

Re: bitops.h ifdef __KERNEL__ cleanup.

[Petr Vandrovec]

On 19 Jul 01 at 12:48, Russell King wrote:
> 
> I totally disagree here.  We already say "user space should not include
> kernel headers".  Why should bitops.h be any different?  Why should atomic.h
> be any different?  They contain architecture specific code, yes, which
> may not work in user space.

Maybe because of I do not know ARM assembler? If you do not want
kernel headers to be used in apps, just move them from asm and linux
into msa and xunil. Then you can simple remove all #ifdef __KERNEL__
from them...

> Oh, and thanks for pointing out ncpfs breaks - I hope the authors will
> fix up their sloppy coding before Davids patch makes it into the kernel.

It will still work. Only resulting binary will be slower. That's what
autoconf is for. If ncpfs does not compile for you, better to contact
me directly, as I'm ncpfs maintainer...
                                            Best regards,
                                                Petr Vandrovec
                                                vandrove@vc.cvut.cz
                                                
P.S.: Part of ncpfs's configure.ac. I do not think that it is that
hard...

  AC_CACHE_CHECK(for working asm/atomic.h,
      ncp_cv_asm_atomic_h,
    AC_TRY_LINK([#define __SMP__
#include <asm/atomic.h>],
      [atomic_t a;
       atomic_set(&a,2);
       atomic_dec(&a);
       if (atomic_read(&a)) {
         if (!atomic_dec_and_test(&a)) {
       atomic_inc(&a);
     }
       }],
      [ncp_cv_asm_atomic_h="yes"],
      [ncp_cv_asm_atomic_h="no"]
    )
  )
  if test "$ncp_cv_asm_atomic_h" = "yes"
  then
    AC_DEFINE(HAVE_ASM_ATOMIC_H, 1, [Define if we have working asm/atomic.h])
  fi

Re: bitops.h ifdef __KERNEL__ cleanup.

[Russell King]

On Thu, Jul 19, 2001 at 07:21:48PM +0000, Petr Vandrovec wrote:
> Maybe because of I do not know ARM assembler? If you do not want
> kernel headers to be used in apps, just move them from asm and linux
> into msa and xunil. Then you can simple remove all #ifdef __KERNEL__
> from them...

Why should the kernel change to please a problem minority in user space
who shouldn't be including kernel headers in the first place?

> It will still work. Only resulting binary will be slower. That's what
> autoconf is for. If ncpfs does not compile for you, better to contact
> me directly, as I'm ncpfs maintainer...

No.  The binary _will_ not do what you expect - these functions are
not atomic in user space on all architecture types.  Yes they may work,
but not with the atomic side effects.  You won't get this from a simple
autoconf test.

I'll give you credit though - you're at least checking that they appear
to exist; I have come across many programs which rely on them existing,
and do not check that they exist.

It is these that David and myself wish to target, and this along with
the general rule of "Never include kernel headers in user code", it
seems to be the most appropriate solution.

Now, there is a nice clean solution to your problem - bug the glibc
people to provide equivalents in their library, hopefully as inline
asm in the systems header files.  Systems which need to do extra stuff
can then have them implemented in the C library.

--
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

bitops.h ifdef __KERNEL__ cleanup.

[David Woodhouse]

Not all architectures put clear_bit et al in asm/bitops.h in a form which 
is usable from userspace. Yet because it happens to work on a PeeCee, 
people do it anyway. 

There's a simple way to fix that :)

Index: include/asm-i386/bitops.h
===================================================================
RCS file: /inst/cvs/linux/include/asm-i386/bitops.h,v
retrieving revision 1.2.2.7
diff -u -r1.2.2.7 bitops.h
--- include/asm-i386/bitops.h	2001/06/02 16:27:54	1.2.2.7
+++ include/asm-i386/bitops.h	2001/07/18 22:52:11
@@ -7,6 +7,8 @@
 
 #include <linux/config.h>
 
+#ifdef __KERNEL__
+
 /*
  * These have to be done with inline assembly: that way the bit-setting
  * is guaranteed to be atomic. All bit operations return 0 if the bit
@@ -329,8 +331,6 @@
 		:"r" (~word));
 	return word;
 }
-
-#ifdef __KERNEL__
 
 /**
  * ffs - find first bit set

--
dwmw2