2003-11-21 19:54:37

by Pavel Machek

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

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?]


2003-11-21 20:14:36

by Jens Axboe

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

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

2003-11-21 22:45:43

by Linus Torvalds

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


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

2003-11-21 23:05:37

by Linus Torvalds

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


[ 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


2003-11-21 23:33:16

by Linus Torvalds

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


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)

2003-11-22 04:42:27

by Alan Modra

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

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

2003-11-22 11:04:27

by Jamie Lokier

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

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

2003-11-24 03:44:45

by Benjamin Herrenschmidt

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


> 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.


2003-11-26 20:09:33

by Nick Clifton

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

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 <[email protected]>

* 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 ----

2003-11-26 20:24:19

by Jakub Jelinek

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

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