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
[email protected]
From: Ulrich Weigand <[email protected]>
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.
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
From: John Levon <[email protected]>
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.
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
From: John Levon <[email protected]>
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.)
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
From: John Levon <[email protected]>
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
[email protected]
From: John Levon <[email protected]>
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.
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;
}
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
From: John Levon <[email protected]>
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".
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
From: John Levon <[email protected]>
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.
:-)
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
From: John Levon <[email protected]>
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?