This patch allows modules to work in Linus' tree for ARM, and is the one
thing which prevents Linus' tree from building for any ARM machine.
After reviewing the /proc/kcore and kclist issues, I've decided that I'm
no longer prepared to even _think_ about supporting /proc/kcore on ARM -
it just gets too ugly, and adds too much code to make it worth the effort,
the time or the energy to implement a solution to that problem. This is
especially true since most people use kgdb or similar rather than
/proc/kcore anyway. /proc/kcore is a "wouldn't it be nice" feature.
The reasons that I'm going with this solution rather than fixing
/proc/kcore is that:
- to add a kclist entry for each module would mean hacking the kclist
structure into vm_struct's ->page pointer with disgusting hacks.
I think we can all agree that this isn't the way to go. The
alternative is to create Yet Another Memory Allocator, and this
isn't something I want to see in what is now an embedded architecture.
- we'd need to find some way to dynamically reserve the virtual mapped
memory regions for the kernel direct mapped RAM. Since ARM uses a
generic memory initialisation implementation which handles contiguous
and discontiguous memory, it doesn't lend itself well to the kclist
approach, and I'm not about to add extra callbacks from init/main.c
(so we have kmalloc available) just to support this.
If someone _else_ wants to put the effort into fixing ARM modules to work
nicely with /proc/kcore, be my guest - I'm just no longer interested in
this problem space.
Maybe in 2.7 a generic "reserve an area of memory in this region"
function like __get_vm_area below is in order?
Therefore, I'm providing a patch which adds the necessary changes to the
core kernel code to make the current modules solution work for ARM.
Please merge.
--- orig/mm/vmalloc.c Tue May 27 10:05:48 2003
+++ linux/mm/vmalloc.c Tue May 27 10:14:45 2003
@@ -178,21 +178,11 @@
return err;
}
-
-/**
- * get_vm_area - reserve a contingous kernel virtual area
- *
- * @size: size of the area
- * @flags: %VM_IOREMAP for I/O mappings or VM_ALLOC
- *
- * Search an area of @size in the kernel virtual mapping area,
- * and reserved it for out purposes. Returns the area descriptor
- * on success or %NULL on failure.
- */
-struct vm_struct *get_vm_area(unsigned long size, unsigned long flags)
+struct vm_struct *__get_vm_area(unsigned long size, unsigned long flags,
+ unsigned long start, unsigned long end)
{
struct vm_struct **p, *tmp, *area;
- unsigned long addr = VMALLOC_START;
+ unsigned long addr = start;
area = kmalloc(sizeof(*area), GFP_KERNEL);
if (unlikely(!area))
@@ -209,12 +199,14 @@
write_lock(&vmlist_lock);
for (p = &vmlist; (tmp = *p) ;p = &tmp->next) {
+ if ((unsigned long)tmp->addr < addr)
+ continue;
if ((size + addr) < addr)
goto out;
if (size + addr <= (unsigned long)tmp->addr)
goto found;
addr = tmp->size + (unsigned long)tmp->addr;
- if (addr > VMALLOC_END-size)
+ if (addr > end - size)
goto out;
}
@@ -239,6 +231,21 @@
}
/**
+ * get_vm_area - reserve a contingous kernel virtual area
+ *
+ * @size: size of the area
+ * @flags: %VM_IOREMAP for I/O mappings or VM_ALLOC
+ *
+ * Search an area of @size in the kernel virtual mapping area,
+ * and reserved it for out purposes. Returns the area descriptor
+ * on success or %NULL on failure.
+ */
+struct vm_struct *get_vm_area(unsigned long size, unsigned long flags)
+{
+ return __get_vm_area(size, flags, VMALLOC_START, VMALLOC_END);
+}
+
+/**
* remove_vm_area - find and remove a contingous kernel virtual area
*
* @addr: base address
--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
On Thu, 14 Aug 2003, Russell King wrote:
>
> After reviewing the /proc/kcore and kclist issues, I've decided that I'm
> no longer prepared to even _think_ about supporting /proc/kcore on ARM -
I suspect we should just remove it altogether.
Does anybody actually _use_ /proc/kcore? It was one of those "cool
feature" things, but I certainly haven't ever used it myself except for
testing, and it's historically often been broken after various kernel
infrastructure updates, and people haven't complained..
Comments?
Linus
On Thu, Aug 14, 2003 at 09:19:17AM -0700, Linus Torvalds wrote:
>
> On Thu, 14 Aug 2003, Russell King wrote:
> >
> > After reviewing the /proc/kcore and kclist issues, I've decided that I'm
> > no longer prepared to even _think_ about supporting /proc/kcore on ARM -
>
> I suspect we should just remove it altogether.
>
> Does anybody actually _use_ /proc/kcore? It was one of those "cool
> feature" things, but I certainly haven't ever used it myself except for
> testing, and it's historically often been broken after various kernel
> infrastructure updates, and people haven't complained..
I was thinking afterwards about a patch to allow /proc/kcore to be
entirely disabled and dropped out of the kernel - we already select
between a.out and ELF.
--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
On Iau, 2003-08-14 at 17:19, Linus Torvalds wrote:
> Does anybody actually _use_ /proc/kcore? It was one of those "cool
> feature" things, but I certainly haven't ever used it myself except for
> testing, and it's historically often been broken after various kernel
> infrastructure updates, and people haven't complained..
>
> Comments?
Now if you'd agree to merge the kgdb stubs to replace it.... ;)
I see no problem with it either going or becoming an arch specific
module someone can go write if they care about it and insmod if they
actually use.
On Thu, Aug 14, 2003 at 05:47:03PM +0100, Alan Cox wrote:
> I see no problem with it either going or becoming an arch specific
> module someone can go write if they care about it and insmod if they
> actually use.
And then I'm stuck with no sensible way to figure out the kernel pointer
size again... all user-space suggestions having the problems listed
in this thread :
http://marc.theaimsgroup.com/?t=104205635900001&r=1&w=2
regards
john
--
Khendon's Law:
If the same point is made twice by the same person, the thread is over.
> > After reviewing the /proc/kcore and kclist issues, I've decided that I'm
> > no longer prepared to even _think_ about supporting /proc/kcore on ARM -
>
> I suspect we should just remove it altogether.
>
> Does anybody actually _use_ /proc/kcore? It was one of those "cool
> feature" things, but I certainly haven't ever used it myself except for
> testing, and it's historically often been broken after various kernel
> infrastructure updates, and people haven't complained..
>
> Comments?
I've used it on a few rare occasions for last-ditch data recovery
before, E.G. an application crashed that had a text file held in RAM
that wasn't ever written to disk. Poking through /proc/kcore could
allow it's recovery.
Probably not a sufficent reason to keep it :-).
John.
On Iau, 2003-08-14 at 17:55, John Levon wrote:
> And then I'm stuck with no sensible way to figure out the kernel pointer
> size again... all user-space suggestions having the problems listed
> in this thread :
And why can't you put the pointer size at the start of the buffer ?
On Thu, 14 Aug 2003, John Levon wrote:
>
> And then I'm stuck with no sensible way to figure out the kernel pointer
> size again...
Why not just fix the oprofile interfaces to contain that information? You
already have to export CPU type, buffer size etc..
Linus
On Thu, Aug 14, 2003 at 05:55:12PM +0100, John Levon wrote:
> On Thu, Aug 14, 2003 at 05:47:03PM +0100, Alan Cox wrote:
>
> > I see no problem with it either going or becoming an arch specific
> > module someone can go write if they care about it and insmod if they
> > actually use.
>
> And then I'm stuck with no sensible way to figure out the kernel pointer
> size again... all user-space suggestions having the problems listed
> in this thread :
>
> http://marc.theaimsgroup.com/?t=104205635900001&r=1&w=2
You could say "oprofile requires /proc/kcore support".
--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
On 14 Aug 2003, Alan Cox wrote:
>
> And why can't you put the pointer size at the start of the buffer ?
Historical aside: this was pretty much _exactly_ the bug that we used to
have in the old style timer tick profiler: it was impossible to figure out
what the "profile shift" value was. So you got a series of "unsigned int"
counts, but you didn't know what the granularity was for the hit counting.
That was fixed by just making read_profile() return the sample step size
as the first word. See
fs/proc/proc_misc.c: read_profile()
and note the small
while (p < sizeof(unsigned int) && count > 0) {
put_user(*((char *)(&sample_step)+p),buf);
buf++; p++; count--; read++;
}
loop (and a few "+1" things for adjusting size comparisons).
Linus
On Thu, Aug 14, 2003 at 10:14:37AM -0700, Linus Torvalds wrote:
> Why not just fix the oprofile interfaces to contain that information? You
> already have to export CPU type, buffer size etc..
I'll quite happily do that. I'd assumed it was also rejected for bloat
reasons based on your previous objections that it was/should be entirely
a userspace issue.
If you'll take /dev/oprofile/pointer_size now, then I'm more than happy
to drop the kcore sniffing (and I don't have the a.out problem any more)
regards
john
--
Khendon's Law:
If the same point is made twice by the same person, the thread is over.
Linus Torvalds wrote:
> On Thu, 14 Aug 2003, Russell King wrote:
>
>>After reviewing the /proc/kcore and kclist issues, I've decided that I'm
>>no longer prepared to even _think_ about supporting /proc/kcore on ARM -
>
>
> I suspect we should just remove it altogether.
>
> Does anybody actually _use_ /proc/kcore? It was one of those "cool
> feature" things, but I certainly haven't ever used it myself except for
> testing, and it's historically often been broken after various kernel
> infrastructure updates, and people haven't complained..
I only fixed it up because someone at SGI complained that
my ia64 kernel virtual space re-arrangements had broken it
(even more ... it had never been right for ia64).
Even now, it's still full of races (e.g. if you start gdb to
look at /proc/kcore, then load or unload modules).
Plus gdb doesn't really understand that kcore is a special
file, so it caches values (read "jiffies" twice and notice
that it doesn't change ... because gdb cached the read).
So don't mistake my recent activity fixing kcore for interest
in its continued existence.
-Tony Luck
On Thu, Aug 14, 2003 at 06:30:24PM +0100, John Levon wrote:
> > Why not just fix the oprofile interfaces to contain that information? You
> > already have to export CPU type, buffer size etc..
Export kernel pointer size from oprofilefs. Also fix the prototype of
oprofilefs_ulong_to_user not to take a pointer for no reason.
Briefly tested on my x86 box with a modified oprofiled.
diff -Naur -X dontdiff linux-cvs/drivers/oprofile/oprofile_files.c linux-fixes/drivers/oprofile/oprofile_files.c
--- linux-cvs/drivers/oprofile/oprofile_files.c 2003-06-15 02:06:38.000000000 +0100
+++ linux-fixes/drivers/oprofile/oprofile_files.c 2003-08-14 19:19:05.000000000 +0100
@@ -19,6 +19,17 @@
unsigned long fs_buffer_watershed = 32768; /* FIXME: tune */
+static ssize_t pointer_size_read(struct file * file, char * buf, size_t count, loff_t * offset)
+{
+ return oprofilefs_ulong_to_user((unsigned long)sizeof(void *), buf, count, offset);
+}
+
+
+static struct file_operations pointer_size_fops = {
+ .read = pointer_size_read,
+};
+
+
static ssize_t cpu_type_read(struct file * file, char * buf, size_t count, loff_t * offset)
{
return oprofilefs_str_to_user(oprofile_ops->cpu_type, buf, count, offset);
@@ -32,7 +43,7 @@
static ssize_t enable_read(struct file * file, char * buf, size_t count, loff_t * offset)
{
- return oprofilefs_ulong_to_user(&oprofile_started, buf, count, offset);
+ return oprofilefs_ulong_to_user(oprofile_started, buf, count, offset);
}
@@ -85,6 +96,7 @@
oprofilefs_create_ulong(sb, root, "buffer_watershed", &fs_buffer_watershed);
oprofilefs_create_ulong(sb, root, "cpu_buffer_size", &fs_cpu_buffer_size);
oprofilefs_create_file(sb, root, "cpu_type", &cpu_type_fops);
+ oprofilefs_create_file(sb, root, "pointer_size", &pointer_size_fops);
oprofile_create_stats_files(sb, root);
if (oprofile_ops->create_files)
oprofile_ops->create_files(sb, root);
diff -Naur -X dontdiff linux-cvs/drivers/oprofile/oprofilefs.c linux-fixes/drivers/oprofile/oprofilefs.c
--- linux-cvs/drivers/oprofile/oprofilefs.c 2003-05-26 02:04:50.000000000 +0100
+++ linux-fixes/drivers/oprofile/oprofilefs.c 2003-08-14 19:21:16.000000000 +0100
@@ -69,7 +69,7 @@
#define TMPBUFSIZE 50
-ssize_t oprofilefs_ulong_to_user(unsigned long * val, char * buf, size_t count, loff_t * offset)
+ssize_t oprofilefs_ulong_to_user(unsigned long val, char * buf, size_t count, loff_t * offset)
{
char tmpbuf[TMPBUFSIZE];
size_t maxlen;
@@ -78,7 +78,7 @@
return 0;
spin_lock(&oprofilefs_lock);
- maxlen = snprintf(tmpbuf, TMPBUFSIZE, "%lu\n", *val);
+ maxlen = snprintf(tmpbuf, TMPBUFSIZE, "%lu\n", val);
spin_unlock(&oprofilefs_lock);
if (maxlen > TMPBUFSIZE)
maxlen = TMPBUFSIZE;
@@ -122,7 +122,8 @@
static ssize_t ulong_read_file(struct file * file, char * buf, size_t count, loff_t * offset)
{
- return oprofilefs_ulong_to_user(file->private_data, buf, count, offset);
+ unsigned long * val = file->private_data;
+ return oprofilefs_ulong_to_user(*val, buf, count, offset);
}
@@ -212,9 +213,8 @@
static ssize_t atomic_read_file(struct file * file, char * buf, size_t count, loff_t * offset)
{
- atomic_t * aval = file->private_data;
- unsigned long val = atomic_read(aval);
- return oprofilefs_ulong_to_user(&val, buf, count, offset);
+ atomic_t * val = file->private_data;
+ return oprofilefs_ulong_to_user(atomic_read(val), buf, count, offset);
}
diff -Naur -X dontdiff linux-cvs/include/linux/oprofile.h linux-fixes/include/linux/oprofile.h
--- linux-cvs/include/linux/oprofile.h 2003-06-15 02:06:38.000000000 +0100
+++ linux-fixes/include/linux/oprofile.h 2003-08-14 19:19:42.000000000 +0100
@@ -92,7 +92,7 @@
* Convert an unsigned long value into ASCII and copy it to the user buffer @buf,
* updating *offset appropriately. Returns bytes written or -EFAULT.
*/
-ssize_t oprofilefs_ulong_to_user(unsigned long * val, char * buf, size_t count, loff_t * offset);
+ssize_t oprofilefs_ulong_to_user(unsigned long val, char * buf, size_t count, loff_t * offset);
/**
* Read an ASCII string for a number from a userspace buffer and fill *val on success.
Alan Cox wrote:
> On Iau, 2003-08-14 at 17:19, Linus Torvalds wrote:
>
>>Does anybody actually _use_ /proc/kcore? It was one of those "cool
>>feature" things, but I certainly haven't ever used it myself except for
>>testing, and it's historically often been broken after various kernel
>>infrastructure updates, and people haven't complained..
>>
>>Comments?
>
>
> Now if you'd agree to merge the kgdb stubs to replace it.... ;)
I for one would like to see that. However:
The simple protocol that it uses needs to be factored out to an
arch-independent place, and the various archs need to be modified to use
it. To get to that point will likely require someone doing it for one
or two archs and then having enough community clout to convince other
archs to merge with it. It could get to the point where it's a pretty
clean implementation (for debugger hooks ;) ).
No, that isn't something I can take on. :/ (Though I did get it
partially working on 2.5 XScale.)
Eli
--------------------. "If it ain't broke now,
Eli Carter \ it will be soon." -- crypto-gram
eli.carter(a)inet.com `-------------------------------------------------
On Thu, Aug 14, 2003 at 03:12:32PM -0500, Eli Carter wrote:
> Alan Cox wrote:
> > Now if you'd agree to merge the kgdb stubs to replace it.... ;)
>
> No, that isn't something I can take on. :/ (Though I did get it
> partially working on 2.5 XScale.)
George Davis did some work in this area. I'm not sure what state it's
in though.
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=1335/1
--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
On Thu, Aug 14, 2003 at 09:19:17AM -0700, Linus Torvalds wrote:
>
> On Thu, 14 Aug 2003, Russell King wrote:
> >
> > After reviewing the /proc/kcore and kclist issues, I've decided that I'm
> > no longer prepared to even _think_ about supporting /proc/kcore on ARM -
>
> I suspect we should just remove it altogether.
>
> Does anybody actually _use_ /proc/kcore? It was one of those "cool
> feature" things, but I certainly haven't ever used it myself except for
> testing, and it's historically often been broken after various kernel
> infrastructure updates, and people haven't complained..
>
> Comments?
Speaking only for me, and all that.
I use it. It's actually pretty handy sometimes, because it lets me
peek at task structures et cetera from userspace; so when I have a
problem with the kernel accessing the wrong memory, I can go figure out
what it's actually looking at.
This is a sort of poor-man's-kgdb. I don't think there's much need for
/proc/kcore if you have a working kgdb, which I'd still like to see
cleaned up and integrated.
--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer
On Thu, Aug 14, 2003 at 05:32:49PM +0100, Russell King wrote:
> I was thinking afterwards about a patch to allow /proc/kcore to be
> entirely disabled and dropped out of the kernel - we already select
> between a.out and ELF.
Ok, it's been a while, but here's the patch which does this - it only
drops /proc/kcore out of the kernel for ARM.
diff -u orig/fs/Kconfig linux/fs/Kconfig
--- orig/fs/Kconfig Tue Sep 9 23:08:10 2003
+++ linux/fs/Kconfig Sat Aug 23 10:12:27 2003
@@ -761,6 +761,10 @@
This option will enlarge your kernel by about 67 KB. Several
programs depend on this, so everyone should say Y here.
+config PROC_KCORE
+ bool
+ default y if !ARM
+
config DEVFS_FS
bool "/dev file system support (EXPERIMENTAL)"
depends on EXPERIMENTAL
diff -u orig/fs/proc/Makefile linux/fs/proc/Makefile
--- orig/fs/proc/Makefile Thu Sep 4 23:58:31 2003
+++ linux/fs/proc/Makefile Thu Aug 14 18:27:31 2003
@@ -8,6 +8,7 @@
proc-$(CONFIG_MMU) := task_mmu.o
proc-y += inode.o root.o base.o generic.o array.o \
- kmsg.o proc_tty.o proc_misc.o kcore.o
+ kmsg.o proc_tty.o proc_misc.o
-proc-$(CONFIG_PROC_DEVICETREE) += proc_devtree.o
+proc-$(CONFIG_PROC_KCORE) += kcore.o
+proc-$(CONFIG_PROC_DEVICETREE) += proc_devtree.o
diff -u orig/fs/proc/proc_misc.c linux/fs/proc/proc_misc.c
--- orig/fs/proc/proc_misc.c Tue Sep 9 23:08:11 2003
+++ linux/fs/proc/proc_misc.c Mon Sep 8 22:52:18 2003
@@ -685,12 +685,14 @@
#ifdef CONFIG_MODULES
create_seq_entry("modules", 0, &proc_modules_operations);
#endif
+#ifdef CONFIG_PROC_KCORE
proc_root_kcore = create_proc_entry("kcore", S_IRUSR, NULL);
if (proc_root_kcore) {
proc_root_kcore->proc_fops = &proc_kcore_operations;
proc_root_kcore->size =
(size_t)high_memory - PAGE_OFFSET + PAGE_SIZE;
}
+#endif
if (prof_on) {
entry = create_proc_entry("profile", S_IWUSR | S_IRUGO, NULL);
if (entry) {
--
Russell King ([email protected]) http://www.arm.linux.org.uk/personal/
Linux kernel maintainer of:
2.6 ARM Linux - http://www.arm.linux.org.uk/
2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core