linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
@ 2002-10-16  1:56 Ulrich Weigand
  2002-10-16  2:00 ` David S. Miller
  0 siblings, 1 reply; 21+ messages in thread
From: Ulrich Weigand @ 2002-10-16  1:56 UTC (permalink / raw)
  To: levon; +Cc: linux-kernel

John Levon wrote:

+               return (u32)dentry;

Um, isn't this supposed to uniquely identify the dentry?
On a platform with 64-bit pointers there's now the theoretical
possibility of different dentries getting the same cookie ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-16  1:56 [PATCH] [8/7] oprofile - dcookies need to use u32 Ulrich Weigand
@ 2002-10-16  2:00 ` David S. Miller
  2002-10-16 16:40   ` John Levon
  0 siblings, 1 reply; 21+ messages in thread
From: David S. Miller @ 2002-10-16  2:00 UTC (permalink / raw)
  To: weigand; +Cc: levon, linux-kernel

   From: Ulrich Weigand <weigand@immd1.informatik.uni-erlangen.de>
   Date: Wed, 16 Oct 2002 03:56:10 +0200 (MET DST)

   +               return (u32)dentry;
   
   Um, isn't this supposed to uniquely identify the dentry?
   On a platform with 64-bit pointers there's now the theoretical
   possibility of different dentries getting the same cookie ...

That's true.

We dealt with this (trying to use a kernel pointer as a cache held by
userspace) in tcp_diag by making the actual object opaque.  It was
actually two u32's, and that way it worked independant of kernel
vs. user word size.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-16  2:00 ` David S. Miller
@ 2002-10-16 16:40   ` John Levon
  2002-10-16 18:29     ` Jeff Garzik
  2002-10-16 21:38     ` David S. Miller
  0 siblings, 2 replies; 21+ messages in thread
From: John Levon @ 2002-10-16 16:40 UTC (permalink / raw)
  To: David S. Miller; +Cc: weigand, linux-kernel

On Tue, Oct 15, 2002 at 07:00:19PM -0700, David S. Miller wrote:

>    +               return (u32)dentry;
>    
>    Um, isn't this supposed to uniquely identify the dentry?
>    On a platform with 64-bit pointers there's now the theoretical
>    possibility of different dentries getting the same cookie ...
> 
> That's true.
> 
> We dealt with this (trying to use a kernel pointer as a cache held by
> userspace) in tcp_diag by making the actual object opaque.  It was
> actually two u32's, and that way it worked independant of kernel
> vs. user word size.

I'm not sure that's an option :

o userspace needs to know the size of the cookie in the event buffer
o userspace would like to use the cookie as a hash value to avoid
  repeated lookups

Perhaps the best solution would be to use a separate u32 ID value,
allocated linearly. I could just refuse to allocate new dcookies in
theoretical case of overflow.

The other possibility is a dcookiefs (cat
/dev/oprofile/dcookie/34343234) but that's a lot of extra
code/complexity ...

regards
john
-- 
"It's a cardboard universe ... and if you lean too hard against it, you fall
 through." 
	- Philip K. Dick 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-16 16:40   ` John Levon
@ 2002-10-16 18:29     ` Jeff Garzik
  2002-10-16 21:38     ` David S. Miller
  1 sibling, 0 replies; 21+ messages in thread
From: Jeff Garzik @ 2002-10-16 18:29 UTC (permalink / raw)
  To: John Levon; +Cc: David S. Miller, weigand, linux-kernel

John Levon wrote:
> The other possibility is a dcookiefs (cat
> /dev/oprofile/dcookie/34343234) but that's a lot of extra
> code/complexity ...


with libfs it's pretty easy these days...


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-16 16:40   ` John Levon
  2002-10-16 18:29     ` Jeff Garzik
@ 2002-10-16 21:38     ` David S. Miller
  2002-10-17  0:57       ` John Levon
  1 sibling, 1 reply; 21+ messages in thread
From: David S. Miller @ 2002-10-16 21:38 UTC (permalink / raw)
  To: levon; +Cc: weigand, linux-kernel

   From: John Levon <levon@movementarian.org>
   Date: Wed, 16 Oct 2002 17:40:57 +0100
   
   I'm not sure that's an option :
   
   o userspace needs to know the size of the cookie in the event buffer
   o userspace would like to use the cookie as a hash value to avoid
     repeated lookups
   
   Perhaps the best solution would be to use a separate u32 ID value,
   allocated linearly. I could just refuse to allocate new dcookies in
   theoretical case of overflow.
   
   The other possibility is a dcookiefs (cat
   /dev/oprofile/dcookie/34343234) but that's a lot of extra
   code/complexity ...
   
I don't understand why using a bigger type is not an option.

Why not just use u64?  That would work too.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-17  0:57       ` John Levon
@ 2002-10-17  0:55         ` David S. Miller
  2002-10-17  1:16           ` John Levon
  0 siblings, 1 reply; 21+ messages in thread
From: David S. Miller @ 2002-10-17  0:55 UTC (permalink / raw)
  To: levon; +Cc: weigand, linux-kernel

   From: John Levon <levon@movementarian.org>
   Date: Thu, 17 Oct 2002 01:57:28 +0100

   The oprofile event buffer is unsigned long [], and stores cookie values.
   Surely that would require us to use u64 there too, doubling the buffer
   sizes on 32-bit machines ?
   
   I suppose we could do so magic to spread the cookie value across two
   buffer entries if necessary, but that's ugly...
   
True.

What if you could query the cookie size at runtime?
Would that help?

Really, if you make it long it's going to be impossible
to support this in 32/64 environments (ppc/sparc/mips/x86_64/
ia64/etc.)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-16 21:38     ` David S. Miller
@ 2002-10-17  0:57       ` John Levon
  2002-10-17  0:55         ` David S. Miller
  0 siblings, 1 reply; 21+ messages in thread
From: John Levon @ 2002-10-17  0:57 UTC (permalink / raw)
  To: David S. Miller; +Cc: weigand, linux-kernel

On Wed, Oct 16, 2002 at 02:38:43PM -0700, David S. Miller wrote:

> I don't understand why using a bigger type is not an option.
> 
> Why not just use u64?  That would work too.

The oprofile event buffer is unsigned long [], and stores cookie values.
Surely that would require us to use u64 there too, doubling the buffer
sizes on 32-bit machines ?

I suppose we could do so magic to spread the cookie value across two
buffer entries if necessary, but that's ugly...

regards
john

-- 
"It's a cardboard universe ... and if you lean too hard against it, you fall
 through." 
	- Philip K. Dick 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-17  1:16           ` John Levon
@ 2002-10-17  1:12             ` David S. Miller
  2002-10-19  0:26               ` John Levon
  0 siblings, 1 reply; 21+ messages in thread
From: David S. Miller @ 2002-10-17  1:12 UTC (permalink / raw)
  To: levon; +Cc: weigand, linux-kernel

   From: John Levon <levon@movementarian.org>
   Date: Thu, 17 Oct 2002 02:16:23 +0100

   On Wed, Oct 16, 2002 at 05:55:15PM -0700, David S. Miller wrote:
   
   > True.
   > 
   > What if you could query the cookie size at runtime?
   
   Not sure what you mean here. The cookie is passed in the syscall, so has
   to be fixed-size no matter what, right ?
   
Right, but you could zero-extend that from u32 if u32
were the size appropriate for the current kernel.

I'm trying to decrease the size of your logfile.

Franks a lot,
David S. Miller
davem@redhat.com

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-17  0:55         ` David S. Miller
@ 2002-10-17  1:16           ` John Levon
  2002-10-17  1:12             ` David S. Miller
  0 siblings, 1 reply; 21+ messages in thread
From: John Levon @ 2002-10-17  1:16 UTC (permalink / raw)
  To: David S. Miller; +Cc: weigand, linux-kernel

On Wed, Oct 16, 2002 at 05:55:15PM -0700, David S. Miller wrote:

>    I suppose we could do so magic to spread the cookie value across two
>    buffer entries if necessary, but that's ugly...
>    
> True.
> 
> What if you could query the cookie size at runtime?

Not sure what you mean here. The cookie is passed in the syscall, so has
to be fixed-size no matter what, right ?

> Really, if you make it long it's going to be impossible
> to support this in 32/64 environments (ppc/sparc/mips/x86_64/
> ia64/etc.)

Sure ... I suppose I'll implement both (allocation of unique ID vs. the
above) and see which is least ugly.

thanks
john
-- 
"It's a cardboard universe ... and if you lean too hard against it, you fall
 through." 
	- Philip K. Dick 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-19  0:26               ` John Levon
@ 2002-10-19  0:23                 ` David S. Miller
  2002-10-19  0:34                   ` John Levon
  0 siblings, 1 reply; 21+ messages in thread
From: David S. Miller @ 2002-10-19  0:23 UTC (permalink / raw)
  To: levon; +Cc: weigand, linux-kernel

   From: John Levon <levon@movementarian.org>
   Date: Sat, 19 Oct 2002 01:26:45 +0100
   
   Now all we need is for whoever ports oprofiled to a 32-bit on 64-bit
   platform to add some check for finding out the kernel's idea of what
   sizeof(unsigned long) is, and using that to read /dev/oprofile/buffer.
   
   Yeah ?

Wouldn't that someone be you?  It's not that hard to code, and if
it's in there from the start it would really save all of us a lot
of time.

Really, the oprofiled work to do this is going to be generic, what
isn't the generic is the "determine kernel pointer size" check.
But you can make x86 return '4' from the get-go and then people like
me will have a simple place to add the proper test for our platform.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-17  1:12             ` David S. Miller
@ 2002-10-19  0:26               ` John Levon
  2002-10-19  0:23                 ` David S. Miller
  0 siblings, 1 reply; 21+ messages in thread
From: John Levon @ 2002-10-19  0:26 UTC (permalink / raw)
  To: David S. Miller; +Cc: weigand, linux-kernel

On Wed, Oct 16, 2002 at 06:12:13PM -0700, David S. Miller wrote:

> Right, but you could zero-extend that from u32 if u32
> were the size appropriate for the current kernel.
> 
> I'm trying to decrease the size of your logfile.

OK, I think I've caught on. Please check the below patch.

Now all we need is for whoever ports oprofiled to a 32-bit on 64-bit
platform to add some check for finding out the kernel's idea of what
sizeof(unsigned long) is, and using that to read /dev/oprofile/buffer.

Yeah ?

thanks
john


diff -X dontdiff -Naur linux-linus/drivers/oprofile/buffer_sync.c linux/drivers/oprofile/buffer_sync.c
--- linux-linus/drivers/oprofile/buffer_sync.c	Wed Oct 16 19:08:46 2002
+++ linux/drivers/oprofile/buffer_sync.c	Thu Oct 17 01:42:47 2002
@@ -118,13 +118,13 @@
  * because we cannot reach this code without at least one
  * dcookie user still being registered (namely, the reader
  * of the event buffer). */
-static inline u32 fast_get_dcookie(struct dentry * dentry,
+static inline unsigned long fast_get_dcookie(struct dentry * dentry,
 	struct vfsmount * vfsmnt)
 {
-	u32 cookie;
+	unsigned long cookie;
  
 	if (dentry->d_cookie)
-		return (u32)dentry;
+		return (unsigned long)dentry;
 	get_dcookie(dentry, vfsmnt, &cookie);
 	return cookie;
 }
@@ -135,9 +135,9 @@
  * not strictly necessary but allows oprofile to associate
  * shared-library samples with particular applications
  */
-static u32 get_exec_dcookie(struct mm_struct * mm)
+static unsigned long get_exec_dcookie(struct mm_struct * mm)
 {
-	u32 cookie = 0;
+	unsigned long cookie = 0;
 	struct vm_area_struct * vma;
  
 	if (!mm)
@@ -163,9 +163,9 @@
  * sure to do this lookup before a mm->mmap modification happens so
  * we don't lose track.
  */
-static u32 lookup_dcookie(struct mm_struct * mm, unsigned long addr, off_t * offset)
+static unsigned long lookup_dcookie(struct mm_struct * mm, unsigned long addr, off_t * offset)
 {
-	u32 cookie = 0;
+	unsigned long cookie = 0;
 	struct vm_area_struct * vma;
 
 	for (vma = find_vma(mm, addr); vma; vma = vma->vm_next) {
@@ -188,7 +188,7 @@
 }
 
 
-static u32 last_cookie = ~0UL;
+static unsigned long last_cookie = ~0UL;
  
 static void add_cpu_switch(int i)
 {
@@ -199,7 +199,7 @@
 }
 
  
-static void add_ctx_switch(pid_t pid, u32 cookie)
+static void add_ctx_switch(pid_t pid, unsigned long cookie)
 {
 	add_event_entry(ESCAPE_CODE);
 	add_event_entry(CTX_SWITCH_CODE); 
@@ -208,7 +208,7 @@
 }
 
  
-static void add_cookie_switch(u32 cookie)
+static void add_cookie_switch(unsigned long cookie)
 {
 	add_event_entry(ESCAPE_CODE);
 	add_event_entry(COOKIE_SWITCH_CODE);
@@ -225,7 +225,7 @@
 
 static void add_us_sample(struct mm_struct * mm, struct op_sample * s)
 {
-	u32 cookie;
+	unsigned long cookie;
 	off_t offset;
  
  	cookie = lookup_dcookie(mm, s->eip, &offset);
@@ -317,7 +317,7 @@
 {
 	struct mm_struct * mm = 0;
 	struct task_struct * new;
-	u32 cookie;
+	unsigned long cookie;
 	int i;
  
 	for (i=0; i < cpu_buf->pos; ++i) {
diff -X dontdiff -Naur linux-linus/fs/dcookies.c linux/fs/dcookies.c
--- linux-linus/fs/dcookies.c	Wed Oct 16 19:08:50 2002
+++ linux/fs/dcookies.c	Sat Oct 19 01:08:02 2002
@@ -8,7 +8,7 @@
  * non-transitory that can be processed at a later date.
  * This is done by locking the dentry/vfsmnt pair in the
  * kernel until released by the tasks needing the persistent
- * objects. The tag is simply an u32 that refers
+ * objects. The tag is simply an unsigned long that refers
  * to the pair and can be looked up from userspace.
  */
 
@@ -46,19 +46,19 @@
 
 
 /* The dentry is locked, its address will do for the cookie */
-static inline u32 dcookie_value(struct dcookie_struct * dcs)
+static inline unsigned long dcookie_value(struct dcookie_struct * dcs)
 {
-	return (u32)dcs->dentry;
+	return (unsigned long)dcs->dentry;
 }
 
 
-static size_t dcookie_hash(u32 dcookie)
+static size_t dcookie_hash(unsigned long dcookie)
 {
 	return (dcookie >> 2) & (hash_size - 1);
 }
 
 
-static struct dcookie_struct * find_dcookie(u32 dcookie)
+static struct dcookie_struct * find_dcookie(unsigned long dcookie)
 {
 	struct dcookie_struct * found = 0;
 	struct dcookie_struct * dcs;
@@ -109,7 +109,7 @@
  * value for a dentry/vfsmnt pair.
  */
 int get_dcookie(struct dentry * dentry, struct vfsmount * vfsmnt,
-	u32 * cookie)
+	unsigned long * cookie)
 {
 	int err = 0;
 	struct dcookie_struct * dcs;
@@ -142,11 +142,12 @@
 /* And here is where the userspace process can look up the cookie value
  * to retrieve the path.
  */
-asmlinkage int sys_lookup_dcookie(u32 cookie, char * buf, size_t len)
+asmlinkage int sys_lookup_dcookie(u64 cookie64, char * buf, size_t len)
 {
+	unsigned long cookie = (unsigned long)cookie64;
+	int err = -EINVAL;
 	char * kbuf;
 	char * path;
-	int err = -EINVAL;
 	size_t pathlen;
 	struct dcookie_struct * dcs;
 
diff -X dontdiff -Naur linux-linus/include/linux/dcookies.h linux/include/linux/dcookies.h
--- linux-linus/include/linux/dcookies.h	Wed Oct 16 19:08:53 2002
+++ linux/include/linux/dcookies.h	Thu Oct 17 01:43:03 2002
@@ -44,7 +44,7 @@
  * Returns 0 on success, with *cookie filled in
  */
 int get_dcookie(struct dentry * dentry, struct vfsmount * vfsmnt,
-	u32 * cookie);
+	unsigned long * cookie);
 
 #else
 
@@ -59,7 +59,7 @@
 }
  
 static inline int get_dcookie(struct dentry * dentry,
-	struct vfsmount * vfsmnt, u32 * cookie)
+	struct vfsmount * vfsmnt, unsigned long * cookie)
 {
 	return -ENOSYS;
 } 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-19  0:34                   ` John Levon
@ 2002-10-19  0:31                     ` David S. Miller
  2002-10-19  0:40                       ` John Levon
  2002-11-01  4:33                       ` John Levon
  0 siblings, 2 replies; 21+ messages in thread
From: David S. Miller @ 2002-10-19  0:31 UTC (permalink / raw)
  To: levon; +Cc: weigand, linux-kernel

   From: John Levon <levon@movementarian.org>
   Date: Sat, 19 Oct 2002 01:34:15 +0100

   Or do I add /dev/oprofile/sizeof or whatever ?
   
That would be a great way to do this portably.

I suspect oprofile won't be the only situation where this value
would be useful, perhaps /proc/sys/kernel/pointer_size?

If you use sizeof(void *) as the initialization value in the kernel
code that implements this read-only sysctl, it will "just work".

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-19  0:23                 ` David S. Miller
@ 2002-10-19  0:34                   ` John Levon
  2002-10-19  0:31                     ` David S. Miller
  0 siblings, 1 reply; 21+ messages in thread
From: John Levon @ 2002-10-19  0:34 UTC (permalink / raw)
  To: David S. Miller; +Cc: weigand, linux-kernel

On Fri, Oct 18, 2002 at 05:23:27PM -0700, David S. Miller wrote:

> Wouldn't that someone be you?  It's not that hard to code, and if
> it's in there from the start it would really save all of us a lot
> of time.

I'm a 64-bit moron: is there a standard way to determine this ?

Or do I add /dev/oprofile/sizeof or whatever ?

thanks
john

-- 
"It's a cardboard universe ... and if you lean too hard against it, you fall
 through." 
	- Philip K. Dick 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-19  0:40                       ` John Levon
@ 2002-10-19  0:35                         ` David S. Miller
  0 siblings, 0 replies; 21+ messages in thread
From: David S. Miller @ 2002-10-19  0:35 UTC (permalink / raw)
  To: levon; +Cc: weigand, linux-kernel

   From: John Levon <levon@movementarian.org>
   Date: Sat, 19 Oct 2002 01:40:22 +0100
   
   Super. I'll do so.

Thanks for all of your hard work fixing this stuff up.
:-)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-19  0:31                     ` David S. Miller
@ 2002-10-19  0:40                       ` John Levon
  2002-10-19  0:35                         ` David S. Miller
  2002-11-01  4:33                       ` John Levon
  1 sibling, 1 reply; 21+ messages in thread
From: John Levon @ 2002-10-19  0:40 UTC (permalink / raw)
  To: David S. Miller; +Cc: weigand, linux-kernel

On Fri, Oct 18, 2002 at 05:31:28PM -0700, David S. Miller wrote:

> I suspect oprofile won't be the only situation where this value
> would be useful, perhaps /proc/sys/kernel/pointer_size?

Super. I'll do so.

thanks
john

-- 
"It's a cardboard universe ... and if you lean too hard against it, you fall
 through." 
	- Philip K. Dick 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-19  0:31                     ` David S. Miller
  2002-10-19  0:40                       ` John Levon
@ 2002-11-01  4:33                       ` John Levon
  2002-11-01 10:27                         ` David S. Miller
  1 sibling, 1 reply; 21+ messages in thread
From: John Levon @ 2002-11-01  4:33 UTC (permalink / raw)
  To: David S. Miller; +Cc: weigand, linux-kernel

On Fri, Oct 18, 2002 at 05:31:28PM -0700, David S. Miller wrote:

> That would be a great way to do this portably.
> 
> I suspect oprofile won't be the only situation where this value
> would be useful, perhaps /proc/sys/kernel/pointer_size?

The problem is this would trivially break cross-compilation. Would it
not be better to stick something in the glibc's bits/types.h
per-platform ?

Not that I particularly fancy going near glibc...

regards
john

-- 
"My first thought was I don't have any makeup. How will I survive
without my makeup ? My second thought was I didn't have any identification.
Who am I ?"
	- Earthquake victim

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-11-01  4:33                       ` John Levon
@ 2002-11-01 10:27                         ` David S. Miller
  0 siblings, 0 replies; 21+ messages in thread
From: David S. Miller @ 2002-11-01 10:27 UTC (permalink / raw)
  To: levon; +Cc: weigand, linux-kernel

   From: John Levon <levon@movementarian.org>
   Date: Fri, 1 Nov 2002 04:33:04 +0000
   
   The problem is this would trivially break cross-compilation. Would it
   not be better to stick something in the glibc's bits/types.h
   per-platform ?
   
   Not that I particularly fancy going near glibc...

No, becuase this is an attribute of the cpu the binary
is executing on, not an attribute of the ABI under which
userland compilation are taking place.

At run time, you probe this /proc/sys/kernel/pointer_size
value.  In fact, this should have no effect whatsoever on
cross-compilation.  I thought we were quite clear on this
solution?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-16  1:33     ` Linus Torvalds
@ 2002-10-16  1:35       ` John Levon
  0 siblings, 0 replies; 21+ messages in thread
From: John Levon @ 2002-10-16  1:35 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: David S. Miller, linux-kernel

On Tue, Oct 15, 2002 at 06:33:51PM -0700, Linus Torvalds wrote:

> Ok, everything applied.

Thanks !

> Quite frankly, as long as the vtune user-level tools are all proprietary,
> I don't really care all that much about vtune compatibility, but if it
> turns out to be easy and convenient we might as well try to be polite
> about it (and apparently they've sorted out all the issues with their
> kernel-level code, and are happy to do that stuff all GPL'd, but since
> it's pretty useless without the tools..).

>From my discussions with the vtune people, their kernel-level stuff has
almost exactly the same needs as oprofile. So I think they have
everything they need already (and I had to go to the effort of a total
rewrite, I think they should as well ...)

In fact I don't see a reason why they couldn't just use the oprofile
code directly anyway, and just have the propietary goop read from
/dev/oprofile/buffer. Especially if that encourages them to port the
PEBS support etc.

regards
john

-- 
"It's a cardboard universe ... and if you lean too hard against it, you fall
 through." 
	- Philip K. Dick 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-16  0:06   ` [PATCH] [8/7] oprofile - dcookies need to use u32 John Levon
  2002-10-16  0:01     ` David S. Miller
@ 2002-10-16  1:33     ` Linus Torvalds
  2002-10-16  1:35       ` John Levon
  1 sibling, 1 reply; 21+ messages in thread
From: Linus Torvalds @ 2002-10-16  1:33 UTC (permalink / raw)
  To: John Levon; +Cc: David S. Miller, linux-kernel


On Wed, 16 Oct 2002, John Levon wrote:
> 
> Look OK ? Applies after the previous 7 patches.

Ok, everything applied.

I only have one more comment - my Intel contacts tell me that the people
working on vtune are trying to make it play together somewhat with
oprofile, and that they are already talking to you.

Quite frankly, as long as the vtune user-level tools are all proprietary,
I don't really care all that much about vtune compatibility, but if it
turns out to be easy and convenient we might as well try to be polite
about it (and apparently they've sorted out all the issues with their
kernel-level code, and are happy to do that stuff all GPL'd, but since
it's pretty useless without the tools..).

Thanks,

		Linus


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-15 23:37 ` David S. Miller
@ 2002-10-16  0:06   ` John Levon
  2002-10-16  0:01     ` David S. Miller
  2002-10-16  1:33     ` Linus Torvalds
  0 siblings, 2 replies; 21+ messages in thread
From: John Levon @ 2002-10-16  0:06 UTC (permalink / raw)
  To: David S. Miller; +Cc: torvalds, linux-kernel

On Tue, Oct 15, 2002 at 04:37:49PM -0700, David S. Miller wrote:

> Can you make the dcookie a fixed sized type such

Look OK ? Applies after the previous 7 patches.

Briefly tested

thanks
john


diff -Naur -X dontdiff linux-linus2/drivers/oprofile/buffer_sync.c linux/drivers/oprofile/buffer_sync.c
--- linux-linus2/drivers/oprofile/buffer_sync.c	Tue Oct 15 23:00:30 2002
+++ linux/drivers/oprofile/buffer_sync.c	Wed Oct 16 00:49:43 2002
@@ -118,13 +118,13 @@
  * because we cannot reach this code without at least one
  * dcookie user still being registered (namely, the reader
  * of the event buffer). */
-static inline unsigned long fast_get_dcookie(struct dentry * dentry,
+static inline u32 fast_get_dcookie(struct dentry * dentry,
 	struct vfsmount * vfsmnt)
 {
-	unsigned long cookie;
+	u32 cookie;
  
 	if (dentry->d_cookie)
-		return (unsigned long)dentry;
+		return (u32)dentry;
 	get_dcookie(dentry, vfsmnt, &cookie);
 	return cookie;
 }
@@ -135,9 +135,9 @@
  * not strictly necessary but allows oprofile to associate
  * shared-library samples with particular applications
  */
-static unsigned long get_exec_dcookie(struct mm_struct * mm)
+static u32 get_exec_dcookie(struct mm_struct * mm)
 {
-	unsigned long cookie = 0;
+	u32 cookie = 0;
 	struct vm_area_struct * vma;
  
 	if (!mm)
@@ -163,9 +163,9 @@
  * sure to do this lookup before a mm->mmap modification happens so
  * we don't lose track.
  */
-static unsigned long lookup_dcookie(struct mm_struct * mm, unsigned long addr, off_t * offset)
+static u32 lookup_dcookie(struct mm_struct * mm, unsigned long addr, off_t * offset)
 {
-	unsigned long cookie = 0;
+	u32 cookie = 0;
 	struct vm_area_struct * vma;
 
 	for (vma = find_vma(mm, addr); vma; vma = vma->vm_next) {
@@ -188,7 +188,7 @@
 }
 
 
-static unsigned long last_cookie = ~0UL;
+static u32 last_cookie = ~0UL;
  
 static void add_cpu_switch(int i)
 {
@@ -199,7 +199,7 @@
 }
 
  
-static void add_ctx_switch(pid_t pid, unsigned long cookie)
+static void add_ctx_switch(pid_t pid, u32 cookie)
 {
 	add_event_entry(ESCAPE_CODE);
 	add_event_entry(CTX_SWITCH_CODE); 
@@ -208,7 +208,7 @@
 }
 
  
-static void add_cookie_switch(unsigned long cookie)
+static void add_cookie_switch(u32 cookie)
 {
 	add_event_entry(ESCAPE_CODE);
 	add_event_entry(COOKIE_SWITCH_CODE);
@@ -225,7 +225,7 @@
 
 static void add_us_sample(struct mm_struct * mm, struct op_sample * s)
 {
-	unsigned long cookie;
+	u32 cookie;
 	off_t offset;
  
  	cookie = lookup_dcookie(mm, s->eip, &offset);
@@ -317,7 +317,7 @@
 {
 	struct mm_struct * mm = 0;
 	struct task_struct * new;
-	unsigned long cookie;
+	u32 cookie;
 	int i;
  
 	for (i=0; i < cpu_buf->pos; ++i) {
diff -Naur -X dontdiff linux-linus2/drivers/oprofile/oprof.c linux/drivers/oprofile/oprof.c
--- linux-linus2/drivers/oprofile/oprof.c	Tue Oct 15 23:00:30 2002
+++ linux/drivers/oprofile/oprof.c	Wed Oct 16 00:48:50 2002
@@ -13,7 +13,6 @@
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/sched.h>
-#include <linux/dcookies.h>
 #include <linux/notifier.h>
 #include <linux/profile.h>
 #include <linux/oprofile.h>
diff -Naur -X dontdiff linux-linus2/fs/dcookies.c linux/fs/dcookies.c
--- linux-linus2/fs/dcookies.c	Tue Oct 15 22:23:29 2002
+++ linux/fs/dcookies.c	Wed Oct 16 00:51:10 2002
@@ -8,7 +8,7 @@
  * non-transitory that can be processed at a later date.
  * This is done by locking the dentry/vfsmnt pair in the
  * kernel until released by the tasks needing the persistent
- * objects. The tag is simply an unsigned long that refers
+ * objects. The tag is simply an u32 that refers
  * to the pair and can be looked up from userspace.
  */
 
@@ -46,19 +46,19 @@
 
 
 /* The dentry is locked, its address will do for the cookie */
-static inline unsigned long dcookie_value(struct dcookie_struct * dcs)
+static inline u32 dcookie_value(struct dcookie_struct * dcs)
 {
-	return (unsigned long)dcs->dentry;
+	return (u32)dcs->dentry;
 }
 
 
-static size_t dcookie_hash(unsigned long dcookie)
+static size_t dcookie_hash(u32 dcookie)
 {
 	return (dcookie >> 2) & (hash_size - 1);
 }
 
 
-static struct dcookie_struct * find_dcookie(unsigned long dcookie)
+static struct dcookie_struct * find_dcookie(u32 dcookie)
 {
 	struct dcookie_struct * found = 0;
 	struct dcookie_struct * dcs;
@@ -109,7 +109,7 @@
  * value for a dentry/vfsmnt pair.
  */
 int get_dcookie(struct dentry * dentry, struct vfsmount * vfsmnt,
-	unsigned long * cookie)
+	u32 * cookie)
 {
 	int err = 0;
 	struct dcookie_struct * dcs;
@@ -142,7 +142,7 @@
 /* And here is where the userspace process can look up the cookie value
  * to retrieve the path.
  */
-asmlinkage int sys_lookup_dcookie(unsigned long cookie, char * buf, size_t len)
+asmlinkage int sys_lookup_dcookie(u32 cookie, char * buf, size_t len)
 {
 	char * kbuf;
 	char * path;
diff -Naur -X dontdiff linux-linus2/include/linux/dcookies.h linux/include/linux/dcookies.h
--- linux-linus2/include/linux/dcookies.h	Tue Oct 15 22:23:29 2002
+++ linux/include/linux/dcookies.h	Wed Oct 16 00:48:25 2002
@@ -44,7 +44,7 @@
  * Returns 0 on success, with *cookie filled in
  */
 int get_dcookie(struct dentry * dentry, struct vfsmount * vfsmnt,
-	unsigned long * cookie);
+	u32 * cookie);
 
 #else
 
@@ -59,7 +59,7 @@
 }
  
 static inline int get_dcookie(struct dentry * dentry,
-	struct vfsmount * vfsmnt, unsigned long * cookie)
+	struct vfsmount * vfsmnt, u32 * cookie)
 {
 	return -ENOSYS;
 } 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [8/7] oprofile - dcookies need to use u32
  2002-10-16  0:06   ` [PATCH] [8/7] oprofile - dcookies need to use u32 John Levon
@ 2002-10-16  0:01     ` David S. Miller
  2002-10-16  1:33     ` Linus Torvalds
  1 sibling, 0 replies; 21+ messages in thread
From: David S. Miller @ 2002-10-16  0:01 UTC (permalink / raw)
  To: levon; +Cc: torvalds, linux-kernel

   From: John Levon <levon@movementarian.org>
   Date: Wed, 16 Oct 2002 01:06:23 +0100

   On Tue, Oct 15, 2002 at 04:37:49PM -0700, David S. Miller wrote:
   
   > Can you make the dcookie a fixed sized type such
   
   Look OK ?

Yes, thank you.

   

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2002-11-01 10:32 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-10-16  1:56 [PATCH] [8/7] oprofile - dcookies need to use u32 Ulrich Weigand
2002-10-16  2:00 ` David S. Miller
2002-10-16 16:40   ` John Levon
2002-10-16 18:29     ` Jeff Garzik
2002-10-16 21:38     ` David S. Miller
2002-10-17  0:57       ` John Levon
2002-10-17  0:55         ` David S. Miller
2002-10-17  1:16           ` John Levon
2002-10-17  1:12             ` David S. Miller
2002-10-19  0:26               ` John Levon
2002-10-19  0:23                 ` David S. Miller
2002-10-19  0:34                   ` John Levon
2002-10-19  0:31                     ` David S. Miller
2002-10-19  0:40                       ` John Levon
2002-10-19  0:35                         ` David S. Miller
2002-11-01  4:33                       ` John Levon
2002-11-01 10:27                         ` David S. Miller
  -- strict thread matches above, loose matches on Subject: below --
2002-10-15 22:32 [PATCH] [2/7] oprofile - dcookies John Levon
2002-10-15 23:37 ` David S. Miller
2002-10-16  0:06   ` [PATCH] [8/7] oprofile - dcookies need to use u32 John Levon
2002-10-16  0:01     ` David S. Miller
2002-10-16  1:33     ` Linus Torvalds
2002-10-16  1:35       ` John Levon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).