All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Gautham R Shenoy <ego@linux.vnet.ibm.com>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>,
	"Gautham R. Shenoy" <ego@linux.vnet.ibm.com>,
	Michael Neuling <mikey@neuling.org>,
	Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-kernel@vger.kernel.org, Nicholas Piggin <npiggin@gmail.com>,
	linuxppc-dev@lists.ozlabs.org,
	Valentin Schneider <valentin.schneider@arm.com>
Subject: Re: [PATCH 1/3] powerpc/smp: Parse ibm, thread-groups with multiple properties
Date: Wed, 09 Dec 2020 14:59:22 +1100	[thread overview]
Message-ID: <87v9dbsk85.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <20201208172540.GA14206@in.ibm.com>

Gautham R Shenoy <ego@linux.vnet.ibm.com> writes:
> Hello Srikar,
>
> Thanks for taking a look at the patch.
>
> On Mon, Dec 07, 2020 at 05:40:42PM +0530, Srikar Dronamraju wrote:
>> * Gautham R. Shenoy <ego@linux.vnet.ibm.com> [2020-12-04 10:18:45]:
>> 
>> > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>> 
>> <snipped>
>> 
>> > 
>> >  static int parse_thread_groups(struct device_node *dn,
>> > -			       struct thread_groups *tg,
>> > -			       unsigned int property)
>> > +			       struct thread_groups_list *tglp)
>> >  {
>> > -	int i;
>> > -	u32 thread_group_array[3 + MAX_THREAD_LIST_SIZE];
>> > +	int i = 0;
>> > +	u32 *thread_group_array;
>> >  	u32 *thread_list;
>> >  	size_t total_threads;
>> > -	int ret;
>> > +	int ret = 0, count;
>> > +	unsigned int property_idx = 0;
>> 
>> NIT:
>> tglx mentions in one of his recent comments to try keep a reverse fir tree
>> ordering of variables where possible.
>
> I suppose you mean moving the longer local variable declarations to to
> the top and shorter ones to the bottom. Thanks. Will fix this.

Yeah. It's called "reverse christmas tree", that's googleable.

I also prefer that style, it makes the locals visually sit with the
beginning of the function body.

cheers

WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <mpe@ellerman.id.au>
To: Gautham R Shenoy <ego@linux.vnet.ibm.com>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>,
	Anton Blanchard <anton@ozlabs.org>,
	Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>,
	Michael Neuling <mikey@neuling.org>,
	Nicholas Piggin <npiggin@gmail.com>,
	Nathan Lynch <nathanl@linux.ibm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Valentin Schneider <valentin.schneider@arm.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] powerpc/smp: Parse ibm,thread-groups with multiple properties
Date: Wed, 09 Dec 2020 14:59:22 +1100	[thread overview]
Message-ID: <87v9dbsk85.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <20201208172540.GA14206@in.ibm.com>

Gautham R Shenoy <ego@linux.vnet.ibm.com> writes:
> Hello Srikar,
>
> Thanks for taking a look at the patch.
>
> On Mon, Dec 07, 2020 at 05:40:42PM +0530, Srikar Dronamraju wrote:
>> * Gautham R. Shenoy <ego@linux.vnet.ibm.com> [2020-12-04 10:18:45]:
>> 
>> > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>> 
>> <snipped>
>> 
>> > 
>> >  static int parse_thread_groups(struct device_node *dn,
>> > -			       struct thread_groups *tg,
>> > -			       unsigned int property)
>> > +			       struct thread_groups_list *tglp)
>> >  {
>> > -	int i;
>> > -	u32 thread_group_array[3 + MAX_THREAD_LIST_SIZE];
>> > +	int i = 0;
>> > +	u32 *thread_group_array;
>> >  	u32 *thread_list;
>> >  	size_t total_threads;
>> > -	int ret;
>> > +	int ret = 0, count;
>> > +	unsigned int property_idx = 0;
>> 
>> NIT:
>> tglx mentions in one of his recent comments to try keep a reverse fir tree
>> ordering of variables where possible.
>
> I suppose you mean moving the longer local variable declarations to to
> the top and shorter ones to the bottom. Thanks. Will fix this.

Yeah. It's called "reverse christmas tree", that's googleable.

I also prefer that style, it makes the locals visually sit with the
beginning of the function body.

cheers

  reply	other threads:[~2020-12-09  4:01 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-04  4:48 [PATCH 0/3] Extend Parsing "ibm, thread-groups" for Shared-L2 information Gautham R. Shenoy
2020-12-04  4:48 ` [PATCH 0/3] Extend Parsing "ibm,thread-groups" " Gautham R. Shenoy
2020-12-04  4:48 ` [PATCH 1/3] powerpc/smp: Parse ibm, thread-groups with multiple properties Gautham R. Shenoy
2020-12-04  4:48   ` [PATCH 1/3] powerpc/smp: Parse ibm,thread-groups " Gautham R. Shenoy
2020-12-07 12:10   ` Srikar Dronamraju
2020-12-07 12:10     ` Srikar Dronamraju
2020-12-08 17:25     ` Gautham R Shenoy
2020-12-08 17:25       ` Gautham R Shenoy
2020-12-09  3:59       ` Michael Ellerman [this message]
2020-12-09  3:59         ` Michael Ellerman
2020-12-09  8:35       ` Srikar Dronamraju
2020-12-09  8:35         ` Srikar Dronamraju
2020-12-09  9:05         ` Gautham R Shenoy
2020-12-09  9:05           ` Gautham R Shenoy
2020-12-04  4:48 ` [PATCH 2/3] powerpc/smp: Add support detecting thread-groups sharing L2 cache Gautham R. Shenoy
2020-12-04  4:48   ` Gautham R. Shenoy
2020-12-07 12:40   ` Srikar Dronamraju
2020-12-07 12:40     ` Srikar Dronamraju
2020-12-08 17:42     ` Gautham R Shenoy
2020-12-08 17:42       ` Gautham R Shenoy
2020-12-09  9:14       ` Srikar Dronamraju
2020-12-09  9:14         ` Srikar Dronamraju
2020-12-04  4:48 ` [PATCH 3/3] powerpc/cacheinfo: Print correct cache-sibling map/list for " Gautham R. Shenoy
2020-12-04  4:48   ` Gautham R. Shenoy
2020-12-04 10:32   ` kernel test robot
2020-12-07 13:11   ` Srikar Dronamraju
2020-12-07 13:11     ` Srikar Dronamraju
2020-12-08 17:56     ` Gautham R Shenoy
2020-12-08 17:56       ` Gautham R Shenoy
2020-12-09  8:39       ` Srikar Dronamraju
2020-12-09  8:39         ` Srikar Dronamraju
2020-12-09  9:07         ` Gautham R Shenoy
2020-12-09  9:07           ` Gautham R Shenoy

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=87v9dbsk85.fsf@mpe.ellerman.id.au \
    --to=mpe@ellerman.id.au \
    --cc=ego@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mikey@neuling.org \
    --cc=nathanl@linux.ibm.com \
    --cc=npiggin@gmail.com \
    --cc=peterz@infradead.org \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=svaidy@linux.vnet.ibm.com \
    --cc=valentin.schneider@arm.com \
    /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.