Re: [kvm-devel] [Qemu-devel] [PATCH] use a thread id variable

2008-03-09 Thread Gilad Ben-Yossef
Glauber Costa wrote:
> This patch introduces a "thread_id" variable to CPUState.
> It's duty will be to hold the process, or more generally, thread
> id of the current executing cpu
> 
>  env->nb_watchpoints = 0;
> +#ifdef __WIN32
> +env->thread_id = GetCurrentProcessId();
> +#else
> +env->thread_id = getpid();
> +#endif
>  *penv = env;


hmm... maybe I'm missing something, but in Linux at least I think you 
would prefer this to be gettid() rather then getpid as each CPU has it's 
own thread, not a different process.

No?

Gilad



-- 
Gilad Ben-Yossef <[EMAIL PROTECTED]>
Chief Coffee Drinker

Codefidence Ltd.| Web: http://codefidence.com
Work: +972-3-7515563 ext. 201   | Mobile: +972-52-8260388

"Your hovercraft is full of eels. For information on
 emptying your hovercraft, turn to Section 2.6.a.17
 of your hovercraft user manual".
- The Monty Python technical writer


-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH] Mark kobjects as unitialized

2008-03-09 Thread Mikael Pettersson
Balaji Rao writes:
 > Yes the idea works. One more memset is needed in sysdev_register. Here's the 
 > final patch.
 > 
 > diff --git a/drivers/base/sys.c b/drivers/base/sys.c
 > index 2f79c55..7c839d9 100644
 > --- a/drivers/base/sys.c
 > +++ b/drivers/base/sys.c
 > @@ -133,6 +133,7 @@ int sysdev_class_register(struct sysdev_class * cls)
 > pr_debug("Registering sysdev class '%s'\n",
 >  kobject_name(&cls->kset.kobj));
 > INIT_LIST_HEAD(&cls->drivers);
 > +   memset(&cls->kset.kobj, 0x00, sizeof(struct kobject));
 > cls->kset.kobj.parent = &system_kset->kobj;
 > cls->kset.kobj.ktype = &ktype_sysdev_class;
 > cls->kset.kobj.kset = system_kset;
 > @@ -227,6 +228,7 @@ int sysdev_register(struct sys_device * sysdev)
 >  
 > pr_debug("Registering sys device '%s'\n", 
 > kobject_name(&sysdev->kobj));
 >  
 > +   memset(&sysdev->kobj, 0x00, sizeof(struct kobject));
 > /* Make sure the kset is set */
 > sysdev->kobj.kset = &cls->kset;
 > 

Thanks, 2.6.25-rc4 + these two memset()s is finally stable for
me with no warnings, BUG()s, or panics.

(However, the patch is whitespace damaged with initial tabs
converted to spaces.)

If you want to pass this on to Linus, you can add a

Tested-by: Mikael Pettersson <[EMAIL PROTECTED]>

/Mikael

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [Qemu-devel] [PATCH] use a thread id variable

2008-03-09 Thread Jamie Lokier
Gilad Ben-Yossef wrote:
> Glauber Costa wrote:
> >This patch introduces a "thread_id" variable to CPUState.
> >It's duty will be to hold the process, or more generally, thread
> >id of the current executing cpu
> >
> > env->nb_watchpoints = 0;
> >+#ifdef __WIN32
> >+env->thread_id = GetCurrentProcessId();
> >+#else
> >+env->thread_id = getpid();
> >+#endif
> > *penv = env;
> 
> hmm... maybe I'm missing something, but in Linux at least I think you 
> would prefer this to be gettid() rather then getpid as each CPU has it's 
> own thread, not a different process.

On most platforms, getpid() returns the same value for all threads, so
it's not useful as a thread id.

On Linux, it depends which version of threads.  The old package,
LinuxThreads, has different getpid() for each thread.  The current one,
NPTL, has them all the same.

What you're supposed to do with pthreads in general is use pthread_self().

Btw, unfortunately pthread_self() is not safe to call from signal
handlers.

-- Jamie

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [Qemu-devel] [PATCH] use a thread id variable

2008-03-09 Thread Gilad Ben-Yossef
Jamie Lokier wrote:
> Gilad Ben-Yossef wrote:
>> Glauber Costa wrote:
>>> This patch introduces a "thread_id" variable to CPUState.
>>> It's duty will be to hold the process, or more generally, thread
>>> id of the current executing cpu
>>>
>>> env->nb_watchpoints = 0;
>>> +#ifdef __WIN32
>>> +env->thread_id = GetCurrentProcessId();
>>> +#else
>>> +env->thread_id = getpid();
>>> +#endif
>>> *penv = env;
>> hmm... maybe I'm missing something, but in Linux at least I think you 
>> would prefer this to be gettid() rather then getpid as each CPU has it's 
>> own thread, not a different process.
> 
> On most platforms, getpid() returns the same value for all threads, so
> it's not useful as a thread id.

Of course it does - this what POSIX says it should do (Linux 2.4 in 
compliance not withstanding). Which is why I suggested to use on Linux 
the non standard gettid() rather then getpid
> 
> On Linux, it depends which version of threads.  The old package,
> LinuxThreads, has different getpid() for each thread.  The current one,
> NPTL, has them all the same.


LinuxThreads behavior is wrong according to the POSIX standard. Of 
course, nothing else was possible with 2.4 kernels, so this is not an 
error on LinuxThreads coders part.


> What you're supposed to do with pthreads in general is use pthread_self().

Unfortunately, AFAIK the opaque handle that pthread_self() returns is 
not  quite meaningless outside of the process whereas what the non 
standard gettid() returns can actually be used to identify a thread from 
"outside" the process, like the shell.


Gilad

-- 
Gilad Ben-Yossef <[EMAIL PROTECTED]>
Chief Coffee Drinker

Codefidence Ltd.| Web: http://codefidence.com
Work: +972-3-7515563 ext. 201   | Mobile: +972-52-8260388

"Your hovercraft is full of eels. For information on
 emptying your hovercraft, turn to Section 2.6.a.17
 of your hovercraft user manual".
- The Monty Python technical writer


-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [Qemu-devel] [PATCH] use a thread id variable

2008-03-09 Thread M. Warner Losh
In message: <[EMAIL PROTECTED]>
Jamie Lokier <[EMAIL PROTECTED]> writes:
: Btw, unfortunately pthread_self() is not safe to call from signal
: handlers.

And also often times meaningless, as signal handlers can run in
arbitrary threads...

Warner

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [Qemu-devel] [PATCH] use a thread id variable

2008-03-09 Thread M. Warner Losh
In message: <[EMAIL PROTECTED]>
Gilad Ben-Yossef <[EMAIL PROTECTED]> writes:
: > What you're supposed to do with pthreads in general is use pthread_self().
: 
: Unfortunately, AFAIK the opaque handle that pthread_self() returns is 
: not  quite meaningless outside of the process whereas what the non 
: standard gettid() returns can actually be used to identify a thread from 
: "outside" the process, like the shell.

gettid() is also non-standard.  If you want to interact with a thread,
you gotta use pthread_self() if you want your code to be portable.

Warner

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [Qemu-devel] [PATCH] use a thread id variable

2008-03-09 Thread Daniel P. Berrange
On Sun, Mar 09, 2008 at 11:26:43AM +0200, Gilad Ben-Yossef wrote:
> Glauber Costa wrote:
> > This patch introduces a "thread_id" variable to CPUState.
> > It's duty will be to hold the process, or more generally, thread
> > id of the current executing cpu
> > 
> >  env->nb_watchpoints = 0;
> > +#ifdef __WIN32
> > +env->thread_id = GetCurrentProcessId();
> > +#else
> > +env->thread_id = getpid();
> > +#endif
> >  *penv = env;
> 
> 
> hmm... maybe I'm missing something, but in Linux at least I think you 
> would prefer this to be gettid() rather then getpid as each CPU has it's 
> own thread, not a different process.

No, this patch is the generic QEMU code, which is single threaded, so
using getpid() is correct. Glauber  has a separate patch in this series
which implements the equivalent for KVM which does indeed use gettid()

Regards,
Dan.
-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [Qemu-devel] [PATCH] use a thread id variable

2008-03-09 Thread Jamie Lokier
M. Warner Losh wrote:
> In message: <[EMAIL PROTECTED]>
> Jamie Lokier <[EMAIL PROTECTED]> writes:
> : Btw, unfortunately pthread_self() is not safe to call from signal
> : handlers.
> 
> And also often times meaningless, as signal handlers can run in
> arbitrary threads...

That's usually the case, but sometimes it is useful.  Some causes of
signals are thread specific, or can be asked to be, and it's nice to
know which thread is receiving them (e.g. thread specific timers,
SIGIOs, write-protection SEGVs, and even sending messages with good
old pthread_kill (same reason as kernel uses IPIs)).

GCC's Boehm garbage collector uses pthread_self() from a signal
handler.  I've used gettid() in a signal handler on a few occasions.


-- Jamie

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] loop in copy_user_generic_string

2008-03-09 Thread Zdenek Kabelac
2008/3/7, Zdenek Kabelac <[EMAIL PROTECTED]>:
> 2008/3/5, Zdenek Kabelac <[EMAIL PROTECTED]>:
>
> > 2008/3/5, Avi Kivity <[EMAIL PROTECTED]>:
>  >
>  > > Andi Kleen wrote:
>  >  >  > Avi Kivity <[EMAIL PROTECTED]> writes:
>  >  >  >
>  >  >  >> Most likely movs emulation is broken for long counts.  Please post a
>  >  >  >> disassembly of copy_user_generic_string to make sure we're looking 
> at
>  >  >  >> the same code.
>  >  >  >>
>  >  >  >
>  >  >  > Be careful -- this code is patched at runtime and what you
>  >  >  > see in the vmlinux is not necessarily the same that is executed
>  >  >  >
>  >  >  >
>  >  >
>  >  >
>  >  > If the disassembled instruction isn't marked as an alternative in the
>  >  >  source, then it can't be patched, right?
>  >  >
>
>
> Hello
>
>  Any progress on this - It looks like I get this bug quite often when I test
>  device-mapper code.
>


Hello

I've made some more experiments and noticed few more things:

a) - it is just enough to run parallel loop with cat LVM partition
>/dev/null and dmsetup status

b) when I insert for() loop for zeroing allocated memory in the
dm-ioctl copy_params function my loop start once the memory crosses
exactly 4KB boundary (visible from register content)

c) in my trace log I could usually always see this pattern:
[  160.634897]  [] preempt_schedule_irq+0x5a/0xa0
[  160.634897]  [] retint_kernel+0x26/0x30

from the look in the arch/x86/kernel/entry64.s I could really see
there is some potentiality for internal loop that may call
preempt_schedule_irq in upon some check in  exit_intr - but having
actually now idea what's this all about...

I've put there just some extra dump_stack trace in the
preempt_schedule_irq - and it's really being printed - but quite
slowly actually considering process eats 100% CPU

So anyone has any idea what might be wrong ?

Zdenek

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel