Date: Tue, 15 Mar 2005 23:07:36 -0800
From: Andrew Morton <akpm@osdl.org>
To: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Cc: torvalds@osdl.org, linux-kernel@vger.kernel.org, rohit.seth@intel.com
Subject: Re: [PATCH] Reading deterministic cache parameters and exporting it
 in /sysfs
Message-Id: <20050315230736.6faa3734.akpm@osdl.org>
In-Reply-To: <20050315152448.A1697@unix-os.sc.intel.com>
References: <20050315152448.A1697@unix-os.sc.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1333
Lines: 47

Venkatesh Pallipadi <venkatesh.pallipadi@intel.com> wrote:
>
>  The attached patch adds support for using cpuid(4) instead of cpuid(2), to get 
>  CPU cache information in a deterministic way for Intel CPUs, whenever 
>  supported.

- find_num_cache_leaves can be marked __init

- Please look for other __init opportunities.  That's quite a lot of code.

- Some functions have a space before the ( and some don't:

	+static ssize_t show_size (struct _cpuid4_info *this_leaf, char *buf)

  omitting the space is preferred.

- Don't cast the return value of kmalloc:

+	cpuid4_info[cpu] = (struct _cpuid4_info *)kmalloc(
+	    sizeof(struct _cpuid4_info) * num_cache_leaves, GFP_KERNEL);

- Sometimes there's a space after an `if', sometimes not.

+		if(cpuid4_info[i])

  a space is preferred.

- kfree(NULL) is permitted:

+	if(cpuid4_info[i])
+		kfree(cpuid4_info[i]);
+	if(cache_kobject[i])
+		kfree(cache_kobject[i]);
+	if(index_kobject[i])
+		kfree(index_kobject[i]);

  (in several places)


Once you've worked through the design issues with davej, please upissue the
patch, thanks.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/