arch/powerpc/platforms/pseries/ras.c - cleanup

PATCH In review Christophe Jaillet 3 participants 5 messages Mar 8, 2010 – Mar 9, 2010

Subsystems (2)

PowerPC Kernel janitors

lore

Share & tools

lore

Christophe Jaillet

author

2010-03-08 21:06 UTC #0 lore raw

PATCH 1/1 arch/powerpc/platforms/pseries/ras.c - cleanup

From: Christophe Jaillet <[email protected]>

Hi, here is a patch against arch/powerpc/platforms/pseries/ras.c.

No need to memset (.., 0, ...) a buffer that is complety filled by a memcpy the line after.

Signed-off-by Christophe Jaillet <[email protected]>

---

(quoted diff) +0 -0

b/arch/powerpc/platforms/pseries/ras.c
index 2b548af..7a401ed 100644

arch/powerpc/platforms/pseries/ras.c +0 -0

@@ -276,7 +276,6 @@ static struct rtas_error_log *fwnmi_get_errinfo(struct

pt_regs *regs)

      (errdata >= rtas.base && errdata < rtas.base + rtas.size - 16)) {

savep = __va(errdata);
regs->gpr[3] = savep[0]; /* restore original r3 */
- memset(mce_data_buf, 0, RTAS_ERROR_LOG_MAX); memcpy(mce_data_buf, (char *)(savep + 1), RTAS_ERROR_LOG_MAX); errhdr = (struct rtas_error_log *)mce_data_buf;
} else {

Julia Lawall

2010-03-08 21:36 UTC #1 lore raw

Quoted 6 lines · Christophe Jaillet

From: Christophe Jaillet <[email protected]>
Hi, here is a patch against arch/powerpc/platforms/pseries/ras.c.
No need to memset (.., 0, ...) a buffer that is complety filled by a memcpy the line after.

I find 14 occurrences of this in linux-next. Could it be useful for something?

julia

Quoted 28 lines

Signed-off-by Christophe Jaillet <[email protected]>
---
arch/powerpc/platforms/pseries/ras.c +0 -1
index 2b548af..7a401ed 100644
@@ -276,7 +276,6 @@ static struct rtas_error_log *fwnmi_get_errinfo(struct
 pt_regs *regs)
      (errdata >= rtas.base && errdata < rtas.base + rtas.size - 16)) {
   savep = __va(errdata);
   regs->gpr[3] = savep[0]; /* restore original r3 */
-  memset(mce_data_buf, 0, RTAS_ERROR_LOG_MAX);
   memcpy(mce_data_buf, (char *)(savep + 1), RTAS_ERROR_LOG_MAX);
   errhdr = (struct rtas_error_log *)mce_data_buf;
  } else {
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Christophe Jaillet

author

2010-03-09 06:14 UTC #2 lore raw

I personaly think that yes, because the smaller, the better.

Could you write one of your semantic match and send the patches ?

"Julia Lawall" <[email protected]> a écrit dans le message de news:[email protected]...

Quoted 7 lines · Christophe Jaillet

Quoted 6 lines
From: Christophe Jaillet <[email protected]>
Hi, here is a patch against arch/powerpc/platforms/pseries/ras.c.
No need to memset (.., 0, ...) a buffer that is complety filled by a

memcpy

Quoted 12 lines

Quoted 1 line
the line after.
I find 14 occurrences of this in linux-next. Could it be useful for something?
julia
Quoted 4 lines
Signed-off-by Christophe Jaillet <[email protected]>

Quoted 8 lines

arch/powerpc/platforms/pseries/ras.c static struct rtas_error_log
Quoted 8 lines
---
arch/powerpc/platforms/pseries/ras.c +0 -0
index 2b548af..7a401ed 100644
@@ -276,7 +276,6 @@ static struct rtas_error_log

*fwnmi_get_errinfo(struct

Quoted 11 lines

arch/powerpc/platforms/pseries/ras.c +0 -1

 pt_regs *regs)
      (errdata >= rtas.base && errdata < rtas.base + rtas.size - 16)) {
   savep = __va(errdata);
   regs->gpr[3] = savep[0]; /* restore original r3 */
-  memset(mce_data_buf, 0, RTAS_ERROR_LOG_MAX);
   memcpy(mce_data_buf, (char *)(savep + 1), RTAS_ERROR_LOG_MAX);
   errhdr = (struct rtas_error_log *)mce_data_buf;

  } else {

Quoted 2 lines

Quoted 2 lines
--
To unsubscribe from this list: send the line "unsubscribe

kernel-janitors" in

Quoted 3 lines

Quoted 3 lines
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Julia Lawall

2010-03-09 17:21 UTC #3 lore raw

Quoted 1 line · Christophe Jaillet

I personaly think that yes, because the smaller, the better.

Perhaps the memset solves some concurrency problem? I just wonder why such a pattern would come up more than one time, in code that looks fairly diverse (ie not copy-paste), if it really has no purpose. But I can always send something and see what happens...

julia

Quoted 60 lines

Could you write one of your semantic match and send the patches ?
CJ
"Julia Lawall" <[email protected]> a écrit dans le message de news:[email protected]...
Quoted 7 lines · Christophe Jaillet
Quoted 6 lines
From: Christophe Jaillet <[email protected]>
Hi, here is a patch against arch/powerpc/platforms/pseries/ras.c.
No need to memset (.., 0, ...) a buffer that is complety filled by a
memcpy
Quoted 12 lines
Quoted 1 line
the line after.
I find 14 occurrences of this in linux-next. Could it be useful for something?
julia
Quoted 4 lines
Signed-off-by Christophe Jaillet <[email protected]>
Quoted 8 lines
arch/powerpc/platforms/pseries/ras.c static struct rtas_error_log
Quoted 8 lines
---
arch/powerpc/platforms/pseries/ras.c +0 -0
index 2b548af..7a401ed 100644
@@ -276,7 +276,6 @@ static struct rtas_error_log
*fwnmi_get_errinfo(struct
Quoted 11 lines
Quoted 11 lines
arch/powerpc/platforms/pseries/ras.c +0 -1
 pt_regs *regs)
      (errdata >= rtas.base && errdata < rtas.base + rtas.size - 16)) {
   savep = __va(errdata);
   regs->gpr[3] = savep[0]; /* restore original r3 */
-  memset(mce_data_buf, 0, RTAS_ERROR_LOG_MAX);
   memcpy(mce_data_buf, (char *)(savep + 1), RTAS_ERROR_LOG_MAX);
   errhdr = (struct rtas_error_log *)mce_data_buf;
  } else {
Quoted 2 lines
Quoted 2 lines
--
To unsubscribe from this list: send the line "unsubscribe
kernel-janitors" in
Quoted 3 lines
Quoted 3 lines
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Bernd Petrovitsch

2010-03-09 10:44 UTC #4 lore raw

In reply to Christophe Jaillet

Quoted 18 lines · Christophe Jaillet

From: Christophe Jaillet <[email protected]>
Hi, here is a patch against arch/powerpc/platforms/pseries/ras.c.
No need to memset (.., 0, ...) a buffer that is complety filled by a memcpy the line after.
Signed-off-by Christophe Jaillet <[email protected]>
---
arch/powerpc/platforms/pseries/ras.c +0 -0
index 2b548af..7a401ed 100644

arch/powerpc/platforms/pseries/ras.c static struct rtas_error_log *fwnmi_get_errinfo(struct · line 280

Quoted 6 lines

arch/powerpc/platforms/pseries/ras.c +0 -1

@@ -276,7 +276,6 @@ static struct rtas_error_log *fwnmi_get_errinfo(struct
 pt_regs *regs)
      (errdata >= rtas.base && errdata < rtas.base + rtas.size - 16)) {
   savep = __va(errdata);
   regs->gpr[3] = savep[0]; /* restore original r3 */
-  memset(mce_data_buf, 0, RTAS_ERROR_LOG_MAX);
   memcpy(mce_data_buf, (char *)(savep + 1), RTAS_ERROR_LOG_MAX);

While you are at it, this ^^^^^^^^ type cast is superfluous too.

Quoted 2 lines

arch/powerpc/platforms/pseries/ras.c +0 -0
   errhdr = (struct rtas_error_log *)mce_data_buf;
  } else {

Bernd

Signature · 2 lines

Bernd Petrovitsch                  Email : [email protected]
                     LUGA : http://www.luga.at