From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S938878AbcIHMjt (ORCPT ); Thu, 8 Sep 2016 08:39:49 -0400 Received: from Galois.linutronix.de ([146.0.238.70]:44064 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755926AbcIHMjs (ORCPT ); Thu, 8 Sep 2016 08:39:48 -0400 Date: Thu, 8 Sep 2016 14:36:54 +0200 (CEST) From: Thomas Gleixner To: Fenghua Yu cc: "H. Peter Anvin" , Ingo Molnar , Tony Luck , Peter Zijlstra , Tejun Heo , Borislav Petkov , Stephane Eranian , Marcelo Tosatti , David Carrillo-Cisneros , Shaohua Li , Ravi V Shankar , Vikas Shivappa , Sai Prakhya , linux-kernel , x86 Subject: Re: [PATCH v2 20/33] x86/intel_rdt.h: Header for inter_rdt.c In-Reply-To: <1473328647-33116-21-git-send-email-fenghua.yu@intel.com> Message-ID: References: <1473328647-33116-1-git-send-email-fenghua.yu@intel.com> <1473328647-33116-21-git-send-email-fenghua.yu@intel.com> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 8 Sep 2016, Fenghua Yu wrote: Subject: x86/intel_rdt.h: Header for inter_rdt.c inter_rdt? I know about Inter Mailand .... > The header mainly provides functions to call from the user interface > file intel_rdt_rdtgroup.c. What the heck? We do not introduce function prototypes and whatever crap without an implementation. We add the stuff when we add a function or implement something which needs a define/struct whatever. > +enum resource_type { > + RESOURCE_L3 = 0, > + RESOURCE_NUM = 1, Why does this need an explicit enum initialization? > +}; > + > +#define MAX_CACHE_LEAVES 4 > +#define MAX_CACHE_DOMAINS 64 > + > +DECLARE_PER_CPU_READ_MOSTLY(int, cpu_l3_domain); > +DECLARE_PER_CPU_READ_MOSTLY(struct rdtgroup *, cpu_rdtgroup); > > extern struct static_key rdt_enable_key; > void __intel_rdt_sched_in(void *dummy); > > +extern bool cdp_enabled; > + > +struct rdt_opts { > + bool cdp_enabled; > + bool verbose; > + bool simulate_cat_l3; > +}; > + > +struct cache_domain { > + cpumask_t shared_cpu_map[MAX_CACHE_DOMAINS]; > + unsigned int max_cache_domains_num; > + unsigned int level; > + unsigned int shared_cache_id[MAX_CACHE_DOMAINS]; > +}; > + > +extern struct rdt_opts rdt_opts; > + > struct clos_cbm_table { > unsigned long cbm; > unsigned int clos_refcnt; > }; > > struct clos_config { > - unsigned long *closmap; > + unsigned long **closmap; > u32 max_closid; > - u32 closids_used; > }; > > +struct shared_domain { > + struct cpumask cpumask; > + int l3_domain; > +}; > + > +#define for_each_cache_domain(domain, start_domain, max_domain) \ > + for (domain = start_domain; domain < max_domain; domain++) > + > +extern struct clos_config cconfig; > +extern struct shared_domain *shared_domain; > +extern int shared_domain_num; > + > +extern struct rdtgroup *root_rdtgrp; > + > +extern struct clos_cbm_table **l3_cctable; > + > +extern unsigned int min_bitmask_len; > +extern void msr_cpu_update(void *arg); > +extern inline void closid_get(u32 closid, int domain); extern inline? > +extern void closid_put(u32 closid, int domain); That's declared static in the source, but sure you do not notice because intel_rdc.c is not hooked up to the Makefile yet..... > +extern void closid_free(u32 closid, int domain, int level); is declared static inline .... > +extern int closid_alloc(u32 *closid, int domain); amd more of this crap to follow. I explicitely asked you last time to do: >> Which is not making the review any simpler. In order to understand the >> modifications I have to go back and page in the original stuff from last >> year once again. So I have to read the original patch first to >> understand the modifications and then get the overall picture of the new >> stuff. Please fold stuff back to the proper places so I can start >> reviewing this thing under the new design idea instead of twisting my >> brain around two designs. And you replied: > Ok. I will do that. Actually you did the reverse. You introduced more crap in the original patches. See 12/32 vs. the previous version http://marc.info/?l=linux-kernel&m=146836100821478 What's the value of mechanically split patches which cannot even compile on their own? Nothing at all except creating the hell for reviewers. Thanks, tglx