RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Tillier, Fabian]

So which is more important to the "Linux kernel" project: i386 backwards
compatibility, or consistent API and functionality across processor
architectures? ;)

- Fab


-----Original Message-----
From: Greg KH [mailto:greg@kroah.com] 
Sent: Thursday, February 05, 2004 12:28 PM
To: Tillier, Fabian
Cc: Randy.Dunlap; sean.hefty@intel.com; linux-kernel@vger.kernel.org;
hozer@hozed.org; woody@co.intel.com; bill.magro@intel.com;
woody@jf.intel.com; infiniband-general@lists.sourceforge.net
Subject: Re: [Infiniband-general] Getting an Infiniband access layer in
theLinux kernel

On Thu, Feb 05, 2004 at 02:26:19PM -0500, Tillier, Fabian wrote:
> Do note that for non x86 architectures, the component library atomic
> abstraction is all #define to the Linux provided functions.  Only x86
> needed help because of i386 backwards compatibility which is not a
goal
> of the InfiniBand project.

But that is a goal of the "Linux kernel" project :)

greg k-h

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Greg KH]

A: No.
Q: Should I include quotations after my reply?

On Thu, Feb 05, 2004 at 03:32:09PM -0500, Tillier, Fabian wrote:
> So which is more important to the "Linux kernel" project: i386 backwards
> compatibility, or consistent API and functionality across processor
> architectures? ;)

Anyway, why not describe what you are trying to accomplish that made you
determine that you _had_ to have these kinds of functions.

Basically, what is lacking in the current kernel locks that the
infiniband project has to have in order to work properly.  We can work
from there.

thanks,

greg k-h

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Chris Friesen]

Greg KH wrote:

> Basically, what is lacking in the current kernel locks that the
> infiniband project has to have in order to work properly.  We can work
> from there.

I think their point is that they want the core device driver code to be 
portable across kernel versions, and across different OS's other than 
linux--which basically requires some kind of abstraction layer.

Chris

-- 
Chris Friesen                    | MailStop: 043/33/F10
Nortel Networks                  | work: (613) 765-0557
3500 Carling Avenue              | fax:  (613) 765-2986
Nepean, ON K2H 8E9 Canada        | email: cfriesen@nortelnetworks.com

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Bill Davidsen]

Tillier, Fabian wrote:
> So which is more important to the "Linux kernel" project: i386 backwards
> compatibility, or consistent API and functionality across processor
> architectures? ;)

For clever programmers they are not incompatible.

> 
> - Fab
> 
> 
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com] 
> Sent: Thursday, February 05, 2004 12:28 PM
> To: Tillier, Fabian
> Cc: Randy.Dunlap; sean.hefty@intel.com; linux-kernel@vger.kernel.org;
> hozer@hozed.org; woody@co.intel.com; bill.magro@intel.com;
> woody@jf.intel.com; infiniband-general@lists.sourceforge.net
> Subject: Re: [Infiniband-general] Getting an Infiniband access layer in
> theLinux kernel
> 
> On Thu, Feb 05, 2004 at 02:26:19PM -0500, Tillier, Fabian wrote:
> 
>>Do note that for non x86 architectures, the component library atomic
>>abstraction is all #define to the Linux provided functions.  Only x86
>>needed help because of i386 backwards compatibility which is not a
> 
> goal
> 
>>of the InfiniBand project.
> 
> 
> But that is a goal of the "Linux kernel" project :)
> 
> greg k-h
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


-- 
bill davidsen <davidsen@tmr.com>
   CTO TMR Associates, Inc
   Doing interesting things with small computers since 1979

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Woodruff, Robert J]

On Thu, Feb 05, 2004 at 03:20:46PM -0800, Greg KH Wrote,

>Great, when is this going to happen?  I think I'm not going to respond
anymore
>to this thread until I see some actual code.

Ok, we now have the InfiniBand Access Layer (IBAL) integrated and
building in 
the 2.6.3 environment and have a patch that we can provide. We have not
yet 
fixed the issues that you have raised so far (such as the abstraction
layer), 
but wanted to submit the code so that we can get additional 
feedback from you and all of the community, as you requested. 

Would you like me to submit the patch directly to linux-kernel email
list ? 

I have also posted it to the InfiniBand sourceforge website at:

http://sourceforge.net/projects/infiniband

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Greg KH]

On Tue, Feb 24, 2004 at 09:55:25AM -0800, Woodruff, Robert J wrote:
> 
> Would you like me to submit the patch directly to linux-kernel email
> list ? 

Yes, please do.  If you want, feel free to cc: me also.

But as you have not cleaned up the stuff that everyone has already told
you to clean up, it might be pretty pointless to do so without fixing
that code first.  In fact, why even post a patch without making those,
necessary changes?

thanks,

greg k-h

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Pete Zaitcev]

On Thu, 5 Feb 2004 17:02:30 -0500
"Tillier, Fabian" <ftillier@infiniconsys.com> wrote:

> That is absolutely correct.  In addition to portability between kernel
> versions and operating systems, there is also portability between
> user-mode and kernel-mode within a single release.

Just for the record, Robert and Asok visited OLS in 2002 and I seem to
recall explaining to them in excuriating detail why an exact compatibility
between user mode and kernel mode API was a crock of shit, pipe dream,
etc. etc., at least in the context of Linux. Failed to make a dent,
apparently. But at least I tried!

On the other hand, I ate Asok's brain about callbacks until he caved,
so it's not all hopeless. Intel people are competent, they just come from
a different cultural background. I think that instead of plonking it would
be more productive to explain them the issues, e.g. the cost of insane
and atrocious abstractions they have in the stack, that ia64 is not the
only 64 bit architecture in the world (no matter how much Intel wants us
to believe it), that IOMMUs actually exist (in HP's Itanic servers, for
one), such kind of thing.

-- Pete

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Arnd Bergmann]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Fri, 6 Feb 2004, Hefty, Sean wrote:

> We want to continue to discuss specific details about what's needed to
> add the code into the kernel.  Here's a list of modifications that I
> think are needed so far:
>
> * Update the code to make direct calls for atomic operations.
> * Verify the use of spinlock calls.
> * Reformat the code for tab spacing and curly brace usage.
> * Elimination of typedefs.

Some more hints:
* Understand how to use ioctls: Your current code can't possibly
  work in 32 bit emulation mode, e.g. when running i386 libs on ia64.
  You are also completely ignoring the ioctl 'command' argument.
* Try to reduce the number of necessary ioctls: 90 different commands
  for a single device is just overkill.
* Don't try to hide system calls behind a character device (/dev/cl_dev).
  Someone will find it sooner or later.
* Don't reimplement devfs (poorly), what your are trying to achieve is
  far easier done with a misc device and udev. call_user_mode_helper
  should only be used for running hotplug.
* Get rid of IN and OUT in declarations. If a pointer is const, it's an
  input.
* Declare functions static when they are only used in the file they are
  defined in. Put extern declarations into headers.
* put EXPORT_SYMBOL statements right next to the symbol, not in a seperate
  file.
* Information about drivers/devices should go to sysfs instead of procfs.
* Don't use vmalloc. 
* aside from spinlocks and atomics, there are some more files in complib
  that can easily be removed by using linux primitives directly (e.g.
  kmalloc, list_head, semaphore, work_queue)
* Use C99 named struct initializers instead of their deprecated gcc 
  counterparts.

> And, yes, knowing some of these issues up front will save the trouble of
> submitting code that will be immediately rejected.

Right, but that should not stop you from providing a BK tree or a diff
file that lets you build ibal in a linux tree. As a starting point, just
drop all of ibal and complib into some tree so people can look at it
without having to figure out how you are building stuff.

	Arnd <><
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (GNU/Linux)

iD8DBQFAJsm/5t5GS2LDRf4RArCIAJ9JEkinmx1yeSZQGS7X6rMaDYjuZwCgl9us
+7w/ZNbFl1ZAiLaIrp/rsso=
=repY
-----END PGP SIGNATURE-----

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Woodruff, Robert J]

On Sunday, February 08, 2004 8:30 AM, Greg KH wrote, 
>Why do you want to run your code in both places?  Does this mean that
it doesn't even really >need to be in the kernel as it works just fine
in userspace?

The reason the Access Layer runs in both kernel and user space is to
support
both kernel and user-mode infiniband clients. For example, in kernel
mode a 
SCSI (SRP) or IPoIB driver would use the Access Layer services. In user
space, a
middleware library, such as DAPL, would use the user-mode access layer
to 
access the infiniband fabric. 

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Hefty, Sean]

On Thu, Feb 5, 2004 at 05:11:00PM -0800, Troy Benjegerdes wrote:
>On Thu, Feb 05, 2004 at 02:26:46PM -0800, Hefty, Sean wrote:
>> Personally, I'm amazed that professional developers have to discuss
or
>> defend modular, portable code.
>
>You're new to linux-kernel, aren't you? ;)

I was not trying to be condescending.  My point was that I think that
everyone on this list knows the purpose and benefits behind an
abstraction layer.  It's not something that needed to be discussed any
further.

I also understand that code in the Linux 2.6 kernel does not need
certain abstractions.  And I agree that because we are targeting the 2.6
kernel specifically, the existing code, some of which was developed 3-5
years ago, should be updated based on what the 2.6 kernel provides.

We want to continue to discuss specific details about what's needed to
add the code into the kernel.  Here's a list of modifications that I
think are needed so far:

* Update the code to make direct calls for atomic operations.
* Verify the use of spinlock calls.
* Reformat the code for tab spacing and curly brace usage.
* Elimination of typedefs.

And, yes, knowing some of these issues up front will save the trouble of
submitting code that will be immediately rejected.

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Richard B. Johnson]

On Fri, 6 Feb 2004, Hefty, Sean wrote:

> On Thu, Feb 5, 2004 at 05:11:00PM -0800, Troy Benjegerdes wrote:
> >On Thu, Feb 05, 2004 at 02:26:46PM -0800, Hefty, Sean wrote:
> >> Personally, I'm amazed that professional developers have to discuss
> or
> >> defend modular, portable code.
> >
> >You're new to linux-kernel, aren't you? ;)
>
> I was not trying to be condescending.  My point was that I think that
> everyone on this list knows the purpose and benefits behind an
> abstraction layer.  It's not something that needed to be discussed any
> further.
>
> I also understand that code in the Linux 2.6 kernel does not need
> certain abstractions.  And I agree that because we are targeting the 2.6
> kernel specifically, the existing code, some of which was developed 3-5
> years ago, should be updated based on what the 2.6 kernel provides.
>
> We want to continue to discuss specific details about what's needed to
> add the code into the kernel.  Here's a list of modifications that I
> think are needed so far:
>
> * Update the code to make direct calls for atomic operations.
> * Verify the use of spinlock calls.
> * Reformat the code for tab spacing and curly brace usage.
> * Elimination of typedefs.
>
> And, yes, knowing some of these issues up front will save the trouble of
> submitting code that will be immediately rejected.

If some major changes are being considered, I think it's time
to get rid of the:

do {  } while(0) stuff that permiates a lot of MACROS and just
use the { } as they were designed.

Before everybody screams, think. It's perfectly correct to
start a new "program unit" without a conditional expression.
You just add a curley-brace, then close the brace when you
are though.

For example (from linux/wait.h):

/*
 * Debugging macros.  We eschew `do { } while (0)' because gcc can generate
 * spurious .aligns.
 */
#if WAITQUEUE_DEBUG
#define WQ_BUG()	BUG()
#define CHECK_MAGIC(x)		\
 do {								\
		if ((x) != (long)&(x)) {			\
			printk("bad magic %lx (should be %lx), ",	\
				(long)x, (long)&(x));			\
			WQ_BUG();					\
		}							\
    } while (0)

Surely, this was some kind of work-around for some compiler bug
in a compiler you are not even allowed to use anymore for the
newer kernels.

All you need is the '{}' and none of the 'do' stuff. Since we
are still allowed to use old compilers in old kernels, I'm
nor suggesting that the headers in 2.4.xxx be changed, just the
new stuff, 2.6

Cheers,
Dick Johnson
Penguin : Linux version 2.4.24 on an i686 machine (797.90 BogoMips).
            Note 96.31% of all statistics are fiction.

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Roland Dreier]

    Richard> If some major changes are being considered, I think it's
    Richard> time to get rid of the:

    Richard> do { } while(0) stuff that permiates a lot of MACROS and
    Richard> just use the { } as they were designed.

    Richard> Before everybody screams, think. It's perfectly correct
    Richard> to start a new "program unit" without a conditional
    Richard> expression.  You just add a curley-brace, then close the
    Richard> brace when you are though.

This is totally, totally wrong.  If you get rid of do { } while (0),
then you can't use the macro in an if statement.  Read any C FAQ for
details, or try the following:

    #define MAC(x) { x = x + 1; }

    int main() {
      int x = 0;

      if (1)
        MAC(x);
      else
        x = x - 1;
    }

I get the following (correct) error:

    $ gcc a.c
    a.c: In function `main':
    a.c:8: syntax error before "else"
    $ gcc --version
    gcc (GCC) 3.2.3 20030502 (Red Hat Linux 3.2.3-20)

because

    if (1)
        { x = x + 1 } ; /* <-- note semicolon
    else
        x = x - 1;

is not correct C.

By the way, it is possible to use parentheses and commas for some
simple macros, so for example the following is OK:

    #define MAC(x) ( x = x + 1, x = x * 2 )

    int main() {
      int x = 0;

      if (1)
        MAC(x);
      else
        x = x - 1;
    }

However I don't see anything wrong with the perfectly standard "do { }
while (0)" idiom.  Certainly if some compiler generates worse code for
that construct that just a plain { }, _that_ is a compiler bug that we
shouldn't have to work around.

 - Roland

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Richard B. Johnson]

On Fri, 6 Feb 2004, Roland Dreier wrote:

>     Richard> If some major changes are being considered, I think it's
>     Richard> time to get rid of the:
>
>     Richard> do { } while(0) stuff that permiates a lot of MACROS and
>     Richard> just use the { } as they were designed.
>
>     Richard> Before everybody screams, think. It's perfectly correct
>     Richard> to start a new "program unit" without a conditional
>     Richard> expression.  You just add a curley-brace, then close the
>     Richard> brace when you are though.
>
> This is totally, totally wrong.  If you get rid of do { } while (0),
> then you can't use the macro in an if statement.  Read any C FAQ for
> details, or try the following:
>

Yes you can. You just don't use an ';' if you are going
to use 'else'.


>     #define MAC(x) { x = x + 1; }
>
>     int main() {
>       int x = 0;
>
>       if (1)
>         MAC(x);
>       else
>         x = x - 1;
>     }
>
> I get the following (correct) error:
>
>     $ gcc a.c
>     a.c: In function `main':
>     a.c:8: syntax error before "else"
>     $ gcc --version
>     gcc (GCC) 3.2.3 20030502 (Red Hat Linux 3.2.3-20)
>
> because
>
>     if (1)
>         { x = x + 1 } ; /* <-- note semicolon
>     else
>         x = x - 1;
>
> is not correct C.
>

As I stated.


> By the way, it is possible to use parentheses and commas for some
> simple macros, so for example the following is OK:
>
>     #define MAC(x) ( x = x + 1, x = x * 2 )
>
>     int main() {
>       int x = 0;
>
>       if (1)
>         MAC(x);
>       else
>         x = x - 1;
>     }
>
> However I don't see anything wrong with the perfectly standard "do { }
> while (0)" idiom.  Certainly if some compiler generates worse code for
> that construct that just a plain { }, _that_ is a compiler bug that we
> shouldn't have to work around.
>
>  - Roland
>



Cheers,
Dick Johnson
Penguin : Linux version 2.4.24 on an i986 machine (797.90 BogoMips).
            Note 96.31% of all statistics are fiction.

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Måns Rullgård]

"Richard B. Johnson" <root@chaos.analogic.com> writes:

> On Fri, 6 Feb 2004, Roland Dreier wrote:
>
>>     Richard> If some major changes are being considered, I think it's
>>     Richard> time to get rid of the:
>>
>>     Richard> do { } while(0) stuff that permiates a lot of MACROS and
>>     Richard> just use the { } as they were designed.
>>
>>     Richard> Before everybody screams, think. It's perfectly correct
>>     Richard> to start a new "program unit" without a conditional
>>     Richard> expression.  You just add a curley-brace, then close the
>>     Richard> brace when you are though.
>>
>> This is totally, totally wrong.  If you get rid of do { } while (0),
>> then you can't use the macro in an if statement.  Read any C FAQ for
>> details, or try the following:
>>
>
> Yes you can. You just don't use an ';' if you are going
> to use 'else'.

Now suppose someone changes the macro into an inline function.  Then
the ; is suddenly required.  How are you going to deal with that?

-- 
Måns Rullgård
mru@kth.se

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Chris Friesen]

Richard B. Johnson wrote:
 > On Fri, 6 Feb 2004, Roland Dreier wrote:

 >> This is totally, totally wrong.  If you get rid of do { } while
 >> (0), then you can't use the macro in an if statement.  Read any C
 >> FAQ for details, or try the following:
 >>
 >>
 >
 > Yes you can. You just don't use an ';' if you are going to use
 > 'else'.

That's just silly.  It means you need to know if something is a macro or
an inline function, and do stuff differently.  If you use the do/while
method, it Just Works.


-- 
Chris Friesen                    | MailStop: 043/33/F10
Nortel Networks                  | work: (613) 765-0557
3500 Carling Avenue              | fax:  (613) 765-2986
Nepean, ON K2H 8E9 Canada        | email: cfriesen@nortelnetworks.com

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 436 bytes --]

On Fri, 06 Feb 2004 13:00:38 EST, "Richard B. Johnson" said:

> Yes you can. You just don't use an ';' if you are going
> to use 'else'.

You did realize we've changed things from macros to inline functions
(and vice versa) on occasion?

Yes, you *can* hack around the "problem".  Is there any actual
evidence that any real performance issues arise from the null
do/while?  Does said issue outweigh the increased fragility of
the code?

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Richard B. Johnson]

On Fri, 6 Feb 2004 Valdis.Kletnieks@vt.edu wrote:

> On Fri, 06 Feb 2004 13:00:38 EST, "Richard B. Johnson" said:
>
> > Yes you can. You just don't use an ';' if you are going
> > to use 'else'.
>
> You did realize we've changed things from macros to inline functions
> (and vice versa) on occasion?
>
> Yes, you *can* hack around the "problem".  Is there any actual
> evidence that any real performance issues arise from the null
> do/while?  Does said issue outweigh the increased fragility of
> the code?
>

Well the 'problem' is a demonstration. The last time I answered
a query about the "do {} while(0)", stuff (as an advocate), there
were tons of 'experts' that jumped on me for helping to propagate
this "atrocious" (as I recall) abuse of the 'C' language. I
was severely tongue-lashed into believing that there was no
actual reason for doing it that way.

Now, after I have been properly "trained" that "do {} while (0)" is
an abuse, we get the other 'experts' who say I'm wrong again!
Can't win.

In spite of the fact that the gcc compiler I'm using doesn't
care, and generates the same code either way, there are others
in the world who have looked at Linux code, in particular
the headers, and turned various shades of grey just before
running off to the head. I have spent a bunch of time looking
at C/C++ headers for Sun and W$ and the only place I've
ever seen the "do {} while(0)" stuff is in Linux. I think
it started with Linux (was a Linux Invention!), as some
kind of work-around, then it became a "Linux Signature".

Cheers,
Dick Johnson
Penguin : Linux version 2.4.24 on an i986 machine (797.90 BogoMips).
            Note 96.31% of all statistics are fiction.

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Matti Aarnio]

On Fri, Feb 06, 2004 at 01:50:42PM -0500, Richard B. Johnson wrote:
....
> In spite of the fact that the gcc compiler I'm using doesn't
> care, and generates the same code either way, there are others
> in the world who have looked at Linux code, in particular
> the headers, and turned various shades of grey just before
> running off to the head. I have spent a bunch of time looking
> at C/C++ headers for Sun and W$ and the only place I've
> ever seen the "do {} while(0)" stuff is in Linux. I think
> it started with Linux (was a Linux Invention!), as some
> kind of work-around, then it became a "Linux Signature".

It is actually  "GCC Signature"  from way back when of 1.x versions
of gcc.  It is also exemplified by rather decent explanations, of why
  #define max(a,b) ((a > b) ? a : b)  
is bad, and should be done with way more complex thing involving
complex contortions of  do ... while(0) ...

> Cheers,
> Dick Johnson
> Penguin : Linux version 2.4.24 on an i986 machine (797.90 BogoMips).
>             Note 96.31% of all statistics are fiction.

/Matti Aarnio

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Petr Vandrovec]

On Fri, Feb 06, 2004 at 01:50:42PM -0500, Richard B. Johnson wrote:
> On Fri, 6 Feb 2004 Valdis.Kletnieks@vt.edu wrote:
> 
> > On Fri, 06 Feb 2004 13:00:38 EST, "Richard B. Johnson" said:
> >
> > > Yes you can. You just don't use an ';' if you are going
> > > to use 'else'.
> >
> > You did realize we've changed things from macros to inline functions
> > (and vice versa) on occasion?
> >
> > Yes, you *can* hack around the "problem".  Is there any actual
> > evidence that any real performance issues arise from the null
> > do/while?  Does said issue outweigh the increased fragility of
> > the code?
> >
> 
> Well the 'problem' is a demonstration. The last time I answered
> a query about the "do {} while(0)", stuff (as an advocate), there
> were tons of 'experts' that jumped on me for helping to propagate
> this "atrocious" (as I recall) abuse of the 'C' language. I
> was severely tongue-lashed into believing that there was no
> actual reason for doing it that way.

> running off to the head. I have spent a bunch of time looking
> at C/C++ headers for Sun and W$ and the only place I've
> ever seen the "do {} while(0)" stuff is in Linux. I think
> it started with Linux (was a Linux Invention!), as some
> kind of work-around, then it became a "Linux Signature".

Yes, because Linux programmers are only one who care about quality. 
Microsoft happilly offers (W2k DDK,inc/ddk/ntddk.h)

/* PLIST_ENTRY RemoveHeadList(PLIST_ENTRY ListHead) */

#define RemoveHeadList(ListHead) \
	(ListHead)->Flink;\
	{RemoveEntryList((ListHead)->Flink)}

and they do not care that you cannot use it in an expression, or
after if () statement, or anywhere else, except directly in an
assignment which is not in if/while body. So you must know that
RemoveHeadList() is macro, even that it is macro built from two
statements, and that you cannot use it as a function at all, as
it has a value only from left side - from right side it is
void :-( And of course it evaluates ListHead two times.

Please do not add such traps into Linux kernel code.
					Petr Vandrovec
					vandrove@vc.cvut.cz

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Jamie Lokier]

Petr Vandrovec wrote:
> Yes, because Linux programmers are only one who care about quality. 
> Microsoft happilly offers (W2k DDK,inc/ddk/ntddk.h)
> 
> /* PLIST_ENTRY RemoveHeadList(PLIST_ENTRY ListHead) */
> 
> #define RemoveHeadList(ListHead) \
> 	(ListHead)->Flink;\
> 	{RemoveEntryList((ListHead)->Flink)}
> 
> and they do not care that you cannot use it in an expression, or
> after if () statement, or anywhere else, except directly in an
> assignment which is not in if/while body. So you must know that
> RemoveHeadList() is macro, even that it is macro built from two
> statements, and that you cannot use it as a function at all, as
> it has a value only from left side - from right side it is
> void :-( And of course it evaluates ListHead two times.


Oh, it is much worse than that.

You _can_ use it after an if() statement.  Your program compiles just
fine.  The only problem is it has a serious bug which is invisible and
may not be noticed for years.

I have seen code shipped with this exact bug, and watched someone
spend days debugging a program that contained it, until they brought
the problem to me.  Then we found it quickly - because I already
know why that macro kind is bad, thanks to the GNU CPP manual.

In short, any programmer managed by me who wrote a macro like that
would be educated why it is not acceptable.  If they still wrote code
like that afterwards, they wouldn't remain with me for long.

-- Jamie

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Måns Rullgård]

Valdis.Kletnieks@vt.edu writes:

> On Fri, 06 Feb 2004 13:00:38 EST, "Richard B. Johnson" said:
>
>> Yes you can. You just don't use an ';' if you are going
>> to use 'else'.
>
> You did realize we've changed things from macros to inline functions
> (and vice versa) on occasion?
>
> Yes, you *can* hack around the "problem".  Is there any actual
> evidence that any real performance issues arise from the null
> do/while?  Does said issue outweigh the increased fragility of
> the code?

I suspect that the compiler authors are well aware of this common
idiom and take care to do whatever is appropriate.

-- 
Måns Rullgård
mru@kth.se

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[somenath]

That's one of the many reasons of using do-while macros..
Other than compilation error, it may generate wrong code too!
this is a FAQ in kernel newbies , look at:
http://www.kernelnewbies.org/faq/index.php3#dowhile

thanks, som.

Roland Dreier wrote:

>    Richard> If some major changes are being considered, I think it's
>    Richard> time to get rid of the:
>
>    Richard> do { } while(0) stuff that permiates a lot of MACROS and
>    Richard> just use the { } as they were designed.
>
>    Richard> Before everybody screams, think. It's perfectly correct
>    Richard> to start a new "program unit" without a conditional
>    Richard> expression.  You just add a curley-brace, then close the
>    Richard> brace when you are though.
>
>This is totally, totally wrong.  If you get rid of do { } while (0),
>then you can't use the macro in an if statement.  Read any C FAQ for
>details, or try the following:
>
>    #define MAC(x) { x = x + 1; }
>
>    int main() {
>      int x = 0;
>
>      if (1)
>        MAC(x);
>      else
>        x = x - 1;
>    }
>
>I get the following (correct) error:
>
>    $ gcc a.c
>    a.c: In function `main':
>    a.c:8: syntax error before "else"
>    $ gcc --version
>    gcc (GCC) 3.2.3 20030502 (Red Hat Linux 3.2.3-20)
>
>because
>
>    if (1)
>        { x = x + 1 } ; /* <-- note semicolon
>    else
>        x = x - 1;
>
>is not correct C.
>
>By the way, it is possible to use parentheses and commas for some
>simple macros, so for example the following is OK:
>
>    #define MAC(x) ( x = x + 1, x = x * 2 )
>
>    int main() {
>      int x = 0;
>
>      if (1)
>        MAC(x);
>      else
>        x = x - 1;
>    }
>
>However I don't see anything wrong with the perfectly standard "do { }
>while (0)" idiom.  Certainly if some compiler generates worse code for
>that construct that just a plain { }, _that_ is a compiler bug that we
>shouldn't have to work around.
>
> - Roland
>
>
>-------------------------------------------------------
>The SF.Net email is sponsored by EclipseCon 2004
>Premiere Conference on Open Tools Development and Integration
>See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
>http://www.eclipsecon.org/osdn
>_______________________________________________
>Infiniband-general mailing list
>Infiniband-general@lists.sourceforge.net
>https://lists.sourceforge.net/lists/listinfo/infiniband-general
>  
>

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Troy Benjegerdes]

On Fri, Feb 06, 2004 at 08:42:14AM -0800, Hefty, Sean wrote:
> On Thu, Feb 5, 2004 at 05:11:00PM -0800, Troy Benjegerdes wrote:
> >On Thu, Feb 05, 2004 at 02:26:46PM -0800, Hefty, Sean wrote:
> >> Personally, I'm amazed that professional developers have to discuss
> or
> >> defend modular, portable code.
> >
> >You're new to linux-kernel, aren't you? ;)
> 
> I was not trying to be condescending.  My point was that I think that
> everyone on this list knows the purpose and benefits behind an
> abstraction layer.  It's not something that needed to be discussed any
> further.

I attempting to make a (poor) joke on the contiual flamewars that happen
on open-source development projects. 

I think someone's said something like this before, but..

"Nothing useful ever gets done on linux-kernel without a good flamewar"

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Greg KH]

On Fri, Feb 06, 2004 at 08:42:14AM -0800, Hefty, Sean wrote:
> 
> We want to continue to discuss specific details about what's needed to
> add the code into the kernel.  Here's a list of modifications that I
> think are needed so far:
> 
> * Update the code to make direct calls for atomic operations.
> * Verify the use of spinlock calls.

Does this include "remove shim around spinlock calls"?  You should just
call the kernel functions and not try to wrap them.

> * Reformat the code for tab spacing and curly brace usage.
> * Elimination of typedefs.

All of these are great things to start with.

thanks,

greg k-h

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Fab Tillier]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Friday, February 06, 2004 10:52 AM
>
> On Fri, Feb 06, 2004 at 08:42:14AM -0800, Hefty, Sean wrote:
> >
> > We want to continue to discuss specific details about what's needed to
> > add the code into the kernel.  Here's a list of modifications that I
> > think are needed so far:
> >
> > * Verify the use of spinlock calls.
> 
> Does this include "remove shim around spinlock calls"?  You should just
> call the kernel functions and not try to wrap them.

The spin lock abstraction was changed a while ago to use the 'spin_lock_bh'
functions rather than the 'spin_lock_irqsave' functions.  Is there still an
issue with using 'spin_lock_bh' that needs to be addressed - is the
abstraction still functionally broken?

There are two things that are accomplished with the spin lock abstraction.
The first is that it resolves to different things between kernel mode and
user mode, allowing code to be shared between both.  The second is debug
hooks that allow tracking who is holding the lock, etc, to aid in debugging.
I'm not a big fan of these debug hooks, so I think the whole spin lock
object could easily (and should) be changed to be a #define to the kernel
structure and functions.  I think there is value in allowing the code to be
shared between kernel mode and user mode.  Would using a macro that resolve
to the native kernel spin lock structure and functions be acceptable?

Thanks for your input,

- Fab

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Greg KH]

On Sun, Feb 08, 2004 at 12:31:56AM -0800, Fab Tillier wrote:
> 
> I think there is value in allowing the code to be shared between
> kernel mode and user mode.  Would using a macro that resolve to the
> native kernel spin lock structure and functions be acceptable?

Probably not, just use the in-kernel call, and be done with it.  If you
_really_ want to share code between userspace and the kernel, keep a
different version of it somewhere else.

Why do you want to run your code in both places?  Does this mean that it
doesn't even really need to be in the kernel as it works just fine in
userspace?

thanks,

greg k-h

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Fab Tillier]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Sunday, February 08, 2004 8:30 AM
> 
> On Sun, Feb 08, 2004 at 12:31:56AM -0800, Fab Tillier wrote:
> >
> > I think there is value in allowing the code to be shared between
> > kernel mode and user mode.  Would using a macro that resolve to the
> > native kernel spin lock structure and functions be acceptable?
> 
> Probably not, just use the in-kernel call, and be done with it.  If you
> _really_ want to share code between userspace and the kernel, keep a
> different version of it somewhere else.

Are you suggesting branching the user mode code from the kernel mode code?
Duplication is not the same as sharing code - you have twice the number of
places that require fixing in the event of a bug.  If we can help it, we'd
like to avoid this.

> 
> Why do you want to run your code in both places?  Does this mean that it
> doesn't even really need to be in the kernel as it works just fine in
> userspace?

IBAL provides APIs for accessing InfiniBand resources in both the kernel and
in user-mode.  Those APIs are identical, and many of the files are shared
between user and kernel mode.  In the kernel, storage drivers, network
drivers, and SDP interface to the kernel IBAL driver.  In user mode, the
subnet manager, uDAPL, MPI, and other IB-enabled applications interface to
the user-mode IBAL library.  The reason a user-mode library is provided is
that InfiniBand provides mechanisms to bypass the kernel for speed path
operations - send, receive, and completion processing - similarly to VI.
Bypassing the kernel for these operations helps provide higher bandwidths
and lower latencies to applications.

The user-mode IBAL library depends on the kernel mode IBAL driver to setup
InfiniBand resources for use, and for the operations that can't be
offloaded, like memory registrations.  Further, the kernel IBAL driver
tracks user-mode resources on a per-process basis to ensure they are freed
if an application seg faults.  This is a must so that things like memory
registrations get cleaned up and the associated memory unlocked even in
abnormal application termination situations.

I hope this clears up why code is shared between user-mode and kernel mode.

- Fab

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Troy Benjegerdes]

> > 
> > Why do you want to run your code in both places?  Does this mean that it
> > doesn't even really need to be in the kernel as it works just fine in
> > userspace?

> The user-mode IBAL library depends on the kernel mode IBAL driver to setup
> InfiniBand resources for use, and for the operations that can't be
> offloaded, like memory registrations.  Further, the kernel IBAL driver
> tracks user-mode resources on a per-process basis to ensure they are freed
> if an application seg faults.  This is a must so that things like memory
> registrations get cleaned up and the associated memory unlocked even in
> abnormal application termination situations.

We really *do* want to share code. We want both the user and kernel
code to use the same code paths for direct access to infiniband storage
devices, for example.

What I think is really needed is a mechanism for user level code to
call code provided by the loaded kernel module. I suspect this could be
done with some proper memory mappings and small user-level shim layers,
but I know there is something I'm forgetting.


-- 
--------------------------------------------------------------------------
Troy Benjegerdes                'da hozer'                hozer@hozed.org  

Somone asked my why I work on this free (http://www.fsf.org/philosophy/)
software stuff and not get a real job. Charles Shultz had the best answer:

"Why do musicians compose symphonies and poets write poems? They do it
because life wouldn't have any meaning for them if they didn't. That's why
I draw cartoons. It's my life." -- Charles Shultz

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Greg KH]

On Sun, Feb 08, 2004 at 08:51:22AM -0800, Fab Tillier wrote:
> > On Sun, Feb 08, 2004 at 12:31:56AM -0800, Fab Tillier wrote:
> > >
> > > I think there is value in allowing the code to be shared between
> > > kernel mode and user mode.  Would using a macro that resolve to the
> > > native kernel spin lock structure and functions be acceptable?
> > 
> > Probably not, just use the in-kernel call, and be done with it.  If you
> > _really_ want to share code between userspace and the kernel, keep a
> > different version of it somewhere else.
> 
> Are you suggesting branching the user mode code from the kernel mode code?
> Duplication is not the same as sharing code - you have twice the number of
> places that require fixing in the event of a bug.  If we can help it, we'd
> like to avoid this.

Do you honestly think that if your code ever makes it into the main
kernel tree, you would be able somehow to extact that and use it from
userspace properly?

No, just don't do this.

Remember, if you _can_ do this from userspace, then your code doesn't
need to be in the kernel at all :)

Oh, and I repeat, let's see some code.  No more bickering about "what
ifs" anymore.

greg k-h

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Perez-Gonzalez, Inaky]

> From: Tillier, Fabian
> ...
>
> Having atomic operations return a value allows one to do something like
> test for zero when decrementing an atomic variable such as a reference
> count, to determine whether final cleanup should proceed.  This removes
> the need for an actual spinlock protecting the reference count.  As you
> know, reading the value post-decrement does not guarantee that said
> value reflects the result of only that decrement operation.  It would be
> catastrophic if two threads checked the value of a reference count
> without proper synchronization - they could both end up running the
> cleanup code with undesired (and perhaps catastrophic) results.

atomic_dec_and_test() does just that (asm/atomic.h).

Iñaky Pérez-González -- Not speaking for Intel -- all opinions are my own (and my fault)

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Woodruff, Robert J]

>Not Found
>The requested URL /iba was not found on this server.

oops, sorry. The web site is, 

http://infiniband.bkbits.net

to clone the data base I think it is, 

bk clone http://infiniband.bkbits.net/iba

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Woodruff, Robert J]

I think what started the discussion was that
if anyone wanted to look at the code and start to comment 
before we have a 2.6 patch ready they can download it from bitkeeper at

http://infiniband.bkbits.net/iba

or if you want, I could post a tar ball of the latest BK change set on
sourceforge,
or you can wait till we make all the changes to the makefiles, etc, to
allow it to 
easily integrate into the 2.6 build environment.

Any preference ?

-----Original Message-----
From: Randy.Dunlap [mailto:rddunlap@osdl.org] 
Sent: Thursday, February 05, 2004 2:40 PM
To: Christoph Hellwig
Cc: Hefty, Sean; ftillier@infiniconsys.com; cfriesen@nortelnetworks.com;
greg@kroah.com; linux-kernel@vger.kernel.org; hozer@hozed.org;
woody@jf.intel.com; Magro, Bill; woody@jf.intel.com;
infiniband-general@lists.sourceforge.net
Subject: Re: [Infiniband-general] Getting an Infiniband access layer in
theLinux kernel


On Thu, 5 Feb 2004 22:40:43 +0000 Christoph Hellwig <hch@infradead.org>
wrote:

| On Thu, Feb 05, 2004 at 02:26:46PM -0800, Hefty, Sean wrote:
| > Personally, I'm amazed that professional developers have to discuss 
| > or defend modular, portable code.
| > 
| > Once the code has been submitted, then specific implementation 
| > problems can be dealt with.
| 
| *plonk*


Christoph, he didn't say merged.  Let them submit it for review... and
then comment on it.

--
~Randy
kernel-janitors project:  http://janitor.kernelnewbies.org/

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Randy.Dunlap]

On Thu, 5 Feb 2004 14:55:54 -0800 "Woodruff, Robert J" <woody@co.intel.com> wrote:

| I think what started the discussion was that
| if anyone wanted to look at the code and start to comment 
| before we have a 2.6 patch ready they can download it from bitkeeper at
| 
| http://infiniband.bkbits.net/iba

Not Found
The requested URL /iba was not found on this server.


| or if you want, I could post a tar ball of the latest BK change set on
| sourceforge,
| or you can wait till we make all the changes to the makefiles, etc, to
| allow it to 
| easily integrate into the 2.6 build environment.
| 
| Any preference ?

yes, tarball for me....

Thanks,   [and please don't top-post]
---
~Randy


| -----Original Message-----
| From: Randy.Dunlap [mailto:rddunlap@osdl.org] 
| Sent: Thursday, February 05, 2004 2:40 PM
| To: Christoph Hellwig
| Cc: Hefty, Sean; ftillier@infiniconsys.com; cfriesen@nortelnetworks.com;
| greg@kroah.com; linux-kernel@vger.kernel.org; hozer@hozed.org;
| woody@jf.intel.com; Magro, Bill; woody@jf.intel.com;
| infiniband-general@lists.sourceforge.net
| Subject: Re: [Infiniband-general] Getting an Infiniband access layer in
| theLinux kernel
| 
| 
| On Thu, 5 Feb 2004 22:40:43 +0000 Christoph Hellwig <hch@infradead.org>
| wrote:
| 
| | On Thu, Feb 05, 2004 at 02:26:46PM -0800, Hefty, Sean wrote:
| | > Personally, I'm amazed that professional developers have to discuss 
| | > or defend modular, portable code.
| | > 
| | > Once the code has been submitted, then specific implementation 
| | > problems can be dealt with.
| | 
| | *plonk*
| 
| 
| Christoph, he didn't say merged.  Let them submit it for review... and
| then comment on it.
| 
| --

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Hefty, Sean]

Personally, I'm amazed that professional developers have to discuss or
defend modular, portable code.

Once the code has been submitted, then specific implementation problems
can be dealt with.

- Sean


-----Original Message-----
From: Tillier, Fabian [mailto:ftillier@infiniconsys.com] 
Sent: Thursday, February 05, 2004 2:03 PM
To: Chris Friesen; Greg KH
Cc: Randy.Dunlap; Hefty, Sean; linux-kernel@vger.kernel.org;
hozer@hozed.org; Woodruff, Robert J; Magro, Bill; woody@jf.intel.com;
infiniband-general@lists.sourceforge.net
Subject: RE: [Infiniband-general] Getting an Infiniband access layer in
theLinux kernel

That is absolutely correct.  In addition to portability between kernel
versions and operating systems, there is also portability between
user-mode and kernel-mode within a single release.

- Fab

-----Original Message-----
From: Chris Friesen [mailto:cfriesen@nortelnetworks.com] 
Sent: Thursday, February 05, 2004 1:57 PM
To: Greg KH
Cc: Tillier, Fabian; Randy.Dunlap; sean.hefty@intel.com;
linux-kernel@vger.kernel.org; hozer@hozed.org; woody@co.intel.com;
bill.magro@intel.com; woody@jf.intel.com;
infiniband-general@lists.sourceforge.net
Subject: Re: [Infiniband-general] Getting an Infiniband access layer in
theLinux kernel

Greg KH wrote:

> Basically, what is lacking in the current kernel locks that the
> infiniband project has to have in order to work properly.  We can work
> from there.

I think their point is that they want the core device driver code to be 
portable across kernel versions, and across different OS's other than 
linux--which basically requires some kind of abstraction layer.

Chris

-- 
Chris Friesen                    | MailStop: 043/33/F10
Nortel Networks                  | work: (613) 765-0557
3500 Carling Avenue              | fax:  (613) 765-2986
Nepean, ON K2H 8E9 Canada        | email: cfriesen@nortelnetworks.com

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Christoph Hellwig]

On Thu, Feb 05, 2004 at 02:26:46PM -0800, Hefty, Sean wrote:
> Personally, I'm amazed that professional developers have to discuss or
> defend modular, portable code.
> 
> Once the code has been submitted, then specific implementation problems
> can be dealt with.

*plonk*

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Randy.Dunlap]

On Thu, 5 Feb 2004 22:40:43 +0000 Christoph Hellwig <hch@infradead.org> wrote:

| On Thu, Feb 05, 2004 at 02:26:46PM -0800, Hefty, Sean wrote:
| > Personally, I'm amazed that professional developers have to discuss or
| > defend modular, portable code.
| > 
| > Once the code has been submitted, then specific implementation problems
| > can be dealt with.
| 
| *plonk*


Christoph, he didn't say merged.  Let them submit it for review...
and then comment on it.

--
~Randy
kernel-janitors project:  http://janitor.kernelnewbies.org/

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Greg KH]

On Thu, Feb 05, 2004 at 02:26:46PM -0800, Hefty, Sean wrote:
> Personally, I'm amazed that professional developers have to discuss or
> defend modular, portable code.

And I can't believe that I have to defend the current Linux kernel
implementation of its modular and portable code to more platforms than
you can shake a stick at.

Who wants to work with who here?

> Once the code has been submitted, then specific implementation problems
> can be dealt with.

Great, when is this going to happen?  I think I'm not going to respond
anymore to this thread until I see some actual code.

That's what I get for trying to point out problems before they happen...

greg k-h

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Troy Benjegerdes]

On Thu, Feb 05, 2004 at 02:26:46PM -0800, Hefty, Sean wrote:
> Personally, I'm amazed that professional developers have to discuss or
> defend modular, portable code.

You're new to linux-kernel, aren't you? ;)

> 
> Once the code has been submitted, then specific implementation problems
> can be dealt with.
> 
> - Sean


--------------------------------------------------------------------------
Troy Benjegerdes                'da hozer'                hozer@hozed.org  

Somone asked my why I work on this free (http://www.fsf.org/philosophy/)
software stuff and not get a real job. Charles Shultz had the best answer:

"Why do musicians compose symphonies and poets write poems? They do it
because life wouldn't have any meaning for them if they didn't. That's why
I draw cartoons. It's my life." -- Charles Shultz

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Tillier, Fabian]

Greg,

I'm not arguing about the spinlocks here, and never have.  I'm arguing
about the atomic abstraction for the x86 platforms.  My last question
was not a yes/no question so I'm not sure what you're answering with
your "No" - your reply makes no sense.  To clarify, the answer to a
"chose one of two things" question is not "No".  Basic XOR logic is all
that's needed here.  I'm not sure what you're asking about with the
whole quotations thing.

Having atomic operations return a value allows one to do something like
test for zero when decrementing an atomic variable such as a reference
count, to determine whether final cleanup should proceed.  This removes
the need for an actual spinlock protecting the reference count.  As you
know, reading the value post-decrement does not guarantee that said
value reflects the result of only that decrement operation.  It would be
catastrophic if two threads checked the value of a reference count
without proper synchronization - they could both end up running the
cleanup code with undesired (and perhaps catastrophic) results.

I'll try a simple example for you assuming atomic_dec returns the
decremented value:

if( atomic_dec( x ) == 0 )
{
    cleanup();
}

In the current implementation of atomic operations for x86 however,
atomic_dec doesn't return anything.  To get the proper behavior would
require a true spinlock because the following code sample would not work
properly since there's no atomicity guaranteed between the decrement and
the read:

atomic_dec( x )
if( atomic_read( x ) == 0 )
{
    cleanup();
}

So without returning the result of the decrement, you lose the ability
to use atomic variables for reference counting.

Hope this clarifies things for you,

- Fab
 

-----Original Message-----
From: Greg KH [mailto:greg@kroah.com] 
Sent: Thursday, February 05, 2004 1:27 PM
To: Tillier, Fabian
Cc: Randy.Dunlap; sean.hefty@intel.com; linux-kernel@vger.kernel.org;
hozer@hozed.org; woody@co.intel.com; bill.magro@intel.com;
woody@jf.intel.com; infiniband-general@lists.sourceforge.net
Subject: Re: [Infiniband-general] Getting an Infiniband access layer in
theLinux kernel

A: No.
Q: Should I include quotations after my reply?

On Thu, Feb 05, 2004 at 03:32:09PM -0500, Tillier, Fabian wrote:
> So which is more important to the "Linux kernel" project: i386
backwards
> compatibility, or consistent API and functionality across processor
> architectures? ;)

Anyway, why not describe what you are trying to accomplish that made you
determine that you _had_ to have these kinds of functions.

Basically, what is lacking in the current kernel locks that the
infiniband project has to have in order to work properly.  We can work
from there.

thanks,

greg k-h

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Brian Gerst]

Tillier, Fabian wrote:
> Greg,
> 
> I'm not arguing about the spinlocks here, and never have.  I'm arguing
> about the atomic abstraction for the x86 platforms.  My last question
> was not a yes/no question so I'm not sure what you're answering with
> your "No" - your reply makes no sense.  To clarify, the answer to a
> "chose one of two things" question is not "No".  Basic XOR logic is all
> that's needed here.  I'm not sure what you're asking about with the
> whole quotations thing.
> 
> Having atomic operations return a value allows one to do something like
> test for zero when decrementing an atomic variable such as a reference
> count, to determine whether final cleanup should proceed.  This removes
> the need for an actual spinlock protecting the reference count.  As you
> know, reading the value post-decrement does not guarantee that said
> value reflects the result of only that decrement operation.  It would be
> catastrophic if two threads checked the value of a reference count
> without proper synchronization - they could both end up running the
> cleanup code with undesired (and perhaps catastrophic) results.
> 
> I'll try a simple example for you assuming atomic_dec returns the
> decremented value:
> 
> if( atomic_dec( x ) == 0 )
> {
>     cleanup();
> }

I guess you missed this then:
/**
  * atomic_dec_and_test - decrement and test
  * @v: pointer of type atomic_t
  *
  * Atomically decrements @v by 1 and
  * returns true if the result is 0, or false for all other
  * cases.  Note that the guaranteed
  * useful range of an atomic_t is only 24 bits.
  */

There is also atomic_dec_and_lock():
/*
  * This is an architecture-neutral, but slow,
  * implementation of the notion of "decrement
  * a reference count, and return locked if it
  * decremented to zero".
  *
  * NOTE NOTE NOTE! This is _not_ equivalent to
  *
  *      if (atomic_dec_and_test(&atomic)) {
  *              spin_lock(&lock);
  *              return 1;
  *      }
  *      return 0;
  *
  * because the spin-lock and the decrement must be
  * "atomic".
  *
  * This slow version gets the spinlock unconditionally,
  * and releases it if it isn't needed. Architectures
  * are encouraged to come up with better approaches,
  * this is trivially done efficiently using a load-locked
  * store-conditional approach, for example.
  */

--
				Brian Gerst

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Bernd Petrovitsch]

On Thu, 2004-02-05 at 23:17, Tillier, Fabian wrote:
[...]
> about the atomic abstraction for the x86 platforms.  My last question
> was not a yes/no question so I'm not sure what you're answering with
> your "No" - your reply makes no sense.  To clarify, the answer to a

You didn't get the point that your quoting style (Text above - full
quote below) is - ahemm - deprecated und really unreadable and the "No"
is not related to the to-include-or-not-include-the-driver question.

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Tillier, Fabian]

That is absolutely correct.  In addition to portability between kernel
versions and operating systems, there is also portability between
user-mode and kernel-mode within a single release.

- Fab

-----Original Message-----
From: Chris Friesen [mailto:cfriesen@nortelnetworks.com] 
Sent: Thursday, February 05, 2004 1:57 PM
To: Greg KH
Cc: Tillier, Fabian; Randy.Dunlap; sean.hefty@intel.com;
linux-kernel@vger.kernel.org; hozer@hozed.org; woody@co.intel.com;
bill.magro@intel.com; woody@jf.intel.com;
infiniband-general@lists.sourceforge.net
Subject: Re: [Infiniband-general] Getting an Infiniband access layer in
theLinux kernel

Greg KH wrote:

> Basically, what is lacking in the current kernel locks that the
> infiniband project has to have in order to work properly.  We can work
> from there.

I think their point is that they want the core device driver code to be 
portable across kernel versions, and across different OS's other than 
linux--which basically requires some kind of abstraction layer.

Chris

-- 
Chris Friesen                    | MailStop: 043/33/F10
Nortel Networks                  | work: (613) 765-0557
3500 Carling Avenue              | fax:  (613) 765-2986
Nepean, ON K2H 8E9 Canada        | email: cfriesen@nortelnetworks.com

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Troy Benjegerdes]

On Thu, Feb 05, 2004 at 05:02:30PM -0500, Tillier, Fabian wrote:
> That is absolutely correct.  In addition to portability between kernel
> versions and operating systems, there is also portability between
> user-mode and kernel-mode within a single release.


This is another issue... I think we should have a mechanism to allow
user code to call functions in user mode that are exported by the
kernel. Like the gettimeofday() syscall bypass stuff which I have heard
about but can't remember the details of.

The linux kernel obviously knows exactly what kind of processor it is
running on, and should be able to know exactly what the best memcpy()
function is. This should be exported to userspace instead of depending
on glibc to ask the kernel what cpu it has, and hope glibc has
reasonably optimized code. 

The same interface would quite easily solve the infiniband atomic
locking issues.

Anyone in the infiniband industry please take note, we probably should 
have had this discussion a year ago, and remember this next time.

RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Tillier, Fabian]

Randy,

All issues are in include/asm-x86/atomic.h

Comments as to the useful range of an atomic_t for x86 states 24-bits
atomic_add is a void.  It should return the incremented value.
atomic_sub is a void.  It should return the decremented value.
atomic_inc is a void.  It should return the incremented value.
atomic_dec is a void.  It should return the decremented value.

There is no atomic exchange.  include/asm-x86/system.h does provide a
compare exchange, but it takes an extra size parameter.

Do note that for non x86 architectures, the component library atomic
abstraction is all #define to the Linux provided functions.  Only x86
needed help because of i386 backwards compatibility which is not a goal
of the InfiniBand project.

Hope that helps.

- Fab

-----Original Message-----
From: Randy.Dunlap [mailto:rddunlap@osdl.org] 
Sent: Thursday, February 05, 2004 10:53 AM
To: Tillier, Fabian
Cc: greg@kroah.com; sean.hefty@intel.com; linux-kernel@vger.kernel.org;
hozer@hozed.org; woody@co.intel.com; bill.magro@intel.com;
woody@jf.intel.com; infiniband-general@lists.sourceforge.net
Subject: Re: [Infiniband-general] Getting an Infiniband access layer in
theLinux kernel

On Thu, 5 Feb 2004 13:31:45 -0500 "Tillier, Fabian"
<ftillier@infiniconsys.com> wrote:

| Greg,
| 
| The atomic operation abstraction is there because atomic support has
| different prototypes between x86 and IA64 (at least it did when it was
| written), with some of the x86 functions not returning values while
the
| IA64 ones did.  Further, comments in the x86 code base indicated that
| only 24-bits are actually valid (probably from some i386 limitation
that
| is no longer relevant).  Thus, the implementation of atomic operations
| for the x86 processor architecture fails a build (via #error) if
you're
| targeting an i386 processor, and provides the same semantics
independent
| of processor architecture.
| 
| To answer your broader question, the reason to have abstraction is to
| facilitate portability.  I'm not just talking about portability
between
| different operating systems, but even portability between different
| versions of Linux distributions and kernels.  Differences between
Linux
| distributions and kernel versions can be handled in a single place,
| avoiding the need to pepper the rest of the code base with #ifdefs.
| This results in more readable and maintainable code for the rest of
the
| project by concentrating platform specific issues to the abstraction
| layer.

Besides Greg's comments:

Can (will) you provide specific examples of these differences/problems?
They need to be quashed.

Thanks.

| Are you suggesting that if there is any abstraction, the code will
never
| be accepted?  Or rather that the abstraction better be correct?  I'm
| hoping for the latter, however please clarify.
| 
| - Fab
| 
| -----Original Message-----
| From: Greg KH [mailto:greg@kroah.com] 
| Sent: Thursday, February 05, 2004 10:10 AM
| To: Hefty, Sean; linux-kernel@vger.kernel.org
| Cc: Troy Benjegerdes; Woodruff, Robert J; Magro, Bill; Woodruff,
Robert
| J; infiniband-general@lists.sourceforge.net
| Subject: Re: [Infiniband-general] Getting an Infiniband access layer
in
| the linux kernel
| 
| On Thu, Feb 05, 2004 at 10:01:13AM -0800, Hefty, Sean wrote:
| > As an FYI, the code is available for download on bitkeeper at
| > http://infiniband.bkbits.net/iba.  We're still working on providing
a
| > tarball and patch for 2.6, but if you would like to see the code
now,
| it
| > is available.
| 
| Oh, I've seen that code, and still feel ill after looking at some of
| it...
| 
| Come on, implementing your own spinlocks (and getting it wrong) and
| atomit_t?  Why in the world would you _ever_ want to do that.
| 
| That code needs a _lot_ of cleanup to make it into the kernel tree.
| 
| Good luck,
| 
| greg k-h


--
~Randy
kernel-janitors project:  http://janitor.kernelnewbies.org/

Re: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel

[Greg KH]

On Thu, Feb 05, 2004 at 02:26:19PM -0500, Tillier, Fabian wrote:
> Do note that for non x86 architectures, the component library atomic
> abstraction is all #define to the Linux provided functions.  Only x86
> needed help because of i386 backwards compatibility which is not a goal
> of the InfiniBand project.

But that is a goal of the "Linux kernel" project :)

greg k-h