Using a local git repository with OProfile CVS

Hi All,

The OProfile source repository is currently CVS-based. I have been using git more lately because I have
found git pretty convenient. Looking around the web I found some instructions on using CVS repository
with a local git repository:

http://drupal.org/node/288873

I thought other people working on OProfile might also be interested in using git for their local
repository. I did some minor tweaks on the commands to use environment variables to minimize typing. You
will need to adjust the environment variables for local conditions. Below are the commands used to setup a
local and use a local OProfile git repository.

-Will

Environment variable setup:

export CVSROOT=:ext:USER_NAME <at> oprofile.cvs.sourceforge.net:/cvsroot/oprofile
export MODULE_NAME=oprofile
export GIT_CVS_CLONE=~/oprofile/oprofile.git
export CVS_WC_MODULE=~/oprofile/oprofile
export WORKING_SAND_BOX=~/oprofile/oprofile.work.git

Importing your module's CVS tree

cvs login
mkdir $GIT_CVS_CLONE
cd $GIT_CVS_CLONE
git cvsimport -a -p -x -v -d $CVSROOT $MODULE_NAME

Re: Using a local git repository with OProfile CVS

Maynard,

Since you are the maintainer for the Oprofile userspace package.

What are your thoughts on moving over to Git at some point during 2011?
 I would really like it if we did :)

Thanks,
John

--

-- 
John Villalovos
Intel Corporation on-site partner engineer at Red Hat, Inc.

http://tabasco.usersys.redhat.com/

------------------------------------------------------------------------------
Learn how Oracle Real Application Clusters (RAC) One Node allows customers
to consolidate database storage, standardize their database environment, and, 
should the need arise, upgrade to a full multi-node Oracle RAC database 
without downtime or disruption
http://p.sf.net/sfu/oracle-sfdevnl

[PATCH] arch/x86/oprofile/op_model_amd.c: perform initialisation on a single CPU

On 30.12.10 12:38:47, Ingo Molnar wrote:
> 
> * Robert Richter <robert.richter <at> amd.com> wrote:
> 
> > On 29.12.10 11:37:43, Ingo Molnar wrote:
> > 
> > > Hm, i'm not sure this fix is correct:
> > > 
> > >  static int op_amd_init(struct oprofile_operations *ops)
> > >  {
> > > +       /*
> > > +        * init_ibs() preforms implictly cpu-local operations, so pin this
> > > +        * thread to its current CPU
> > > +        */
> > > +       preempt_disable();
> > >         init_ibs();
> > > +       preempt_enable();
> > > 
> > > If init_ibs() is indeed CPU local, then it needs to be called on all CPUs. Does 
> > > that happen and if not why not? AFAICS it's only called on one CPU.
> > 
> > It is correct to run init_ibs() only on one cpu.  It only checks the ibs 
> > capabilities and sets up pci devices (if necessary). It runs only on one cpu but 
> > operates with the local APIC and some MSRs, thus it is better to disable 
> > preemption.
> 
> Ok, but in that case the prempt_disable()/enable() should be put into init_ibs(), 
> not be open-coded at the caller like that.
> 
> The comment about its cpu-localness could move to init_ibs() as well.

Re: [PATCH] arch/x86/oprofile/op_model_amd.c: perform initialisation on a single CPU

Ingo,

I tested your patch and it fixes the bug too.

On 03.01.11 07:03:10, Ingo Molnar wrote:
> 
> * Robert Richter <robert.richter <at> amd.com> wrote:
> 
> >  static void init_ibs(void)
> >  {
> >  	ibs_caps = get_ibs_caps();
> 
> this get_ibs_caps() call is percpu too - still it now runs with preempt off.

get_ibs_caps() uses the cpuid instruction and this is percpu. But
mixed silicon support does not allow the use of different cpus with
different cpuid features in one system, so it should be save to use it
with preemption enabled.

Also, since the check uses a combination of boot_cpu_has() and
cpuid_eax(), disabling preemption would not help here. We would have
to pin the init code to the boot cpu then. Suppose a boot cpu with ibs
enabled and a secondary (init) cpu with ibs disabled. This will crash
the system when accessing ibs msrs.

Anyway, I am fine with your change.

> 
> > +		printk(KERN_INFO "oprofile: AMD IBS detected (0x%08x)\n",
> > +		       (unsigned)ibs_caps);

Re: [PATCH] arch/x86/oprofile/op_model_amd.c: perform initialisation on a single CPU

* Robert Richter <robert.richter <at> amd.com> wrote:

>  static void init_ibs(void)
>  {
>  	ibs_caps = get_ibs_caps();

this get_ibs_caps() call is percpu too - still it now runs with preempt off.

> +		printk(KERN_INFO "oprofile: AMD IBS detected (0x%08x)\n",
> +		       (unsigned)ibs_caps);

that cast looks ugly and unnecessary.

I fixed both in the commit below. (not tested yet)

Thanks,

	Ingo

--------------->
>From c7c25802b39c443b3745cfa973dc49a97a3491f8 Mon Sep 17 00:00:00 2001
From: Robert Richter <robert.richter <at> amd.com>
Date: Mon, 3 Jan 2011 12:15:14 +0100
Subject: [PATCH] arch/x86/oprofile/op_model_amd.c: Perform initialisation on a single CPU

Disable preemption in init_ibs(). The function only checks the
ibs capabilities and sets up pci devices (if necessary). It runs
only on one cpu but operates with the local APIC and some MSRs,
thus it is better to disable preemption.

Patch "arch/x86/oprofile/op_model_amd.c: Perform initialisation on a single CPU" has been added to the 2.6.36-stable tree

This is a note to let you know that I've just added the patch titled

    arch/x86/oprofile/op_model_amd.c: Perform initialisation on a single CPU

to the 2.6.36-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     arch-x86-oprofile-op_model_amd.c-perform-initialisation-on-a-single-cpu.patch
and it can be found in the queue-2.6.36 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable <at> kernel.org> know about it.

>From c7c25802b39c443b3745cfa973dc49a97a3491f8 Mon Sep 17 00:00:00 2001
From: Robert Richter <robert.richter <at> amd.com>
Date: Mon, 3 Jan 2011 12:15:14 +0100
Subject: arch/x86/oprofile/op_model_amd.c: Perform initialisation on a single CPU

From: Robert Richter <robert.richter <at> amd.com>

commit c7c25802b39c443b3745cfa973dc49a97a3491f8 upstream.

Disable preemption in init_ibs(). The function only checks the
ibs capabilities and sets up pci devices (if necessary). It runs
only on one cpu but operates with the local APIC and some MSRs,
thus it is better to disable preemption.

[    7.034377] BUG: using smp_processor_id() in preemptible [00000000] code: modprobe/483

[PATCH] Numerical argument checking for opcontrol

There are a number of options for opcontrol that take numerical arguments. opcontrol just blindly copies
them into the various /dev/oprofile file. During testing of oprofile the following bug was filed
describing the issue:

https://bugzilla.redhat.com/show_bug.cgi?id=589638

The attached patch adds argument checking so people know when a bogus value is used.

Sign-off-by: William Cohen <wcohen <at> redhat.com>

2010-12-15  William Cohen  <wcohen <at> redhat.com>

	* utils/opcontrol: Add argument checking for numerical arguments.

-Will

------------------------------------------------------------------------------
Learn how Oracle Real Application Clusters (RAC) One Node allows customers
to consolidate database storage, standardize their database environment, and, 
should the need arise, upgrade to a full multi-node Oracle RAC database 
without downtime or disruption
http://p.sf.net/sfu/oracle-sfdevnl

_______________________________________________
oprofile-list mailing list
oprofile-list <at> lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/oprofile-list

Re: Using a local git repository with OProfile CVS

John Villalovos wrote:
> Maynard,
> 
> Since you are the maintainer for the Oprofile userspace package.
> 
> What are your thoughts on moving over to Git at some point during 2011?
Unless someone can make a case that this is really needed for the overall oprofile community's benefit,
it'll be on my back burner until I have nothing more important to do.  If you like using a git repo for your
local work, it seems that Will's Jan 3 posting ("Using a local git repository with OProfile CVS") is the
solution for now.

-Maynard

>  I would really like it if we did :)
> 
> Thanks,
> John
> 

------------------------------------------------------------------------------
Learn how Oracle Real Application Clusters (RAC) One Node allows customers
to consolidate database storage, standardize their database environment, and, 
should the need arise, upgrade to a full multi-node Oracle RAC database 
without downtime or disruption
http://p.sf.net/sfu/oracle-sfdevnl

Re: [PATCH] Numerical argument checking for opcontrol

William Cohen wrote:
> There are a number of options for opcontrol that take numerical arguments. opcontrol just blindly copies
them into the various /dev/oprofile file. During testing of oprofile the following bug was filed
describing the issue:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=589638
> 
> The attached patch adds argument checking so people know when a bogus value is used.
> 
> Sign-off-by: William Cohen <wcohen <at> redhat.com>
> 
> 2010-12-15  William Cohen  <wcohen <at> redhat.com>
> 
> 	* utils/opcontrol: Add argument checking for numerical arguments.
Patch applied.  Thanks!

-Maynard
> 
> -Will
> 
> 
> 
> ------------------------------------------------------------------------------
> Learn how Oracle Real Application Clusters (RAC) One Node allows customers
> to consolidate database storage, standardize their database environment, and, 
> should the need arise, upgrade to a full multi-node Oracle RAC database 
> without downtime or disruption
> http://p.sf.net/sfu/oracle-sfdevnl
> 
> 

Re: Using a local git repository with OProfile CVS

On 1/5/2011 12:24 PM, Maynard Johnson wrote:
> John Villalovos wrote:
>> Maynard,
>>
>> Since you are the maintainer for the Oprofile userspace package.
>>
>> What are your thoughts on moving over to Git at some point during 2011?
> Unless someone can make a case that this is really needed for the overall oprofile community's benefit,
it'll be on my back burner until I have nothing more important to do.  If you like using a git repo for your
local work, it seems that Will's Jan 3 posting ("Using a local git repository with OProfile CVS") is the
solution for now.

The only case that I have is that I think it would improve development.

I'm guessing quite a few of the people who would contribute to oprofile 
development are kernel developers.  Since they are already using git 
then it makes it easier for them to also contribute to the userspace 
code.  A common source control tool for the userspace and kernel 
component seems to make sense.  And since we know that the kernel won't 
be switching to CVS, it would require the userspace to switch to git in 
order to have a common source control tool for both pieces.

On a personal note.  I haven't used CVS in years and had to re-acquaint 
myself with it to work on submitting some patches.  And I expect others 
may have that same issue if they also haven't used CVS in years, since 
we tend to forget what was once 2nd nature.

I personally believe that switching to git would be to the overall 
oprofile community's benefit.  But maybe there are arguments against.  I 
have heard other people express interest in git for the userspace.