[PATCH][2.5] introduce list_move macros

[PATCH][2.5] introduce list_move macros

[Lightweight patch manager]

This is the only _global_ patch about the list_move macros, which means 
introducing them. Here they are:

--- linus-2.5/include/linux/list.h	Sun Jun  9 04:17:14 2002
+++ thunder-2.5/include/linux/list.h	Sun Jun  9 05:07:02 2002
@@ -174,6 +174,24 @@
 	for (pos = (head)->next, n = pos->next; pos != (head); \
 		pos = n, n = pos->next)
 
+/**
+ * list_move           - move a list entry from a right after b
+ * @list       the entry to move
+ * @head       the entry to move after
+ */
+#define list_move(list,head) \
+        list_del(list); \
+        list_add(list,head)
+
+/**
+ * list_move_tail      - move a list entry from a right before b
+ * @list       the entry to move
+ * @head       the entry that will come after ours
+ */
+#define list_move(list,head) \
+        list_del(list); \
+        list_add_tail(list,head)
+
 #endif /* __KERNEL__ || _LVM_H_INCLUDE */
 
 #endif

-- 
Lightweight patch manager using pine. If you have any objections, tell me.

Re: [PATCH][2.5] introduce list_move macros

[Mark Zealey]

On Sun, Jun 09, 2002 at 05:09:54AM -0600, Lightweight patch manager wrote:

> +#define list_move(list,head) \
> +        list_del(list); \
> +        list_add(list,head)
> +
> +/**
> + * list_move_tail      - move a list entry from a right before b

> +#define list_move(list,head) \

I guess that should be:
+#define list_move_tail(list,head) \

You should enclose the ops in a do { ... } while(0) block too.

-- 

Mark Zealey (aka JALH on irc.openprojects.net: #zealos and many more)
mark@zealos.org; mark@itsolve.co.uk

UL++++>$ G!>(GCM/GCS/GS/GM) dpu? s:-@ a17! C++++>$ P++++>+++++$ L+++>+++++$
!E---? W+++>$ !w--- r++ !t---?@ !X---?  !R- !tv b+ G+++ e>+++++ !h++* r!-- y--

Re: [PATCH][2.5] introduce list_move macros

[Thunder from the hill]

Hi,

On Sun, 9 Jun 2002, Mark Zealey wrote:
> I guess that should be:
> +#define list_move_tail(list,head) \
> 
> You should enclose the ops in a do { ... } while(0) block too.

There was a "revisited" version just a short time later. Please complain 
about that one ;-)

Regards,
Thunder
-- 
German attitude becoming        |	Thunder from the hill at ngforever
rightaway popular:		|
       "Get outa my way,  	|	free inhabitant not directly
    for I got a mobile phone!"	|	belonging anywhere

Re: [PATCH][2.5] introduce list_move macros

[OGAWA Hirofumi]

Lightweight patch manager <patch@luckynet.dynu.com> writes:

> This is the only _global_ patch about the list_move macros, which means 
> introducing them. Here they are:
> 
> --- linus-2.5/include/linux/list.h	Sun Jun  9 04:17:14 2002
> +++ thunder-2.5/include/linux/list.h	Sun Jun  9 05:07:02 2002
> @@ -174,6 +174,24 @@
>  	for (pos = (head)->next, n = pos->next; pos != (head); \
>  		pos = n, n = pos->next)
>  
> +/**
> + * list_move           - move a list entry from a right after b
> + * @list       the entry to move
> + * @head       the entry to move after
> + */
> +#define list_move(list,head) \
> +        list_del(list); \
> +        list_add(list,head)
> +
> +/**
> + * list_move_tail      - move a list entry from a right before b
> + * @list       the entry to move
> + * @head       the entry that will come after ours
> + */
> +#define list_move(list,head) \
                ^^^^
Probably typo.  list_move_tail.

> +        list_del(list); \
> +        list_add_tail(list,head)
> +
>  #endif /* __KERNEL__ || _LVM_H_INCLUDE */
>  
>  #endif

	if (check_something(x))
		list_move(p, head);

In the above case, these seems unsafe. So, shouldn't these use
`do {} while(0)' or `inline function'?
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH][2.5] introduce list_move macros

[Thomas 'Dent' Mirlacher]

On Sun, 9 Jun 2002, OGAWA Hirofumi wrote:

> Lightweight patch manager <patch@luckynet.dynu.com> writes:
> 
> > This is the only _global_ patch about the list_move macros, which means 
> > introducing them. Here they are:
> > 
> > --- linus-2.5/include/linux/list.h	Sun Jun  9 04:17:14 2002
> > +++ thunder-2.5/include/linux/list.h	Sun Jun  9 05:07:02 2002
> > @@ -174,6 +174,24 @@
> >  	for (pos = (head)->next, n = pos->next; pos != (head); \
> >  		pos = n, n = pos->next)
> >  
> > +/**
> > + * list_move           - move a list entry from a right after b
> > + * @list       the entry to move
> > + * @head       the entry to move after
> > + */
> > +#define list_move(list,head) \
> > +        list_del(list); \
> > +        list_add(list,head)
> > +
> > +/**
> > + * list_move_tail      - move a list entry from a right before b
> > + * @list       the entry to move
> > + * @head       the entry that will come after ours
> > + */
> > +#define list_move(list,head) \
>                 ^^^^
> Probably typo.  list_move_tail.
> 
> > +        list_del(list); \
> > +        list_add_tail(list,head)
> > +
> >  #endif /* __KERNEL__ || _LVM_H_INCLUDE */
> >  
> >  #endif
> 
> 	if (check_something(x))
> 		list_move(p, head);
> 
> In the above case, these seems unsafe. So, shouldn't these use
> `do {} while(0)' or `inline function'?

another possible solution would be:

static __inline__ void list_move(list_t *list, struct list_t *head)
{
	__list_del(list->prev, list->next);
	list_add(list, head);
}

static __inline__ void list_move_tail(list_t *list, struct list_t *head)
{
	__list_del(list->prev, list->next);
	list_add_tail(list, head);
}

this gets rid of the problem described above,
	adds inline advantages
	removes zeroing of entry->(next|prev) ... which is not needed for move

	tm
-- 
in some way i do, and in some way i don't.

Re: [PATCH][2.5] introduce list_move macros

[Russell King]

On Sun, Jun 09, 2002 at 05:09:54AM -0600, Lightweight patch manager wrote:
> +/**
> + * list_move           - move a list entry from a right after b
> + * @list       the entry to move
> + * @head       the entry to move after
> + */
> +#define list_move(list,head) \
> +        list_del(list); \
> +        list_add(list,head)

Yuck.  What if someone does:

	if (foo)
		list_move(list,head);

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

[PATCH][2.5] introduce list_move macros (revisited)

[Lightweight patch manager]

This is the only _global_ patch about the list_move macros, which means 
introducing them. Here they are, revisited:

--- linus-2.5/include/linux/list.h	Sun Jun  9 04:17:14 2002
+++ thunder-2.5/include/linux/list.h	Sun Jun  9 06:40:14 2002
@@ -174,6 +174,26 @@
 	for (pos = (head)->next, n = pos->next; pos != (head); \
 		pos = n, n = pos->next)
 
+/**
+ * list_move           - move a list entry from a right after b
+ * @list       the entry to move
+ * @head       the entry to move after
+ */
+#define list_move(list,head) \
+        do { __list_del(list->prev, list->next); \
+             list_add(list,head); } \
+        while(0)
+
+/**
+ * list_move_tail      - move a list entry from a right before b
+ * @list       the entry to move
+ * @head       the entry that will come after ours
+ */
+#define list_move_tail(list,head) \
+        do { __list_del(list->prev, list->next); \
+             list_add_tail(list,head); } \
+        while(0)
+
 #endif /* __KERNEL__ || _LVM_H_INCLUDE */
 
 #endif

-- 
Lightweight patch manager using pine. If you have any objections, tell me.

Re: [PATCH][2.5] introduce list_move macros (revisited)

[Dan Aloni]

On Sun, Jun 09, 2002 at 06:42:17AM -0600, Lightweight patch manager wrote:
>
[snip]
> + */
> +#define list_move(list,head) \
> +        do { __list_del(list->prev, list->next); \
> +             list_add(list,head); } \
> +        while(0)
> +
[snip]
>

I have a few comments about this, that were not already been said on this thread:

 * How about making it safer by assigning 'list' to something first,
   instead of letting the preprocessor evaluate it 3 times, which 
   can cause problems like, when 'list' evaluates to a function call?
 * At least surround 'list' with (), as in (list)->prev, so operator
   priorities would not give you trouble.
 * Why are you adding the definitions in the end of list.h? It would
   have been more originized if it was just after list_del_init, i.e,
   along with the other list mutators.
 * The comments for the macros are not clear at all. They don't tell
   what's the purpose of these macros, which is to delete an entry from
   a list and add it to another - which is what an average user will seek
   when reading list.h.

And about macros, I quote from Documentation/SubmittingPatches:

	3) 'static inline' is better than a macro

	Static inline functions are greatly preferred over macros.
	They provide type safety, have no length limitations, no formatting
	limitations, and under gcc they are as cheap as macros.

	Macros should only be used for cases where a static inline is clearly
	suboptimal [there a few, isolated cases of this in fast paths],
	or where it is impossible to use a static inline function [such as
	string-izing].

	'static inline' is preferred over 'static __inline__', 'extern inline',
	and 'extern __inline__'.

--
Dan Aloni
da-x@gmx.net

[PATCH] 2.5.21 - list.h cleanup

[Dan Aloni]

This patch is against 2.5.21 vanilla. 

 + replace __inline__ with inline.
 + use list_t intead of struct list_head (no bytes we harmed, bla.. bla..)
 + add the new list_move and list_move_tail mutators as inline functions.

--- linux-2.5.21/include/linux/list.h	Mon Jun 10 17:28:26 2002
+++ linux-2.5.21/include/linux/list.h	Mon Jun 10 18:08:58 2002
@@ -24,7 +24,7 @@
 #define LIST_HEAD_INIT(name) { &(name), &(name) }
 
 #define LIST_HEAD(name) \
-	struct list_head name = LIST_HEAD_INIT(name)
+	list_t name = LIST_HEAD_INIT(name)
 
 #define INIT_LIST_HEAD(ptr) do { \
 	(ptr)->next = (ptr); (ptr)->prev = (ptr); \
@@ -36,9 +36,7 @@
  * This is only for internal list manipulation where we know
  * the prev/next entries already!
  */
-static __inline__ void __list_add(struct list_head * new,
-	struct list_head * prev,
-	struct list_head * next)
+static inline void __list_add(list_t *new, list_t *prev, list_t *next)
 {
 	next->prev = new;
 	new->next = next;
@@ -54,7 +52,7 @@
  * Insert a new entry after the specified head.
  * This is good for implementing stacks.
  */
-static __inline__ void list_add(struct list_head *new, struct list_head *head)
+static inline void list_add(list_t *new, list_t *head)
 {
 	__list_add(new, head, head->next);
 }
@@ -67,7 +65,7 @@
  * Insert a new entry before the specified head.
  * This is useful for implementing queues.
  */
-static __inline__ void list_add_tail(struct list_head *new, struct list_head *head)
+static inline void list_add_tail(list_t *new, list_t *head)
 {
 	__list_add(new, head->prev, head);
 }
@@ -79,8 +77,7 @@
  * This is only for internal list manipulation where we know
  * the prev/next entries already!
  */
-static __inline__ void __list_del(struct list_head * prev,
-				  struct list_head * next)
+static inline void __list_del(list_t * prev, list_t * next)
 {
 	next->prev = prev;
 	prev->next = next;
@@ -91,7 +88,7 @@
  * @entry: the element to delete from the list.
  * Note: list_empty on entry does not return true after this, the entry is in an undefined state.
  */
-static __inline__ void list_del(struct list_head *entry)
+static inline void list_del(list_t *entry)
 {
 	__list_del(entry->prev, entry->next);
 	entry->next = (void *) 0;
@@ -102,17 +99,39 @@
  * list_del_init - deletes entry from list and reinitialize it.
  * @entry: the element to delete from the list.
  */
-static __inline__ void list_del_init(struct list_head *entry)
+static inline void list_del_init(list_t *entry)
 {
 	__list_del(entry->prev, entry->next);
 	INIT_LIST_HEAD(entry); 
 }
 
 /**
+ * list_move - delete from one list and add as another's head
+ * @list: the entry to move
+ * @head: the head that will precede our entry
+ */
+static inline void list_move(list_t *list, list_t *head)
+{
+        __list_del(list->prev, list->next);
+        list_add(list, head);
+}
+
+/**
+ * list_move_tail - delete from one list and add as another's tail
+ * @list: the entry to move
+ * @head: the head that will follow our entry
+ */
+static inline void list_move_tail(list_t *list, list_t *head)
+{
+        __list_del(list->prev, list->next);
+        list_add_tail(list, head);
+}
+
+/**
  * list_empty - tests whether a list is empty
  * @head: the list to test.
  */
-static __inline__ int list_empty(struct list_head *head)
+static inline int list_empty(list_t *head)
 {
 	return head->next == head;
 }
@@ -122,13 +141,13 @@
  * @list: the new list to add.
  * @head: the place to add it in the first list.
  */
-static __inline__ void list_splice(struct list_head *list, struct list_head *head)
+static inline void list_splice(list_t *list, list_t *head)
 {
-	struct list_head *first = list->next;
+	list_t *first = list->next;
 
 	if (first != list) {
-		struct list_head *last = list->prev;
-		struct list_head *at = head->next;
+		list_t *last = list->prev;
+		list_t *at = head->next;
 
 		first->prev = head;
 		head->next = first;
@@ -140,7 +159,7 @@
 
 /**
  * list_entry - get the struct for this entry
- * @ptr:	the &struct list_head pointer.
+ * @ptr:	the &list_t pointer.
  * @type:	the type of the struct this is embedded in.
  * @member:	the name of the list_struct within the struct.
  */
@@ -149,7 +168,7 @@
 
 /**
  * list_for_each	-	iterate over a list
- * @pos:	the &struct list_head to use as a loop counter.
+ * @pos:	the &list_t to use as a loop counter.
  * @head:	the head for your list.
  */
 #define list_for_each(pos, head) \
@@ -157,7 +176,7 @@
         	pos = pos->next, prefetch(pos->next))
 /**
  * list_for_each_prev	-	iterate over a list backwards
- * @pos:	the &struct list_head to use as a loop counter.
+ * @pos:	the &list_t to use as a loop counter.
  * @head:	the head for your list.
  */
 #define list_for_each_prev(pos, head) \
@@ -166,8 +185,8 @@
         	
 /**
  * list_for_each_safe	-	iterate over a list safe against removal of list entry
- * @pos:	the &struct list_head to use as a loop counter.
- * @n:		another &struct list_head to use as temporary storage
+ * @pos:	the &list_t to use as a loop counter.
+ * @n:		another &list_t to use as temporary storage
  * @head:	the head for your list.
  */
 #define list_for_each_safe(pos, n, head) \


-- 
Dan Aloni
da-x@gmx.net

Re: [PATCH] 2.5.21 - list.h cleanup

[Thunder from the hill]

Hi,

On Mon, 10 Jun 2002, Dan Aloni wrote:
> This patch is against 2.5.21 vanilla. 
> 
>  + replace __inline__ with inline.
>  + use list_t intead of struct list_head (no bytes we harmed, bla.. bla..)
>  + add the new list_move and list_move_tail mutators as inline functions.

Applied.

I just didn't dare to say inline here, because I was beaten some times for 
that.

Regards,
Thunder
-- 
German attitude becoming        |	Thunder from the hill at ngforever
rightaway popular:		|
       "Get outa my way,  	|	free inhabitant not directly
    for I got a mobile phone!"	|	belonging anywhere

Re: [PATCH] 2.5.21 - list.h cleanup

[Andreas Dilger]

On Jun 10, 2002  18:28 +0300, Dan Aloni wrote:
> This patch is against 2.5.21 vanilla. 
>  + replace __inline__ with inline.

Good.

>  + add the new list_move and list_move_tail mutators as inline functions.

Good.

>  + use list_t intead of struct list_head (no bytes we harmed, bla.. bla..)

I think you will find that the "struct list_head" is the preferred way
to go (which is why there are lots of "struct list_head" users in the
code and few "list_t" users.

Cheers, Andreas
--
Andreas Dilger
http://www-mddsp.enel.ucalgary.ca/People/adilger/
http://sourceforge.net/projects/ext2resize/

Re: [PATCH] 2.5.21 - list.h cleanup

[Thomas 'Dent' Mirlacher]

--snip/snip

> I think you will find that the "struct list_head" is the preferred way
> to go (which is why there are lots of "struct list_head" users in the
> code and few "list_t" users.

ok, the point that *_t is hiding implementation details (when used for
structs is valid). but is there a general consens on not using typedefs
for structs?

if yes, can we _please_ get rid of the *_t for structs.
if no, shouldn't we use the types already defined?

a similar thing will be unsigned (int|short|long|...)

just my $0.02 for the day,

	tm

-- 
in sometimes i don't, this time i do. 

Re: [PATCH] 2.5.21 - list.h cleanup

[Linus Torvalds]

On Mon, 10 Jun 2002, Thomas 'Dent' Mirlacher wrote:
>
> ok, the point that *_t is hiding implementation details (when used for
> structs is valid). but is there a general consens on not using typedefs
> for structs?

The linux coding style _tends_ to avoid using typedefs. It's not a hard
rule (and I did in fact apply this patch, since it otherwise looked fine),
but it's fairly common to use an explicit "struct xxxx" instead of
"xxxx_t".

I dislike type abstraction if it has no real reason. And saving on typing
is not a good reason - if your typing speed is the main issue when you're
coding, you're doing something seriously wrong.

(Similarly, if you are trying to compress lines to be shorter, you have
other problems, nothing to do with type names).

Does code look "prettier" with a "_t" rather than a "struct "? I don't
know. I personally don't think so, and I also hate the "_p" (or even more
the just plain "p") convention for "pointer".

Hiding the fact that it's a struct causes bad things if people don't
realize it: allocating structs on the stack is something you should be
aware of (and careful with), and passing them as parameters is is much
better done explicitly as a "pointer to struct".

There are _some_ exceptions. For example, "pte_t" etc might well be a
struct on most architectures, and that's ok: it's expressly designed to be
an opaque (and still fairly small) thing. This is an example of where
there are clear _reasons_ for the abstraction, not just abstraction for
its own sake.

But in the end, maintainership matters. I personally don't want the
typedef culture to get the upper hand, but I don't mind a few of them, and
people who maintain their own code usually get the last word.

		Linus

Re: [PATCH] 2.5.21 - list.h cleanup

[Thomas 'Dent' Mirlacher]

On Mon, 10 Jun 2002, Linus Torvalds wrote:

--snip/snip
> But in the end, maintainership matters. I personally don't want the
> typedef culture to get the upper hand, but I don't mind a few of them, and
> people who maintain their own code usually get the last word.

to sum it up:

using the "struct mystruct" is _recommended_, but not a must.

	tm

-- 
in some way i do, and in some way i don't.

Re: [PATCH] 2.5.21 - list.h cleanup

[Linus Torvalds]

On Mon, 10 Jun 2002, Thomas 'Dent' Mirlacher wrote:
>
> On Mon, 10 Jun 2002, Linus Torvalds wrote:
>
> --snip/snip
> > But in the end, maintainership matters. I personally don't want the
> > typedef culture to get the upper hand, but I don't mind a few of them, and
> > people who maintain their own code usually get the last word.
>
> to sum it up:
>
> using the "struct mystruct" is _recommended_, but not a must.

Well, it's more than just "struct xx". It's really typedefs in general.

For example, some people like to do things like

	typedef unsigned int counter_t;

and then use "counter_t" all over the place. I think that's not just ugly,
but stupid and counter-productive. It makes it much harder to do things
like "printk()" portably, for example ("should I use %u, %l or just %d?"),
and generally adds no value. It only _hides_ information, like whether the
type is signed or not.

There is nothing wrong with just using something like "unsigned long"
directly, even if it is a few characters longer than you might like. And
if you care about the number of bits, use "u32" or something. Don't make
up useless types that have no added advantage.

We actually have real _problems_ due to this in the kernel, where people
use "off_t", and it's not easily printk'able across different
architectures (we used to have this same problem with size_t). We should
have just used "unsigned long" inside the kernel, and be done with it (and
"unsigned long long" for loff_t).

We should also have some format for printing out "u32/u64" etc, but that's
another issue and has the problem that gcc won't understand them, so
adding new formats is _hard_ from a maintenance standpoint.

			Linus

PS. And never _ever_ make the "pointerness" part of the type. People who
write

	typedef struct urb_struct * urbp_t;

(or whatever the name was) should just be shot. I was _soo_ happy to see
that crap get excised from the kernel USB drivers.

Re: [PATCH] 2.5.21 - list.h cleanup

[Rusty Russell]

On Mon, 10 Jun 2002 10:21:36 -0700 (PDT)
Linus Torvalds <torvalds@transmeta.com> wrote:
> Well, it's more than just "struct xx". It's really typedefs in general.

Worst sin is that you can't predeclare typedefs.  For many uses (not the
list macros of course):
	struct xx;
is sufficient and avoids the #include hell,

Cheers
Rusty.
-- 
   there are those who do and those who hang on and you don't see too
   many doers quoting their contemporaries.  -- Larry McVoy

Re: [PATCH] 2.5.21 - list.h cleanup

[Linus Torvalds]

On Tue, 11 Jun 2002, Rusty Russell wrote:
>
> Worst sin is that you can't predeclare typedefs.  For many uses (not the
> list macros of course):
> 	struct xx;
> is sufficient and avoids the #include hell,

True.

However, that only works for function declarations.

typedefs are easy to avoid.

The real #include hell comes, to a large degree, from the fact that we
like inline functions. Which have many wonderful properties, but they have
the same nasty property typedefs have: they require full type information
and cannot be predeclared.

And while I'd like to avoid #include hell, I'm not willing to replace
inline functions with #define's to avoid it ;^p

		Linus

Re: [PATCH] 2.5.21 - list.h cleanup

[Martin Dalecki]

Użytkownik Linus Torvalds napisał:
> 
> On Tue, 11 Jun 2002, Rusty Russell wrote:
> 
>>Worst sin is that you can't predeclare typedefs.  For many uses (not the
>>list macros of course):
>>	struct xx;
>>is sufficient and avoids the #include hell,
> 
> 
> True.
> 
> However, that only works for function declarations.
> 
> typedefs are easy to avoid.
> 
> The real #include hell comes, to a large degree, from the fact that we
> like inline functions. Which have many wonderful properties, but they have
> the same nasty property typedefs have: they require full type information
> and cannot be predeclared.
> 
> And while I'd like to avoid #include hell, I'm not willing to replace
> inline functions with #define's to avoid it ;^p

That's true, but please note that there is quite a lot
of inadequate include abuse in Linux. People are just too
lazy to check whatever inlining something really speeds things
up. For example the functions used to copy data between
userspace and kernel are very likely to be executed not much slower
if *not* included. In fact they should not turn up
on *any* benchmark - becouse if they do we have other problems...
Namely we have a system call which is basically doing nothing.
But they show up significantly on the .text size.

And then we have quite a lot if silly include code like the following:

static inline function()
{
           seme_function();
		some_other_function();
}

Which doesn't really make much sense, quite frequently, becouse
We will trash caches anyway...

Or

static inline int iterator()
{
}

This doesn't make sense too, becouse
the CPUs tends to be good at branch prediction and if
a function is mainly used inside loops again and again
it doesn't make sense to make them inline frequently too.

And so on and so on...

Re: [PATCH] 2.5.21 - list.h cleanup

[Martin Dalecki]

> And while I'd like to avoid #include hell, I'm not willing to replace
> inline functions with #define's to avoid it ;^p


A more real example from blk.h:


extern inline struct request *elv_next_request(request_queue_t *q)
{
	struct request *rq;

	while ((rq = __elv_next_request(q))) {
		rq->flags |= REQ_STARTED;

		if (&rq->queuelist == q->last_merge)
			q->last_merge = NULL;

		if ((rq->flags & REQ_DONTPREP) || !q->prep_rq_fn)
			break;

		/*
		 * all ok, break and return it
		 */
		if (!q->prep_rq_fn(q, rq))
			break;

		/*
		 * prep said no-go, kill it
		 */
		blkdev_dequeue_request(rq);
		if (end_that_request_first(rq, 0, rq->nr_sectors))
			BUG();

		end_that_request_last(rq);
	}

	return rq;
}

The only thing this achvies is kernel bload, since
elv_next_request is:

1. Calling tons of functions, which could be inlined as well.

2. Not precisely on any ultra fast path.

3. Bound by the device speed.

4. Increasing the pressure on branch prediction due
to the while construct.

Re: [PATCH] 2.5.21 - list.h cleanup

[Rusty Russell]

In message <Pine.LNX.4.44.0206110128130.1987-100000@home.transmeta.com> you wri
te:
> 
> 
> On Tue, 11 Jun 2002, Rusty Russell wrote:
> >
> > Worst sin is that you can't predeclare typedefs.  For many uses (not the
> > list macros of course):
> > 	struct xx;
> > is sufficient and avoids the #include hell,
> 
> True.
> 
> However, that only works for function declarations.

Our headers basically consist of:

1) Macros
2) Structure declarations
3) Function declarations
4) Inline functions

The number of structures and functions which need only "struct xxx *"
is very high: removing typedefs is something about with ~zero pain
(unlike dropping the sometimes-dubious loveaffair with inlines).

Rusty.
PS.  I blame Ingo: list_t indeed!
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] 2.5.21 - list.h cleanup

[Ingo Molnar]

On Tue, 11 Jun 2002, Rusty Russell wrote:

> The number of structures and functions which need only "struct xxx *"
> is very high: removing typedefs is something about with ~zero pain
> (unlike dropping the sometimes-dubious loveaffair with inlines).
> 
> Rusty.
> PS.  I blame Ingo: list_t indeed!

the reason why i added list_t to the scheduler code was mainly for
aesthetic reasons. I'm still using 80x25 text consoles mainly, which are
more sensitive to code length. Also, 'struct list_head' did not reflect
the kind of lightweight list type we have, 'list_t' does that better. Eg.:

unsigned int void some_function(list_t *head, list_t *next, list_t *prev)
{
}

instead of:

unsigned int some_function(struct list_head *head, struct list_head *next,
			 struct list_head *prev)
{
}

but if typedefs create other problems then these arguments are secondary i
guess. I'm completely against redefining base types for no particular
reason, like counter_t.

But i think it would be useful to introduce some sort of '_t convention',
where _t always means a complex (or potentially complex - opaque) type. It
makes code so much more compact and readable, and it does not hide
anything - _t *always* means a complex type in the way i use it.

To see this in action check out 2.5's drivers/md/raid5.c for example,
replace all the _t types with their full-blown struct equivalents and
compare code readability. And this is not broken code in any way, it's
just code that uses lots of complex types.

	Ingo

Re: [PATCH] 2.5.21 - list.h cleanup

[Rusty Russell]

In message <Pine.LNX.4.44.0206122125210.17567-100000@elte.hu> you write:
> On Tue, 11 Jun 2002, Rusty Russell wrote:
> > PS.  I blame Ingo: list_t indeed!
> 
> the reason why i added list_t to the scheduler code was mainly for
> aesthetic reasons. I'm still using 80x25 text consoles mainly, which are
> more sensitive to code length. Also, 'struct list_head' did not reflect
> the kind of lightweight list type we have, 'list_t' does that better. Eg.:

I agree calling it "struct list" or "struct dlist" would have been
nicer, but we're stuck with it now.

> but if typedefs create other problems then these arguments are secondary i
> guess. I'm completely against redefining base types for no particular
> reason, like counter_t.

Patch below.  There aren't many uses of list_t at all, but thousands
of "struct list_head" users.

> But i think it would be useful to introduce some sort of '_t convention',
> where _t always means a complex (or potentially complex - opaque) type. It
> makes code so much more compact and readable, and it does not hide
> anything - _t *always* means a complex type in the way i use it.

OTOH, I've thought of adding a kerrno_t which is an int, and only
useful for documentation purposes (meaning: I return 0 or -errno).
This conflicts with your _t definition 8(

Cheers,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

diff -urN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.21/include/linux/device.h tmp/include/linux/device.h
--- linux-2.5.21/include/linux/device.h	Mon Jun 10 16:03:55 2002
+++ tmp/include/linux/device.h	Thu Jun 13 15:44:58 2002
@@ -56,9 +56,9 @@
 	rwlock_t		lock;
 	atomic_t		refcount;
 
-	list_t			node;
-	list_t			devices;
-	list_t			drivers;
+	struct list_head	node;
+	struct list_head	devices;
+	struct list_head	drivers;
 
 	struct driver_dir_entry	dir;
 	struct driver_dir_entry	device_dir;
@@ -92,8 +92,8 @@
 	rwlock_t		lock;
 	atomic_t		refcount;
 
-	list_t			bus_list;
-	list_t			devices;
+	struct list_head	bus_list;
+	struct list_head	devices;
 
 	struct driver_dir_entry	dir;
 
diff -urN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.21/include/linux/fs.h tmp/include/linux/fs.h
--- linux-2.5.21/include/linux/fs.h	Mon Jun 10 16:03:55 2002
+++ tmp/include/linux/fs.h	Thu Jun 13 15:44:01 2002
@@ -323,8 +323,8 @@
 	struct list_head	io_pages;	/* being prepared for I/O */
 	unsigned long		nrpages;	/* number of total pages */
 	struct address_space_operations *a_ops;	/* methods */
-	list_t			i_mmap;		/* list of private mappings */
-	list_t			i_mmap_shared;	/* list of private mappings */
+	struct list_head	i_mmap;		/* list of private mappings */
+	struct list_head	i_mmap_shared;	/* list of private mappings */
 	spinlock_t		i_shared_lock;  /* and spinlock protecting it */
 	unsigned long		dirtied_when;	/* jiffies of first page dirtying */
 	int			gfp_mask;	/* how to allocate the pages */
diff -urN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.21/include/linux/list.h tmp/include/linux/list.h
--- linux-2.5.21/include/linux/list.h	Mon Jun  3 12:21:28 2002
+++ tmp/include/linux/list.h	Thu Jun 13 15:44:13 2002
@@ -19,8 +19,6 @@
 	struct list_head *next, *prev;
 };
 
-typedef struct list_head list_t;
-
 #define LIST_HEAD_INIT(name) { &(name), &(name) }
 
 #define LIST_HEAD(name) \
diff -urN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.21/include/linux/mm.h tmp/include/linux/mm.h
--- linux-2.5.21/include/linux/mm.h	Fri Jun  7 13:59:08 2002
+++ tmp/include/linux/mm.h	Thu Jun 13 15:44:09 2002
@@ -61,7 +61,7 @@
 	 * one of the address_space->i_mmap{,shared} lists,
 	 * for shm areas, the list of attaches, otherwise unused.
 	 */
-	list_t shared;
+	struct list_head shared;
 
 	/* Function pointers to deal with this struct. */
 	struct vm_operations_struct * vm_ops;
diff -urN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.21/include/linux/sched.h tmp/include/linux/sched.h
--- linux-2.5.21/include/linux/sched.h	Mon Jun 10 16:03:55 2002
+++ tmp/include/linux/sched.h	Thu Jun 13 15:43:48 2002
@@ -259,7 +259,7 @@
 	int lock_depth;		/* Lock depth */
 
 	int prio, static_prio;
-	list_t run_list;
+	struct list_head run_list;
 	prio_array_t *array;
 
 	unsigned long sleep_avg;
diff -urN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.21/kernel/sched.c tmp/kernel/sched.c
--- linux-2.5.21/kernel/sched.c	Mon Jun 10 16:03:56 2002
+++ tmp/kernel/sched.c	Thu Jun 13 15:45:44 2002
@@ -123,7 +123,7 @@
 struct prio_array {
 	int nr_active;
 	unsigned long bitmap[BITMAP_SIZE];
-	list_t queue[MAX_PRIO];
+	struct list_head queue[MAX_PRIO];
 };
 
 /*
@@ -142,7 +142,7 @@
 	prio_array_t *active, *expired, arrays[2];
 	int prev_nr_running[NR_CPUS];
 	task_t *migration_thread;
-	list_t migration_queue;
+	struct list_head migration_queue;
 } ____cacheline_aligned;
 
 static struct runqueue runqueues[NR_CPUS] __cacheline_aligned;
@@ -499,7 +499,7 @@
 	task_t *next = this_rq->idle, *tmp;
 	runqueue_t *busiest, *rq_src;
 	prio_array_t *array;
-	list_t *head, *curr;
+	struct list_head *head, *curr;
 
 	/*
 	 * We search all runqueues to find the most busy one.
@@ -759,7 +759,7 @@
 	task_t *prev, *next;
 	runqueue_t *rq;
 	prio_array_t *array;
-	list_t *queue;
+	struct list_head *queue;
 	int idx;
 
 	if (unlikely(in_interrupt()))
@@ -1652,7 +1652,7 @@
  */
 
 typedef struct {
-	list_t list;
+	struct list_head list;
 	task_t *task;
 	struct semaphore sem;
 } migration_req_t;
diff -urN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.21/mm/memory.c tmp/mm/memory.c
--- linux-2.5.21/mm/memory.c	Mon Jun  3 12:21:28 2002
+++ tmp/mm/memory.c	Thu Jun 13 15:43:23 2002
@@ -1028,11 +1028,11 @@
 	return -1;
 }
 
-static void vmtruncate_list(list_t *head, unsigned long pgoff)
+static void vmtruncate_list(struct list_head *head, unsigned long pgoff)
 {
 	unsigned long start, end, len, diff;
 	struct vm_area_struct *vma;
-	list_t *curr;
+	struct list_head *curr;
 
 	list_for_each(curr, head) {
 		vma = list_entry(curr, struct vm_area_struct, shared);
diff -urN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.21/mm/page_alloc.c tmp/mm/page_alloc.c
--- linux-2.5.21/mm/page_alloc.c	Mon Jun 10 16:03:56 2002
+++ tmp/mm/page_alloc.c	Thu Jun 13 15:43:29 2002
@@ -235,7 +235,7 @@
         zone_t *zone = page_zone(page);
         unsigned long flags;
 	int order;
-	list_t *curr;
+	struct list_head *curr;
 
 	/*
 	 * Should not matter as we need quiescent system for

Re: [PATCH] 2.5.21 - list.h cleanup

[Ingo Molnar]

On Thu, 13 Jun 2002, Rusty Russell wrote:

> > But i think it would be useful to introduce some sort of '_t convention',
> > where _t always means a complex (or potentially complex - opaque) type. It
> > makes code so much more compact and readable, and it does not hide
> > anything - _t *always* means a complex type in the way i use it.
> 
> OTOH, I've thought of adding a kerrno_t which is an int, and only useful
> for documentation purposes (meaning: I return 0 or -errno). This
> conflicts with your _t definition 8(

well, good rules have exceptions, and kerrno_t is not purely equivalent to
int, it's rather a restricted range of integers. This makes it more
eligible for the _t usage than counter_t for example.

	Ingo

Re: [PATCH] 2.5.21 - list.h cleanup

[george anzinger]

Linus Torvalds wrote:
> 
> On Tue, 11 Jun 2002, Rusty Russell wrote:
> >
> > Worst sin is that you can't predeclare typedefs.  For many uses (not the
> > list macros of course):
> >       struct xx;
> > is sufficient and avoids the #include hell,
> 
> True.
> 
> However, that only works for function declarations.
> 
> typedefs are easy to avoid.
> 
> The real #include hell comes, to a large degree, from the fact that we
> like inline functions. Which have many wonderful properties, but they have
> the same nasty property typedefs have: they require full type information
> and cannot be predeclared.
> 
> And while I'd like to avoid #include hell, I'm not willing to replace
> inline functions with #define's to avoid it ;^p

On wonders if it might be useful to split header files into
say for example, list_d.h and list_i.h with the declarations
in the "_d.h" and inlines in the "_i.h".  Then we could move
the "_i.h" includes to the end of the include list.  Yeah, I
know, too many includes in includes to work.  

By the way, my reading of the C standard indicates that they
don't _have to_ compile the inlines when they are found, but
_may_ defer the compile until they are referenced by
non-inline code.  This change would fix the problem.
-- 
George Anzinger   george@mvista.com
High-res-timers: 
http://sourceforge.net/projects/high-res-timers/
Real time sched:  http://sourceforge.net/projects/rtsched/
Preemption patch:
http://www.kernel.org/pub/linux/kernel/people/rml

Re: [PATCH] 2.5.21 - list.h cleanup

[Daniel Phillips]

On Tuesday 11 June 2002 16:52, george anzinger wrote:
> Linus Torvalds wrote:
> > The real #include hell comes, to a large degree, from the fact that we
> > like inline functions. Which have many wonderful properties, but they have
> > the same nasty property typedefs have: they require full type information
> > and cannot be predeclared.
> > 
> > And while I'd like to avoid #include hell, I'm not willing to replace
> > inline functions with #define's to avoid it ;^p
> 
> On wonders if it might be useful to split header files into
> say for example, list_d.h and list_i.h with the declarations
> in the "_d.h" and inlines in the "_i.h".  Then we could move
> the "_i.h" includes to the end of the include list.  Yeah, I
> know, too many includes in includes to work.  

No, it does work, and it works very well.  The winning strategy is to
split out heavily referenced data declarations from any dependent
function declarations, and give them their own headers, also included
from the orginal headers.  The normal 'compile this header once'
mechanism makes this work smoothly and that's optimized by cpp so
compile speed impact is negigible.  In many cases only the data
declaration is needed, so it's possible we might even see a slight
speedup.

The tradeoff of a few more header files (and it's only a few) against
a lot more sanity is well worth it.  I demonstrated the technique in
my 'early_page' patch set, which lets you declare struct page as the
first thing in mm.h. and I suppose I should dust that off, update it
to 2.5 and submit it.

-- 
Daniel

Re: [PATCH] 2.5.21 - list.h cleanup

[Rusty Russell]

In message <3D060EC8.321A0D66@mvista.com> you write:
> On wonders if it might be useful to split header files into
> say for example, list_d.h and list_i.h with the declarations
> in the "_d.h" and inlines in the "_i.h".  Then we could move
> the "_i.h" includes to the end of the include list.  Yeah, I
> know, too many includes in includes to work.  

The only really sane way to implement "CONFIG_SMALL_NO_INLINES" that I
can think of is to have headers do

#include <linux/inline.h>

inline_me int function(int x) { return x++; }

Then inline.h contain:

#include <linux/config.h>
#ifdef CONFIG_SMALL_NO_INLINES
#define inline_me
#else
#define inline_me static inline
#endif

And if do a final compile of a file "inlines.c" like so if
CONFIG_SMALL_NO_INLINES is set:

#include <linux/config.h>
#undef CONFIG_SMALL_NO_INLINES

/* Instantiate one of each inline for real: auto-generated list */

#include <linux/header1.h>
#include <linux/header2.h>
#include <linux/header3.h>
#include <linux/header4.h>

Expect an implementation in... um... well, someone else perhaps?
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] 2.5.21 - list.h cleanup

[Linus Torvalds]

On Wed, 12 Jun 2002, Rusty Russell wrote:
>
> The only really sane way to implement "CONFIG_SMALL_NO_INLINES" that I
> can think of is to have headers do

inlines, when used properly, are _not_ larger than not inlining.

Quite often, inlining means that much of the code goes away due to
constants etc (alloc_pages(), for example), while in other cases the code
itself ends up being smaller because a small function takes up space due
to clobbering registers, memory, and all the function calling sequence
itself.

This is _very_ much the case with the list.h inlines we're talking about.
They'd be larger, slower, and the resulting assembly code would be less
readable if they weren't inlined.

However, we do have some inlines that are just too large, and have often
grown up over time. They should not be a CONFIG_SMALL_NO_INLINES thing,
though, they should just be moved into proper .c file.s

(There are also inlines that _look_ large, but which just go away when
attacked by an optimizing compiler. See all the "get_user()" stuff with
case statements etc)

None of these ugly header file tricks, please.

		Linus

Re: [PATCH] 2.5.21 - list.h cleanup

[Rusty Russell]

In message <Pine.LNX.4.44.0206111824050.16686-100000@home.transmeta.com> you wr
ite:
> 
> 
> On Wed, 12 Jun 2002, Rusty Russell wrote:
> >
> > The only really sane way to implement "CONFIG_SMALL_NO_INLINES" that I
> > can think of is to have headers do
> 
> inlines, when used properly, are _not_ larger than not inlining.

Good, now we have a benchmark for inline elimination, if people feel
so strongly about it.

Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] 2.5.21 - list.h cleanup

[Martin Dalecki]

Użytkownik Linus Torvalds napisał:
> 
> On Wed, 12 Jun 2002, Rusty Russell wrote:
> 
>>The only really sane way to implement "CONFIG_SMALL_NO_INLINES" that I
>>can think of is to have headers do
> 
> 
> inlines, when used properly, are _not_ larger than not inlining.

Actually I have monitored linux/include/linux/*.h for improper
and inadequate inlining. Well what have I to say. In comparision
to the last time I checked (around 4 years ago) the situation
got *much* better. I was not able to save more then around 1k of
code from the kernel... How ever some offenders are:

1. elv_next_request(request_queue_t *q)

-  just too big

2. seq_putc(struct seq_file *m, char c)

- just not worth the trobule.

3. seq_puts(struct seq_file *m, const char *s)

- calls strlen and memset - really not worth inlining.

4. void DQUOT_INIT(struct inode *inode)
  void DQUOT_DROP(struct inode *inode)
DQUOT_PREALLOC_SPACE_NODIRTY(struct in
int DQUOT_PREALLOC_SPACE(struct inode *inode, qsize_t nr)ode *inode, qsize_t

and friends from quotaops - all to be to be inlines.

But the situation got really really better over time!
Please note that I didn't look at include/asm/*.h files, where
memset copy_from/to_user strlen and friends reside, which
result in quite a lot of inline code.

Re: [PATCH] 2.5.21 - list.h cleanup

[Andrew Morton]

Martin Dalecki wrote:
> 
> U¿ytkownik Linus Torvalds napisa³:
> >
> > On Wed, 12 Jun 2002, Rusty Russell wrote:
> >
> >>The only really sane way to implement "CONFIG_SMALL_NO_INLINES" that I
> >>can think of is to have headers do
> >
> >
> > inlines, when used properly, are _not_ larger than not inlining.
> 
> Actually I have monitored linux/include/linux/*.h for improper
> and inadequate inlining. Well what have I to say. In comparision
> to the last time I checked (around 4 years ago) the situation
> got *much* better. I was not able to save more then around 1k of
> code from the kernel...

You looked in the wrong place...

The below patch shrunk 2.4.17-pre2 by 11 kbytes.  That's
a ton of L1 cache, and most of it is fastpath.

 drivers/block/ll_rw_blk.c |   18 ++-------------
 fs/binfmt_elf.c           |    4 +--
 fs/block_dev.c            |    2 -
 fs/dcache.c               |    8 +++---
 fs/inode.c                |    6 ++---
 fs/locks.c                |    8 +++---
 fs/namei.c                |   14 ++++++------
 fs/namespace.c            |   42 ++++++++++++++++++++++++++++++++++++
 fs/open.c                 |    4 +--
 fs/read_write.c           |    2 -
 fs/stat.c                 |    2 -
 fs/super.c                |    2 -
 include/linux/fs_struct.h |   53 +++-------------------------------------------
 kernel/exit.c             |   10 ++++----
 kernel/fork.c             |    4 +--
 kernel/module.c           |    2 -
 kernel/sched.c            |    6 ++---
 kernel/signal.c           |    3 --
 kernel/sys.c              |    2 -
 kernel/timer.c            |    2 -
 lib/rwsem.c               |    4 +--
 mm/filemap.c              |    4 +--
 mm/highmem.c              |    2 -
 mm/memory.c               |    2 -
 mm/mmap.c                 |    4 +--
 mm/slab.c                 |   14 ++++++------

--- linux-2.4.17-pre2/fs/binfmt_elf.c	Sat Oct 20 19:16:59 2001
+++ linux-akpm/fs/binfmt_elf.c	Fri Nov 30 21:21:26 2001
@@ -223,7 +223,7 @@ create_elf_tables(char *p, int argc, int
 
 #ifndef elf_map
 
-static inline unsigned long
+static unsigned long
 elf_map (struct file *filep, unsigned long addr, struct elf_phdr *eppnt, int prot, int type)
 {
 	unsigned long map_addr;
@@ -914,7 +914,7 @@ static int dump_seek(struct file *file, 
  *
  * I think we should skip something. But I am not sure how. H.J.
  */
-static inline int maydump(struct vm_area_struct *vma)
+static int maydump(struct vm_area_struct *vma)
 {
 	/*
 	 * If we may not read the contents, don't allow us to dump
--- linux-2.4.17-pre2/fs/dcache.c	Wed Oct  3 22:57:36 2001
+++ linux-akpm/fs/dcache.c	Fri Nov 30 21:21:26 2001
@@ -56,7 +56,7 @@ static LIST_HEAD(dentry_unused);
 struct dentry_stat_t dentry_stat = {0, 0, 45, 0,};
 
 /* no dcache_lock, please */
-static inline void d_free(struct dentry *dentry)
+static void d_free(struct dentry *dentry)
 {
 	if (dentry->d_op && dentry->d_op->d_release)
 		dentry->d_op->d_release(dentry);
@@ -71,7 +71,7 @@ static inline void d_free(struct dentry 
  * d_iput() operation if defined.
  * Called with dcache_lock held, drops it.
  */
-static inline void dentry_iput(struct dentry * dentry)
+static void dentry_iput(struct dentry * dentry)
 {
 	struct inode *inode = dentry->d_inode;
 	if (inode) {
@@ -215,7 +215,7 @@ int d_invalidate(struct dentry * dentry)
 
 /* This should be called _only_ with dcache_lock held */
 
-static inline struct dentry * __dget_locked(struct dentry *dentry)
+static struct dentry * __dget_locked(struct dentry *dentry)
 {
 	atomic_inc(&dentry->d_count);
 	if (atomic_read(&dentry->d_count) == 1) {
@@ -291,7 +291,7 @@ restart:
  * removed.
  * Called with dcache_lock, drops it and then regains.
  */
-static inline void prune_one_dentry(struct dentry * dentry)
+static void prune_one_dentry(struct dentry * dentry)
 {
 	struct dentry * parent;
 
--- linux-2.4.17-pre2/fs/locks.c	Thu Oct 11 07:52:18 2001
+++ linux-akpm/fs/locks.c	Fri Nov 30 21:21:26 2001
@@ -146,7 +146,7 @@ static struct file_lock *locks_alloc_loc
 }
 
 /* Free a lock which is not in use. */
-static inline void locks_free_lock(struct file_lock *fl)
+static void locks_free_lock(struct file_lock *fl)
 {
 	if (fl == NULL) {
 		BUG();
@@ -422,7 +422,7 @@ static void locks_insert_block(struct fi
 	list_add(&waiter->fl_link, &blocked_list);
 }
 
-static inline
+static
 void locks_notify_blocked(struct file_lock *waiter)
 {
 	if (waiter->fl_notify)
@@ -490,7 +490,7 @@ static inline void _unhash_lock(struct f
  * notify the FS that the lock has been cleared and
  * finally free the lock.
  */
-static inline void _delete_lock(struct file_lock *fl, unsigned int wait)
+static void _delete_lock(struct file_lock *fl, unsigned int wait)
 {
 	fasync_helper(0, fl->fl_file, 0, &fl->fl_fasync);
 	if (fl->fl_fasync != NULL){
@@ -521,7 +521,7 @@ static void locks_delete_lock(struct fil
  * then delete lock. Essentially useful only in locks_remove_*().
  * Note: this must be called with the semaphore already held!
  */
-static inline void locks_unlock_delete(struct file_lock **thisfl_p)
+static void locks_unlock_delete(struct file_lock **thisfl_p)
 {
 	struct file_lock *fl = *thisfl_p;
 	int (*lock)(struct file *, int, struct file_lock *);
--- linux-2.4.17-pre2/fs/namei.c	Wed Oct 17 14:46:29 2001
+++ linux-akpm/fs/namei.c	Fri Nov 30 21:21:26 2001
@@ -331,7 +331,7 @@ static struct dentry * real_lookup(struc
  * Without that kind of total limit, nasty chains of consecutive
  * symlinks can cause almost arbitrarily long lookups. 
  */
-static inline int do_follow_link(struct dentry *dentry, struct nameidata *nd)
+static int do_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
 	int err;
 	if (current->link_count >= 5)
@@ -378,7 +378,7 @@ int follow_up(struct vfsmount **mnt, str
 	return __follow_up(mnt, dentry);
 }
 
-static inline int __follow_down(struct vfsmount **mnt, struct dentry **dentry)
+static int __follow_down(struct vfsmount **mnt, struct dentry **dentry)
 {
 	struct vfsmount *mounted;
 
@@ -401,7 +401,7 @@ int follow_down(struct vfsmount **mnt, s
 	return __follow_down(mnt,dentry);
 }
  
-static inline void follow_dotdot(struct nameidata *nd)
+static void follow_dotdot(struct nameidata *nd)
 {
 	while(1) {
 		struct vfsmount *parent;
@@ -704,7 +704,7 @@ void set_fs_altroot(void)
 }
 
 /* SMP-safe */
-static inline int
+static int
 walk_init_root(const char *name, struct nameidata *nd)
 {
 	read_lock(&current->fs->lock);
@@ -867,7 +867,7 @@ static inline int check_sticky(struct in
  *  8. If we were asked to remove a non-directory and victim isn't one - EISDIR.
  *  9. We can't remove a root or mountpoint.
  */
-static inline int may_delete(struct inode *dir,struct dentry *victim, int isdir)
+static int may_delete(struct inode *dir,struct dentry *victim, int isdir)
 {
 	int error;
 	if (!victim->d_inode || victim->d_parent->d_inode != dir)
@@ -898,7 +898,7 @@ static inline int may_delete(struct inod
  *  3. We should have write and exec permissions on dir
  *  4. We can't do it if dir is immutable (done in permission())
  */
-static inline int may_create(struct inode *dir, struct dentry *child) {
+static int may_create(struct inode *dir, struct dentry *child) {
 	if (child->d_inode)
 		return -EEXIST;
 	if (IS_DEADDIR(dir))
@@ -1942,7 +1942,7 @@ out:
 	return len;
 }
 
-static inline int
+static int
 __vfs_follow_link(struct nameidata *nd, const char *link)
 {
 	int res = 0;
--- linux-2.4.17-pre2/fs/open.c	Fri Oct 12 13:48:42 2001
+++ linux-akpm/fs/open.c	Fri Nov 30 21:21:26 2001
@@ -89,7 +89,7 @@ int do_truncate(struct dentry *dentry, l
 	return error;
 }
 
-static inline long do_sys_truncate(const char * path, loff_t length)
+static long do_sys_truncate(const char * path, loff_t length)
 {
 	struct nameidata nd;
 	struct inode * inode;
@@ -155,7 +155,7 @@ asmlinkage long sys_truncate(const char 
 	return do_sys_truncate(path, (long)length);
 }
 
-static inline long do_sys_ftruncate(unsigned int fd, loff_t length, int small)
+static long do_sys_ftruncate(unsigned int fd, loff_t length, int small)
 {
 	struct inode * inode;
 	struct dentry *dentry;
--- linux-2.4.17-pre2/fs/read_write.c	Sun Aug  5 13:12:41 2001
+++ linux-akpm/fs/read_write.c	Fri Nov 30 21:21:26 2001
@@ -76,7 +76,7 @@ loff_t default_llseek(struct file *file,
 	return retval;
 }
 
-static inline loff_t llseek(struct file *file, loff_t offset, int origin)
+static loff_t llseek(struct file *file, loff_t offset, int origin)
 {
 	loff_t (*fn)(struct file *, loff_t, int);
 	loff_t retval;
--- linux-2.4.17-pre2/fs/stat.c	Thu Sep 13 16:04:43 2001
+++ linux-akpm/fs/stat.c	Fri Nov 30 21:21:26 2001
@@ -16,7 +16,7 @@
 /*
  * Revalidate the inode. This is required for proper NFS attribute caching.
  */
-static __inline__ int
+static int
 do_revalidate(struct dentry *dentry)
 {
 	struct inode * inode = dentry->d_inode;
--- linux-2.4.17-pre2/fs/super.c	Mon Nov 26 11:52:07 2001
+++ linux-akpm/fs/super.c	Fri Nov 30 21:21:26 2001
@@ -314,7 +314,7 @@ static void put_super(struct super_block
 	__put_super(sb);
 }
 
-static inline void write_super(struct super_block *sb)
+static void write_super(struct super_block *sb)
 {
 	lock_super(sb);
 	if (sb->s_root && sb->s_dirt)
--- linux-2.4.17-pre2/fs/inode.c	Fri Nov 30 14:32:14 2001
+++ linux-akpm/fs/inode.c	Fri Nov 30 21:21:26 2001
@@ -178,7 +178,7 @@ repeat:
 	current->state = TASK_RUNNING;
 }
 
-static inline void wait_on_inode(struct inode *inode)
+static void wait_on_inode(struct inode *inode)
 {
 	if (inode->i_state & I_LOCK)
 		__wait_on_inode(inode);
@@ -191,7 +191,7 @@ static inline void write_inode(struct in
 		inode->i_sb->s_op->write_inode(inode, sync);
 }
 
-static inline void __iget(struct inode * inode)
+static void __iget(struct inode * inode)
 {
 	if (atomic_read(&inode->i_count)) {
 		atomic_inc(&inode->i_count);
@@ -245,7 +245,7 @@ static inline void __sync_one(struct ino
 	wake_up(&inode->i_wait);
 }
 
-static inline void sync_one(struct inode *inode, int sync)
+static void sync_one(struct inode *inode, int sync)
 {
 	if (inode->i_state & I_LOCK) {
 		__iget(inode);
--- linux-2.4.17-pre2/fs/block_dev.c	Thu Nov 22 23:02:58 2001
+++ linux-akpm/fs/block_dev.c	Fri Nov 30 21:21:26 2001
@@ -344,7 +344,7 @@ struct block_device *bdget(dev_t dev)
 	return bdev;
 }
 
-static inline void __bd_forget(struct inode *inode)
+static void __bd_forget(struct inode *inode)
 {
 	list_del_init(&inode->i_devices);
 	inode->i_bdev = NULL;
--- linux-2.4.17-pre2/kernel/exit.c	Thu Nov 22 23:02:59 2001
+++ linux-akpm/kernel/exit.c	Fri Nov 30 21:21:26 2001
@@ -131,7 +131,7 @@ int is_orphaned_pgrp(int pgrp)
 	return will_become_orphaned_pgrp(pgrp, 0);
 }
 
-static inline int has_stopped_jobs(int pgrp)
+static int has_stopped_jobs(int pgrp)
 {
 	int retval = 0;
 	struct task_struct * p;
@@ -211,7 +211,7 @@ void put_files_struct(struct files_struc
 	}
 }
 
-static inline void __exit_files(struct task_struct *tsk)
+static void __exit_files(struct task_struct *tsk)
 {
 	struct files_struct * files = tsk->files;
 
@@ -228,7 +228,7 @@ void exit_files(struct task_struct *tsk)
 	__exit_files(tsk);
 }
 
-static inline void __put_fs_struct(struct fs_struct *fs)
+static void __put_fs_struct(struct fs_struct *fs)
 {
 	/* No need to hold fs->lock if we are killing it */
 	if (atomic_dec_and_test(&fs->count)) {
@@ -249,7 +249,7 @@ void put_fs_struct(struct fs_struct *fs)
 	__put_fs_struct(fs);
 }
 
-static inline void __exit_fs(struct task_struct *tsk)
+static void __exit_fs(struct task_struct *tsk)
 {
 	struct fs_struct * fs = tsk->fs;
 
@@ -296,7 +296,7 @@ void end_lazy_tlb(struct mm_struct *mm)
  * Turn us into a lazy TLB process if we
  * aren't already..
  */
-static inline void __exit_mm(struct task_struct * tsk)
+static void __exit_mm(struct task_struct * tsk)
 {
 	struct mm_struct * mm = tsk->mm;
 
--- linux-2.4.17-pre2/kernel/fork.c	Thu Nov 22 23:02:59 2001
+++ linux-akpm/kernel/fork.c	Fri Nov 30 21:21:26 2001
@@ -246,7 +246,7 @@ struct mm_struct * mm_alloc(void)
  * is dropped: either by a lazy thread or by
  * mmput. Free the page directory and the mm.
  */
-inline void __mmdrop(struct mm_struct *mm)
+void __mmdrop(struct mm_struct *mm)
 {
 	if (mm == &init_mm) BUG();
 	pgd_free(mm->pgd);
@@ -359,7 +359,7 @@ fail_nomem:
 	return retval;
 }
 
-static inline struct fs_struct *__copy_fs_struct(struct fs_struct *old)
+static struct fs_struct *__copy_fs_struct(struct fs_struct *old)
 {
 	struct fs_struct *fs = kmem_cache_alloc(fs_cachep, GFP_KERNEL);
 	/* We don't need to lock fs - think why ;-) */
--- linux-2.4.17-pre2/kernel/module.c	Thu Nov 22 23:02:59 2001
+++ linux-akpm/kernel/module.c	Fri Nov 30 21:21:26 2001
@@ -254,7 +254,7 @@ void __init init_modules(void)
  * Copy the name of a module from user space.
  */
 
-static inline long
+static long
 get_mod_name(const char *user_name, char **buf)
 {
 	unsigned long page;
--- linux-2.4.17-pre2/kernel/sched.c	Thu Nov 22 23:02:59 2001
+++ linux-akpm/kernel/sched.c	Fri Nov 30 21:21:26 2001
@@ -333,7 +333,7 @@ static inline void move_first_runqueue(s
  * "current->state = TASK_RUNNING" to mark yourself runnable
  * without the overhead of this.
  */
-static inline int try_to_wake_up(struct task_struct * p, int synchronous)
+static int try_to_wake_up(struct task_struct * p, int synchronous)
 {
 	unsigned long flags;
 	int success = 0;
@@ -449,7 +449,7 @@ signed long schedule_timeout(signed long
  * cleans up all remaining scheduler things, without impacting the
  * common case.
  */
-static inline void __schedule_tail(struct task_struct *prev)
+static void __schedule_tail(struct task_struct *prev)
 {
 #ifdef CONFIG_SMP
 	int policy;
@@ -698,7 +698,7 @@ same_process:
  * started to run but is not in state TASK_RUNNING.  try_to_wake_up() returns zero
  * in this (rare) case, and we handle it by contonuing to scan the queue.
  */
-static inline void __wake_up_common (wait_queue_head_t *q, unsigned int mode,
+static void __wake_up_common (wait_queue_head_t *q, unsigned int mode,
 			 	     int nr_exclusive, const int sync)
 {
 	struct list_head *tmp;
--- linux-2.4.17-pre2/kernel/signal.c	Thu Nov 22 23:02:59 2001
+++ linux-akpm/kernel/signal.c	Fri Nov 30 21:21:26 2001
@@ -630,8 +630,7 @@ kill_sl_info(int sig, struct siginfo *in
 	return retval;
 }
 
-inline int
-kill_proc_info(int sig, struct siginfo *info, pid_t pid)
+int kill_proc_info(int sig, struct siginfo *info, pid_t pid)
 {
 	int error;
 	struct task_struct *p;
--- linux-2.4.17-pre2/kernel/sys.c	Tue Sep 18 14:10:43 2001
+++ linux-akpm/kernel/sys.c	Fri Nov 30 21:21:26 2001
@@ -473,7 +473,7 @@ asmlinkage long sys_setgid(gid_t gid)
  * files..
  * Thanks to Olaf Kirch and Peter Benie for spotting this.
  */
-static inline void cap_emulate_setxuid(int old_ruid, int old_euid, 
+static void cap_emulate_setxuid(int old_ruid, int old_euid, 
 				       int old_suid)
 {
 	if ((old_ruid == 0 || old_euid == 0 || old_suid == 0) &&
--- linux-2.4.17-pre2/kernel/timer.c	Mon Oct  8 10:41:41 2001
+++ linux-akpm/kernel/timer.c	Fri Nov 30 21:21:26 2001
@@ -119,7 +119,7 @@ void init_timervecs (void)
 
 static unsigned long timer_jiffies;
 
-static inline void internal_add_timer(struct timer_list *timer)
+static void internal_add_timer(struct timer_list *timer)
 {
 	/*
 	 * must be cli-ed when calling this
--- linux-2.4.17-pre2/mm/filemap.c	Mon Nov 26 11:52:07 2001
+++ linux-akpm/mm/filemap.c	Fri Nov 30 21:21:26 2001
@@ -670,7 +670,7 @@ void add_to_page_cache_locked(struct pag
  * This adds a page to the page cache, starting out as locked,
  * owned by us, but unreferenced, not uptodate and with no errors.
  */
-static inline void __add_to_page_cache(struct page * page,
+static void __add_to_page_cache(struct page * page,
 	struct address_space *mapping, unsigned long offset,
 	struct page **hash)
 {
@@ -2220,7 +2220,7 @@ out:
 	return error;
 }
 
-static inline void setup_read_behavior(struct vm_area_struct * vma,
+static void setup_read_behavior(struct vm_area_struct * vma,
 	int behavior)
 {
 	VM_ClearReadHint(vma);
--- linux-2.4.17-pre2/mm/highmem.c	Mon Oct 22 15:01:57 2001
+++ linux-akpm/mm/highmem.c	Fri Nov 30 21:21:26 2001
@@ -233,7 +233,7 @@ static inline void copy_to_high_bh_irq (
 	__restore_flags(flags);
 }
 
-static inline void bounce_end_io (struct buffer_head *bh, int uptodate)
+static void bounce_end_io (struct buffer_head *bh, int uptodate)
 {
 	struct page *page;
 	struct buffer_head *bh_orig = (struct buffer_head *)(bh->b_private);
--- linux-2.4.17-pre2/mm/memory.c	Thu Nov 22 23:02:59 2001
+++ linux-akpm/mm/memory.c	Fri Nov 30 21:21:26 2001
@@ -864,7 +864,7 @@ int remap_page_range(unsigned long from,
  *
  * We hold the mm semaphore for reading and vma->vm_mm->page_table_lock
  */
-static inline void establish_pte(struct vm_area_struct * vma, unsigned long address, pte_t *page_table, pte_t entry)
+static void establish_pte(struct vm_area_struct * vma, unsigned long address, pte_t *page_table, pte_t entry)
 {
 	set_pte(page_table, entry);
 	flush_tlb_page(vma, address);
--- linux-2.4.17-pre2/mm/mmap.c	Mon Nov  5 21:01:12 2001
+++ linux-akpm/mm/mmap.c	Fri Nov 30 21:21:26 2001
@@ -107,7 +107,7 @@ static inline void __remove_shared_vm_st
 	}
 }
 
-static inline void remove_shared_vm_struct(struct vm_area_struct *vma)
+static void remove_shared_vm_struct(struct vm_area_struct *vma)
 {
 	lock_vma_mappings(vma);
 	__remove_shared_vm_struct(vma);
@@ -333,7 +333,7 @@ static void __vma_link(struct mm_struct 
 	__vma_link_file(vma);
 }
 
-static inline void vma_link(struct mm_struct * mm, struct vm_area_struct * vma, struct vm_area_struct * prev,
+static void vma_link(struct mm_struct * mm, struct vm_area_struct * vma, struct vm_area_struct * prev,
 			    rb_node_t ** rb_link, rb_node_t * rb_parent)
 {
 	lock_vma_mappings(vma);
--- linux-2.4.17-pre2/mm/slab.c	Fri Nov 30 14:32:15 2001
+++ linux-akpm/mm/slab.c	Fri Nov 30 21:21:26 2001
@@ -521,7 +521,7 @@ static inline void kmem_freepages (kmem_
 }
 
 #if DEBUG
-static inline void kmem_poison_obj (kmem_cache_t *cachep, void *addr)
+static void kmem_poison_obj (kmem_cache_t *cachep, void *addr)
 {
 	int size = cachep->objsize;
 	if (cachep->flags & SLAB_RED_ZONE) {
@@ -532,7 +532,7 @@ static inline void kmem_poison_obj (kmem
 	*(unsigned char *)(addr+size-1) = POISON_END;
 }
 
-static inline int kmem_check_poison_obj (kmem_cache_t *cachep, void *addr)
+static int kmem_check_poison_obj (kmem_cache_t *cachep, void *addr)
 {
 	int size = cachep->objsize;
 	void *end;
@@ -1219,7 +1219,7 @@ static inline void kmem_cache_alloc_head
 	}
 }
 
-static inline void * kmem_cache_alloc_one_tail (kmem_cache_t *cachep,
+static void * kmem_cache_alloc_one_tail (kmem_cache_t *cachep,
 						slab_t *slabp)
 {
 	void *objp;
@@ -1316,7 +1316,7 @@ void* kmem_cache_alloc_batch(kmem_cache_
 }
 #endif
 
-static inline void * __kmem_cache_alloc (kmem_cache_t *cachep, int flags)
+static void * __kmem_cache_alloc (kmem_cache_t *cachep, int flags)
 {
 	unsigned long save_flags;
 	void* objp;
@@ -1392,7 +1392,7 @@ alloc_new_slab_nolock:
 # define CHECK_PAGE(pg)	do { } while (0)
 #endif
 
-static inline void kmem_cache_free_one(kmem_cache_t *cachep, void *objp)
+static void kmem_cache_free_one(kmem_cache_t *cachep, void *objp)
 {
 	slab_t* slabp;
 
@@ -1452,7 +1452,7 @@ static inline void kmem_cache_free_one(k
 }
 
 #ifdef CONFIG_SMP
-static inline void __free_block (kmem_cache_t* cachep,
+static void __free_block (kmem_cache_t* cachep,
 							void** objpp, int len)
 {
 	for ( ; len > 0; len--, objpp++)
@@ -1471,7 +1471,7 @@ static void free_block (kmem_cache_t* ca
  * __kmem_cache_free
  * called with disabled ints
  */
-static inline void __kmem_cache_free (kmem_cache_t *cachep, void* objp)
+static void __kmem_cache_free (kmem_cache_t *cachep, void* objp)
 {
 #ifdef CONFIG_SMP
 	cpucache_t *cc = cc_data(cachep);
--- linux-2.4.17-pre2/drivers/block/ll_rw_blk.c	Mon Nov  5 21:01:11 2001
+++ linux-akpm/drivers/block/ll_rw_blk.c	Fri Nov 30 21:21:26 2001
@@ -420,7 +420,7 @@ void blk_init_queue(request_queue_t * q,
  * Get a free request. io_request_lock must be held and interrupts
  * disabled on the way in.
  */
-static inline struct request *get_request(request_queue_t *q, int rw)
+static struct request *get_request(request_queue_t *q, int rw)
 {
 	struct request *rq = NULL;
 	struct request_list *rl = q->rq + rw;
@@ -460,18 +460,6 @@ static struct request *__get_request_wai
 	return rq;
 }
 
-static inline struct request *get_request_wait(request_queue_t *q, int rw)
-{
-	register struct request *rq;
-
-	spin_lock_irq(&io_request_lock);
-	rq = get_request(q, rw);
-	spin_unlock_irq(&io_request_lock);
-	if (rq)
-		return rq;
-	return __get_request_wait(q, rw);
-}
-
 /* RO fail safe mechanism */
 
 static long ro_bits[MAX_BLKDEV][8];
@@ -497,7 +485,7 @@ void set_device_ro(kdev_t dev,int flag)
 	else ro_bits[major][minor >> 5] &= ~(1 << (minor & 31));
 }
 
-inline void drive_stat_acct (kdev_t dev, int rw,
+void drive_stat_acct (kdev_t dev, int rw,
 				unsigned long nr_sectors, int new_io)
 {
 	unsigned int major = MAJOR(dev);
@@ -546,7 +534,7 @@ static inline void add_request(request_q
 /*
  * Must be called with io_request_lock held and interrupts disabled
  */
-inline void blkdev_release_request(struct request *req)
+void blkdev_release_request(struct request *req)
 {
 	request_queue_t *q = req->q;
 	int rw = req->cmd;
--- linux-2.4.17-pre2/lib/rwsem.c	Tue Jul 10 20:08:51 2001
+++ linux-akpm/lib/rwsem.c	Fri Nov 30 21:21:26 2001
@@ -35,7 +35,7 @@ void rwsemtrace(struct rw_semaphore *sem
  * - the spinlock must be held by the caller
  * - woken process blocks are discarded from the list after having flags zeroised
  */
-static inline struct rw_semaphore *__rwsem_do_wake(struct rw_semaphore *sem)
+static struct rw_semaphore *__rwsem_do_wake(struct rw_semaphore *sem)
 {
 	struct rwsem_waiter *waiter;
 	struct list_head *next;
@@ -110,7 +110,7 @@ static inline struct rw_semaphore *__rws
 /*
  * wait for a lock to be granted
  */
-static inline struct rw_semaphore *rwsem_down_failed_common(struct rw_semaphore *sem,
+static struct rw_semaphore *rwsem_down_failed_common(struct rw_semaphore *sem,
 								 struct rwsem_waiter *waiter,
 								 signed long adjustment)
 {
--- linux-2.4.17-pre2/include/linux/fs_struct.h	Fri Jul 13 15:10:44 2001
+++ linux-akpm/include/linux/fs_struct.h	Fri Nov 30 21:21:26 2001
@@ -17,55 +17,10 @@ struct fs_struct {
 	NULL, NULL, NULL, NULL, NULL, NULL \
 }
 
-extern void exit_fs(struct task_struct *);
-extern void set_fs_altroot(void);
-
-/*
- * Replace the fs->{rootmnt,root} with {mnt,dentry}. Put the old values.
- * It can block. Requires the big lock held.
- */
-
-static inline void set_fs_root(struct fs_struct *fs,
-	struct vfsmount *mnt,
-	struct dentry *dentry)
-{
-	struct dentry *old_root;
-	struct vfsmount *old_rootmnt;
-	write_lock(&fs->lock);
-	old_root = fs->root;
-	old_rootmnt = fs->rootmnt;
-	fs->rootmnt = mntget(mnt);
-	fs->root = dget(dentry);
-	write_unlock(&fs->lock);
-	if (old_root) {
-		dput(old_root);
-		mntput(old_rootmnt);
-	}
-}
-
-/*
- * Replace the fs->{pwdmnt,pwd} with {mnt,dentry}. Put the old values.
- * It can block. Requires the big lock held.
- */
-
-static inline void set_fs_pwd(struct fs_struct *fs,
-	struct vfsmount *mnt,
-	struct dentry *dentry)
-{
-	struct dentry *old_pwd;
-	struct vfsmount *old_pwdmnt;
-	write_lock(&fs->lock);
-	old_pwd = fs->pwd;
-	old_pwdmnt = fs->pwdmnt;
-	fs->pwdmnt = mntget(mnt);
-	fs->pwd = dget(dentry);
-	write_unlock(&fs->lock);
-	if (old_pwd) {
-		dput(old_pwd);
-		mntput(old_pwdmnt);
-	}
-}
-
+void exit_fs(struct task_struct *);
+void set_fs_altroot(void);
+void set_fs_root(struct fs_struct *fs, struct vfsmount *mnt, struct dentry *dentry);
+void set_fs_pwd(struct fs_struct *fs, struct vfsmount *mnt, struct dentry *dentry);
 struct fs_struct *copy_fs_struct(struct fs_struct *old);
 void put_fs_struct(struct fs_struct *fs);
 
--- linux-2.4.17-pre2/fs/namespace.c	Fri Nov 30 14:32:14 2001
+++ linux-akpm/fs/namespace.c	Fri Nov 30 21:21:26 2001
@@ -398,6 +398,48 @@ out:
 }
 
 /*
+ * Replace the fs->{rootmnt,root} with {mnt,dentry}. Put the old values.
+ * It can block. Requires the big lock held.
+ */
+void set_fs_root(struct fs_struct *fs, struct vfsmount *mnt,
+			struct dentry *dentry)
+{
+	struct dentry *old_root;
+	struct vfsmount *old_rootmnt;
+	write_lock(&fs->lock);
+	old_root = fs->root;
+	old_rootmnt = fs->rootmnt;
+	fs->rootmnt = mntget(mnt);
+	fs->root = dget(dentry);
+	write_unlock(&fs->lock);
+	if (old_root) {
+		dput(old_root);
+		mntput(old_rootmnt);
+	}
+}
+
+/*
+ * Replace the fs->{pwdmnt,pwd} with {mnt,dentry}. Put the old values.
+ * It can block. Requires the big lock held.
+ */
+void set_fs_pwd(struct fs_struct *fs, struct vfsmount *mnt,
+			struct dentry *dentry)
+{
+	struct dentry *old_pwd;
+	struct vfsmount *old_pwdmnt;
+	write_lock(&fs->lock);
+	old_pwd = fs->pwd;
+	old_pwdmnt = fs->pwdmnt;
+	fs->pwdmnt = mntget(mnt);
+	fs->pwd = dget(dentry);
+	write_unlock(&fs->lock);
+	if (old_pwd) {
+		dput(old_pwd);
+		mntput(old_pwdmnt);
+	}
+}
+
+/*
  *	The 2.0 compatible umount. No flags. 
  */

Re: [PATCH] 2.5.21 - list.h cleanup

[Gerrit Huizenga]

In message <Pine.LNX.4.44.0206110128130.1987-100000@home.transmeta.com>, > : Li
nus Torvalds writes:
> 
> 
> On Tue, 11 Jun 2002, Rusty Russell wrote:
> >
> > Worst sin is that you can't predeclare typedefs.  For many uses (not the
> > list macros of course):
> > 	struct xx;
> > is sufficient and avoids the #include hell,
> 
> True.

Untrue.  Or partially true (yes, you *can* use struct xx;).

But you can also use:

typedef foo_t;

to predefine, just like you can use struct xx;

Also avoids some aspects of #include <hell>.

gerrit

Re: [PATCH] 2.5.21 - list.h cleanup

[Rusty Russell]

In message <E17HpqG-000454-00@w-gerrit2> you write:
> In message <Pine.LNX.4.44.0206110128130.1987-100000@home.transmeta.com>, > : 
Li
> nus Torvalds writes:
> > 
> > 
> > On Tue, 11 Jun 2002, Rusty Russell wrote:
> > >
> > > Worst sin is that you can't predeclare typedefs.  For many uses (not the
> > > list macros of course):
> > > 	struct xx;
> > > is sufficient and avoids the #include hell,
> > 
> > True.
> 
> Untrue.  Or partially true (yes, you *can* use struct xx;).
> 
> But you can also use:
> 
> typedef foo_t;

Huh?  In what language?  Try it with -Wall to see what you're really
doing here, and think about what happens when you put that in one
header, and the real typedef in another.

Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] 2.5.21 - list.h cleanup

[Gerrit Huizenga]

In message <E17Hz8A-0003oC-00@wagner.rustcorp.com.au>, > : Rusty Russell writes
:
> In message <E17HpqG-000454-00@w-gerrit2> you write:
> > In message <Pine.LNX.4.44.0206110128130.1987-100000@home.transmeta.com>, > : 
> Li
> > nus Torvalds writes:
> > > 
> > > 
> > > On Tue, 11 Jun 2002, Rusty Russell wrote:
> > > >
> > > > Worst sin is that you can't predeclare typedefs.  For many uses (not the
> > > > list macros of course):
> > > > 	struct xx;
> > > > is sufficient and avoids the #include hell,
> > > 
> > > True.
> > 
> > Untrue.  Or partially true (yes, you *can* use struct xx;).
> > 
> > But you can also use:
> > 
> > typedef foo_t;
> 
> Huh?  In what language?  Try it with -Wall to see what you're really
> doing here, and think about what happens when you put that in one
> header, and the real typedef in another.

I sit corrected.  Synapse must have misfired.  The only references
I see to incomplete typedefs in our code or in the ANSI spec are
related to:

typedef struct foo foo_t;

No, we didn't use gcc for our kernel but it was an ANSI compiler.
My test didn't use -Wall for gcc, duh...

I was thinking that incomplete typedefs would be okay as long as you
only used them in prototypes, but only struct */union * pointers
are guaranteed to be compatible, not int */struct * pointer, so
this couldn't have been of any use.

I'll be sure to consume one extra beer this weekend to kill off that
bad synapse.

gerrit

Re: [PATCH] 2.5.21 - list.h cleanup

[Manik Raina]

> The linux coding style _tends_ to avoid using typedefs. It's not a hard
> rule (and I did in fact apply this patch, since it otherwise looked fine),
> but it's fairly common to use an explicit "struct xxxx" instead of
> "xxxx_t".
> 


	Besides , if someone's browsing the code using 'gid' one would
	have to first discover where xxxx_t is defined and realise it's
	typedef'ed to struct _xxxx_t and then we'd start to find where
	the heck _xxxx_t is .

	I'm not saying code needs to be written keeping in mind ease
	with which we can run code browsing tools, but avoiding unnecessary
	typedefs can certainly help here.

Re: [PATCH] 2.5.21 - list.h cleanup

[Dan Aloni]

On Mon, Jun 10, 2002 at 10:02:21AM -0700, Linus Torvalds wrote:
> The linux coding style _tends_ to avoid using typedefs. It's not a hard
> rule (and I did in fact apply this patch, since it otherwise looked fine),
> but it's fairly common to use an explicit "struct xxxx" instead of
> "xxxx_t".

I agree. For a better future:

--- linux-2.5.21/Documentation/CodingStyle	Mon Jun 10 20:29:26 2002
+++ linux-2.5.21/Documentation/CodingStyle	Mon Jun 10 20:46:06 2002
@@ -264,3 +264,35 @@
 
 Remember: if another thread can find your data structure, and you don't
 have a reference count on it, you almost certainly have a bug.
+
+
+		Chapter 9: Typedefs
+
+You should avoid using typedefs. It's not a hard rule but it's fairly 
+common to use an explicit "struct xxxx" instead of "xxxx_t".
+
+Hiding the fact that it's a struct causes bad things if people don't
+realize it: allocating structs on the stack is something you should be
+aware of (and careful with), and passing them as parameters is is much
+better done explicitly as a "pointer to struct".
+
+There are _some_ exceptions. For example, "pte_t" etc might well be a
+struct on most architectures, and that's ok: it's expressly designed to be
+an opaque (and still fairly small) thing. This is an example of where
+there are clear _reasons_ for the abstraction, not just abstraction for
+its own sake.
+
+For example, some people like to do things like: 
+
+ /* BAD!!! */    typedef unsigned int counter_t;    /* BAD!!! */
+
+and then use "counter_t" all over the place. I think that's not just ugly,
+but stupid and counter-productive. It makes it much harder to do things
+like "printk()" portably, for example ("should I use %u, %l or just %d?"),
+and generally adds no value. It only _hides_ information, like whether the
+type is signed or not.
+
+There is nothing wrong with just using something like "unsigned long"
+directly, even if it is a few characters longer than you might like. And
+if you care about the number of bits, use "u32" or something. Don't make
+up useless types that have no added advantage.

-- 
Dan Aloni
da-x@gmx.net


P.S.
list_t was added between 2.5.1-pre9 and 2.5.1-pre10, when you applied 
Ingo's scalable scheduler code. Since then it gained a few users: device.h, 
fs.h, sched.h, and mm.h. It can be kicked out easily.

Re: [PATCH] 2.5.21 - list.h cleanup

[Thunder from the hill]

Hi,

On Mon, 10 Jun 2002, Dan Aloni wrote:
>  + replace __inline__ with inline.
>  + use list_t intead of struct list_head (no bytes we harmed, bla.. bla..)
>  + add the new list_move and list_move_tail mutators as inline functions.

Deja vu feeling...

It's already in the Linus tree, why do you resend it?!

Regards,
Thunder
-- 
German attitude becoming        |	Thunder from the hill at ngforever
rightaway popular:		|
       "Get outa my way,  	|	free inhabitant not directly
    for I got a mobile phone!"	|	belonging anywhere