[patch] option to quite unaligned trap warnings

[patch] option to quite unaligned trap warnings

[Jes Sorensen]

Hi,

This patch is an attempt to find a good compromise to the somewhat
contentious issue of the unaligned usertrap warnings.

Every engineer obviously agrees that these warnings should be taken
serious and acted upon. However there are cases where systems are
running applications where they do not have control of the
applications or simply don't care, ie. if you're running some Java app
and the JVM is spitting out a ton of these warnings (yes, I've seen
this in system logs - no, I didn't run the Java apps ;-). In these
situations administrators at times get really annoyed with these
warnings chewing up their syslog.

What this patch does is to provide an option for the system
administrator to switch off the warnings system wide, overruling the
prctl setting. By doing so he/she will get a single warning in the
syslog strongly recommending that they reenable it and have the users
fix their apps since it will lead to improved performance.

Note that the default setting in this patch is the same as we have
right now. Admins needs to take action to disable the messages.

IMHO this seems a reasonable compromise - anybody have objections
against this going into the tree?

Cheers,
Jes

Some sites have no option to recompile their applications or do not
care to spend time fixing them to eliminate unaligned accesses and
therefore find the kernel's warnings highly annoying.

This patch will allow a site to disable the unaligned warnings for all
userland apps as an alternative to the individual user using prctl to
do so. It does however print a strong encouragement to the syslog for
the site administrator to re-enable and fix the applications on the
first occurrence of such a trap.

Sample message as it would be found in the syslog:

test-unaligned(13233) encountered an unaligned exception which required
kernel assistance, which degrades the performance of the application.
Unaligned exception warnings have been disabled by the system administrator
and you will see no more of these messages. It is recommend that the
warnings be re-enabled and the information used to fix the applications
which will lead to better performance of the applications in question!
echo 0 > /proc/sys/kernel/ignore-unaligned-usertrap to re-enable!

Signed-off-by: Jes Sorensen <jes@sgi.com>

----

 arch/ia64/kernel/unaligned.c |   38 +++++++++++++++++++++++++++++++++++---
 include/linux/sysctl.h       |    1 +
 kernel/sysctl.c              |   14 ++++++++++++++
 3 files changed, 50 insertions(+), 3 deletions(-)

Index: linux-2.6/arch/ia64/kernel/unaligned.c
=================================--- linux-2.6.orig/arch/ia64/kernel/unaligned.c
+++ linux-2.6/arch/ia64/kernel/unaligned.c
@@ -53,6 +53,15 @@
 #define SIGN_EXT9		0xffffffffffffff00ul
 
 /*
+ *  sysctl settable hook which tells the kernel whether to honor the
+ *  IA64_THREAD_UAC_NOPRINT prctl.  Because this is user settable, we want
+ *  to allow the super user to enable/disable this for security reasons
+ *  (i.e. don't allow attacker to fill up logs with unaligned accesses).
+ */
+int no_unaligned_warning;
+static int noprint_warning;
+
+/*
  * For M-unit:
  *
  *  opcode |   m  |   x6    |
@@ -1324,8 +1333,9 @@
 		if ((current->thread.flags & IA64_THREAD_UAC_SIGBUS) != 0)
 			goto force_sigbus;
 
-		if (!(current->thread.flags & IA64_THREAD_UAC_NOPRINT)
-		    && within_logging_rate_limit())
+		if (!no_unaligned_warning &&
+		    !(current->thread.flags & IA64_THREAD_UAC_NOPRINT) &&
+		    within_logging_rate_limit())
 		{
 			char buf[200];	/* comm[] is at most 16 bytes... */
 			size_t len;
@@ -1340,7 +1350,29 @@
 			if (user_mode(regs))
 				tty_write_message(current->signal->tty, buf);
 			buf[len-1] = '\0';	/* drop '\r' */
-			printk(KERN_WARNING "%s", buf);	/* watch for command names containing %s */
+			/* watch for command names containing %s */
+			printk(KERN_WARNING "%s", buf);
+		} else {
+			if (no_unaligned_warning && !noprint_warning) {
+				noprint_warning = 1;
+				printk(KERN_WARNING "%s(%d) encountered an "
+				       "unaligned exception which required\n"
+				       "kernel assistance, which degrades "
+				       "the performance of the application.\n"
+				       "Unaligned exception warnings have "
+				       "been disabled by the system "
+				       "administrator\n"
+				       "and you will see no more of these "
+				       "messages. It is recommend that the\n"
+				       "warnings be re-enabled and the "
+				       "information used to fix the "
+				       "applications\n"
+				       "which will lead to better performance "
+				       "of the applications in question!\n"
+				       "echo 0 > /proc/sys/kernel/ignore-"
+				       "unaligned-usertrap to re-enable!\n",
+				       current->comm, current->pid);
+			}
 		}
 	} else {
 		if (within_logging_rate_limit())
Index: linux-2.6/include/linux/sysctl.h
=================================--- linux-2.6.orig/include/linux/sysctl.h
+++ linux-2.6/include/linux/sysctl.h
@@ -146,6 +146,7 @@
 	KERN_RANDOMIZEh, /* int: randomize virtual address space */
 	KERN_SETUID_DUMPABLEi, /* int: behaviour of dumps for setuid core */
 	KERN_SPIN_RETRYp,	/* int: number of spinlock retries */
+	KERN_IA64_UNALIGNEDq,	/* int: ia64 unaligned userland trap enable */
 };
 
 
Index: linux-2.6/kernel/sysctl.c
=================================--- linux-2.6.orig/kernel/sysctl.c
+++ linux-2.6/kernel/sysctl.c
@@ -126,6 +126,10 @@
 extern int acct_parm[];
 #endif
 
+#ifdef CONFIG_IA64
+extern int no_unaligned_warning;
+#endif
+
 int randomize_va_space = 1;
 
 static int parse_table(int __user *, int, void __user *, size_t __user *, void __user *, size_t,
@@ -658,6 +662,16 @@
 		.proc_handler	= &proc_dointvec,
 	},
 #endif
+#ifdef CONFIG_IA64
+	{
+		.ctl_name	= KERN_IA64_UNALIGNED,
+		.procname	= "ignore-unaligned-usertrap",
+		.data		= &no_unaligned_warning,
+		.maxlen		= sizeof (int),
+	 	.mode		= 0644,
+		.proc_handler	= &proc_dointvec,
+	},
+#endif
 	{ .ctl_name = 0 }
 };
 

Re: [patch] option to quite unaligned trap warnings

[Jes Sorensen]

>>>>> "Jes" = Jes Sorensen <jes@sgi.com> writes:

Jes> Hi, This patch is an attempt to find a good compromise to the
Jes> somewhat contentious issue of the unaligned usertrap warnings.

Hi,

I haven't seen any negative reactions to this one yet and it's been a
week so far. Tony any chance you would apply it to your tree?

Thanks,
Jes

RE: [patch] option to quite unaligned trap warnings

[Luck, Tony]

Jes> Hi, This patch is an attempt to find a good compromise to the
Jes> somewhat contentious issue of the unaligned usertrap warnings.

> I haven't seen any negative reactions to this one yet and it's been a
> week so far. Tony any chance you would apply it to your tree?

Yes, though I might trim the printk() message back a bit, it seems
a bit[1] over the top.  How about just:

Unaligned exception warnings have been disabled
echo 0 > /proc/sys/kernel/ignore-unaligned-usertrap to re-enable

-Tony

[1] more than a bit, hundreds of bytes

Re: [patch] option to quite unaligned trap warnings

[Jes Sorensen]

Luck, Tony wrote:
> Jes> Hi, This patch is an attempt to find a good compromise to the
> Jes> somewhat contentious issue of the unaligned usertrap warnings.
> 
> 
>>I haven't seen any negative reactions to this one yet and it's been a
>>week so far. Tony any chance you would apply it to your tree?
> 
> 
> Yes, though I might trim the printk() message back a bit, it seems
> a bit[1] over the top.  How about just:
> 
> Unaligned exception warnings have been disabled
> echo 0 > /proc/sys/kernel/ignore-unaligned-usertrap to re-enable

I kinda added that deliberately to make it a strong point to the users
that they really should consider fixing their apps. If we do a simple
message like that they may not realize the cost of it.

I can live with the simple message, but I think it would be good to
swing the cluebat a bit ;-)

Cheers,
Jes

RE: [patch] option to quite unaligned trap warnings

[Luck, Tony]

> I kinda added that deliberately to make it a strong point to the users
> that they really should consider fixing their apps. If we do a simple
> message like that they may not realize the cost of it.
>
> I can live with the simple message, but I think it would be good to
> swing the cluebat a bit ;-)

The clueless users won't ever see your message as the default is for
them to still keep seeing messages every (rate limited) time their
broken applications make a misaligned access.  They need to have
somehow found a clue someplace to work out how to turn these messages
off ... hopefully there is some correlation between them finding that
clue and knowing that an unaligned access is bad for performance.

-Tony

Re: [patch] option to quite unaligned trap warnings

[Jes Sorensen]

>>>>> "Tony" = Luck, Tony <tony.luck@intel.com> writes:

>> I kinda added that deliberately to make it a strong point to the
>> users that they really should consider fixing their apps. If we do
>> a simple message like that they may not realize the cost of it.
>> 
>> I can live with the simple message, but I think it would be good to
>> swing the cluebat a bit ;-)

Tony> The clueless users won't ever see your message as the default is
Tony> for them to still keep seeing messages every (rate limited) time
Tony> their broken applications make a misaligned access.  They need
Tony> to have somehow found a clue someplace to work out how to turn
Tony> these messages off ... hopefully there is some correlation
Tony> between them finding that clue and knowing that an unaligned
Tony> access is bad for performance.

I guess the risk is that there are clueful users and clueless admins,
the latter being the ones disabling this feature.

Anyway if you prefer a simpler text then I am ok with it, getting the
functionality in solves the problem I was facing.

Thanks,
Jes

Re: [patch] option to quite unaligned trap warnings

[Jes Sorensen]

>>>>> "Tony" = Luck, Tony <tony.luck@intel.com> writes:

Jes> Hi, This patch is an attempt to find a good compromise to the
Jes> somewhat contentious issue of the unaligned usertrap warnings.

>> I haven't seen any negative reactions to this one yet and it's been
>> a week so far. Tony any chance you would apply it to your tree?

Tony> Yes, though I might trim the printk() message back a bit, it
Tony> seems a bit[1] over the top.  How about just:

Hi Tony,

I didn't see this one go in yet. Were you waiting for me to do an
updated patch?

Admittedly it's something I'd like to see go into 2.6.16.

Cheers,
Jes