Skip to contentSkip to conversation

softirq bugs in pre2

PATCH Andrea Arcangeli 13 participants 20 messages Jun 11, 2001 – Jun 15, 2001

lore
Share & tools
lore

Andrea Arcangeli

author
#0 lore raw

in pre2 ret_from_exception reschedule and handle signals even if it is returning to kernel code. rescheduling was not so obviously broken (at least not for the page fault ;) but it just slowdown the code as I just have all the necessary explicit checks in copy-user itself (which are needed anyways exactly because there may not be any page fault).

arch/i386/kernel/entry.S +6 -11
diff -urN 2.4.6pre2/arch/i386/kernel/entry.S softirq-smp-fixes/arch/i386/kernel/entry.S
@@ -246,16 +246,9 @@
 	jmp ret_from_sys_call
 
 	ALIGN
-ret_from_exception:
-	cli
-	cmpl $0,need_resched(%ebx)
-	jne reschedule
-	cmpl $0,sigpending(%ebx)
-	jne signal_return
-	jmp restore_all
-
 ENTRY(ret_from_intr)
 	GET_CURRENT(%ebx)
+ret_from_exception:
 	movl EFLAGS(%esp),%eax		# mix EFLAGS and CS
 	movb CS(%esp),%al
 	testl $(VM_MASK | 3),%eax	# return to VM86 mode or non-supervisor?

@@ -313,14 +306,16 @@
 	pushl $-1		# mark this as an int
 	SAVE_ALL
 	GET_CURRENT(%ebx)
-	pushl $ret_from_exception
 	movl %cr0,%eax
 	testl $0x4,%eax			# EM (math emulation bit)
-	je SYMBOL_NAME(math_state_restore)
+	jne device_not_available_emulate
+	call SYMBOL_NAME(math_state_restore)
+	jmp ret_from_exception
+device_not_available_emulate:
 	pushl $0		# temporary storage for ORIG_EIP
 	call  SYMBOL_NAME(math_emulate)
 	addl $4,%esp
-	ret
+	jmp ret_from_exception
 
 ENTRY(debug)
 	pushl $0

(side note, I also removed the call prediction invalidation so fpu restore is faster)

Since I mentioned the copy-user latency fixes (even if offtopic with the above) this is the URL for trivial merging:

	ftp://ftp.us.kernel.org/pub/linux/kernel/people/andrea/kernels/v2.4/2.4.5aa3/00_copy-user-lat-2

00_copy-user-lat-2:

arch/i386/lib/usercopy.c +4 -0
diff -urN 2.4.4/arch/i386/lib/usercopy.c lowlat/arch/i386/lib/usercopy.c
@@ -6,6 +6,7 @@
  * Copyright 1997 Linus Torvalds
  */
 #include <linux/config.h>
+#include <linux/condsched.h>
 #include <asm/uaccess.h>
 #include <asm/mmx.h>
 

@@ -91,6 +92,7 @@
 		  "=&D" (__d2)						   \
 		: "i"(-EFAULT), "0"(count), "1"(count), "3"(src), "4"(dst) \
 		: "memory");						   \
+	conditional_schedule();						   \
 } while (0)
 
 long

@@ -134,6 +136,7 @@
 		".previous"						\
 		: "=&c"(size), "=&D" (__d0)				\
 		: "r"(size & 3), "0"(size / 4), "1"(addr), "a"(0));	\
+	conditional_schedule();						\
 } while (0)
 
 unsigned long

@@ -180,5 +183,6 @@
 		:"=r" (n), "=D" (s), "=a" (res), "=c" (tmp)
 		:"0" (n), "1" (s), "2" (0), "3" (mask)
 		:"cc");
+	conditional_schedule();
 	return res & mask;
 }
include/asm-alpha/uaccess.h +34 -6
diff -urN 2.4.4/include/asm-alpha/uaccess.h lowlat/include/asm-alpha/uaccess.h
@@ -3,6 +3,7 @@
 
 #include <linux/errno.h>
 #include <linux/sched.h>
+#include <linux/condsched.h>
 
 
 /*

@@ -384,19 +385,36 @@
 	return len;
 }
 
-#define __copy_to_user(to,from,n)   __copy_tofrom_user_nocheck((to),(from),(n))
-#define __copy_from_user(to,from,n) __copy_tofrom_user_nocheck((to),(from),(n))
+static inline long
+__copy_to_user(void *to, const void *from, long n)
+{
+	n = __copy_tofrom_user_nocheck(to, from, n);
+	conditional_schedule();
+	return n;
+}
+
+static inline long
+__copy_from_user(void *to, const void *from, long n)
+{
+	n = __copy_tofrom_user_nocheck(to, from, n);
+	conditional_schedule();
+	return n;
+}
 
 extern inline long
 copy_to_user(void *to, const void *from, long n)
 {
-	return __copy_tofrom_user(to, from, n, to);
+	n = __copy_tofrom_user(to, from, n, to);
+	conditional_schedule();
+	return n;
 }
 
 extern inline long
 copy_from_user(void *to, const void *from, long n)
 {
-	return __copy_tofrom_user(to, from, n, from);
+	n = __copy_tofrom_user(to, from, n, from);
+	conditional_schedule();
+	return n;
 }
 
 extern void __do_clear_user(void);

@@ -434,6 +452,7 @@
 			: "$1","$2","$3","$4","$5","$28","memory");
 		len = __cl_len;
 	}
+	conditional_schedule();
 	return len;
 }
 

@@ -448,6 +467,7 @@
 	long ret = -EFAULT;
 	if (__access_ok((long)from, 0, get_fs()))
 		ret = __strncpy_from_user(to, from, n);
+	conditional_schedule();
 	return ret;
 }
 

@@ -456,7 +476,11 @@
 
 extern inline long strlen_user(const char *str)
 {
-	return access_ok(VERIFY_READ,str,0) ? __strlen_user(str) : 0;
+	long ret = 0;
+	if (access_ok(VERIFY_READ,str,0))
+		ret = __strlen_user(str);
+	conditional_schedule();
+	return ret;
 }
 
 /* Returns: 0 if exception before NUL or reaching the supplied limit (N),

@@ -465,7 +489,11 @@
 
 extern inline long strnlen_user(const char *str, long n)
 {
-	return access_ok(VERIFY_READ,str,0) ? __strnlen_user(str, n) : 0;
+	long ret = 0;
+	if (access_ok(VERIFY_READ,str,0))
+		ret = __strnlen_user(str, n);
+	conditional_schedule();
+	return ret;
 }
 
 /*
include/asm-i386/uaccess.h +5 -0
diff -urN 2.4.4/include/asm-i386/uaccess.h lowlat/include/asm-i386/uaccess.h
@@ -6,6 +6,7 @@
  */
 #include <linux/config.h>
 #include <linux/sched.h>
+#include <linux/condsched.h>
 #include <asm/page.h>
 
 #define VERIFY_READ 0

@@ -258,6 +259,7 @@
 		: "=&c"(size), "=&D" (__d0), "=&S" (__d1)		\
 		: "r"(size & 3), "0"(size / 4), "1"(to), "2"(from)	\
 		: "memory");						\
+	conditional_schedule();						\
 } while (0)
 
 #define __copy_user_zeroing(to,from,size)				\

@@ -286,6 +288,7 @@
 		: "=&c"(size), "=&D" (__d0), "=&S" (__d1)		\
 		: "r"(size & 3), "0"(size / 4), "1"(to), "2"(from)	\
 		: "memory");						\
+	conditional_schedule();						\
 } while (0)
 
 /* We let the __ versions of copy_from/to_user inline, because they're often

@@ -326,6 +329,7 @@
 			: "=c"(size), "=&S" (__d0), "=&D" (__d1)\
 			: "1"(from), "2"(to), "0"(size/4)	\
 			: "memory");				\
+		conditional_schedule();				\
 		break;						\
 	case 1:							\
 		__asm__ __volatile__(				\

@@ -416,6 +420,7 @@
 			: "=c"(size), "=&S" (__d0), "=&D" (__d1)\
 			: "1"(from), "2"(to), "0"(size/4)	\
 			: "memory");				\
+		conditional_schedule();				\
 		break;						\
 	case 1:							\
 		__asm__ __volatile__(				\
include/linux/condsched.h +14 -0
diff -urN 2.4.4/include/linux/condsched.h lowlat/include/linux/condsched.h
@@ -0,0 +1,14 @@
+#ifndef _LINUX_CONDSCHED_H
+#define _LINUX_CONDSCHED_H
+
+#ifndef __ASSEMBLY__
+#define conditional_schedule() \
+do { \
+	if (current->need_resched) { \
+		current->state = TASK_RUNNING; \
+		schedule(); \
+	} \
+} while(0)
+#endif
+
+#endif

Andrea

#1 lore raw
Quoted 3 lines · Andrea Arcangeli

Since I mentioned the copy-user latency fixes (even if offtopic with the above) this is the URL for trivial merging:

The copy-user latency fixes only make sense for out-of-line copies. If we're going to have a conditional function call to "schedule()", we do not want to inline the dang thing any more - we've just destroyed our register set etc anyway.

Linus

#2 lore raw
Quoted 4 lines

The copy-user latency fixes only make sense for out-of-line copies. If we're going to have a conditional function call to "schedule()", we do not want to inline the dang thing any more - we've just destroyed our register set etc anyway.

Is there any reason we still inline so many copy_* and put/get user calls. I can't benchmark a difference out of line and the code shrinkage is more than measurable (AMD Athlon 550 and AMD Athlon 1GHz)

#3 lore raw

Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

In reply to Linus Torvalds

How good is the linux kernel driver for the Intel gigabit Ethernet NIC (copper) with the TL82543GC chipset? The card says it's a "PRO/1000T" server adapter, and it looks like the part number A19845-003.

The sales guy who is promoting it says this is apparently a new card and he claims he can get specs from engineering. Not sure about NDA status.

So my question is... is it worth calling this guy's bluff?

Shawn.

#4 lore raw

Re: Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

Quoted 2 lines

The sales guy who is promoting it says this is apparently a new card and he claims he can get specs from engineering.

Rotfl. Nobody has been able to get remotely useful docs out of intel on their 1Gbit ethernet.

Alan

#5 lore raw

Re: Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

Hi Shawn.

Quoted 5 lines · Alan Cox
Quoted 2 lines

The sales guy who is promoting it says this is apparently a new card and he claims he can get specs from engineering.

Rotfl. Nobody has been able to get remotely useful docs out of Intel on their 1Gbit ethernet.

Shawn, I'd suggest you tell the said sales guy that IF he can get you the FULL specs TOGETHER WITH permission to freely distribute them, AND "a friend of yours who knows how to read such specs" confirms that they are indeed FULL specs, then you'll consider buying some of the cards, but if not, you're not interested...

Best wishes from Riley.

#6 lore raw

Re: Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

Quoted 2 lines · Riley Williams

Shawn, I'd suggest you tell the said sales guy that IF he can get you the FULL specs TOGETHER WITH permission to freely distribute them, AND

Permission to freely distribute the specs isn't necessary, although it is nice indeed. All that's needed is permission to GPL the driver sources written using knowledge from said specs.

Ion

Signature · 2 lines
  It is better to keep your mouth shut and be thought a fool,
            than to open it and remove all doubt.
#7 lore raw

Re: Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

Hi Ion.

Quoted 7 lines
Quoted 3 lines

Shawn, I'd suggest you tell the said sales guy that IF he can get you the FULL specs TOGETHER WITH permission to freely distribute them...

Permission to freely distribute the specs isn't necessary, although it is nice indeed. All that's needed is permission to GPL the driver sources written using knowledge from said specs.

That presupposes that the person they give the specs to is the person writing the driver. I don't remember shawn offering to write a driver or anything approaching that.

As I see it, if Shawn has permission to freely distribute the specs, he can send a copy to Alan Cox for forwarding to the relevant driver developers. However, if he has to sign an NDA to get them, they're useless...

Alan: Am I right in assuming this?

Best wishes from Riley.

#8 lore raw

Re: Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

Quoted 15 lines · Riley Williams

Hi Ion.

Quoted 7 lines
Quoted 3 lines

Shawn, I'd suggest you tell the said sales guy that IF he can get you the FULL specs TOGETHER WITH permission to freely distribute them...

Permission to freely distribute the specs isn't necessary, although it is nice indeed. All that's needed is permission to GPL the driver sources written using knowledge from said specs.

That presupposes that the person they give the specs to is the person writing the driver. I don't remember shawn offering to write a driver or anything approaching that.

Quoted 6 lines

As I see it, if Shawn has permission to freely distribute the specs, he can send a copy to Alan Cox for forwarding to the relevant driver developers. However, if he has to sign an NDA to get them, they're useless...

Alan: Am I right in assuming this?

If you can find specs, I have tested cards and help write a driver...

Jeff

Signature · 3 lines
Jeff Garzik      | Andre the Giant has a posse.
Building 1024    |
MandrakeSoft     |
#9 lore raw

Re: Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

In reply to Riley Williams

Sorry for jumping in late in this thread....

With respect to Intel- I've had a number of meetings with these folks on just this topic- mostly trying to work these issues from the *BSD side of our house.

Basically- as far as I understand what they've said, is that they're trying to figure out how they can:

Formulate a different NDA than they currently have set up for companies. This NDA means that they can release documents to engineers writing software *and* allow the engineers to release a driver under some kind of Open Source- *possibly* with a feature set less than that described in the documents.

My personal estimation is that they're serious about trying to do this. As the engineers I've talked to have put it (roughly), "The Intel IP is *not* in the )!*$)!*$!)*$ Chip APIs!". This will probably take a Very Long Time for them to implement though. They may not, ultimately, be able to figure it out. The jury's still out.

If any of you want to talk to the Intel folks who are dealing with this (at least from the marketing side of the Lan group), contact me and I can try and put you in touch with those people.

-matt

Quoted 29 lines · Riley Williams

Hi Ion.

Quoted 7 lines
Quoted 3 lines

Shawn, I'd suggest you tell the said sales guy that IF he can get you the FULL specs TOGETHER WITH permission to freely distribute them...

Permission to freely distribute the specs isn't necessary, although it is nice indeed. All that's needed is permission to GPL the driver sources written using knowledge from said specs.

That presupposes that the person they give the specs to is the person writing the driver. I don't remember shawn offering to write a driver or anything approaching that.

As I see it, if Shawn has permission to freely distribute the specs, he can send a copy to Alan Cox for forwarding to the relevant driver developers. However, if he has to sign an NDA to get them, they're useless...

Alan: Am I right in assuming this?

Best wishes from Riley.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#10 lore raw

Re: Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

In reply to Riley Williams
Quoted 6 lines

As I see it, if Shawn has permission to freely distribute the specs, he can send a copy to Alan Cox for forwarding to the relevant driver developers. However, if he has to sign an NDA to get them, they're useless...

Alan: Am I right in assuming this?

It depends on the NDA. Lots of vendors NDA their docs because they give enough info to cloen the hardware, or explain how to fab the chip, or include personal details of employees .. and a million other reasons. Many sensible, many quite daft.

But in my experience you have a better chance of getting a straight answer out of a politician than intels networking folks. Maybe they have reformed

Alan

#11 lore raw

Re: Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

In reply to Riley Williams

I know, jumping in at this late-stage is bad form... but if we're talking about the Intel 82543GC Gigabit MAC, why doesn't someone just use the FreeBSD if_wx.c driver as a starting point?

It took me a while to find, as they refer to it as the LIVENGOOD instead of the 82543, but the PCI ProductID values seem to match...

Matt

Quoted 22 lines · Alan Cox
Quoted 6 lines

As I see it, if Shawn has permission to freely distribute the specs, he can send a copy to Alan Cox for forwarding to the relevant driver developers. However, if he has to sign an NDA to get them, they're useless...

Alan: Am I right in assuming this?

It depends on the NDA. Lots of vendors NDA their docs because they give enough info to cloen the hardware, or explain how to fab the chip, or include personal details of employees .. and a million other reasons. Many sensible, many quite daft.

But in my experience you have a better chance of getting a straight answer out of a politician than intels networking folks. Maybe they have reformed

Alan

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 6 lines
Matthew Dharm                              Home: [email protected] 
Maintainer, Linux USB Mass Storage Driver

It was a new hope.
					-- Dust Puppy
User Friendly, 12/25/1998
#12 lore raw

Re: Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

Quoted 3 lines

I know, jumping in at this late-stage is bad form... but if we're talking about the Intel 82543GC Gigabit MAC, why doesn't someone just use the FreeBSD if_wx.c driver as a starting point?

Intel have released a BSD like licensed gig-E driver for Linux. It needs a serious cleanup but the hardware interface is very very clean so that is a doable job. There are some patent related licensing issues but the discussion on resolving that and getting them into the kernel does look promising

#13 lore raw

Re: Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

In reply to Matthew Dharm
Quoted 8 lines · Matthew Dharm

I know, jumping in at this late-stage is bad form... but if we're talking about the Intel 82543GC Gigabit MAC, why doesn't someone just use the FreeBSD if_wx.c driver as a starting point?

It took me a while to find, as they refer to it as the LIVENGOOD instead of the 82543, but the PCI ProductID values seem to match...

Matt

Let me stir the pot a bit and report that the Intel driver folks (yes, they *do* have them) claim to have a driver that blows the doors off of wx. I told them that with their access to docs and to actual chip engineers that if it *didn't* do so, they really were in sad shape.

-matt

#14 lore raw

Re: Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

In reply to Riley Williams

No word yet. I would be extremely surprised to get the docs WITHOUT an NDA or so. It never hurts to try though :-)

I wouldn't write the driver no, my C skills aren't very good and lack of kernel API knowledge would render that idea dead ;-)

Shawn.

Quoted 25 lines · Riley Williams

Hi Ion.

Quoted 7 lines
Quoted 3 lines

Shawn, I'd suggest you tell the said sales guy that IF he can get you the FULL specs TOGETHER WITH permission to freely distribute them...

Permission to freely distribute the specs isn't necessary, although it is nice indeed. All that's needed is permission to GPL the driver sources written using knowledge from said specs.

That presupposes that the person they give the specs to is the person writing the driver. I don't remember shawn offering to write a driver or anything approaching that.

As I see it, if Shawn has permission to freely distribute the specs, he can send a copy to Alan Cox for forwarding to the relevant driver developers. However, if he has to sign an NDA to get them, they're useless...

Alan: Am I right in assuming this?

Best wishes from Riley.

#15 lore raw

Re: Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

In reply to Riley Williams
Quoted 7 lines · Ion Badulescu

Date: Wed, 13 Jun 2001 03:25:22 -0700
From: Ion Badulescu <[email protected]>
To: Riley Williams <[email protected]>
Cc: Shawn Starr <[email protected]>, <[email protected]>,
Alan Cox <[email protected]>
Subject: Re: Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

Quoted 8 lines
Quoted 2 lines · Riley Williams

Shawn, I'd suggest you tell the said sales guy that IF he can get you the FULL specs TOGETHER WITH permission to freely distribute them, AND

Permission to freely distribute the specs isn't necessary, although it is nice indeed. All that's needed is permission to GPL the driver sources written using knowledge from said specs.

Which would still be a problem. You then have a GPL'ed driver which still cannot be sanely modified in the way the GPL would like to guarantee.

Ralf

#16 lore raw

Re: Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

Quoted 13 lines · Ralf Baechle
Quoted 12 lines · Ion Badulescu

Date: Wed, 13 Jun 2001 03:25:22 -0700
From: Ion Badulescu <[email protected]>
To: Riley Williams <[email protected]>
Cc: Shawn Starr <[email protected]>, <[email protected]>,
Alan Cox <[email protected]>
Subject: Re: Gigabit Intel NIC? - Intel Gigabit Ethernet Pro/1000T

Quoted 2 lines · Riley Williams

Shawn, I'd suggest you tell the said sales guy that IF he can get you the FULL specs TOGETHER WITH permission to freely distribute them, AND

Quoted 6 lines
Quoted 3 lines

Permission to freely distribute the specs isn't necessary, although it is nice indeed. All that's needed is permission to GPL the driver sources written using knowledge from said specs.

Which would still be a problem. You then have a GPL'ed driver which still cannot be sanely modified in the way the GPL would like to guarantee.

That isn't a problem - the GPL doesn't attempt to guarantee users the INFORMATION needed to make sane changes, just that they have the facility to do so. Just as the kernel doesn't come with a copy of the POSIX specs, the RFCs, etc. - some of the standards the kernel implements aren't available publicly, but that doesn't stop the kernel being freely modifiable!

James.

author
#17 lore raw
In reply to Andrea Arcangeli
Quoted 4 lines · Linus Torvalds

The copy-user latency fixes only make sense for out-of-line copies. If we're going to have a conditional function call to "schedule()", we do not want to inline the dang thing any more - we've just destroyed our register set etc anyway.

I think we should override conditional_schedule() with asm, ala local_bh_enable in pre2, then it should be ok to left the copies inline (btw I found more bugs in the softirq stuff in pre2, I will keep sorting it out later today or tomorrow).

Andrea

#18 lore raw
In reply to Andrea Arcangeli
Quoted 10 lines · Linus Torvalds
Quoted 3 lines · Andrea Arcangeli

Since I mentioned the copy-user latency fixes (even if offtopic with the above) this is the URL for trivial merging:

The copy-user latency fixes only make sense for out-of-line copies. If we're going to have a conditional function call to "schedule()", we do not want to inline the dang thing any more - we've just destroyed our register set etc anyway.

It's overkill. This adds many hundreds of scheduling points to the kernel, of which we need only five. It makes more sense to simply open-code those five.

- generic_file_read/write
- read /dev/zero, /dev/mem
- memcpy_to_iovec()

This will by no means provide a low-latency kernel, but it will fix the most common causes of poor interactivity in normal use.

Just doing generic_file_read/write would suffice, actually.

#19 lore raw
In reply to Andrea Arcangeli

Hi!

Quoted 8 lines

diff -urN 2.4.4/include/linux/condsched.h lowlat/include/linux/condsched.h

include/linux/condsched.h +4 -0
@@ -0,0 +1,14 @@
+#ifndef _LINUX_CONDSCHED_H
+#define _LINUX_CONDSCHED_H
+
+#ifndef __ASSEMBLY__

include/linux/condsched.h line 14

Quoted 10 lines
include/linux/condsched.h +10 -0
+#define conditional_schedule() \
+do { \
+	if (current->need_resched) { \
+		current->state = TASK_RUNNING; \
+		schedule(); \
+	} \
+} while(0)
+#endif
+
+#endif

I do not think that conditional schedule is worth its own header file. I see you want to potentiatlly replace it with assembly, but can't that be done in, say, system.h too? [Ok, you'd have to create linux/system.h, but that can be called cleanup.]
Pavel

Signature · 2 lines
I'm [email protected]. "In my country we have almost anarchy and I don't care."
Panos Katsaloulis describing me w.r.t. patents at [email protected]