Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1300331 > unrolled thread

[PATCH] staging-slicoss: Replace variable initialisations by assignments in slic_if_init()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH] staging-slicoss: Replace variable initialisations by  assignments in slic_if_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 17:40 +0100
    Re: [PATCH] staging-slicoss: Replace variable initialisations by  assignments in slic_if_init() Julia Lawall <julia.lawall@lip6.fr> - 2016-01-03 17:50 +0100
      Re: [PATCH] staging-slicoss: Replace variable initialisations by  assignments in slic_if_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 18:50 +0100
        Re: [PATCH] staging-slicoss: Replace variable initialisations by  assignments in slic_if_init() Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-01-03 19:00 +0100
          Re: [PATCH] staging-slicoss: Replace variable initialisations by  assignments in slic_if_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 19:20 +0100
            Re: [PATCH] staging-slicoss: Replace variable initialisations by  assignments in slic_if_init() Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-01-03 19:30 +0100
              Re: [PATCH] staging-slicoss: Replace variable initialisations by  assignments in slic_if_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 20:00 +0100
                Re: [PATCH] staging-slicoss: Replace variable initialisations by  assignments in slic_if_init() Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-01-03 20:50 +0100
                  Re: staging-slicoss: Replace variable initialisations by assignments  in slic_if_init() Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-01-03 21:20 +0100
                    Re: staging-slicoss: Replace variable initialisations by assignments  in slic_if_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 21:30 +0100
                  Re: staging-slicoss: Replace variable initialisations by assignments  in slic_if_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 21:20 +0100

#1300331 — [PATCH] staging-slicoss: Replace variable initialisations by assignments in slic_if_init()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 3 Jan 2016 17:25:59 +0100

Replace explicit initialisation for two local variables at the beginning
by assignments.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/slicoss/slicoss.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index b23a2d1..8fdcac8 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -2301,9 +2301,9 @@ static int slic_adapter_allocresources(struct adapter *adapter,
  */
 static int slic_if_init(struct adapter *adapter, unsigned long *flags)
 {
-	struct sliccard *card = adapter->card;
+	struct sliccard *card;
 	struct net_device *dev = adapter->netdev;
-	__iomem struct slic_regs *slic_regs = adapter->slic_regs;
+	__iomem struct slic_regs *slic_regs;
 	struct slic_shmem *pshmem;
 	int rc;
 
@@ -2348,6 +2348,7 @@ static int slic_if_init(struct adapter *adapter, unsigned long *flags)
 		adapter->queues_initialized = 1;
 	}
 
+	slic_regs = adapter->slic_regs;
 	slic_reg32_write(&slic_regs->slic_icr, ICR_INT_OFF, FLUSH);
 	mdelay(1);
 
@@ -2374,6 +2375,7 @@ static int slic_if_init(struct adapter *adapter, unsigned long *flags)
 	}
 
 	adapter->state = ADAPT_UP;
+	card = adapter->card;
 	if (!card->loadtimerset) {
 		setup_timer(&card->loadtimer, &slic_timer_load_check,
 			    (ulong)card);
-- 
2.6.3

--
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/

[toc] | [next] | [standalone]

#1300333

On Sun, 3 Jan 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 3 Jan 2016 17:25:59 +0100
> 
> Replace explicit initialisation for two local variables at the beginning
> by assignments.

Why?

julia

> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/staging/slicoss/slicoss.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
> index b23a2d1..8fdcac8 100644
> --- a/drivers/staging/slicoss/slicoss.c
> +++ b/drivers/staging/slicoss/slicoss.c
> @@ -2301,9 +2301,9 @@ static int slic_adapter_allocresources(struct adapter *adapter,
>   */
>  static int slic_if_init(struct adapter *adapter, unsigned long *flags)
>  {
> -	struct sliccard *card = adapter->card;
> +	struct sliccard *card;
>  	struct net_device *dev = adapter->netdev;
> -	__iomem struct slic_regs *slic_regs = adapter->slic_regs;
> +	__iomem struct slic_regs *slic_regs;
>  	struct slic_shmem *pshmem;
>  	int rc;
>  
> @@ -2348,6 +2348,7 @@ static int slic_if_init(struct adapter *adapter, unsigned long *flags)
>  		adapter->queues_initialized = 1;
>  	}
>  
> +	slic_regs = adapter->slic_regs;
>  	slic_reg32_write(&slic_regs->slic_icr, ICR_INT_OFF, FLUSH);
>  	mdelay(1);
>  
> @@ -2374,6 +2375,7 @@ static int slic_if_init(struct adapter *adapter, unsigned long *flags)
>  	}
>  
>  	adapter->state = ADAPT_UP;
> +	card = adapter->card;
>  	if (!card->loadtimerset) {
>  		setup_timer(&card->loadtimer, &slic_timer_load_check,
>  			    (ulong)card);
> -- 
> 2.6.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
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/

[toc] | [prev] | [next] | [standalone]

#1300349

>> Replace explicit initialisation for two local variables at the beginning
>> by assignments.
> 
> Why?

I prefer that assignments for variables like "card" and "slic_regs"
will only be performed immediately before the corresponding content will be
read again (after a few condition checks were executed).

Another description could be this view:
I suggest to move the variable initialisation a bit.

Regards,
Markus
--
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/

[toc] | [prev] | [next] | [standalone]

#1300350

On Sun, Jan 03, 2016 at 06:48:17PM +0100, SF Markus Elfring wrote:
> >> Replace explicit initialisation for two local variables at the beginning
> >> by assignments.
> > 
> > Why?
> 
> I prefer that assignments for variables like "card" and "slic_regs"
> will only be performed immediately before the corresponding content will be
> read again (after a few condition checks were executed).
> 
> Another description could be this view:
> I suggest to move the variable initialisation a bit.

And like David Miller and others just said, please don't bother us with
pointless patches such as this, if you keep it up, I'll have to add you
to my killfile as patches like this are a waste of everyone's valuable
time.

greg k-h
--
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/

[toc] | [prev] | [next] | [standalone]

#1300355

>> I prefer that assignments for variables like "card" and "slic_regs"
>> will only be performed immediately before the corresponding content will be
>> read again (after a few condition checks were executed).
>>
>> Another description could be this view:
>> I suggest to move the variable initialisation a bit.
> 
> And like David Miller and others just said, please don't bother us with
> pointless patches such as this, if you keep it up, I'll have to add you
> to my killfile as patches like this are a waste of everyone's valuable time.

I am a bit surprised that you do not like such source code fine-tuning.
Will related software improvements get another chance later (eventually together
with other changes)?

Regards,
Markus
--
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/

[toc] | [prev] | [next] | [standalone]

#1300358

On Sun, Jan 03, 2016 at 07:16:49PM +0100, SF Markus Elfring wrote:
> >> I prefer that assignments for variables like "card" and "slic_regs"
> >> will only be performed immediately before the corresponding content will be
> >> read again (after a few condition checks were executed).
> >>
> >> Another description could be this view:
> >> I suggest to move the variable initialisation a bit.
> > 
> > And like David Miller and others just said, please don't bother us with
> > pointless patches such as this, if you keep it up, I'll have to add you
> > to my killfile as patches like this are a waste of everyone's valuable time.
> 
> I am a bit surprised that you do not like such source code fine-tuning.

It's moving stuff around for no real reason, why would I like it?
Reading and reviewing and applying this type of stuff takes away from
the time I have to spend reviewing and applying actual code fixes from
other developers who are doing real and useful work.

Remember maintainer's time is our most limited resource right now.  You
are abusing that by wasting their time for no valid reason.

> Will related software improvements get another chance later (eventually together
> with other changes)?

Define "improvements".  Did you fix an obvious bug?  Did you speed up
the code in a measurable way?  Did you make the code easier to
understand somehow?  For this patch you did none of these things.

Code in staging needs to be moved out of staging, and this patch does
nothing toward achieving that goal and it wastes people's time reviewing
it to see if it is correct or not.  Please stop or again, you will end
up in some killfiles, if you haven't already been placed there.

thanks,

greg k-h
--
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/

[toc] | [prev] | [next] | [standalone]

#1300363

>> I am a bit surprised that you do not like such source code fine-tuning.
> 
> It's moving stuff around for no real reason, why would I like it?

Can such fine-tuning result in positive effects for the run-time behaviour?


> Reading and reviewing and applying this type of stuff takes away from
> the time I have to spend reviewing and applying actual code fixes from
> other developers who are doing real and useful work.

I am aware that a lot of open issues are competing for your precious
software development attention.


> Remember maintainer's time is our most limited resource right now.

That is mostly usual.


> You are abusing that by wasting their time for no valid reason.

I find a couple of my update suggestions still valid. I agree that
the importance of proposed changes is varying.


>> Will related software improvements get another chance later (eventually together
>> with other changes)?
> 
> Define "improvements".  Did you fix an obvious bug?

Maybe. - It depends on the error classes you are interested in at the moment.


> Did you speed up the code in a measurable way?

My suggestions can result in measurable differences.


> Did you make the code easier to understand somehow?
> For this patch you did none of these things.

Thanks for your view on my approach.

Will it become acceptable to reduce the scope for any more variable
definitions in further function implementations?


> Code in staging needs to be moved out of staging, and this patch does
> nothing toward achieving that goal and it wastes people's time reviewing
> it to see if it is correct or not.

I am curious on the ways the discussed software can evolve further.

Regards,
Markus
--
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/

[toc] | [prev] | [next] | [standalone]

#1300376

On Sun, Jan 03, 2016 at 07:50:18PM +0100, SF Markus Elfring wrote:
> >> I am a bit surprised that you do not like such source code fine-tuning.
> > 
> > It's moving stuff around for no real reason, why would I like it?
> 
> Can such fine-tuning result in positive effects for the run-time behaviour?

If you can not benchmark and show the proof, don't even start to claim
such a thing.

> > Did you speed up the code in a measurable way?
> 
> My suggestions can result in measurable differences.

Show the proof please.  That's the only way I will ever accept anything
else from you like this.

> Will it become acceptable to reduce the scope for any more variable
> definitions in further function implementations?

No.

> > Code in staging needs to be moved out of staging, and this patch does
> > nothing toward achieving that goal and it wastes people's time reviewing
> > it to see if it is correct or not.
> 
> I am curious on the ways the discussed software can evolve further.

That's nice, but that's not my concern.

greg k-h
--
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/

[toc] | [prev] | [next] | [standalone]

#1300381 — Re: staging-slicoss: Replace variable initialisations by assignments in slic_if_init()

On Sun, Jan 03, 2016 at 09:10:45PM +0100, SF Markus Elfring wrote:
> >> Can such fine-tuning result in positive effects for the run-time behaviour?
> > 
> > If you can not benchmark and show the proof, don't even start to claim
> > such a thing.
> 
> Which measurement results would you accept for further discussion?

If you don't know how to show your results, then this whole discussion
is pointless.

*plonk*
--
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/

[toc] | [prev] | [next] | [standalone]

#1300385 — Re: staging-slicoss: Replace variable initialisations by assignments in slic_if_init()

>> Which measurement results would you accept for further discussion?
> 
> If you don't know how to show your results, then this whole discussion
> is pointless.

I could show test results. But it could happen that I would choose insufficient
variations for execution environments.

I am unsure about the requested detail and size around test parameters.

Regards,
Markus
--
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/

[toc] | [prev] | [next] | [standalone]

#1300383 — Re: staging-slicoss: Replace variable initialisations by assignments in slic_if_init()

>> Can such fine-tuning result in positive effects for the run-time behaviour?
> 
> If you can not benchmark and show the proof, don't even start to claim
> such a thing.

Which measurement results would you accept for further discussion?


>> My suggestions can result in measurable differences.
> 
> Show the proof please.

Which information presentations would finally qualify for "the proof"
you request now?

Which test systems will be representative for you?


> That's the only way I will ever accept anything else from you like this.

Are there any update suggestions remaining in the waiting queue
which have a higher chance of acceptance because their value might be
a bit more obvious?

Regards,
Markus
--
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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web