All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Barry Kasindorf <barry.kasindorf@amd.com>
Cc: linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Robert Richter <robert.richter@amd.com>,
	the arch/x86 maintainers <x86@kernel.org>
Subject: Re: [PATCH 1/3] AMD Family10h+ IBS support for oProfile driver: Setup routines
Date: Thu, 3 Jul 2008 11:20:52 +0200	[thread overview]
Message-ID: <20080703092052.GA5548@elte.hu> (raw)
In-Reply-To: <20080609175030.2844.77365.sendpatchset@localhost.localdomain>


* Barry Kasindorf <barry.kasindorf@amd.com> wrote:

> This patchset supports the new profiling hardware available in the 
> latest AMD CPUs in the oProfile driver.

Looks interesting! There are some patch logistics issues:

1) please rebase your patches ontop of -mm, which carries the oprofile 
multiplexing cleanups and enhancements already which heavily interact 
with your patchset.

2) -mm is also based on linux-next which has a few other changes in 
drivers/oprofile as well: i did a test-merge of your patches and 3/3 
interacted heavily with API changes in linux-next. Part of the problem 
was that you mixed whitespace changes into your patch which randomly 
iteracted with other stuff. Cleanups are nice and welcome, but try to 
keep them in a separate patch. (multiple patches if the situation 
requires it)

3) please Cc: the x86 maintainers too because these patches change quite 
a few things in the x86 architecture code.

also:

>  		case 0x10:
>  			model = &op_athlon_spec;
> -			cpu_type = "x86-64/family10";
> +			cpu_type = "x86-64/family10h";
> +			break;

please just keep the ABI string at "x86-64/family10" - we can live with 
this small quirk forever. The mess to remove that quirk is much larger 
than the benefit it brings (which is small to non-existent).

[ you really dont want such a small issue block your patches.]

the next one is OK:

> +		case 0x11:
> +			model = &op_athlon_spec;
> +			cpu_type = "x86-64/family11h";

as it's really 0x11.

	Ingo

  parent reply	other threads:[~2008-07-03 12:57 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-09 17:50 [PATCH 1/3] AMD Family10h+ IBS support for oProfile driver: Setup routines Barry Kasindorf
2008-06-09 17:50 ` [PATCH 2/3] AMD Family10h+ IBS support for oProfile driver: Interrupt routines Barry Kasindorf
2008-06-09 17:50 ` [PATCH 3/3] AMD Family10h+ IBS support for oProfile driver: buffer management Barry Kasindorf
2008-06-11 10:53   ` Pavel Machek
2008-06-11 13:25     ` Kasindorf, Barry
2008-06-11 10:51 ` [PATCH 1/3] AMD Family10h+ IBS support for oProfile driver: Setup routines Pavel Machek
2008-06-11 12:07   ` Andi Kleen
2008-06-11 13:29     ` Kasindorf, Barry
2008-06-11 18:01       ` Pavel Machek
2008-06-11 18:10         ` Kasindorf, Barry
2008-07-03  9:20 ` Ingo Molnar [this message]
2008-07-03  9:37   ` Andrew Morton
2008-07-03 10:01     ` Ingo Molnar
2008-07-03 15:36       ` [PATCH 1/3] AMD Family10h+ IBS support for oProfile driver:Setup routines Kasindorf, Barry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080703092052.GA5548@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@linux-foundation.org \
    --cc=barry.kasindorf@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robert.richter@amd.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.