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

Groups > linux.kernel > #1300283 > unrolled thread

[PATCH 0/2] 390/qeth: Fine-tuning for qeth_core_set_online()

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 0/2] 390/qeth: Fine-tuning for qeth_core_set_online() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 11:10 +0100
    [PATCH 1/2] 390/qeth: Delete an unnecessary variable initialisation  in qeth_core_set_online() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 11:10 +0100
      Re: [PATCH 1/2] 390/qeth: Delete an unnecessary variable  initialisation in qeth_core_set_online() Heiko Carstens <heiko.carstens@de.ibm.com> - 2016-01-04 12:30 +0100
      Re: [PATCH 1/2] 390/qeth: Delete an unnecessary variable  initialisation in qeth_core_set_online() Ursula Braun <ubraun@linux.vnet.ibm.com> - 2016-01-07 15:40 +0100
        Re: 390/qeth: Delete an unnecessary variable initialisation in  qeth_core_set_online() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-08 08:20 +0100
          Re: 390/qeth: Delete an unnecessary variable initialisation in  qeth_core_set_online() Ursula Braun <ubraun@linux.vnet.ibm.com> - 2016-01-08 09:30 +0100
            Re: 390/qeth: Delete an unnecessary variable initialisation in  qeth_core_set_online() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-08 13:10 +0100
    [PATCH 2/2] 390/qeth: Refactoring for qeth_core_set_online() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 11:10 +0100
      Re: [PATCH 2/2] 390/qeth: Refactoring for qeth_core_set_online() Heiko Carstens <heiko.carstens@de.ibm.com> - 2016-01-04 12:40 +0100
        Re: [PATCH 2/2] 390/qeth: Refactoring for qeth_core_set_online() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-04 14:20 +0100
          Re: [PATCH 2/2] 390/qeth: Refactoring for qeth_core_set_online() Heiko Carstens <heiko.carstens@de.ibm.com> - 2016-01-04 15:10 +0100
            Re: 390/qeth: Refactoring for qeth_core_set_online() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-04 15:20 +0100
              Re: 390/qeth: Refactoring for qeth_core_set_online() Heiko Carstens <heiko.carstens@de.ibm.com> - 2016-01-05 09:00 +0100

#1300283 — [PATCH 0/2] 390/qeth: Fine-tuning for qeth_core_set_online()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 3 Jan 2016 10:56:45 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Delete an unnecessary variable initialisation
  Refactoring

 drivers/s390/net/qeth_core_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

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

#1300284 — [PATCH 1/2] 390/qeth: Delete an unnecessary variable initialisation in qeth_core_set_online()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 3 Jan 2016 10:48:05 +0100

Omit explicit initialisation at the beginning for one local variable
that is redefined before its first use.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/s390/net/qeth_core_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 7871537..54fde2e 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -5637,7 +5637,7 @@ static void qeth_core_remove_device(struct ccwgroup_device *gdev)
 static int qeth_core_set_online(struct ccwgroup_device *gdev)
 {
 	struct qeth_card *card = dev_get_drvdata(&gdev->dev);
-	int rc = 0;
+	int rc;
 	int def_discipline;
 
 	if (!card->discipline) {
-- 
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] | [prev] | [next] | [standalone]

#1300647 — Re: [PATCH 1/2] 390/qeth: Delete an unnecessary variable initialisation in qeth_core_set_online()

On Sun, Jan 03, 2016 at 11:02:00AM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 3 Jan 2016 10:48:05 +0100
> 
> Omit explicit initialisation at the beginning for one local variable
> that is redefined before its first use.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/s390/net/qeth_core_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
> index 7871537..54fde2e 100644
> --- a/drivers/s390/net/qeth_core_main.c
> +++ b/drivers/s390/net/qeth_core_main.c
> @@ -5637,7 +5637,7 @@ static void qeth_core_remove_device(struct ccwgroup_device *gdev)
>  static int qeth_core_set_online(struct ccwgroup_device *gdev)
>  {
>  	struct qeth_card *card = dev_get_drvdata(&gdev->dev);
> -	int rc = 0;
> +	int rc;
>  	int def_discipline;

You can generate hundreds of patches like this one. There are even plenty
more opportunities within this same file. I don't think we need this.

If at all then change all occurrences within a file at once, but that is
Ursula's call.

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

#1303615 — Re: [PATCH 1/2] 390/qeth: Delete an unnecessary variable initialisation in qeth_core_set_online()

On Sun, 2016-01-03 at 11:02 +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 3 Jan 2016 10:48:05 +0100
> 
> Omit explicit initialisation at the beginning for one local variable
> that is redefined before its first use.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/s390/net/qeth_core_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
> index 7871537..54fde2e 100644
> --- a/drivers/s390/net/qeth_core_main.c
> +++ b/drivers/s390/net/qeth_core_main.c
> @@ -5637,7 +5637,7 @@ static void qeth_core_remove_device(struct ccwgroup_device *gdev)
>  static int qeth_core_set_online(struct ccwgroup_device *gdev)
>  {
>  	struct qeth_card *card = dev_get_drvdata(&gdev->dev);
> -	int rc = 0;
> +	int rc;
>  	int def_discipline;
> 
>  	if (!card->discipline) {

As Heiko already answered, you could propose a lot of this kind of
changes with just minor benefit. I do not want to push them in single
patches. Either there is a cleanup patch for explicit initialisation of
local variables in the whole qeth driver, or we take care about such
minor changes, once we touch the code anyway due to other reasons.

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

#1304224 — Re: 390/qeth: Delete an unnecessary variable initialisation in qeth_core_set_online()

> As Heiko already answered, you could propose a lot of this kind of changes
> with just minor benefit. I do not want to push them in single patches.

Thanks for your clarification.


> Either there is a cleanup patch for explicit initialisation of
> local variables in the whole qeth driver,

Is there any more fine-tuning cooking in the background?


> or we take care about such minor changes, once we touch the code anyway

How often will this really happen?


> due to other reasons.

I am curious which ones will trigger further related software improvements.

Regards,
Markus

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

#1304257 — Re: 390/qeth: Delete an unnecessary variable initialisation in qeth_core_set_online()

On Fri, 2016-01-08 at 08:18 +0100, SF Markus Elfring wrote:
> > As Heiko already answered, you could propose a lot of this kind of changes
> > with just minor benefit. I do not want to push them in single patches.
> 
> Thanks for your clarification.
> 
> 
> > Either there is a cleanup patch for explicit initialisation of
> > local variables in the whole qeth driver,
> 
> Is there any more fine-tuning cooking in the background?
Not yet; qeth is an important driver for Linux on System z; there are
lots of investigation ideas for improvements, which we will take care
about according to their priorities. I regard your proposed fine-tuning
code change as valid, but prioritize it as one with lowest benefit,
since it does not really make a difference once compiled.
> 
> 
> > or we take care about such minor changes, once we touch the code anyway
> 
> How often will this really happen?
There is no general rule. Check our git history to answer this question.
> 
> 
> > due to other reasons.
> 
> I am curious which ones will trigger further related software improvements.
> 
> Regards,
> Markus
> 

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

#1304454 — Re: 390/qeth: Delete an unnecessary variable initialisation in qeth_core_set_online()

>> Is there any more fine-tuning cooking in the background?
> Not yet;

I am a bit surprised by this information.


> qeth is an important driver for Linux on System z;

Good to know …


> there are lots of investigation ideas for improvements,
> which we will take care about according to their priorities.

Software development as usual …


> I regard your proposed fine-tuning code change as valid,

Thanks for a bit of positive feedback.


> but prioritize it as one with lowest benefit,

This is fine in principle.


> since it does not really make a difference once compiled.

Would you like to help in the determination if deletion of unnecessary variable
initialisations (besides in the implementation of the function "qeth_core_set_online")
will result in measurable effects?

Regards,
Markus

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

#1300285 — [PATCH 2/2] 390/qeth: Refactoring for qeth_core_set_online()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 3 Jan 2016 10:50:11 +0100

Reduce the scope for the local variable "def_discipline" to one branch
of an if statement.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/s390/net/qeth_core_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 54fde2e..3261977 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -5638,9 +5638,10 @@ static int qeth_core_set_online(struct ccwgroup_device *gdev)
 {
 	struct qeth_card *card = dev_get_drvdata(&gdev->dev);
 	int rc;
-	int def_discipline;
 
 	if (!card->discipline) {
+		int def_discipline;
+
 		if (card->info.type == QETH_CARD_TYPE_IQD)
 			def_discipline = QETH_DISCIPLINE_LAYER3;
 		else
-- 
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] | [prev] | [next] | [standalone]

#1300651 — Re: [PATCH 2/2] 390/qeth: Refactoring for qeth_core_set_online()

On Sun, Jan 03, 2016 at 11:02:56AM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 3 Jan 2016 10:50:11 +0100
> 
> Reduce the scope for the local variable "def_discipline" to one branch
> of an if statement.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/s390/net/qeth_core_main.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
> index 54fde2e..3261977 100644
> --- a/drivers/s390/net/qeth_core_main.c
> +++ b/drivers/s390/net/qeth_core_main.c
> @@ -5638,9 +5638,10 @@ static int qeth_core_set_online(struct ccwgroup_device *gdev)
>  {
>  	struct qeth_card *card = dev_get_drvdata(&gdev->dev);
>  	int rc;
> -	int def_discipline;
>  
>  	if (!card->discipline) {
> +		int def_discipline;
> +
>  		if (card->info.type == QETH_CARD_TYPE_IQD)
>  			def_discipline = QETH_DISCIPLINE_LAYER3;

Same here: I don't think we want to start with patches like this. This
going to be a never ending story without much benefit.

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

#1300742 — Re: [PATCH 2/2] 390/qeth: Refactoring for qeth_core_set_online()

>> +++ b/drivers/s390/net/qeth_core_main.c
>> @@ -5638,9 +5638,10 @@ static int qeth_core_set_online(struct ccwgroup_device *gdev)
>>  {
>>  	struct qeth_card *card = dev_get_drvdata(&gdev->dev);
>>  	int rc;
>> -	int def_discipline;
>>  
>>  	if (!card->discipline) {
>> +		int def_discipline;
>> +
>>  		if (card->info.type == QETH_CARD_TYPE_IQD)
>>  			def_discipline = QETH_DISCIPLINE_LAYER3;
> 
> Same here: I don't think we want to start with patches like this.

Thanks for your feedback.


> This going to be a never ending story without much benefit.

Is the source code a bit clearer and safer if it will be expressed
directly that the use of a specific variable is not intended for
a complete function implementation but for the smaller scope
of an if branch?

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]

#1300760 — Re: [PATCH 2/2] 390/qeth: Refactoring for qeth_core_set_online()

On Mon, Jan 04, 2016 at 02:10:34PM +0100, SF Markus Elfring wrote:
> >> +++ b/drivers/s390/net/qeth_core_main.c
> >> @@ -5638,9 +5638,10 @@ static int qeth_core_set_online(struct ccwgroup_device *gdev)
> >>  {
> >>  	struct qeth_card *card = dev_get_drvdata(&gdev->dev);
> >>  	int rc;
> >> -	int def_discipline;
> >>  
> >>  	if (!card->discipline) {
> >> +		int def_discipline;
> >> +
> >>  		if (card->info.type == QETH_CARD_TYPE_IQD)
> >>  			def_discipline = QETH_DISCIPLINE_LAYER3;
> > 
> > Same here: I don't think we want to start with patches like this.
> 
> Thanks for your feedback.
> 
> 
> > This going to be a never ending story without much benefit.
> 
> Is the source code a bit clearer and safer if it will be expressed
> directly that the use of a specific variable is not intended for
> a complete function implementation but for the smaller scope
> of an if branch?

This depends on the function and what the author prefers. In this case the
function body is very small so I don't see any benefit at all.

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

#1300766 — Re: 390/qeth: Refactoring for qeth_core_set_online()

> In this case the function body is very small
> so I don't see any benefit at all.

Do you care for fine-tuning of variable placement occasionally?

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]

#1301316 — Re: 390/qeth: Refactoring for qeth_core_set_online()

On Mon, Jan 04, 2016 at 03:10:41PM +0100, SF Markus Elfring wrote:
> > In this case the function body is very small
> > so I don't see any benefit at all.
> 
> Do you care for fine-tuning of variable placement occasionally?

No.

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