Skip to contentSkip to conversation

tip.git regression from "vsprintf: unify the format decoding layer for its 3 users"

DISCUSSION Jeremy Fitzhardinge 4 participants 11 messages Mar 14, 2009 – Mar 16, 2009

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Change fef20d9c1380f04ba9492d6463148db07b413708, "vsprintf: unify the format decoding layer for its 3 users", causes a regression in xenbus which results in no devices getting attached to a new domain. Reverting fef20d9c1380f04ba9492d6463148db07b413708 and 39e874f8afbdb3745e2406ce4ecbde9ac4cbaa78 fixes the problem.

I haven't identified what format string is being handled wrongly, so I don't know what the precise bug is. The most complex looking format in use seems to be %.*s; there's also "%s/%s", "%i" and "%lX".

J

#1 lore raw

Two PV DomUs (CentOS 5.2 , F10) have been tested with new kernel
During shutdown of any one of mentioned DomUs usual sequence of daemon's messages drops into stack trace with following messages at the end:-

xenbus_dev_shutdown:   device/vif/0 timeout closing device xenbus_dev_shutdown:   device/vbd/51712 timeout closing device System halted.

After that Xen Host stop responding at all, just dies.

Boris

--- On Sat, 3/14/09, Jeremy Fitzhardinge <[email protected]> wrote:
From: Jeremy Fitzhardinge <[email protected]>
Subject: [Xen-devel] tip.git regression from "vsprintf: unify the format decoding layer for its 3 users"
To: "Frederic Weisbecker" <[email protected]>
Cc: "Xen-devel" <[email protected]>, "Ingo Molnar" <[email protected]>, "the arch/x86 maintainers" <[email protected]>, "Linux Kernel Mailing List" <[email protected]>
Date: Saturday, March 14, 2009, 3:04 AM

Change fef20d9c1380f04ba9492d6463148db07b413708, "vsprintf: unify the format decoding layer for its 3 users", causes a regression in xenbus which results in no devices getting attached to a new domain. Reverting fef20d9c1380f04ba9492d6463148db07b413708 and 39e874f8afbdb3745e2406ce4ecbde9ac4cbaa78 fixes the problem.

I haven't identified what format string is being handled wrongly, so I don't know what the precise bug is. The most complex looking format in use seems to be %.*s; there's also "%s/%s", "%i" and
"%lX".

J

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#2 lore raw
In reply to Jeremy Fitzhardinge

2009/3/14 Jeremy Fitzhardinge <[email protected]>:

Quoted 9 lines

Change fef20d9c1380f04ba9492d6463148db07b413708, "vsprintf: unify the format decoding layer for its 3 users", causes a regression in xenbus which results in no devices getting attached to a new domain. Reverting fef20d9c1380f04ba9492d6463148db07b413708 and 39e874f8afbdb3745e2406ce4ecbde9ac4cbaa78 fixes the problem.

I haven't identified what format string is being handled wrongly, so I don't know what the precise bug is. The most complex looking format in use seems to be %.*s; there's also "%s/%s", "%i" and "%lX".

Hi,

At least %.*s seems to be broken. How about this patch?

Vegard

lib/vsprintf.c +7 -7
index dc16743..be3001f 100644
@@ -398,7 +398,7 @@ static noinline char* put_dec(char *buf, unsigned long long num)
 
 enum format_type {
 	FORMAT_TYPE_NONE, /* Just a string part */
-	FORMAT_TYPE_WITDH,
+	FORMAT_TYPE_WIDTH,
 	FORMAT_TYPE_PRECISION,
 	FORMAT_TYPE_CHAR,
 	FORMAT_TYPE_STR,
@@ -770,7 +770,7 @@ static int format_decode(const char *fmt, struct printf_spec *spec)
 	const char *start = fmt;
 
 	/* we finished early by reading the field width */
-	if (spec->type == FORMAT_TYPE_WITDH) {
+	if (spec->type == FORMAT_TYPE_WIDTH) {
 		if (spec->field_width < 0) {
 			spec->field_width = -spec->field_width;
 			spec->flags |= LEFT;
@@ -828,7 +828,7 @@ static int format_decode(const char *fmt, struct printf_spec *spec)
 		spec->field_width = skip_atoi(&fmt);
 	else if (*fmt == '*') {
 		/* it's the next argument */
-		spec->type = FORMAT_TYPE_WITDH;
+		spec->type = FORMAT_TYPE_WIDTH;
 		return ++fmt - start;
 	}
 
@@ -843,7 +843,7 @@ precision:
 				spec->precision = 0;
 		} else if (*fmt == '*') {
 			/* it's the next argument */
-			spec->type = FORMAT_TYPE_WITDH;
+			spec->type = FORMAT_TYPE_PRECISION;
 			return ++fmt - start;
 		}
 	}
@@ -1002,7 +1002,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
 			break;
 		}
 
-		case FORMAT_TYPE_WITDH:
+		case FORMAT_TYPE_WIDTH:
 			spec.field_width = va_arg(args, int);
 			break;
 
@@ -1306,7 +1306,7 @@ do {									\
 		case FORMAT_TYPE_NONE:
 			break;
 
-		case FORMAT_TYPE_WITDH:
+		case FORMAT_TYPE_WIDTH:
 		case FORMAT_TYPE_PRECISION:
 			save_arg(int);
 			break;
@@ -1472,7 +1472,7 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
 			break;
 		}
 
-		case FORMAT_TYPE_WITDH:
+		case FORMAT_TYPE_WIDTH:
 			spec.field_width = get_arg(int);
 			break;
 
#3 lore raw
Quoted 8 lines · Vegard Nossum

2009/3/14 Jeremy Fitzhardinge <[email protected]>:

Quoted 6 lines

Change fef20d9c1380f04ba9492d6463148db07b413708, "vsprintf: unify the format decoding layer for its 3 users", causes a regression in xenbus which results in no devices getting attached to a new domain. Reverting fef20d9c1380f04ba9492d6463148db07b413708 and 39e874f8afbdb3745e2406ce4ecbde9ac4cbaa78 fixes the problem.

Quoted 9 lines
Quoted 4 lines

I haven't identified what format string is being handled wrongly, so I don't know what the precise bug is. The most complex looking format in use seems to be %.*s; there's also "%s/%s", "%i" and "%lX".

Hi,

At least %.*s seems to be broken. How about this patch?

Thanks, that does the trick.

J

#4 lore raw

What is the issue status in meantime ?
git pull reports conflicting merge ./lib/vsprintf.c PV DomUs  may be brought up , but cannot be shutdown.

Boris.

--- On Sat, 3/14/09, Jeremy Fitzhardinge <[email protected]> wrote:

From: Jeremy Fitzhardinge <[email protected]>
Subject: [Xen-devel] Re: tip.git regression from "vsprintf: unify the format decoding layer for its 3 users"
To: "Vegard Nossum" <[email protected]>
Cc: "Xen-devel" <[email protected]>, "Frederic Weisbecker" <[email protected]>, "Ingo Molnar" <[email protected]>, "the arch/x86 maintainers" <[email protected]>, "Linux Kernel Mailing List" <[email protected]>
Date: Saturday, March 14, 2009, 12:11 PM

Quoted 3 lines · Vegard Nossum

2009/3/14 Jeremy Fitzhardinge <[email protected]>:

Quoted 1 line

Change fef20d9c1380f04ba9492d6463148db07b413708, "vsprintf: unify

the format

Quoted 1 line
Quoted 1 line

decoding layer for its 3 users", causes a regression in xenbus

which results

Quoted 5 lines
Quoted 5 lines

in no devices getting attached to a new domain. Reverting fef20d9c1380f04ba9492d6463148db07b413708 and 39e874f8afbdb3745e2406ce4ecbde9ac4cbaa78 fixes the problem.

I haven't identified what format string is being handled wrongly,

so I don't

Quoted 1 line
Quoted 1 line

know what the precise bug is. The most complex looking format in use

seems

Quoted 1 line
Quoted 1 line

to be %.*s; there's also "%s/%s", "%i" and

"%lX".

Quoted 6 lines
Quoted 1 line

Hi,

At least %.*s seems to be broken. How about this patch?

Thanks, that does the trick.

J

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#5 lore raw
Quoted 5 lines · Boris Derzhavets

What is the issue status in meantime ?
git pull reports conflicting merge ./lib/vsprintf.c PV DomUs may be brought up , but cannot be shutdown.

Do a "git checkout xen/dom0/hackery; git reset --hard remotes/xen/xen/dom0/hackery" to rebase your local hackery branch to the upstream one. I did a bit of a history-rewrite to replace the reverts with the proper fix.

J

#6 lore raw

how to enable shadow page table? Do I have to run HVM guest systems for shadow paging mode?

I am working on a research project based on Xen 3.3.1. I want to use the shadow page table for setting guest system memory pages as read-only and/or dirty, and perform a copy-on-write mechanism when these memory pages are updated. I heard that auto_translated_physmap is not supported any more.

Then how can I enable shadow page table? Do I have to run HVM guest systems to have shadow paging mode? Can I enable shadow paging mode in PV?

I look forward to your help on this problem. Thank you very much,

Sincerely,
Long Wang

#7 lore raw
In reply to Jeremy Fitzhardinge

It's not working:-

root@ServerIntrepid:/usr/src/linux-2.6-xen# git checkout xen/dom0/hackery
Already on "xen/dom0/hackery"
root@ServerIntrepid:/usr/src/linux-2.6-xen# git reset --hard remotes/xen/xen/dom0/hackery
fatal: ambiguous argument 'remotes/xen/xen/dom0/hackery': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions

Boris

--- On Mon, 3/16/09, Jeremy Fitzhardinge <[email protected]> wrote:
From: Jeremy Fitzhardinge <[email protected]>
Subject: Re: [Xen-devel] Re: tip.git regression from "vsprintf: unify the format decoding layer for its 3 users"
To: [email protected]
Cc: "Xen-devel" <[email protected]>, "Vegard Nossum" <[email protected]>, "Frederic Weisbecker" <[email protected]>, "the arch/x86 maintainers" <[email protected]>, "Linux Kernel Mailing List" <[email protected]>, "Ingo Molnar" <[email protected]>
Date: Monday, March 16, 2009, 10:20 AM

Quoted 5 lines · Boris Derzhavets

What is the issue status in meantime ?
git pull reports conflicting merge ./lib/vsprintf.c PV DomUs may be brought up , but cannot be shutdown.

Do a "git checkout xen/dom0/hackery; git reset --hard remotes/xen/xen/dom0/hackery" to rebase your local hackery branch to the upstream one. I did a bit of a history-rewrite to replace the reverts with the proper fix.

J

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#8 lore raw
Quoted 7 lines · Boris Derzhavets

It's not working:-

root@ServerIntrepid:/usr/src/linux-2.6-xen# git checkout xen/dom0/hackery
Already on "xen/dom0/hackery"
root@ServerIntrepid:/usr/src/linux-2.6-xen# git reset --hard remotes/xen/xen/dom0/hackery
fatal: ambiguous argument 'remotes/xen/xen/dom0/hackery': unknown revision or path not in the working tree.

The proper name is "remotes/<whatever you called your xen remote>/xen/dom0/hackery" (it might be "origin").

J

#9 lore raw

Performed:-

root@ServerIntrepid:/usr/src/linux-2.6-xen# git reset --hard remotes/origin/xen/dom0/hackery
HEAD is now at a38db50 Merge commit 'tip/core/printk' into xen/dom0/hackery
make clean
make
make modules_install install
mkinitramfs -o /boot/initrd-2.6.29-rc7-tip.img 2.6.29-rc7-tip

Booted with new kernel under Xen (19355) and get same bug in place. Cannot shutdown any PV DomU . Xen Host dies.

Should i download from scratch :-

# git clone git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git linux-2.6-xen
# cd linux-2.6-xen
# git checkout origin/xen/dom0/hackery -b xen/dom0/hackery
# git reset --hard remotes/origin/xen/dom0/hackery

Boris

--- On Mon, 3/16/09, Jeremy Fitzhardinge <[email protected]> wrote:
From: Jeremy Fitzhardinge <[email protected]>
Subject: Re: [Xen-devel] Re: tip.git regression from "vsprintf: unify the format decoding layer for its 3 users"
To: [email protected]
Cc: "Xen-devel" <[email protected]>, "Vegard Nossum" <[email protected]>, "Frederic Weisbecker" <[email protected]>, "the arch/x86 maintainers" <[email protected]>, "Linux Kernel Mailing List" <[email protected]>, "Ingo Molnar" <[email protected]>
Date: Monday, March 16, 2009, 12:09 PM

Quoted 5 lines · Boris Derzhavets

It's not working:-

root@ServerIntrepid:/usr/src/linux-2.6-xen# git checkout xen/dom0/hackery
Already on "xen/dom0/hackery"
root@ServerIntrepid:/usr/src/linux-2.6-xen# git reset --hard

remotes/xen/xen/dom0/hackery

Quoted 1 line

fatal: ambiguous argument 'remotes/xen/xen/dom0/hackery': unknown

revision or path not in the working tree.

Quoted 1 line

The proper name is "remotes/<whatever you called your xen remote>/xen/dom0/hackery" (it might be "origin").

J

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#10 lore raw
Quoted 9 lines · Boris Derzhavets

Performed:-

root@ServerIntrepid:/usr/src/linux-2.6-xen# git reset --hard remotes/origin/xen/dom0/hackery
HEAD is now at a38db50 Merge commit 'tip/core/printk' into xen/dom0/hackery
make clean
make
make modules_install install
mkinitramfs -o /boot/initrd-2.6.29-rc7-tip.img 2.6.29-rc7-tip
Quoted 3 lines

Booted with new kernel under Xen (19355) and get same bug in place. Cannot shutdown any PV DomU . Xen Host dies.

OK, so we're talking about a new bug now, right?

When you say "Xen Host dies", do you mean that Xen itself crashes? Or that the dom0 kernel crashes? Hangs? Non-responsive?

Do you have a serial console?

J