Date: Sun, 17 Mar 2002 14:40:03 GMT From: Hiten Pandya <hiten@uk.FreeBSD.org> To: FreeBSD-gnats-submit@FreeBSD.org Cc: re@FreeBSD.org, phk@FreeBSD.org Subject: kern/36008: [PATCH] Critical PERFMON fix for DEVFS users Message-ID: <200203171440.g2HEe3400609@hpdi.ath.cx>
next in thread | raw e-mail | index | archive | help
>Number: 36008
>Category: kern
>Synopsis: [PATCH] Critical PERFMON fix for DEVFS users
>Confidential: no
>Severity: critical
>Priority: high
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Sun Mar 17 06:50:00 PST 2002
>Closed-Date:
>Last-Modified:
>Originator: Hiten Pandya
>Release: FreeBSD 5.0-CURRENT i386
>Organization:
>Environment:
System: FreeBSD hpdi.ath.cx 5.0-CURRENT FreeBSD 5.0-CURRENT #0: Sat Mar 16 19:06:14 GMT 2002 hitenp@hpdi.ath.cx:/c1/obj/data/dev/src/sys/CURRENT5 i386
>Description:
This PR has be opened to address a pretty critical issue. The
PERFMON driver is kinda broken for people using DEVFS because of
some device initialisation error.
I myself use 5.0-CURRENT with DEVFS, and have found out that the
PERFMON driver was not loading even though I had the PERFMON option
in my kernel configuration.
The following warnings are seen when PERFMON was enabled with DEVFS:
-- DMESG output --
Preloaded elf module "/boot/kernel/acpi.ko" at 0xc03be0a8.
Timecounter "i8254" frequency 1193182 Hz
Timecounter "TSC" frequency 730807620 Hz
CPU: Pentium III/Pentium III Xeon/Celeron (730.81-MHz 686-class CPU)
Origin =3D "GenuineIntel" Id =3D 0x683 Stepping =3D 3
Features=3D0x387fbff<FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE=
,MCA,CMOV,PAT,PSE36,PN,MMX,FXSR,SSE>
WARNING: Driver mistake: make_dev(perfmon) called before SI_SUB_DRIVERS
-- DMESG output --
The last line is of pretty much interest, as it shouws that the
perfmon driver failed to load because the Device subsystem
i.e. DEVFS was not initialised. A discussion about this was held
on the current@ list, and Terry suggested me a fix, which I have turned
into a patch, and it does the following (also commented in the code for
clarity):
o Split the perfmon_init() function into perfmon_init_dev();
o perfmon_init_dev() is passed as a helper function SYSINIT()
as follows (wrapped to 80 columns):
SYSINIT(cpu, SI_SUB_DRIVERS,
SI_ORDER_ANY, perfmon_init_dev, NULL);
o The above SYSINIT is used to initialise the perfmon driver,
after the DEVFS subsystem has loaded, by using SI_SUB_DRIVERS,
and also making it a low priority task, by using SI_ORDER_ANY.
I have tested my patch, and it has also been approved by Terry. I have
CC'ed this PR to phk@, who maintains the DEVFS subsystem I beleive, and
re@ because of the neccessary changes which needs to go into the:
5.0 Developer pre-release (-CURRENT code slush is over!)
I hope this patch makes it way through. I would surely not mind any
comments, compliments, or rants on it. ;) BTW, I haven't tested this
patch on -STABLE, as my area of interest is -CURRENT. :-)
References:
- Message-ID: 3C8D44BE.260E044B@mindspring.com
(Re: warnings in my bootup (dmesg.boot)) posted to current@FreeBSD.org
Thank You,
Regards,
-- Hiten Pandya
-- <hiten@uk.FreeBSD.org>
>How-To-Repeat:
Compile a -CURRENT kernel which has the following two options in it:
options PERFMON # Perfmon driver for the i686
options DEVFS # The Device Filesystem
It will produce a dmesg(1) output similar to the one which I have
provided above.
>Fix:
Apply the following patch, which will resolve the issue, as commented
in the patch itself:
Index: perfmon.c
===================================================================
RCS file: /home/ncvs/src/sys/i386/i386/perfmon.c,v
retrieving revision 1.26
diff -u -r1.26 perfmon.c
--- perfmon.c 2001/12/18 00:27:15 1.26
+++ perfmon.c 2002/03/16 19:42:03
@@ -58,6 +58,16 @@
static d_open_t perfmon_open;
static d_ioctl_t perfmon_ioctl;
+/*
+ * XXX perfmon_init_dev(void *) is a split from the perfmon_init() funtion.
+ * This solves a problem for DEVFS users. It loads the "perfmon" driver after
+ * the DEVFS subsystem has been kicked into action. The SI_ORDER_ANY is to
+ * assure that it is the most lowest priority task which, guarantees the
+ * above.
+ */
+static void perfmon_init_dev __P((void *));
+SYSINIT(cpu, SI_SUB_DRIVERS, SI_ORDER_ANY, perfmon_init_dev, NULL);
+
#define CDEV_MAJOR 2 /* We're really a minor of mem.c */
static struct cdevsw perfmon_cdevsw = {
/* open */ perfmon_open,
@@ -105,6 +115,12 @@
break;
}
#endif /* SMP */
+}
+
+static void
+perfmon_init_dev(dummy)
+ void *dummy;
+{
make_dev(&perfmon_cdevsw, 32, UID_ROOT, GID_KMEM, 0640, "perfmon");
}
>Release-Note:
>Audit-Trail:
>Unformatted:
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200203171440.g2HEe3400609>