ionice kills vanilla 2.6.0-test9 was [Re: [PATCH] cfq + io priorities (fwd)]

ionice kills vanilla 2.6.0-test9 was [Re: [PATCH] cfq + io priorities (fwd)]

[Pavel Machek]

Hi!

> > I'm attaching the simple ionice tool. It's used as follows:
> 
> Here's one that works, sorry about that. To compile:
> 
> # gcc -Wall -D__X86 -o ionice ionice.c
> 
> or other define for PPC or X86_64.

Well, did that, run it on vanilla kernel, and it kills the
machine. Can someone reproduce it?
							Pavel
[What is needed to start using cfq? Is your patch, this utility and
elevator=cfq enough?]

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <getopt.h>
#include <unistd.h>
#include <sys/ptrace.h>
#include <asm/unistd.h>

extern int sys_ioprio_set(int);
extern int sys_ioprio_get(void);

#ifdef __X86
#define __NR_ioprio_set		274
#define __NR_ioprio_get		275
#endif

#ifdef __X86_64
#define __NR_ioprio_set		237
#define __NR_ioprio_get		238
#endif

#ifdef __PPC
#define __NR_ioprio_set		255
#define __NR_ioprio_get		256
#endif

#ifndef __NR_ioprio_set
#error set arch
#endif

_syscall1(int, ioprio_set, int, ioprio);
_syscall0(int, ioprio_get);

int main(int argc, char *argv[])
{
	int ioprio = 2, set = 0;
	int c;

	while ((c = getopt(argc, argv, "+n:")) != EOF) {
		switch (c) {
		case 'n':
			ioprio = strtol(optarg, NULL, 10);
			set = 1;
			break;
		}
	}

	if (!set)
		printf("%d\n", ioprio_get());
	else if (argv[optind]) {
		if (ioprio_set(ioprio) == -1) {
			perror("ioprio_set");
			return 1;
		}

		execvp(argv[optind], &argv[optind]);
	}
	return 0;
}


----- End forwarded message -----

-- 
				Pavel
Written on sharp zaurus, because my Velo1 broke. If you have Velo you don't need...



-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: ionice kills vanilla 2.6.0-test9 was [Re: [PATCH] cfq + io priorities (fwd)]

[Jens Axboe]

On Fri, Nov 21 2003, Pavel Machek wrote:
> Hi!
> 
> > > I'm attaching the simple ionice tool. It's used as follows:
> > 
> > Here's one that works, sorry about that. To compile:
> > 
> > # gcc -Wall -D__X86 -o ionice ionice.c
> > 
> > or other define for PPC or X86_64.
> 
> Well, did that, run it on vanilla kernel, and it kills the
> machine. Can someone reproduce it?

I saw that on ppc too, btw, didn't trace it yet. But there definitely is
a problem with calling syscalls that don't exist.

> [What is needed to start using cfq? Is your patch, this utility and
> elevator=cfq enough?]

Yes, that is it. Your result may vary though, I have a newer cfq that's
almost ready for release that fixes varies performance problems.

-- 
Jens Axboe

Re: ionice kills vanilla 2.6.0-test9 was [Re: [PATCH] cfq + io priorities (fwd)]

[Benjamin Herrenschmidt]

> I saw that on ppc too, btw, didn't trace it yet. But there definitely is
> a problem with calling syscalls that don't exist.
> 
> > [What is needed to start using cfq? Is your patch, this utility and
> > elevator=cfq enough?]

We had an additional bug on PPC with signals above NR_signal that
paulus recent patches should have fixed

Ben.

Re: ionice kills vanilla 2.6.0-test9 was [Re: [PATCH] cfq + io priorities (fwd)]

[Linus Torvalds]

On Fri, 21 Nov 2003, Pavel Machek wrote:
> 
> Well, did that, run it on vanilla kernel, and it kills the
> machine. Can someone reproduce it?

Ok, looks like a binutils bug to me.

"nr_syscalls" is mis-assembled at least on x86. The code says

	nr_syscalls=(.-sys_call_table)/4

but if you actually look at the disassembly of entry.S, you'll notice that 
the constant used in the object file is the one without the division.

Just do "gdb vmlinux" and do "disassemble system_call" to see this. It 
says something like

	cmp    $0x448,%eax
	jae    syscall_badsys

and it _should_ use just 274 (0x112 rather than 0x448) on x86.

Now, I wonder if this has ever worked, of which binutils version broke 
it..

		Linus

Re: ionice kills vanilla 2.6.0-test9 was [Re: [PATCH] cfq + io priorities (fwd)]

[Linus Torvalds]

[ Trivial test input file included, and "bug-binutils" added to the CC ]

On Fri, 21 Nov 2003, Linus Torvalds wrote:
> 
> Just do "gdb vmlinux" and do "disassemble system_call" to see this. It 
> says something like
> 
> 	cmp    $0x448,%eax
> 	jae    syscall_badsys
> 
> and it _should_ use just 274 (0x112 rather than 0x448) on x86.
> 
> Now, I wonder if this has ever worked, of which binutils version broke 
> it..

Some looking around shows that it works correctly on a SuSE box with

	GNU assembler version 2.13.90.0.18 (i486-suse-linux) using BFD version 2.13.90.0.18 20030121 (SuSE Linux)
	GNU ld version 2.13.90.0.18 20030121 (SuSE Linux)

but is broken on RH Fedora core 1:

	GNU assembler version 2.14.90.0.6 (i386-redhat-linux) using BFD version 2.14.90.0.6 20030820
	GNU ld version 2.14.90.0.6 20030820

So it definitely _does_ work in some versions, and the bug appears to be 
new to binutils 2.14, with 2.13 doing the right thing.

You can trivially see if with a simple assembly file like

	start:
		.long 1,2,3,a
	a=(.-start)/4

where 2.13.90 as shipped by SuSE will get it right (and generate a list of
1,2,3,4), while 2.14.90 from Fedora core will generate 1,2,3,16.

		Linus

Re: ionice kills vanilla 2.6.0-test9 was [Re: [PATCH] cfq + io priorities (fwd)]

[Linus Torvalds]

On Fri, 21 Nov 2003, Linus Torvalds wrote:
> 
> So it definitely _does_ work in some versions, and the bug appears to be 
> new to binutils 2.14, with 2.13 doing the right thing.

It looks like we can work around it with this silly syntactic sugar.. Does 
this work for you?

		Linus

---
===== arch/i386/kernel/entry.S 1.69 vs edited =====
--- 1.69/arch/i386/kernel/entry.S	Wed Oct  1 06:53:17 2003
+++ edited/arch/i386/kernel/entry.S	Fri Nov 21 15:32:00 2003
@@ -49,6 +49,8 @@
 #include <asm/page.h>
 #include "irq_vectors.h"
 
+#define nr_syscalls ((syscall_table_size)/4)
+
 EBX		= 0x00
 ECX		= 0x04
 EDX		= 0x08
@@ -881,4 +883,4 @@
  	.long sys_fadvise64_64
 	.long sys_ni_syscall	/* sys_vserver */
 
-nr_syscalls=(.-sys_call_table)/4
+syscall_table_size=(.-sys_call_table)

Re: ionice kills vanilla 2.6.0-test9 was [Re: [PATCH] cfq + io priorities (fwd)]

[Jamie Lokier]

Linus Torvalds wrote:
> It looks like we can work around it with this silly syntactic sugar.. Does 
> this work for you?

Or try this: nr_syscalls=(.-sys_call_table)>>2

-- Jamie

Re: ionice kills vanilla 2.6.0-test9 was [Re: [PATCH] cfq + io priorities (fwd)]

[Alan Modra]

On Fri, Nov 21, 2003 at 03:05:23PM -0800, Linus Torvalds wrote:
> So it definitely _does_ work in some versions, and the bug appears to be 
> new to binutils 2.14, with 2.13 doing the right thing.
> 
> You can trivially see if with a simple assembly file like
> 
> 	start:
> 		.long 1,2,3,a
> 	a=(.-start)/4

Broken with http://sources.redhat.com/ml/binutils/2003-04/msg00412.html
and http://sources.redhat.com/ml/binutils/2003-04/msg00414.html.
That '/' is being treated as a start of line comment char, thus trimming
the rest of the line.

I think gas/app.c:do_scrub_chars is such an awful mess that it's
impossible to get right.  Needs to be tossed out and rewritten.  The
fundamental problem is that you can't track which component of an
assember input line you're preprocessing without more information on the
particular target syntax.  And most of the current complexity is just
for deciding whether to remove whitespace!  That at least needs to go.

For now, I'm reverting HJ's patches and including your testcase in
the gas testsuite.

gas/ChangeLog
	* app.c (do_scrub_chars): Revert 2003-04-23 and 2003-04-22.

gas/testsuite/ChangeLog
	* gas/i386/divide.s: New.
	* gas/i386/divide.d: New.
	* gas/i386/i386.exp (gas_32_check): Run it.

Index: app.c
===================================================================
RCS file: /cvs/src/src/gas/app.c,v
retrieving revision 1.26
diff -u -p -r1.26 app.c
--- app.c	21 Nov 2003 01:52:16 -0000	1.26
+++ app.c	22 Nov 2003 04:24:01 -0000
@@ -1308,13 +1308,11 @@ do_scrub_chars (int (*get) (char *, int)
 	  /* Some relatively `normal' character.  */
 	  if (state == 0)
 	    {
-	      if (IS_SYMBOL_COMPONENT (ch))
-		state = 11;	/* Now seeing label definition.  */
+	      state = 11;	/* Now seeing label definition.  */
 	    }
 	  else if (state == 1)
 	    {
-	      if (IS_SYMBOL_COMPONENT (ch))
-		state = 2;	/* Ditto.  */
+	      state = 2;	/* Ditto.  */
 	    }
 	  else if (state == 9)
 	    {
Index: testsuite/gas/i386/divide.d
===================================================================
RCS file: testsuite/gas/i386/divide.d
diff -N testsuite/gas/i386/divide.d
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gas/i386/divide.d	22 Nov 2003 04:41:09 -0000
@@ -0,0 +1,8 @@
+#objdump: -s
+#name: i386 divide
+
+.*: +file format .*
+
+Contents of section .*
+ 0000 01000000 02000000 03000000 04000000 .*
+ 0010 05000000 .*
Index: testsuite/gas/i386/divide.s
===================================================================
RCS file: testsuite/gas/i386/divide.s
diff -N testsuite/gas/i386/divide.s
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gas/i386/divide.s	22 Nov 2003 04:41:09 -0000
@@ -0,0 +1,4 @@
+start:
+ .long 1,2,3,a,b
+ a=(.-start)/4-1
+b=(.-start)/4
Index: testsuite/gas/i386/i386.exp
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/i386/i386.exp,v
retrieving revision 1.19
diff -u -p -r1.19 i386.exp
--- testsuite/gas/i386/i386.exp	23 Jun 2003 20:15:33 -0000	1.19
+++ testsuite/gas/i386/i386.exp	22 Nov 2003 04:41:09 -0000
@@ -57,6 +57,7 @@ if [expr ([istarget "i*86-*-*"] ||  [ist
     run_dump_test "pcrel"
     run_dump_test "sub"
     run_dump_test "prescott"
+    run_dump_test "divide"
 
     # PIC is only supported on ELF targets.
     if { ([istarget "*-*-elf*"] || [istarget "*-*-linux*"] )

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: ionice kills vanilla 2.6.0-test9 was [Re: [PATCH] cfq + io priorities (fwd)]

[Nick Clifton]

Hi Linus,

> You can trivially see if with a simple assembly file like
>
> 	start:
> 		.long 1,2,3,a
> 	a=(.-start)/4
>
> where 2.13.90 as shipped by SuSE will get it right (and generate a
> list of 1,2,3,4), while 2.14.90 from Fedora core will generate
> 1,2,3,16.

It appears that the 2.14.90.0.6 release of binutils used with the
Fedora code needs the patch below applied.  This patch has been
committed to the 2_14 branch and the mainline binutils sources.

Cheers
        Nick Clifton
        Binutils Maintainer

-----------------------------------------------------------------------
gas/ChangeLog
2003-04-30  Alan Modra  <amodra@bigpond.net.au>

	* app.c (do_scrub_chars): Revert 2003-04-23 and 2003-04-22 changes.

Index: gas/app.c
===================================================================
RCS file: /repositories/repositories/sourceware/src/gas/app.c,v
retrieving revision 1.23
diff -c -3 -p -r1.23 app.c
*** gas/app.c	23 Apr 2003 17:51:41 -0000	1.23
--- gas/app.c	26 Nov 2003 20:02:32 -0000
*************** do_scrub_chars (get, tostart, tolen)
*** 1294,1300 ****
  	    }
  	  else if (state == 1)
  	    {
- 	      if (IS_SYMBOL_COMPONENT (ch))
  		state = 2;	/* Ditto.  */
  	    }
  	  else if (state == 9)
--- 1294,1299 ----

Re: ionice kills vanilla 2.6.0-test9 was [Re: [PATCH] cfq + io priorities (fwd)]

[Jakub Jelinek]

On Wed, Nov 26, 2003 at 08:06:00PM +0000, Nick Clifton wrote:
> Hi Linus,
> 
> > You can trivially see if with a simple assembly file like
> >
> > 	start:
> > 		.long 1,2,3,a
> > 	a=(.-start)/4
> >
> > where 2.13.90 as shipped by SuSE will get it right (and generate a
> > list of 1,2,3,4), while 2.14.90 from Fedora core will generate
> > 1,2,3,16.
> 
> It appears that the 2.14.90.0.6 release of binutils used with the
> Fedora code needs the patch below applied.  This patch has been
> committed to the 2_14 branch and the mainline binutils sources.

That's not the only change needed.
https://www.redhat.com/archives/fedora-test-list/2003-November/msg01380.html

	Jakub