Re: [RFC PATCH v2] do_exit(): panic() recursion detected
I see 2 paths how "bad things" can cause recursive do_exit - various
traps that go through die() and therefore covered by panic_on_oops; and
do_group_exit() as result of fatal signal.
Provided one add "panic on coredump" functionality, path through
do_group_exit() covered as well.
Let's drop this patch.
Thanks, Vladimir
On 12/7/20 5:49 PM, Eric W. Biederman wrote:
Vladimir Kondratiev writes:
Please ignore version 1 of the patch - it was sent from wrong mail address.
To clarify the reason:
Situation where do_exit() re-entered, discovered by static code analysis.
For safety critical system, it is better to panic() when minimal chance of
corruption detected. For this reason, we also panic on fatal signal delivery -
patch for this not submitted yet.
What did the static code analysis say? What triggers the recursion.
What makes it safe to even call panic on this code path? Is there
enough kernel stack?
My sense is that if this actually can happen and is a real concern,
and that it is safe to do something on this code path it is probably
better just to ooops. That way if someone is trying to debug such
a recursion they will have a backtrace to work with. Plus panic
on oops will work.
Eric
On 12/7/20 2:44 PM, Vladimir Kondratiev wrote:
Recursive do_exit() is symptom of compromised kernel integrity.
For safety critical systems, it may be better to
panic() in this case to minimize risk.
Signed-off-by: Vladimir Kondratiev
Change-Id: I42f45900a08c4282c511b05e9e6061360d07db60
---
Documentation/admin-guide/kernel-parameters.txt | 6 ++
include/linux/kernel.h | 1 +
kernel/exit.c | 7 +++
kernel/sysctl.c | 9 +
4 files changed, 23 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index 44fde25bb221..6e12a6804557 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3508,6 +3508,12 @@
bit 4: print ftrace buffer
bit 5: print all printk messages in buffer
+panic_on_exit_recursion
+ panic() when do_exit() recursion detected, rather then
+ try to stay running whenever possible.
+ Useful on safety critical systems; re-entry in do_exit
+ is a symptom of compromised kernel integrity.
+
panic_on_taint= Bitmask for conditionally calling panic() in add_taint()
Format: [,nousertaint]
Hexadecimal bitmask representing the set of TAINT flags
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 2f05e9128201..5afb20534cb2 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -539,6 +539,7 @@ extern int sysctl_panic_on_rcu_stall;
extern int sysctl_panic_on_stackoverflow;
extern bool crash_kexec_post_notifiers;
+extern int panic_on_exit_recursion;
/*
* panic_cpu is used for synchronizing panic() and crash_kexec() execution.
It
diff --git a/kernel/exit.c b/kernel/exit.c
index 1f236ed375f8..162799a8b539 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -68,6 +68,9 @@
#include
#include
+int panic_on_exit_recursion __read_mostly;
+core_param(panic_on_exit_recursion, panic_on_exit_recursion, int, 0644);
+
static void __unhash_process(struct task_struct *p, bool group_dead)
{
nr_threads--;
@@ -757,6 +760,10 @@ void __noreturn do_exit(long code)
*/
if (unlikely(tsk->flags & PF_EXITING)) {
pr_alert("Fixing recursive fault but reboot is needed!\n");
+ if (panic_on_exit_recursion)
+ panic("Recursive do_exit() detected in %s[%d]\n",
+ current->comm, task_pid_nr(current));
+
futex_exit_recursive(tsk);
set_current_state(TASK_UNINTERRUPTIBLE);
schedule();
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index afad085960b8..bb397fba2c42 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2600,6 +2600,15 @@ static struct ctl_table kern_table[] = {
.extra2 = _thousand,
},
#endif
+ {
+ .procname = "panic_on_exit_recursion",
+ .data = _on_exit_recursion,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ },
{
.procname = "panic_on_warn",
.data = _on_warn,
Re: [RFC PATCH] do_exit(): panic() recursion detected
We do panic on oops as well. We panic on anything that may point to system stability issues. I can't proof this code can't be reached without oops, so I want to panic here as well. On 12/7/20 2:51 PM, Peter Zijlstra wrote: On Mon, Dec 07, 2020 at 02:40:49PM +0200, Vladimir Kondratiev wrote: From: Vladimir Kondratiev Recursive do_exit() is symptom of compromised kernel integrity. For safety critical systems, it may be better to panic() in this case to minimize risk. You've not answered the previously posed question on why panic_on_oops isn't more suitable for your type of systems. Signed-off-by: Vladimir Kondratiev Change-Id: I42f45900a08c4282c511b05e9e6061360d07db60 This Change-ID crap doesn't belong in kernel patches.
Re: [RFC PATCH v2] do_exit(): panic() recursion detected
Please ignore version 1 of the patch - it was sent from wrong mail address.
To clarify the reason:
Situation where do_exit() re-entered, discovered by static code analysis.
For safety critical system, it is better to panic() when minimal chance
of corruption detected. For this reason, we also panic on fatal signal
delivery - patch for this not submitted yet.
On 12/7/20 2:44 PM, Vladimir Kondratiev wrote:
Recursive do_exit() is symptom of compromised kernel integrity.
For safety critical systems, it may be better to
panic() in this case to minimize risk.
Signed-off-by: Vladimir Kondratiev
Change-Id: I42f45900a08c4282c511b05e9e6061360d07db60
---
Documentation/admin-guide/kernel-parameters.txt | 6 ++
include/linux/kernel.h | 1 +
kernel/exit.c | 7 +++
kernel/sysctl.c | 9 +
4 files changed, 23 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index 44fde25bb221..6e12a6804557 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3508,6 +3508,12 @@
bit 4: print ftrace buffer
bit 5: print all printk messages in buffer
+ panic_on_exit_recursion
+ panic() when do_exit() recursion detected, rather then
+ try to stay running whenever possible.
+ Useful on safety critical systems; re-entry in do_exit
+ is a symptom of compromised kernel integrity.
+
panic_on_taint= Bitmask for conditionally calling panic() in add_taint()
Format: [,nousertaint]
Hexadecimal bitmask representing the set of TAINT flags
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 2f05e9128201..5afb20534cb2 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -539,6 +539,7 @@ extern int sysctl_panic_on_rcu_stall;
extern int sysctl_panic_on_stackoverflow;
extern bool crash_kexec_post_notifiers;
+extern int panic_on_exit_recursion;
/*
* panic_cpu is used for synchronizing panic() and crash_kexec() execution. It
diff --git a/kernel/exit.c b/kernel/exit.c
index 1f236ed375f8..162799a8b539 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -68,6 +68,9 @@
#include
#include
+int panic_on_exit_recursion __read_mostly;
+core_param(panic_on_exit_recursion, panic_on_exit_recursion, int, 0644);
+
static void __unhash_process(struct task_struct *p, bool group_dead)
{
nr_threads--;
@@ -757,6 +760,10 @@ void __noreturn do_exit(long code)
*/
if (unlikely(tsk->flags & PF_EXITING)) {
pr_alert("Fixing recursive fault but reboot is needed!\n");
+ if (panic_on_exit_recursion)
+ panic("Recursive do_exit() detected in %s[%d]\n",
+ current->comm, task_pid_nr(current));
+
futex_exit_recursive(tsk);
set_current_state(TASK_UNINTERRUPTIBLE);
schedule();
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index afad085960b8..bb397fba2c42 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2600,6 +2600,15 @@ static struct ctl_table kern_table[] = {
.extra2 = _thousand,
},
#endif
+ {
+ .procname = "panic_on_exit_recursion",
+ .data = _on_exit_recursion,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ },
{
.procname = "panic_on_warn",
.data = _on_warn,
[RFC PATCH v2] do_exit(): panic() recursion detected
Recursive do_exit() is symptom of compromised kernel integrity.
For safety critical systems, it may be better to
panic() in this case to minimize risk.
Signed-off-by: Vladimir Kondratiev
Change-Id: I42f45900a08c4282c511b05e9e6061360d07db60
---
Documentation/admin-guide/kernel-parameters.txt | 6 ++
include/linux/kernel.h | 1 +
kernel/exit.c | 7 +++
kernel/sysctl.c | 9 +
4 files changed, 23 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index 44fde25bb221..6e12a6804557 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3508,6 +3508,12 @@
bit 4: print ftrace buffer
bit 5: print all printk messages in buffer
+ panic_on_exit_recursion
+ panic() when do_exit() recursion detected, rather then
+ try to stay running whenever possible.
+ Useful on safety critical systems; re-entry in do_exit
+ is a symptom of compromised kernel integrity.
+
panic_on_taint= Bitmask for conditionally calling panic() in add_taint()
Format: [,nousertaint]
Hexadecimal bitmask representing the set of TAINT flags
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 2f05e9128201..5afb20534cb2 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -539,6 +539,7 @@ extern int sysctl_panic_on_rcu_stall;
extern int sysctl_panic_on_stackoverflow;
extern bool crash_kexec_post_notifiers;
+extern int panic_on_exit_recursion;
/*
* panic_cpu is used for synchronizing panic() and crash_kexec() execution. It
diff --git a/kernel/exit.c b/kernel/exit.c
index 1f236ed375f8..162799a8b539 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -68,6 +68,9 @@
#include
#include
+int panic_on_exit_recursion __read_mostly;
+core_param(panic_on_exit_recursion, panic_on_exit_recursion, int, 0644);
+
static void __unhash_process(struct task_struct *p, bool group_dead)
{
nr_threads--;
@@ -757,6 +760,10 @@ void __noreturn do_exit(long code)
*/
if (unlikely(tsk->flags & PF_EXITING)) {
pr_alert("Fixing recursive fault but reboot is needed!\n");
+ if (panic_on_exit_recursion)
+ panic("Recursive do_exit() detected in %s[%d]\n",
+ current->comm, task_pid_nr(current));
+
futex_exit_recursive(tsk);
set_current_state(TASK_UNINTERRUPTIBLE);
schedule();
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index afad085960b8..bb397fba2c42 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2600,6 +2600,15 @@ static struct ctl_table kern_table[] = {
.extra2 = _thousand,
},
#endif
+ {
+ .procname = "panic_on_exit_recursion",
+ .data = _on_exit_recursion,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ },
{
.procname = "panic_on_warn",
.data = _on_warn,
--
2.27.0
[RFC PATCH] do_exit(): panic() recursion detected
From: Vladimir Kondratiev
Recursive do_exit() is symptom of compromised kernel integrity.
For safety critical systems, it may be better to
panic() in this case to minimize risk.
Signed-off-by: Vladimir Kondratiev
Change-Id: I42f45900a08c4282c511b05e9e6061360d07db60
---
Documentation/admin-guide/kernel-parameters.txt | 6 ++
include/linux/kernel.h | 1 +
kernel/exit.c | 7 +++
kernel/sysctl.c | 9 +
4 files changed, 23 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index 44fde25bb221..6e12a6804557 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3508,6 +3508,12 @@
bit 4: print ftrace buffer
bit 5: print all printk messages in buffer
+ panic_on_exit_recursion
+ panic() when do_exit() recursion detected, rather then
+ try to stay running whenever possible.
+ Useful on safety critical systems; re-entry in do_exit
+ is a symptom of compromised kernel integrity.
+
panic_on_taint= Bitmask for conditionally calling panic() in add_taint()
Format: [,nousertaint]
Hexadecimal bitmask representing the set of TAINT flags
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 2f05e9128201..5afb20534cb2 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -539,6 +539,7 @@ extern int sysctl_panic_on_rcu_stall;
extern int sysctl_panic_on_stackoverflow;
extern bool crash_kexec_post_notifiers;
+extern int panic_on_exit_recursion;
/*
* panic_cpu is used for synchronizing panic() and crash_kexec() execution. It
diff --git a/kernel/exit.c b/kernel/exit.c
index 1f236ed375f8..162799a8b539 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -68,6 +68,9 @@
#include
#include
+int panic_on_exit_recursion __read_mostly;
+core_param(panic_on_exit_recursion, panic_on_exit_recursion, int, 0644);
+
static void __unhash_process(struct task_struct *p, bool group_dead)
{
nr_threads--;
@@ -757,6 +760,10 @@ void __noreturn do_exit(long code)
*/
if (unlikely(tsk->flags & PF_EXITING)) {
pr_alert("Fixing recursive fault but reboot is needed!\n");
+ if (panic_on_exit_recursion)
+ panic("Recursive do_exit() detected in %s[%d]\n",
+ current->comm, task_pid_nr(current));
+
futex_exit_recursive(tsk);
set_current_state(TASK_UNINTERRUPTIBLE);
schedule();
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index afad085960b8..bb397fba2c42 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2600,6 +2600,15 @@ static struct ctl_table kern_table[] = {
.extra2 = _thousand,
},
#endif
+ {
+ .procname = "panic_on_exit_recursion",
+ .data = _on_exit_recursion,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ },
{
.procname = "panic_on_warn",
.data = _on_warn,
--
2.27.0
[PATCH] do_exit(): panic() when double fault detected
Double fault detected in do_exit() is symptom of integrity
compromised. For safety critical systems, it may be better to
panic() in this case to minimize risk.
Signed-off-by: Vladimir Kondratiev
---
Documentation/admin-guide/kernel-parameters.txt | 5 +
include/linux/kernel.h | 1 +
kernel/exit.c | 7 +++
kernel/sysctl.c | 9 +
4 files changed, 22 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index 44fde25bb221..6cb2a63c47f3 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3521,6 +3521,11 @@
extra details on the taint flags that users can pick
to compose the bitmask to assign to panic_on_taint.
+ panic_on_double_fault
+ panic() when double fault detected in do_exit().
+ Useful on safety critical systems; double fault is
+ a symptom of kernel integrity compromised.
+
panic_on_warn panic() instead of WARN(). Useful to cause kdump
on a WARN().
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 2f05e9128201..0d8822259a36 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -539,6 +539,7 @@ extern int sysctl_panic_on_rcu_stall;
extern int sysctl_panic_on_stackoverflow;
extern bool crash_kexec_post_notifiers;
+extern int panic_on_double_fault;
/*
* panic_cpu is used for synchronizing panic() and crash_kexec() execution. It
diff --git a/kernel/exit.c b/kernel/exit.c
index 1f236ed375f8..e67ae43644f9 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -68,6 +68,9 @@
#include
#include
+int panic_on_double_fault __read_mostly;
+core_param(panic_on_double_fault, panic_on_double_fault, int, 0644);
+
static void __unhash_process(struct task_struct *p, bool group_dead)
{
nr_threads--;
@@ -757,6 +760,10 @@ void __noreturn do_exit(long code)
*/
if (unlikely(tsk->flags & PF_EXITING)) {
pr_alert("Fixing recursive fault but reboot is needed!\n");
+ if (panic_on_double_fault)
+ panic("Double fault detected in %s[%d]\n",
+ current->comm, task_pid_nr(current));
+
futex_exit_recursive(tsk);
set_current_state(TASK_UNINTERRUPTIBLE);
schedule();
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index afad085960b8..869a2ca41e8e 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2600,6 +2600,15 @@ static struct ctl_table kern_table[] = {
.extra2 = _thousand,
},
#endif
+ {
+ .procname = "panic_on_double_fault",
+ .data = _on_double_fault,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ },
{
.procname = "panic_on_warn",
.data = _on_warn,
--
2.27.0
-
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
[PATCH] mmc: sdhci-cadence: fix PHY write
Accordingly to Cadence documentation, PHY write procedure is: 1. Software sets the PHY Register Address (HRS04[5:0]) and the PHY Write Data (HRS04[15:8]) fields. 2. Software sets the PHY Write Transaction Request (HRS04[24]) field to 1. 3. Software waits as the PHY Write Transaction Acknowledge (HRS04[26]) field is equal to 0. 4. Hardware performs the write transaction to PHY register where HRS04[15:8] is a data written to register under HRS04[5:0] address. 5. Hardware sets the PHY Transaction Acknowledge (HRS04[26]) to 1 when transaction is completed. 6. Software clears the PHY Write Transaction Request (HRS04[24]) to 1 after noticing that the PHY Write Transaction Acknowledge (HRS04[26]) field is equal to 1. 7. Software waits for the PHY Acknowledge Register (HRS04[26]) field is equal to 0. Add missing steps 3 and 7. Lack of these steps causes integrity errors detested by hardware. Signed-off-by: Vladimir Kondratiev --- drivers/mmc/host/sdhci-cadence.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c index e474d3fa099e..6b2e7c43cbc1 100644 --- a/drivers/mmc/host/sdhci-cadence.c +++ b/drivers/mmc/host/sdhci-cadence.c @@ -114,6 +114,11 @@ static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, u32 tmp; int ret; + ret = readl_poll_timeout(reg, tmp, !(tmp & SDHCI_CDNS_HRS04_ACK), +0, 10); + if (ret) + return ret; + tmp = FIELD_PREP(SDHCI_CDNS_HRS04_WDATA, data) | FIELD_PREP(SDHCI_CDNS_HRS04_ADDR, addr); writel(tmp, reg); @@ -128,7 +133,10 @@ static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, tmp &= ~SDHCI_CDNS_HRS04_WR; writel(tmp, reg); - return 0; + ret = readl_poll_timeout(reg, tmp, !(tmp & SDHCI_CDNS_HRS04_ACK), +0, 10); + + return ret; } static unsigned int sdhci_cdns_phy_param_count(struct device_node *np) -- 2.20.1 - Intel Israel (74) Limited This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
[PATCH] mips: fix cacheinfo
Because CONFIG_OF defined for MIPS, cacheinfo attempts to fill information
from DT, ignoring data filled by architecture routine. This leads to error
reported
cacheinfo: Unable to detect cache hierarchy for CPU 0
Way to fix this provided in
commit fac51482577d ("drivers: base: cacheinfo: fix x86 with
CONFIG_OF enabled")
Utilize same mechanism to report that cacheinfo set by architecture
specific function
Signed-off-by: Vladimir Kondratiev
---
arch/mips/kernel/cacheinfo.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/mips/kernel/cacheinfo.c b/arch/mips/kernel/cacheinfo.c
index e0dd66881da6..f777e44653d5 100644
--- a/arch/mips/kernel/cacheinfo.c
+++ b/arch/mips/kernel/cacheinfo.c
@@ -69,6 +69,8 @@ static int __populate_cache_leaves(unsigned int cpu)
if (c->tcache.waysize)
populate_cache(tcache, this_leaf, 3, CACHE_TYPE_UNIFIED);
+ this_cpu_ci->cpu_map_populated = true;
+
return 0;
}
--
2.20.1
[PATCH v3] mips: cm: reprime error cause
Accordingly to the documentation
---cut---
The GCR_ERROR_CAUSE.ERR_TYPE field and the GCR_ERROR_MULT.ERR_TYPE
fields can be cleared by either a reset or by writing the current
value of GCR_ERROR_CAUSE.ERR_TYPE to the
GCR_ERROR_CAUSE.ERR_TYPE register.
---cut---
Do exactly this. Original value of cm_error may be safely written back;
it clears error cause and keeps other bits untouched.
Fixes: 3885c2b463f6 ("MIPS: CM: Add support for reporting CM cache errors")
Signed-off-by: Vladimir Kondratiev
---
arch/mips/kernel/mips-cm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/kernel/mips-cm.c b/arch/mips/kernel/mips-cm.c
index 8f5bd04f320a..7f3f136572de 100644
--- a/arch/mips/kernel/mips-cm.c
+++ b/arch/mips/kernel/mips-cm.c
@@ -457,5 +457,5 @@ void mips_cm_error_report(void)
}
/* reprime cause register */
- write_gcr_error_cause(0);
+ write_gcr_error_cause(cm_error);
}
--
2.19.1
[PATCH v2] mips: cm: reprime error cause
From: Vladimir Kondratiev
Accordingly to the documentation
---cut---
The GCR_ERROR_CAUSE.ERR_TYPE field and the GCR_ERROR_MULT.ERR_TYPE
fields can be cleared by either a reset or by writing the current
value of GCR_ERROR_CAUSE.ERR_TYPE to the
GCR_ERROR_CAUSE.ERR_TYPE register.
---cut---
Do exactly this. Original value of cm_error may be safely written back;
it clears error cause and keeps other bits untouched.
Fixes: 3885c2b463f6 ("MIPS: CM: Add support for reporting CM cache errors")
Signed-off-by: Vladimir Kondratiev
---
arch/mips/kernel/mips-cm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/kernel/mips-cm.c b/arch/mips/kernel/mips-cm.c
index 8f5bd04f320a..7f3f136572de 100644
--- a/arch/mips/kernel/mips-cm.c
+++ b/arch/mips/kernel/mips-cm.c
@@ -457,5 +457,5 @@ void mips_cm_error_report(void)
}
/* reprime cause register */
- write_gcr_error_cause(0);
+ write_gcr_error_cause(cm_error);
}
--
2.19.1
[PATCH] mips: cm: reprime error cause
Accordingly to the documentation
---cut---
The GCR_ERROR_CAUSE.ERR_TYPE field and the GCR_ERROR_MULT.ERR_TYPE
fields can be cleared by either a reset or by writing the current
value of GCR_ERROR_CAUSE.ERR_TYPE to the
GCR_ERROR_CAUSE.ERR_TYPE register.
---cut---
Do exactly this
Fixes: 3885c2b463f6 ("MIPS: CM: Add support for reporting CM cache errors")
Signed-off-by: Vladimir Kondratiev
---
arch/mips/kernel/mips-cm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/kernel/mips-cm.c b/arch/mips/kernel/mips-cm.c
index 8f5bd04f320a..f54ba99dc7d9 100644
--- a/arch/mips/kernel/mips-cm.c
+++ b/arch/mips/kernel/mips-cm.c
@@ -457,5 +457,5 @@ void mips_cm_error_report(void)
}
/* reprime cause register */
- write_gcr_error_cause(0);
+ write_gcr_error_cause(cm_error & CM3_GCR_ERROR_CAUSE_ERRTYPE);
}
--
2.19.1
RFC: striving for automotive grade certification
Hi, I am looking how can we get kernel certified for life critical applications, in particular for automotive industry. Mean drive train, not infotainment. To begin with, all certification processes are talking about cleaning compilation warnings at level higher then usual. Example would be unused parameter in function. This is what I want to start with. There are lots of warnings triggered in kernel compilation by -Wunused-parameter, it is perhaps most frequent warning at all. Technically it is not hard to fix all such warnings by adding __always_unused when needed. However this will produce huge patch touching lots of files for kind of nothing. So, before starting this effort, I want to consult: - is this (massive cleanup) right direction in general? - Any ideas better then marking __always_unused? - what to do in cases where parameter is unused depending on some pre-processor conditions? - is it better to do one huge patch or split into pieces? Thanks, Vladimir
Re: [PATCH] kbuild: gitignore output directory
Agree; sending v2
On 2/1/19 6:18 AM, Masahiro Yamada wrote:
On Wed, Jan 30, 2019 at 8:15 PM Vladimir Kondratiev
wrote:
When compiling into output directory using O=, many files
created under KBUILD_OUTPUT that git considers
as new ones; git clients, ex. "git gui" lists it, and it clutters
file list making it difficult to see what was really changed
Generate .gitignore in output directory that ignores all
its content
Signed-off-by: Vladimir Kondratiev
---
Makefile | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Makefile b/Makefile
index 141653226f3c..ee66ea28869b 100644
--- a/Makefile
+++ b/Makefile
@@ -483,10 +483,13 @@ PHONY += outputmakefile
# outputmakefile generates a Makefile in the output directory, if using a
# separate output directory. This allows convenient use of make in the
# output directory.
+# At the same time when output Makefile generated, generate .gitignore to
+# ignore whole output directory
outputmakefile:
ifneq ($(KBUILD_SRC),)
$(Q)ln -fsn $(srctree) source
$(Q)$(CONFIG_SHELL) $(srctree)/scripts/mkmakefile $(srctree)
+ echo "# this is build directory, ignore it\n*" > .gitignore
endif
ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
--
2.19.1
The idea looks OK to me.
The implementation must be improved.
You need to add $(Q) to suppress the annoying command echo.
Also, this patch does not work for all distributions because
echo "\n"
is not portable.
GNU Make runs recipes in /bin/sh (unless SHELL variable is changed),
but the implementation of /bin/sh depends on distributions.
This patch works on Ubuntu etc.
because /bin/sh is a symbolic link to dash.
But, in some distributions,
/bin/sh is a symbolic link to bash.
Docker is useful for quick tests of
various distributions. :)
See the result of echo "hello\nworld"
[Ubuntu]
foo@8ad1275125c5:~$ cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=18.10
DISTRIB_CODENAME=cosmic
DISTRIB_DESCRIPTION="Ubuntu 18.10"
foo@8ad1275125c5:~$ ls -l /bin/sh
lrwxrwxrwx 1 root root 4 Nov 14 23:00 /bin/sh -> dash
foo@8ad1275125c5:~$ /bin/sh
$ type echo
echo is a shell builtin
$ echo "hello\nworld"
hello
world
[CentOS]
[foo@c3fbaa4b6f72 ~]$ cat /etc/redhat-release
CentOS Linux release 7.6.1810 (Core)
[foo@c3fbaa4b6f72 ~]$ ls -l /bin/sh
lrwxrwxrwx 1 root root 4 Dec 5 01:36 /bin/sh -> bash
[foo@c3fbaa4b6f72 ~]$ /bin/sh
sh-4.2$ type echo
echo is a shell builtin
sh-4.2$ echo "hello\nworld"
hello\nworld
On example for workaround might be:
diff --git a/Makefile b/Makefile
index ee66ea2..010c1c6 100644
--- a/Makefile
+++ b/Makefile
@@ -489,7 +489,7 @@ outputmakefile:
ifneq ($(KBUILD_SRC),)
$(Q)ln -fsn $(srctree) source
$(Q)$(CONFIG_SHELL) $(srctree)/scripts/mkmakefile $(srctree)
- echo "# this is build directory, ignore it\n*" > .gitignore
+ $(Q){ echo "# this is build directory, ignore it"; echo "*"; }
.gitignore
endif
ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
[PATCH v2] kbuild: gitignore output directory
From: Vladimir Kondratiev
When compiling into output directory using O=, many files
created under KBUILD_OUTPUT that git considers
as new ones; git clients, ex. "git gui" lists it, and it clutters
file list making it difficult to see what was really changed
Generate .gitignore in output directory that ignores all
its content
Signed-off-by: Vladimir Kondratiev
---
Makefile | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Makefile b/Makefile
index 141653226f3c..b1d651e822b1 100644
--- a/Makefile
+++ b/Makefile
@@ -483,10 +483,13 @@ PHONY += outputmakefile
# outputmakefile generates a Makefile in the output directory, if using a
# separate output directory. This allows convenient use of make in the
# output directory.
+# At the same time when output Makefile generated, generate .gitignore to
+# ignore whole output directory
outputmakefile:
ifneq ($(KBUILD_SRC),)
$(Q)ln -fsn $(srctree) source
$(Q)$(CONFIG_SHELL) $(srctree)/scripts/mkmakefile $(srctree)
+ $(Q){ echo "# this is build directory, ignore it"; echo "*"; } >
.gitignore
endif
ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
--
2.19.1
-
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
[PATCH] kbuild: gitignore output directory
When compiling into output directory using O=, many files created under KBUILD_OUTPUT that git considers as new ones; git clients, ex. "git gui" lists it, and it clutters file list making it difficult to see what was really changed Generate .gitignore in output directory that ignores all its content Signed-off-by: Vladimir Kondratiev --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index 141653226f3c..ee66ea28869b 100644 --- a/Makefile +++ b/Makefile @@ -483,10 +483,13 @@ PHONY += outputmakefile # outputmakefile generates a Makefile in the output directory, if using a # separate output directory. This allows convenient use of make in the # output directory. +# At the same time when output Makefile generated, generate .gitignore to +# ignore whole output directory outputmakefile: ifneq ($(KBUILD_SRC),) $(Q)ln -fsn $(srctree) source $(Q)$(CONFIG_SHELL) $(srctree)/scripts/mkmakefile $(srctree) + echo "# this is build directory, ignore it\n*" > .gitignore endif ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),) -- 2.19.1
Re: [PATCH] lib: 64bit IO
On 1/29/19 6:01 PM, Greg Kroah-Hartman wrote:
On Tue, Jan 29, 2019 at 05:41:02PM +0200, Vladimir Kondratiev wrote:
implement missing io{read|write}64
For 64-bit platforms, these 64-bit io functions are defined in
include/asm-generic/iomap.h,
but actual implementation missing. Provide it.
What is in include/asm-generic/io.h?
$ git grep iowrite64be | grep generic
include/asm-generic/io.h:#ifndef iowrite64be
include/asm-generic/io.h:#define iowrite64be iowrite64be
include/asm-generic/io.h:static inline void iowrite64be(u64 value, volatile
void __iomem *addr)
include/asm-generic/iomap.h:extern void iowrite64be(u64, void __iomem *);
so are you sure this is needed?
What code is failing to build for you?
The code you mentioned is under
#ifndef CONFIG_GENERIC_IOMAP
i.e. it works for platforms that does not define CONFIG_GENERIC_IOMAP
This currently defined for quite a lot of platforms, all these should
not use these inlines and use lib/iomap.c - it selected by the same
symbol, from lib/Makefile:
obj-$(CONFIG_GENERIC_IOMAP) += iomap.o
linux$ git grep GENERIC_IOMAP | grep Kconfig
arch/hexagon/Kconfig: select GENERIC_IOMAP
arch/ia64/Kconfig: select GENERIC_IOMAP
arch/m68k/Kconfig: select GENERIC_IOMAP
arch/mips/Kconfig: select GENERIC_IOMAP
arch/openrisc/Kconfig: select GENERIC_IOMAP
arch/powerpc/platforms/Kconfig: select GENERIC_IOMAP
arch/unicore32/Kconfig: select GENERIC_IOMAP
arch/x86/Kconfig: select GENERIC_IOMAP
lib/Kconfig:config GENERIC_IOMAP
Actually, I am using 64-bit MIPS platform for now, and some pieces of
code was not compiling without this patch; but this looks like code all
platforms above need in order to use 64-bit transactions in
"io{redd|write}xx" style
Also, I figured out most platforms does not bother providing PIO in
64-bit quantities, so see below version 2 with MMIO-only implementation.
thanks,
greg k-h
From 7f0d31a7cdcc535e0248cb4c4cf8f1d16dc0133d Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev
Date: Thu, 24 Jan 2019 16:59:20 +0200
Subject: [PATCH v2] lib: 64bit IO
implement missing io{read|write}64
For 64-bit platforms, these 64-bit io functions are defined in
include/asm-generic/iomap.h,
but actual implementation missing. Provide it.
Most platforms does not provide 64-bit PIO functions, thus
implement 64-bit transactions as MMIO only
Signed-off-by: Vladimir Kondratiev
---
lib/iomap.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/lib/iomap.c b/lib/iomap.c
index 541d926da95e..86501bccdf72 100644
--- a/lib/iomap.c
+++ b/lib/iomap.c
@@ -67,6 +67,9 @@ static void bad_io_access(unsigned long port, const
char *access)
#ifndef mmio_read16be
#define mmio_read16be(addr) be16_to_cpu(__raw_readw(addr))
#define mmio_read32be(addr) be32_to_cpu(__raw_readl(addr))
+#ifdef CONFIG_64BIT
+#define mmio_read64be(addr) be64_to_cpu(__raw_readq(addr))
+#endif
#endif
unsigned int ioread8(void __iomem *addr)
@@ -100,6 +103,22 @@ EXPORT_SYMBOL(ioread16be);
EXPORT_SYMBOL(ioread32);
EXPORT_SYMBOL(ioread32be);
+#ifdef CONFIG_64BIT
+
+u64 ioread64(void __iomem *addr)
+{
+ return readq(addr);
+}
+EXPORT_SYMBOL(ioread64);
+
+u64 ioread64be(void __iomem *addr)
+{
+ return mmio_read64be(addr);
+}
+EXPORT_SYMBOL(ioread64be);
+
+#endif /* CONFIG_64BIT */
+
#ifndef pio_write16be
#define pio_write16be(val,port) outw(swab16(val),port)
#define pio_write32be(val,port) outl(swab32(val),port)
@@ -108,6 +127,9 @@ EXPORT_SYMBOL(ioread32be);
#ifndef mmio_write16be
#define mmio_write16be(val,port) __raw_writew(be16_to_cpu(val),port)
#define mmio_write32be(val,port) __raw_writel(be32_to_cpu(val),port)
+#ifdef CONFIG_64BIT
+#define mmio_write64be(val, port) __raw_writeq(be64_to_cpu(val), port)
+#endif
#endif
void iowrite8(u8 val, void __iomem *addr)
@@ -136,6 +158,22 @@ EXPORT_SYMBOL(iowrite16be);
EXPORT_SYMBOL(iowrite32);
EXPORT_SYMBOL(iowrite32be);
+#ifdef CONFIG_64BIT
+
+void iowrite64(u64 val, void __iomem *addr)
+{
+ writeq(val, addr);
+}
+EXPORT_SYMBOL(iowrite64);
+
+void iowrite64be(u64 val, void __iomem *addr)
+{
+ mmio_write64be(val, addr);
+}
+EXPORT_SYMBOL(iowrite64be);
+
+#endif /* CONFIG_64BIT */
+
/*
* These are the "repeat MMIO read/write" functions.
* Note the "__raw" accesses, since we don't want to
--
2.19.1
[PATCH] lib: 64bit IO
implement missing io{read|write}64
For 64-bit platforms, these 64-bit io functions are defined in
include/asm-generic/iomap.h,
but actual implementation missing. Provide it.
Signed-off-by: Vladimir Kondratiev
---
lib/iomap.c | 46 ++
1 file changed, 46 insertions(+)
diff --git a/lib/iomap.c b/lib/iomap.c
index 541d926da95e..5e2b0bcc5561 100644
--- a/lib/iomap.c
+++ b/lib/iomap.c
@@ -62,11 +62,17 @@ static void bad_io_access(unsigned long port, const char
*access)
#ifndef pio_read16be
#define pio_read16be(port) swab16(inw(port))
#define pio_read32be(port) swab32(inl(port))
+#ifdef CONFIG_64BIT
+#define pio_read64be(port) swab64(inq(port))
+#endif
#endif
#ifndef mmio_read16be
#define mmio_read16be(addr) be16_to_cpu(__raw_readw(addr))
#define mmio_read32be(addr) be32_to_cpu(__raw_readl(addr))
+#ifdef CONFIG_64BIT
+#define mmio_read64be(addr) be64_to_cpu(__raw_readq(addr))
+#endif
#endif
unsigned int ioread8(void __iomem *addr)
@@ -100,14 +106,38 @@ EXPORT_SYMBOL(ioread16be);
EXPORT_SYMBOL(ioread32);
EXPORT_SYMBOL(ioread32be);
+#ifdef CONFIG_64BIT
+
+u64 ioread64(void __iomem *addr)
+{
+ IO_COND(addr, return inq(port), return readq(addr));
+ return 0xULL;
+}
+EXPORT_SYMBOL(ioread64);
+
+u64 ioread64be(void __iomem *addr)
+{
+ IO_COND(addr, return pio_read64be(port), return mmio_read64be(addr));
+ return 0xULL;
+}
+EXPORT_SYMBOL(ioread64be);
+
+#endif /* CONFIG_64BIT */
+
#ifndef pio_write16be
#define pio_write16be(val,port) outw(swab16(val),port)
#define pio_write32be(val,port) outl(swab32(val),port)
+#ifdef CONFIG_64BIT
+#define pio_write64be(val, port) outq(swab64(val), port)
+#endif
#endif
#ifndef mmio_write16be
#define mmio_write16be(val,port) __raw_writew(be16_to_cpu(val),port)
#define mmio_write32be(val,port) __raw_writel(be32_to_cpu(val),port)
+#ifdef CONFIG_64BIT
+#define mmio_write64be(val, port) __raw_writeq(be64_to_cpu(val), port)
+#endif
#endif
void iowrite8(u8 val, void __iomem *addr)
@@ -136,6 +166,22 @@ EXPORT_SYMBOL(iowrite16be);
EXPORT_SYMBOL(iowrite32);
EXPORT_SYMBOL(iowrite32be);
+#ifdef CONFIG_64BIT
+
+void iowrite64(u64 val, void __iomem *addr)
+{
+ IO_COND(addr, outq(val, port), writeq(val, addr));
+}
+EXPORT_SYMBOL(iowrite64);
+
+void iowrite64be(u64 val, void __iomem *addr)
+{
+ IO_COND(addr, pio_write64be(val, port), mmio_write64be(val, addr));
+}
+EXPORT_SYMBOL(iowrite64be);
+
+#endif /* CONFIG_64BIT */
+
/*
* These are the "repeat MMIO read/write" functions.
* Note the "__raw" accesses, since we don't want to
--
2.19.1
[PATCH] rmem: support for dma_addr different from phys_addr
On some systems, dma address differs from physical one due to
various reasons.
dma_init_coherent_memory is aware of it, it takes both
phys_addr_t and dma_addt_t arguments for memory block.
However, rmem_dma_device_init passes physical address for DMA one.
Fix this, using phys_to_dma(). This assumes the whole buffer has
same offset between physical and dma addresses.
Signed-off-by: Vladimir Kondratiev <vladimir.kondrat...@intel.com>
---
drivers/base/dma-coherent.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/base/dma-coherent.c b/drivers/base/dma-coherent.c
index 640a7e63c453..5371cb19c918 100644
--- a/drivers/base/dma-coherent.c
+++ b/drivers/base/dma-coherent.c
@@ -292,7 +292,8 @@ static int rmem_dma_device_init(struct reserved_mem *rmem,
struct device *dev)
struct dma_coherent_mem *mem = rmem->priv;
if (!mem &&
- !dma_init_coherent_memory(rmem->base, rmem->base, rmem->size,
+ !dma_init_coherent_memory(rmem->base, phys_to_dma(dev, rmem->base),
+ rmem->size,
DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE,
)) {
pr_err("Reserved memory: failed to init DMA memory pool at %pa,
size %ld MiB\n",
--
2.11.0
[PATCH] rmem: support for dma_addr different from phys_addr
On some systems, dma address differs from physical one due to
various reasons.
dma_init_coherent_memory is aware of it, it takes both
phys_addr_t and dma_addt_t arguments for memory block.
However, rmem_dma_device_init passes physical address for DMA one.
Fix this, using phys_to_dma(). This assumes the whole buffer has
same offset between physical and dma addresses.
Signed-off-by: Vladimir Kondratiev
---
drivers/base/dma-coherent.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/base/dma-coherent.c b/drivers/base/dma-coherent.c
index 640a7e63c453..5371cb19c918 100644
--- a/drivers/base/dma-coherent.c
+++ b/drivers/base/dma-coherent.c
@@ -292,7 +292,8 @@ static int rmem_dma_device_init(struct reserved_mem *rmem,
struct device *dev)
struct dma_coherent_mem *mem = rmem->priv;
if (!mem &&
- !dma_init_coherent_memory(rmem->base, rmem->base, rmem->size,
+ !dma_init_coherent_memory(rmem->base, phys_to_dma(dev, rmem->base),
+ rmem->size,
DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE,
)) {
pr_err("Reserved memory: failed to init DMA memory pool at %pa,
size %ld MiB\n",
--
2.11.0
[PATCH] rmem: support for dma_addr different from phys_addr
On some systems, dma address differs from physical one due to
various reasons.
dma_init_coherent_memory is aware of it, it takes both
phys_addr_t and dma_addt_t arguments for memory block.
However, rmem_dma_device_init passes physical address for DMA one.
Fix this, using phys_to_dma(). This assumes the whole buffer has
same offset between physical and dma addresses.
Change-Id: Ic709312941ee4e1a37afb2ea25cbd759eabc009b
Signed-off-by: Vladimir Kondratiev <vladimir.kondrat...@intel.com>
---
drivers/base/dma-coherent.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/base/dma-coherent.c b/drivers/base/dma-coherent.c
index 640a7e63c453..5371cb19c918 100644
--- a/drivers/base/dma-coherent.c
+++ b/drivers/base/dma-coherent.c
@@ -292,7 +292,8 @@ static int rmem_dma_device_init(struct reserved_mem *rmem,
struct device *dev)
struct dma_coherent_mem *mem = rmem->priv;
if (!mem &&
- !dma_init_coherent_memory(rmem->base, rmem->base, rmem->size,
+ !dma_init_coherent_memory(rmem->base, phys_to_dma(dev, rmem->base),
+ rmem->size,
DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE,
)) {
pr_err("Reserved memory: failed to init DMA memory pool at %pa,
size %ld MiB\n",
--
2.11.0
-
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
[PATCH] rmem: support for dma_addr different from phys_addr
On some systems, dma address differs from physical one due to
various reasons.
dma_init_coherent_memory is aware of it, it takes both
phys_addr_t and dma_addt_t arguments for memory block.
However, rmem_dma_device_init passes physical address for DMA one.
Fix this, using phys_to_dma(). This assumes the whole buffer has
same offset between physical and dma addresses.
Change-Id: Ic709312941ee4e1a37afb2ea25cbd759eabc009b
Signed-off-by: Vladimir Kondratiev
---
drivers/base/dma-coherent.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/base/dma-coherent.c b/drivers/base/dma-coherent.c
index 640a7e63c453..5371cb19c918 100644
--- a/drivers/base/dma-coherent.c
+++ b/drivers/base/dma-coherent.c
@@ -292,7 +292,8 @@ static int rmem_dma_device_init(struct reserved_mem *rmem,
struct device *dev)
struct dma_coherent_mem *mem = rmem->priv;
if (!mem &&
- !dma_init_coherent_memory(rmem->base, rmem->base, rmem->size,
+ !dma_init_coherent_memory(rmem->base, phys_to_dma(dev, rmem->base),
+ rmem->size,
DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE,
)) {
pr_err("Reserved memory: failed to init DMA memory pool at %pa,
size %ld MiB\n",
--
2.11.0
-
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
Re: [PATCH] wil6210: increase cmd buffer size to avoid sscanf buffer overflow
On 03/01/2015 07:48 PM, Colin King wrote: From: Colin Ian King cppcheck detected a buffer overflow: [drivers/net/wireless/ath/wil6210/debugfs.c:634]: (error) Width 8 given in format string (no. 1) is larger than destination buffer 'cmd[8]', use %7s to prevent overflowing it. For the current %8s sscanf we require cmd to be 9 chars long so increase it by 1 byte to prevent the sscan overflow (rather than reduce the %8s specifier to %7s as cppcheck recommends). Signed-off-by: Colin Ian King --- drivers/net/wireless/ath/wil6210/debugfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c index 45c3558e..29aab12 100644 --- a/drivers/net/wireless/ath/wil6210/debugfs.c +++ b/drivers/net/wireless/ath/wil6210/debugfs.c @@ -618,7 +618,7 @@ static ssize_t wil_write_back(struct file *file, const char __user *buf, struct wil6210_priv *wil = file->private_data; int rc; char *kbuf = kmalloc(len + 1, GFP_KERNEL); - char cmd[8]; + char cmd[9]; int p1, p2, p3; if (!kbuf) Thanks for finding this. Here is my Acked-by: Vladimir Kondratiev Thanks, Vladimir -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] wil6210: increase cmd buffer size to avoid sscanf buffer overflow
On 03/01/2015 07:48 PM, Colin King wrote: From: Colin Ian King colin.k...@canonical.com cppcheck detected a buffer overflow: [drivers/net/wireless/ath/wil6210/debugfs.c:634]: (error) Width 8 given in format string (no. 1) is larger than destination buffer 'cmd[8]', use %7s to prevent overflowing it. For the current %8s sscanf we require cmd to be 9 chars long so increase it by 1 byte to prevent the sscan overflow (rather than reduce the %8s specifier to %7s as cppcheck recommends). Signed-off-by: Colin Ian King colin.k...@canonical.com --- drivers/net/wireless/ath/wil6210/debugfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c index 45c3558e..29aab12 100644 --- a/drivers/net/wireless/ath/wil6210/debugfs.c +++ b/drivers/net/wireless/ath/wil6210/debugfs.c @@ -618,7 +618,7 @@ static ssize_t wil_write_back(struct file *file, const char __user *buf, struct wil6210_priv *wil = file-private_data; int rc; char *kbuf = kmalloc(len + 1, GFP_KERNEL); - char cmd[8]; + char cmd[9]; int p1, p2, p3; if (!kbuf) Thanks for finding this. Here is my Acked-by: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com Thanks, Vladimir -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ath: change logging functions to return void
On Monday, September 22, 2014 10:35:34 AM Joe Perches wrote: > Other miscellanea: > > o add __printf verification to wil6210 logging functions > No format/argument mismatches found > > Signed-off-by: Joe Perches > For wil6210: Acked-by: Vladimir Kondratiev -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ath: change logging functions to return void
On Monday, September 22, 2014 10:35:34 AM Joe Perches wrote: Other miscellanea: o add __printf verification to wil6210 logging functions No format/argument mismatches found Signed-off-by: Joe Perches j...@perches.com For wil6210: Acked-by: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] wil6210: Fix switch operator "missing break?" warning
On Friday, February 07, 2014 03:46:40 PM Alexander Gordeev wrote:
> This update fixes a warning introduced with commit bc977ba1
> ("wil6210: Use pci_enable_msi_range() instead of pci_enable_msi_block()")
>
> drivers/net/wireless/ath/wil6210/pcie_bus.c:65 wil_if_pcie_enable()
> warn: missing break? reassigning 'use_msi'
>
I can't reproduce this warning. What tools used to get it?
Neither gcc (I have 4.8.1) nor sparse report it.
Anyway, I am fine with both 'switch' and 'if'.
Thanks, Vladimir.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] wil6210: Fix switch operator missing break? warning
On Friday, February 07, 2014 03:46:40 PM Alexander Gordeev wrote: This update fixes a warning introduced with commit bc977ba1 (wil6210: Use pci_enable_msi_range() instead of pci_enable_msi_block()) drivers/net/wireless/ath/wil6210/pcie_bus.c:65 wil_if_pcie_enable() warn: missing break? reassigning 'use_msi' I can't reproduce this warning. What tools used to get it? Neither gcc (I have 4.8.1) nor sparse report it. Anyway, I am fine with both 'switch' and 'if'. Thanks, Vladimir. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/7] wil6210: Use new interfaces for MSI enablement
On Wednesday, January 08, 2014 12:54:01 PM Alexander Gordeev wrote: > Vladimir, > > This series is against pci/msi branch in Bjorn Helgaas's repo: > git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git > > Commit 302a252 "PCI/MSI: Add pci_enable_msi_range() and > pci_enable_msix_range()" > Thanks, see it. New code don't distinguish between negative and positive error values, same as old code. I'll fix it. Meanwhile, below my Acked-by: Vladimir Kondratiev -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/7] wil6210: Use new interfaces for MSI enablement
On Tuesday, January 07, 2014 07:05:42 PM Alexander Gordeev wrote: > Signed-off-by: Alexander Gordeev > --- > drivers/net/wireless/ath/wil6210/pcie_bus.c | 36 ++ > 1 files changed, 19 insertions(+), 17 deletions(-) > Patch looks fine, but I can't validate it as I can't find patch introducing pci_enable_msi_range(). Where this patch landed on? I am working with: git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-testing.git I also checked git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git and, of course git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git Thanks, Vladimir -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/7] wil6210: Use new interfaces for MSI enablement
On Tuesday, January 07, 2014 07:05:42 PM Alexander Gordeev wrote: Signed-off-by: Alexander Gordeev agord...@redhat.com --- drivers/net/wireless/ath/wil6210/pcie_bus.c | 36 ++ 1 files changed, 19 insertions(+), 17 deletions(-) Patch looks fine, but I can't validate it as I can't find patch introducing pci_enable_msi_range(). Where this patch landed on? I am working with: git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-testing.git I also checked git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git and, of course git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git Thanks, Vladimir -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/7] wil6210: Use new interfaces for MSI enablement
On Wednesday, January 08, 2014 12:54:01 PM Alexander Gordeev wrote: Vladimir, This series is against pci/msi branch in Bjorn Helgaas's repo: git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git Commit 302a252 PCI/MSI: Add pci_enable_msi_range() and pci_enable_msix_range() Thanks, see it. New code don't distinguish between negative and positive error values, same as old code. I'll fix it. Meanwhile, below my Acked-by: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ 044/102] ath: wil6210: Fix build error
On Thursday, August 08, 2013 06:57:20 PM Greg Kroah-Hartman wrote: > 3.10-stable review patch. If anyone has any objections, please let me know. This one is good for stable. I'd suggest to apply also commit c6c7788fe26fdc91e729f60742815ccdb505fd81 - wil6210: drop -Werror compiler flag it is not in Linus's tree yet, but in Linville's wireless-next Thanks, Vladimir -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ 044/102] ath: wil6210: Fix build error
On Thursday, August 08, 2013 06:57:20 PM Greg Kroah-Hartman wrote: 3.10-stable review patch. If anyone has any objections, please let me know. This one is good for stable. I'd suggest to apply also commit c6c7788fe26fdc91e729f60742815ccdb505fd81 - wil6210: drop -Werror compiler flag it is not in Linus's tree yet, but in Linville's wireless-next Thanks, Vladimir -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/net/wireless/ath/wil6210: Makefile, only -Werror when no -W* in EXTRA_CFLAGS
On Wednesday, February 27, 2013 04:56:57 PM Chen Gang wrote:
> 于 2013年02月27日 16:45, Vladimir Kondratiev 写道:
> > On Wednesday, February 27, 2013 02:55:06 PM Chen Gang wrote:
> >>
> >> When make with EXTRA_CFLAGS=-W, it will report error.
> >> so give a check in Makefile.
> >>
> >> Signed-off-by: Chen Gang
> >> ---
> >> drivers/net/wireless/ath/wil6210/Makefile |4 +++-
> >> 1 files changed, 3 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/net/wireless/ath/wil6210/Makefile
> >> b/drivers/net/wireless/ath/wil6210/Makefile
> >> index 9396dc9..d288eea 100644
> >> --- a/drivers/net/wireless/ath/wil6210/Makefile
> >> +++ b/drivers/net/wireless/ath/wil6210/Makefile
> >> @@ -9,5 +9,7 @@ wil6210-objs += wmi.o
> >> wil6210-objs += interrupt.o
> >> wil6210-objs += txrx.o
> >>
> >> -subdir-ccflags-y += -Werror
> >> +ifeq (, $(findstring -W,$(EXTRA_CFLAGS)))
> >> + subdir-ccflags-y += -Werror
> >> +endif
> >> subdir-ccflags-y += -D__CHECK_ENDIAN__
> >>
> > Acked-by: Vladimir Kondratiev
> >
> > Well, agree; -W triggers lots of warnings, I suspect all other places that
> > use
> > -Werror should be problematic. Worth fixing all others? Quick look for v3.8
> > raises:
> >
>
> thank you for your suggestion.
> after this patch applied, I will do for others (let you as signed-of-by,
> too).
Perhaps, it would be good idea to fight the original problem.
I mean, fix warnings where possible. Example: there are lots of
"unused parameter" ones. Where it is false warning, add __maybe_unused
attribute. Like this:
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index c566927..83e43b7 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -163,8 +163,9 @@ extern int _cond_resched(void);
# define might_sleep() \
do { __might_sleep(__FILE__, __LINE__, 0); might_resched(); } while (0)
#else
- static inline void __might_sleep(const char *file, int line,
- int preempt_offset) { }
+ static inline void __might_sleep(const char *file __maybe_unused,
+ int line __maybe_unused,
+ int preempt_offset __maybe_unused) { }
# define might_sleep() do { might_resched(); } while (0)
#endif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/net/wireless/ath/wil6210: Makefile, only -Werror when no -W* in EXTRA_CFLAGS
On Wednesday, February 27, 2013 04:56:57 PM Chen Gang wrote:
于 2013年02月27日 16:45, Vladimir Kondratiev 写道:
On Wednesday, February 27, 2013 02:55:06 PM Chen Gang wrote:
When make with EXTRA_CFLAGS=-W, it will report error.
so give a check in Makefile.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/net/wireless/ath/wil6210/Makefile |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/Makefile
b/drivers/net/wireless/ath/wil6210/Makefile
index 9396dc9..d288eea 100644
--- a/drivers/net/wireless/ath/wil6210/Makefile
+++ b/drivers/net/wireless/ath/wil6210/Makefile
@@ -9,5 +9,7 @@ wil6210-objs += wmi.o
wil6210-objs += interrupt.o
wil6210-objs += txrx.o
-subdir-ccflags-y += -Werror
+ifeq (, $(findstring -W,$(EXTRA_CFLAGS)))
+ subdir-ccflags-y += -Werror
+endif
subdir-ccflags-y += -D__CHECK_ENDIAN__
Acked-by: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com
Well, agree; -W triggers lots of warnings, I suspect all other places that
use
-Werror should be problematic. Worth fixing all others? Quick look for v3.8
raises:
thank you for your suggestion.
after this patch applied, I will do for others (let you as signed-of-by,
too).
Perhaps, it would be good idea to fight the original problem.
I mean, fix warnings where possible. Example: there are lots of
unused parameter ones. Where it is false warning, add __maybe_unused
attribute. Like this:
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index c566927..83e43b7 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -163,8 +163,9 @@ extern int _cond_resched(void);
# define might_sleep() \
do { __might_sleep(__FILE__, __LINE__, 0); might_resched(); } while (0)
#else
- static inline void __might_sleep(const char *file, int line,
- int preempt_offset) { }
+ static inline void __might_sleep(const char *file __maybe_unused,
+ int line __maybe_unused,
+ int preempt_offset __maybe_unused) { }
# define might_sleep() do { might_resched(); } while (0)
#endif
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers: net: Remove remaining alloc/OOM messages
On Thursday, February 07, 2013 01:46:27 PM Joe Perches wrote: > alloc failures already get standardized OOM > messages and a dump_stack. > > For the affected mallocs around these OOM messages: > > Converted kmallocs with multiplies to kmalloc_array. > Converted a kmalloc/memcpy to kmemdup. > Removed now unused stack variables. > Removed unnecessary parentheses. > Neatened alignment. > > Signed-off-by: Joe Perches > For wil6210: Acked--by: Vladimir Kondratiev Thanks, Vladimir -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers: net: Remove remaining alloc/OOM messages
On Thursday, February 07, 2013 01:46:27 PM Joe Perches wrote: alloc failures already get standardized OOM messages and a dump_stack. For the affected mallocs around these OOM messages: Converted kmallocs with multiplies to kmalloc_array. Converted a kmalloc/memcpy to kmemdup. Removed now unused stack variables. Removed unnecessary parentheses. Neatened alignment. Signed-off-by: Joe Perches j...@perches.com For wil6210: Acked--by: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com Thanks, Vladimir -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] dynamic_debug: introduce debug_hex_dump()
On Tuesday, December 11, 2012 02:55:55 PM Luis R. Rodriguez wrote:
> Vladimir, to be clear, you can continue with waiting for the patches
> to get into 3.9 but that will take a while but given that you want
> your driver in 3.8 you can fold those routines into your driver as
> you had before but with a rename by adding a prefix and sumbit that
> driver and specify it is for 3.8.
>
I have internally patch that will compensate not merged yet functionality,
without any rename, see below. I can massage commit message and include it
with my driver as separate patch to be easier to undo later.
Comments?
>From c4e7254d0cba51c8b43d36a317a1b847fffb18d8 Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev
Date: Wed, 21 Nov 2012 13:57:21 +0200
Subject: [PATCH] [DEBUG] Allow to compile with mainstream kernel
Unless patch for dynamic hexdump merged into mainstream kernel,
need to provide replacement for the dynamic hexdump functionality
Do not hurt compilation with kernel that have patch above merged.
REMOVE when dynamic hexdump merged into mainstream kernel!!
Change-Id: I6abb8f91ed529ffc84bd9cc21444a59ee15fe456
---
dbg_hexdump.h | 32
wil6210.h |2 ++
2 files changed, 34 insertions(+)
create mode 100644 dbg_hexdump.h
diff --git a/dbg_hexdump.h b/dbg_hexdump.h
new file mode 100644
index 000..7144eed
--- /dev/null
+++ b/dbg_hexdump.h
@@ -0,0 +1,32 @@
+#ifndef DBG_HEXDUMP_H_
+#define DBG_HEXDUMP_H_
+
+#if !defined(print_hex_dump_debug)
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+do { \
+ DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, \
+ __builtin_constant_p(prefix_str) ? prefix_str : "hexdump");\
+ if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
+ print_hex_dump(KERN_DEBUG, prefix_str, \
+ prefix_type, rowsize, groupsize, \
+ buf, len, ascii);\
+} while (0)
+
+#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+ dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)
+
+#define print_hex_dump_bytes(prefix_str, prefix_type, buf, len)\
+ dynamic_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true)
+#else /* defined(CONFIG_DYNAMIC_DEBUG) */
+#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+ print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, rowsize,\
+ groupsize, buf, len, ascii)
+#endif /* defined(CONFIG_DYNAMIC_DEBUG) */
+#endif /* !defined(print_hex_dump_debug) */
+
+#endif /* DBG_HEXDUMP_H_ */
diff --git a/wil6210.h b/wil6210.h
index b37c3d2..4797da0 100644
--- a/wil6210.h
+++ b/wil6210.h
@@ -21,6 +21,8 @@
#include
#include
+#include "dbg_hexdump.h"
+
#define WIL_NAME "wil6210"
/**
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] dynamic_debug: introduce debug_hex_dump()
On Tuesday, December 11, 2012 02:55:55 PM Luis R. Rodriguez wrote:
Vladimir, to be clear, you can continue with waiting for the patches
to get into 3.9 but that will take a while but given that you want
your driver in 3.8 you can fold those routines into your driver as
you had before but with a rename by adding a prefix and sumbit that
driver and specify it is for 3.8.
I have internally patch that will compensate not merged yet functionality,
without any rename, see below. I can massage commit message and include it
with my driver as separate patch to be easier to undo later.
Comments?
From c4e7254d0cba51c8b43d36a317a1b847fffb18d8 Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com
Date: Wed, 21 Nov 2012 13:57:21 +0200
Subject: [PATCH] [DEBUG] Allow to compile with mainstream kernel
Unless patch for dynamic hexdump merged into mainstream kernel,
need to provide replacement for the dynamic hexdump functionality
Do not hurt compilation with kernel that have patch above merged.
REMOVE when dynamic hexdump merged into mainstream kernel!!
Change-Id: I6abb8f91ed529ffc84bd9cc21444a59ee15fe456
---
dbg_hexdump.h | 32
wil6210.h |2 ++
2 files changed, 34 insertions(+)
create mode 100644 dbg_hexdump.h
diff --git a/dbg_hexdump.h b/dbg_hexdump.h
new file mode 100644
index 000..7144eed
--- /dev/null
+++ b/dbg_hexdump.h
@@ -0,0 +1,32 @@
+#ifndef DBG_HEXDUMP_H_
+#define DBG_HEXDUMP_H_
+
+#if !defined(print_hex_dump_debug)
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+do { \
+ DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, \
+ __builtin_constant_p(prefix_str) ? prefix_str : hexdump);\
+ if (unlikely(descriptor.flags _DPRINTK_FLAGS_PRINT)) \
+ print_hex_dump(KERN_DEBUG, prefix_str, \
+ prefix_type, rowsize, groupsize, \
+ buf, len, ascii);\
+} while (0)
+
+#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+ dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)
+
+#define print_hex_dump_bytes(prefix_str, prefix_type, buf, len)\
+ dynamic_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true)
+#else /* defined(CONFIG_DYNAMIC_DEBUG) */
+#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+ print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, rowsize,\
+ groupsize, buf, len, ascii)
+#endif /* defined(CONFIG_DYNAMIC_DEBUG) */
+#endif /* !defined(print_hex_dump_debug) */
+
+#endif /* DBG_HEXDUMP_H_ */
diff --git a/wil6210.h b/wil6210.h
index b37c3d2..4797da0 100644
--- a/wil6210.h
+++ b/wil6210.h
@@ -21,6 +21,8 @@
#include linux/wireless.h
#include net/cfg80211.h
+#include dbg_hexdump.h
+
#define WIL_NAME wil6210
/**
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] dynamic_debug: introduce debug_hex_dump()
Hi Jason, Do you have any update on the status for patches below? Where is it now? When do you expect it to merge? 3.8? I am waiting for this to merge before I can go on with my driver. Thanks, Vladimir On Tuesday, November 20, 2012 11:08:45 AM Jason Baron wrote: > On Sun, Nov 18, 2012 at 10:35:40AM -0800, Joe Perches wrote: > > On Sun, 2012-11-18 at 19:01 +0200, Vladimir Kondratiev wrote: > > > In case this option wins, patch follows. There is no need for > > > 2-nd one to fix existing drivers. > > > > Thanks Vladimir. > > > > > > Yes, thanks. I've pulled in this patch, and posted a dynamic debug tree > at: git clone git://github.com/jibaron/ddebug.git > > I will push the collected set of patches to Greg. > > I also added a couple of patches there to convert dynamic debug to use > 'jump labels', which shows good results. > > Thanks, > > -Jason -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] dynamic_debug: introduce debug_hex_dump()
Hi Jason, Do you have any update on the status for patches below? Where is it now? When do you expect it to merge? 3.8? I am waiting for this to merge before I can go on with my driver. Thanks, Vladimir On Tuesday, November 20, 2012 11:08:45 AM Jason Baron wrote: On Sun, Nov 18, 2012 at 10:35:40AM -0800, Joe Perches wrote: On Sun, 2012-11-18 at 19:01 +0200, Vladimir Kondratiev wrote: In case this option wins, patch follows. There is no need for 2-nd one to fix existing drivers. Thanks Vladimir. Yes, thanks. I've pulled in this patch, and posted a dynamic debug tree at: git clone git://github.com/jibaron/ddebug.git I will push the collected set of patches to Greg. I also added a couple of patches there to convert dynamic debug to use 'jump labels', which shows good results. Thanks, -Jason -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] dynamic_debug: introduce debug_hex_dump()
On Sunday, November 18, 2012 05:43:07 PM Vladimir Kondratiev wrote:
> On Sunday, November 18, 2012 06:38:51 AM Joe Perches wrote:
> > Another option could be testing __builtin_constant_p(prefix)
>
> You mean something like below? Yes, it will work as well.
>
> Pro: don't need to change existing code
> (drop [PATCH 2/2] dynamic_debug: use constant format in
> print_hex_dump_bytes())
>
> Cons: format in dynamic metadata will be useless
>
> What looks better?
>
> ---
> #define dynamic_hex_dump(prefix_str, prefix_type, rowsize,\
>groupsize, buf, len, ascii)\
> do { \
> DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, \
> __builtin_constant_p(prefix_str) ? prefix_str : "hexdump");\
> if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
> print_hex_dump(KERN_DEBUG, prefix_str, \
> prefix_type, rowsize, groupsize, \
> buf, len, ascii);\
> } while (0)
In case this option wins, patch follows. There is no need for
2-nd one to fix existing drivers.
>From 42bea6be2b3899eac1ed3f48eb955a4d83960cf5 Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev
Date: Sun, 18 Nov 2012 18:56:29 +0200
Subject: [PATCH] dynamic_debug: dynamic hex dump
Introduce print_hex_dump_debug() that can be dynamically controlled, similar to
pr_debug.
Also, make print_hex_dump_bytes() dynamically controlled
Implement only 'p' flag (_DPRINTK_FLAGS_PRINT) to keep it simple since hex dump
prints
multiple lines and long prefix would impact readability.
To provide line/file etc. information, use pr_debug or similar
before/after print_hex_dump_debug()
Signed-off-by: Vladimir Kondratiev
---
Documentation/dynamic-debug-howto.txt | 15 +--
include/linux/dynamic_debug.h | 11 +++
include/linux/printk.h| 17 +
lib/hexdump.c |4 +++-
4 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/Documentation/dynamic-debug-howto.txt
b/Documentation/dynamic-debug-howto.txt
index 6e16849..72322c6 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -6,8 +6,16 @@ This document describes how to use the dynamic debug (dyndbg)
feature.
Dynamic debug is designed to allow you to dynamically enable/disable
kernel code to obtain additional kernel information. Currently, if
-CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() calls can
-be dynamically enabled per-callsite.
+CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() and
+print_hex_dump_debug()/print_hex_dump_bytes() calls can be dynamically
+enabled per-callsite.
+
+If CONFIG_DYNAMIC_DEBUG is not set, print_hex_dump_debug() is just
+shortcut for print_hex_dump(KERN_DEBUG).
+
+For print_hex_dump_debug()/print_hex_dump_bytes(), format string is
+its 'prefix_str' argument, if it is constant string; or "hexdump"
+in case 'prefix_str' is build dynamically.
Dynamic debug has even more useful features:
@@ -202,6 +210,9 @@ The flags are:
tInclude thread ID in messages not generated from interrupt context
_No flags are set. (Or'd with others on input)
+For print_hex_dump_debug() and print_hex_dump_bytes(), only 'p' flag
+have meaning, other flags ignored.
+
For display, the flags are preceded by '='
(mnemonic: what the flags are currently equal to).
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 6dd4787..2fe93b2 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -95,6 +95,17 @@ do {
\
##__VA_ARGS__);\
} while (0)
+#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+do { \
+ DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, \
+ __builtin_constant_p(prefix_str) ? prefix_str : "hexdump");\
+ if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
+ print_hex_dump(KERN_DEBUG, prefix_str, \
+ prefix_type, rowsize, groupsize, \
+ buf, len, ascii);\
+} while (0)
+
#else
#include
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 9afc01e..02c95cf 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -321,8 +321,13 @@ extern void hex_dump_to_buffer(const void *buf, size_t len,
extern void print_hex_dump(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsi
Re: [RFC] dynamic_debug: introduce debug_hex_dump()
On Sunday, November 18, 2012 06:38:51 AM Joe Perches wrote:
> Another option could be testing __builtin_constant_p(prefix)
You mean something like below? Yes, it will work as well.
Pro: don't need to change existing code
(drop [PATCH 2/2] dynamic_debug: use constant format in print_hex_dump_bytes())
Cons: format in dynamic metadata will be useless
What looks better?
---
#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
groupsize, buf, len, ascii)\
do {\
DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, \
__builtin_constant_p(prefix_str) ? prefix_str : "hexdump");\
if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
print_hex_dump(KERN_DEBUG, prefix_str, \
prefix_type, rowsize, groupsize, \
buf, len, ascii);\
} while (0)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] dynamic_debug: dynamic hex dump
>From 26bb3835fe438622d108716420c3cc191f2e881b Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev
Date: Sun, 18 Nov 2012 15:37:20 +0200
Subject: [PATCH 1/2] dynamic_debug: dynamic hex dump
Introduce print_hex_dump_debug() that can be dynamically controlled, similar to
pr_debug.
Also, make print_hex_dump_bytes() dynamically controlled
Implement only 'p' flag (_DPRINTK_FLAGS_PRINT) to keep it simple since hex dump
prints
multiple lines and long prefix would impact readability.
To provide line/file etc. information, use pr_debug or similar
before/after print_hex_dump_debug()
Signed-off-by: Vladimir Kondratiev
---
Documentation/dynamic-debug-howto.txt | 11 +--
include/linux/dynamic_debug.h | 10 ++
include/linux/printk.h| 17 +
lib/hexdump.c |4 +++-
4 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/Documentation/dynamic-debug-howto.txt
b/Documentation/dynamic-debug-howto.txt
index 6e16849..b39a771 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -6,8 +6,12 @@ This document describes how to use the dynamic debug (dyndbg)
feature.
Dynamic debug is designed to allow you to dynamically enable/disable
kernel code to obtain additional kernel information. Currently, if
-CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() calls can
-be dynamically enabled per-callsite.
+CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() and
+print_hex_dump_debug()/print_hex_dump_bytes() calls can be dynamically
+enabled per-callsite.
+
+If CONFIG_DYNAMIC_DEBUG is not set, print_hex_dump_debug() is just
+shortcut for print_hex_dump(KERN_DEBUG).
Dynamic debug has even more useful features:
@@ -202,6 +206,9 @@ The flags are:
tInclude thread ID in messages not generated from interrupt context
_No flags are set. (Or'd with others on input)
+For print_hex_dump_debug() and print_hex_dump_bytes(), only 'p' flag
+have meaning, other flags ignored.
+
For display, the flags are preceded by '='
(mnemonic: what the flags are currently equal to).
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 6dd4787..17565ab 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -95,6 +95,16 @@ do {
\
##__VA_ARGS__);\
} while (0)
+#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+do { \
+ DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, prefix_str); \
+ if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
+ print_hex_dump(KERN_DEBUG, prefix_str, \
+ prefix_type, rowsize, groupsize, \
+ buf, len, ascii);\
+} while (0)
+
#else
#include
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 9afc01e..02c95cf 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -321,8 +321,13 @@ extern void hex_dump_to_buffer(const void *buf, size_t len,
extern void print_hex_dump(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
const void *buf, size_t len, bool ascii);
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define print_hex_dump_bytes(prefix_str, prefix_type, buf, len)\
+ dynamic_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true)
+#else
extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
const void *buf, size_t len);
+#endif /* defined(CONFIG_DYNAMIC_DEBUG) */
#else
static inline void print_hex_dump(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
@@ -336,4 +341,16 @@ static inline void print_hex_dump_bytes(const char
*prefix_str, int prefix_type,
#endif
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+ dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)
+#else
+#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+ print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, rowsize,\
+ groupsize, buf, len, ascii)
+#endif /* defined(CONFIG_DYNAMIC_DEBUG) */
+
#endif
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 6540d65..3f0494c 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -227,6 +227,7 @@ void print_hex_dump(const char *level, const char
*prefix_str, int prefix_type,
}
EXPORT
[PATCH 2/2] dynamic_debug: use constant format in print_hex_dump_bytes()
>From cff77ecaa9a1f875327a0530f0ebc7998427bbf4 Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev
Date: Sun, 18 Nov 2012 15:47:09 +0200
Subject: [PATCH 2/2] dynamic_debug: use constant format in
print_hex_dump_bytes()
In order to convert print_hex_dump_bytes() to dynamic debug,
it should be wrapped in macro (DEFINE_DYNAMIC_DEBUG_METADATA)
to define metadata. This, in turn, require 'format' to be constant string.
For practical purposes, it is reasonable to use as 'format' the
'prefix_str' argument for the print_hex_dump_bytes()
This patch fixes usage of dynamically generated prefixes. General approach
is to print desired prefix using pr_debug(), following with hex dump using
"short"
prefix string. This will preserve desired information with little impact on
readability.
In fact, result should be even better for human, albeit it may break automatic
log
parser, should it be based on prefix string.
Signed-off-by: Vladimir Kondratiev
---
drivers/isdn/hardware/mISDN/avmfritz.c | 12
drivers/isdn/hardware/mISDN/mISDNipac.c | 22 --
drivers/isdn/hardware/mISDN/netjet.c| 16 ++--
drivers/isdn/hardware/mISDN/w6692.c | 19 ---
drivers/net/wireless/ath/ath6kl/debug.c |3 ++-
drivers/net/wireless/libertas_tf/deb_defs.h |6 ++
drivers/tty/ipwireless/hardware.c |9 -
7 files changed, 34 insertions(+), 53 deletions(-)
diff --git a/drivers/isdn/hardware/mISDN/avmfritz.c
b/drivers/isdn/hardware/mISDN/avmfritz.c
index dceaec8..d24f85b 100644
--- a/drivers/isdn/hardware/mISDN/avmfritz.c
+++ b/drivers/isdn/hardware/mISDN/avmfritz.c
@@ -104,7 +104,6 @@ enum {
#define AVM_ISACX_DATA 0x08
/* data struct */
-#define LOG_SIZE 63
struct hdlc_stat_reg {
#ifdef __BIG_ENDIAN
@@ -141,7 +140,6 @@ struct fritzcard {
struct isac_hw isac;
struct hdlc_hw hdlc[2];
struct bchannel bch[2];
- charlog[LOG_SIZE + 1];
};
static LIST_HEAD(Cards);
@@ -438,9 +436,8 @@ hdlc_empty_fifo(struct bchannel *bch, int count)
cnt += 4;
}
if (p && (debug & DEBUG_HW_BFIFO)) {
- snprintf(fc->log, LOG_SIZE, "B%1d-recv %s %d ",
-bch->nr, fc->name, count);
- print_hex_dump_bytes(fc->log, DUMP_PREFIX_OFFSET, p, count);
+ pr_debug("B%1d-recv %s %d\n", bch->nr, fc->name, count);
+ print_hex_dump_bytes("recv", DUMP_PREFIX_OFFSET, p, count);
}
}
@@ -509,9 +506,8 @@ hdlc_fill_fifo(struct bchannel *bch)
}
}
if ((debug & DEBUG_HW_BFIFO) && !fillempty) {
- snprintf(fc->log, LOG_SIZE, "B%1d-send %s %d ",
-bch->nr, fc->name, count);
- print_hex_dump_bytes(fc->log, DUMP_PREFIX_OFFSET, p, count);
+ pr_debug("B%1d-send %s %d\n", bch->nr, fc->name, count);
+ print_hex_dump_bytes("send", DUMP_PREFIX_OFFSET, p, count);
}
}
diff --git a/drivers/isdn/hardware/mISDN/mISDNipac.c
b/drivers/isdn/hardware/mISDN/mISDNipac.c
index ccd7d85..1e35dcb 100644
--- a/drivers/isdn/hardware/mISDN/mISDNipac.c
+++ b/drivers/isdn/hardware/mISDN/mISDNipac.c
@@ -137,11 +137,8 @@ isac_empty_fifo(struct isac_hw *isac, int count)
isac->read_fifo(isac->dch.hw, isac->off, ptr, count);
WriteISAC(isac, ISAC_CMDR, 0x80);
if (isac->dch.debug & DEBUG_HW_DFIFO) {
- charpfx[MISDN_MAX_IDLEN + 16];
-
- snprintf(pfx, MISDN_MAX_IDLEN + 15, "D-recv %s %d ",
-isac->name, count);
- print_hex_dump_bytes(pfx, DUMP_PREFIX_OFFSET, ptr, count);
+ pr_debug("D-recv %s %d\n", isac->name, count);
+ print_hex_dump_bytes("recv", DUMP_PREFIX_OFFSET, ptr, count);
}
}
@@ -175,11 +172,8 @@ isac_fill_fifo(struct isac_hw *isac)
isac->dch.timer.expires = jiffies + ((DBUSY_TIMER_VALUE * HZ)/1000);
add_timer(>dch.timer);
if (isac->dch.debug & DEBUG_HW_DFIFO) {
- charpfx[MISDN_MAX_IDLEN + 16];
-
- snprintf(pfx, MISDN_MAX_IDLEN + 15, "D-send %s %d ",
-isac->name, count);
- print_hex_dump_bytes(pfx, DUMP_PREFIX_OFFSET, ptr, count);
+ pr_debug("D-send %s %d\n", isac->name, count);
+ print_hex_dump_bytes("send", DUMP_PREFIX_OFFSET, ptr, count);
}
}
@@ -962,9 +956,9 @@ hscx_empty_fifo(struct hscx_hw *hscx, u8 count)
hscx_cmdr(hscx, 0x80); /* RMC */
if (hscx->bch.debug & DEBUG_HW_BFIFO) {
- snprintf(hscx->log, 64, "
Re: [RFC] dynamic_debug: introduce debug_hex_dump()
On Sunday, November 18, 2012 11:47:58 AM Vladimir Kondratiev wrote:
> On Wednesday, November 14, 2012 07:27:47 PM Vladimir Kondratiev wrote:
> > +#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
> > +groupsize, buf, len, ascii)\
> > +do { \
> > + DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, prefix_str); \
> > + if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
> > + print_hex_dump(KERN_DEBUG, prefix_str, \
> > + prefix_type, rowsize, groupsize, \
> > + buf, len, ascii);\
> > +} while (0)
>
> There is some problem with the code above. There are existing drivers
> that use print_hex_dump_bytes() with the prefix_str built dynamically,
> like this (see drivers/tty/ipwireless/hardware.c:362)
>
> static void dump_data_bytes(const char *type, const unsigned char *data,
> unsigned length)
> {
> char prefix[56];
>
> sprintf(prefix, IPWIRELESS_PCCARD_NAME ": %s %s ",
> type, data_type(data, length));
> print_hex_dump_bytes(prefix, 0, (void *)data,
> length < DUMP_MAX_BYTES ? length : DUMP_MAX_BYTES);
> }
>
> In this case, prefix_str is not constant, and this solution will not
> work.
>
> I see 2 drivers that use dynamic prefix:
>
> - mentioned above drivers/tty/ipwireless/hardware.c
> - drivers/isdn/hardware/mISDN/
>
> I see several options:
>
> 1) require prefix_str to be constant. Patch code mentioned. For example,
> code above may reads:
>
> static void dump_data_bytes(const char *type, const unsigned char *data,
> unsigned length)
> {
> pr_debug(IPWIRELESS_PCCARD_NAME ": %s %s\n",
> type, data_type(data, length));
> print_hex_dump_bytes(IPWIRELESS_PCCARD_NAME, 0, (void *)data,
> length < DUMP_MAX_BYTES ? length : DUMP_MAX_BYTES);
> }
>
> 2) introduce fixed format string in DEFINE_DYNAMIC_DEBUG_METADATA, like
>
> DEFINE_DYNAMIC_DEBUG_METADATA((descriptor, "hexdump");
>
> 3) remove print_hex_dump_bytes from this patch
>
> I think 1) is the best, opinions?
>
> Thanks, Vladimir
In case of option 1), patches follows. Note there is change in the initial
patch as well (I forgot to exclude print_hex_dump_bytes() from compilation in
hexdump.c. Please, comment.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] dynamic_debug: introduce debug_hex_dump()
On Sunday, November 18, 2012 11:47:58 AM Vladimir Kondratiev wrote:
On Wednesday, November 14, 2012 07:27:47 PM Vladimir Kondratiev wrote:
+#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+do { \
+ DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, prefix_str); \
+ if (unlikely(descriptor.flags _DPRINTK_FLAGS_PRINT)) \
+ print_hex_dump(KERN_DEBUG, prefix_str, \
+ prefix_type, rowsize, groupsize, \
+ buf, len, ascii);\
+} while (0)
There is some problem with the code above. There are existing drivers
that use print_hex_dump_bytes() with the prefix_str built dynamically,
like this (see drivers/tty/ipwireless/hardware.c:362)
static void dump_data_bytes(const char *type, const unsigned char *data,
unsigned length)
{
char prefix[56];
sprintf(prefix, IPWIRELESS_PCCARD_NAME : %s %s ,
type, data_type(data, length));
print_hex_dump_bytes(prefix, 0, (void *)data,
length DUMP_MAX_BYTES ? length : DUMP_MAX_BYTES);
}
In this case, prefix_str is not constant, and this solution will not
work.
I see 2 drivers that use dynamic prefix:
- mentioned above drivers/tty/ipwireless/hardware.c
- drivers/isdn/hardware/mISDN/
I see several options:
1) require prefix_str to be constant. Patch code mentioned. For example,
code above may reads:
static void dump_data_bytes(const char *type, const unsigned char *data,
unsigned length)
{
pr_debug(IPWIRELESS_PCCARD_NAME : %s %s\n,
type, data_type(data, length));
print_hex_dump_bytes(IPWIRELESS_PCCARD_NAME, 0, (void *)data,
length DUMP_MAX_BYTES ? length : DUMP_MAX_BYTES);
}
2) introduce fixed format string in DEFINE_DYNAMIC_DEBUG_METADATA, like
DEFINE_DYNAMIC_DEBUG_METADATA((descriptor, hexdump);
3) remove print_hex_dump_bytes from this patch
I think 1) is the best, opinions?
Thanks, Vladimir
In case of option 1), patches follows. Note there is change in the initial
patch as well (I forgot to exclude print_hex_dump_bytes() from compilation in
hexdump.c. Please, comment.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] dynamic_debug: use constant format in print_hex_dump_bytes()
From cff77ecaa9a1f875327a0530f0ebc7998427bbf4 Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com
Date: Sun, 18 Nov 2012 15:47:09 +0200
Subject: [PATCH 2/2] dynamic_debug: use constant format in
print_hex_dump_bytes()
In order to convert print_hex_dump_bytes() to dynamic debug,
it should be wrapped in macro (DEFINE_DYNAMIC_DEBUG_METADATA)
to define metadata. This, in turn, require 'format' to be constant string.
For practical purposes, it is reasonable to use as 'format' the
'prefix_str' argument for the print_hex_dump_bytes()
This patch fixes usage of dynamically generated prefixes. General approach
is to print desired prefix using pr_debug(), following with hex dump using
short
prefix string. This will preserve desired information with little impact on
readability.
In fact, result should be even better for human, albeit it may break automatic
log
parser, should it be based on prefix string.
Signed-off-by: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com
---
drivers/isdn/hardware/mISDN/avmfritz.c | 12
drivers/isdn/hardware/mISDN/mISDNipac.c | 22 --
drivers/isdn/hardware/mISDN/netjet.c| 16 ++--
drivers/isdn/hardware/mISDN/w6692.c | 19 ---
drivers/net/wireless/ath/ath6kl/debug.c |3 ++-
drivers/net/wireless/libertas_tf/deb_defs.h |6 ++
drivers/tty/ipwireless/hardware.c |9 -
7 files changed, 34 insertions(+), 53 deletions(-)
diff --git a/drivers/isdn/hardware/mISDN/avmfritz.c
b/drivers/isdn/hardware/mISDN/avmfritz.c
index dceaec8..d24f85b 100644
--- a/drivers/isdn/hardware/mISDN/avmfritz.c
+++ b/drivers/isdn/hardware/mISDN/avmfritz.c
@@ -104,7 +104,6 @@ enum {
#define AVM_ISACX_DATA 0x08
/* data struct */
-#define LOG_SIZE 63
struct hdlc_stat_reg {
#ifdef __BIG_ENDIAN
@@ -141,7 +140,6 @@ struct fritzcard {
struct isac_hw isac;
struct hdlc_hw hdlc[2];
struct bchannel bch[2];
- charlog[LOG_SIZE + 1];
};
static LIST_HEAD(Cards);
@@ -438,9 +436,8 @@ hdlc_empty_fifo(struct bchannel *bch, int count)
cnt += 4;
}
if (p (debug DEBUG_HW_BFIFO)) {
- snprintf(fc-log, LOG_SIZE, B%1d-recv %s %d ,
-bch-nr, fc-name, count);
- print_hex_dump_bytes(fc-log, DUMP_PREFIX_OFFSET, p, count);
+ pr_debug(B%1d-recv %s %d\n, bch-nr, fc-name, count);
+ print_hex_dump_bytes(recv, DUMP_PREFIX_OFFSET, p, count);
}
}
@@ -509,9 +506,8 @@ hdlc_fill_fifo(struct bchannel *bch)
}
}
if ((debug DEBUG_HW_BFIFO) !fillempty) {
- snprintf(fc-log, LOG_SIZE, B%1d-send %s %d ,
-bch-nr, fc-name, count);
- print_hex_dump_bytes(fc-log, DUMP_PREFIX_OFFSET, p, count);
+ pr_debug(B%1d-send %s %d\n, bch-nr, fc-name, count);
+ print_hex_dump_bytes(send, DUMP_PREFIX_OFFSET, p, count);
}
}
diff --git a/drivers/isdn/hardware/mISDN/mISDNipac.c
b/drivers/isdn/hardware/mISDN/mISDNipac.c
index ccd7d85..1e35dcb 100644
--- a/drivers/isdn/hardware/mISDN/mISDNipac.c
+++ b/drivers/isdn/hardware/mISDN/mISDNipac.c
@@ -137,11 +137,8 @@ isac_empty_fifo(struct isac_hw *isac, int count)
isac-read_fifo(isac-dch.hw, isac-off, ptr, count);
WriteISAC(isac, ISAC_CMDR, 0x80);
if (isac-dch.debug DEBUG_HW_DFIFO) {
- charpfx[MISDN_MAX_IDLEN + 16];
-
- snprintf(pfx, MISDN_MAX_IDLEN + 15, D-recv %s %d ,
-isac-name, count);
- print_hex_dump_bytes(pfx, DUMP_PREFIX_OFFSET, ptr, count);
+ pr_debug(D-recv %s %d\n, isac-name, count);
+ print_hex_dump_bytes(recv, DUMP_PREFIX_OFFSET, ptr, count);
}
}
@@ -175,11 +172,8 @@ isac_fill_fifo(struct isac_hw *isac)
isac-dch.timer.expires = jiffies + ((DBUSY_TIMER_VALUE * HZ)/1000);
add_timer(isac-dch.timer);
if (isac-dch.debug DEBUG_HW_DFIFO) {
- charpfx[MISDN_MAX_IDLEN + 16];
-
- snprintf(pfx, MISDN_MAX_IDLEN + 15, D-send %s %d ,
-isac-name, count);
- print_hex_dump_bytes(pfx, DUMP_PREFIX_OFFSET, ptr, count);
+ pr_debug(D-send %s %d\n, isac-name, count);
+ print_hex_dump_bytes(send, DUMP_PREFIX_OFFSET, ptr, count);
}
}
@@ -962,9 +956,9 @@ hscx_empty_fifo(struct hscx_hw *hscx, u8 count)
hscx_cmdr(hscx, 0x80); /* RMC */
if (hscx-bch.debug DEBUG_HW_BFIFO) {
- snprintf(hscx-log, 64, B%1d-recv %s %d ,
+ pr_debug(B%1d-recv %s %d\n,
hscx-bch.nr, hscx-ip-name, count);
- print_hex_dump_bytes(hscx-log, DUMP_PREFIX_OFFSET, p, count);
+ print_hex_dump_bytes(recv
[PATCH 1/2] dynamic_debug: dynamic hex dump
From 26bb3835fe438622d108716420c3cc191f2e881b Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com
Date: Sun, 18 Nov 2012 15:37:20 +0200
Subject: [PATCH 1/2] dynamic_debug: dynamic hex dump
Introduce print_hex_dump_debug() that can be dynamically controlled, similar to
pr_debug.
Also, make print_hex_dump_bytes() dynamically controlled
Implement only 'p' flag (_DPRINTK_FLAGS_PRINT) to keep it simple since hex dump
prints
multiple lines and long prefix would impact readability.
To provide line/file etc. information, use pr_debug or similar
before/after print_hex_dump_debug()
Signed-off-by: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com
---
Documentation/dynamic-debug-howto.txt | 11 +--
include/linux/dynamic_debug.h | 10 ++
include/linux/printk.h| 17 +
lib/hexdump.c |4 +++-
4 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/Documentation/dynamic-debug-howto.txt
b/Documentation/dynamic-debug-howto.txt
index 6e16849..b39a771 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -6,8 +6,12 @@ This document describes how to use the dynamic debug (dyndbg)
feature.
Dynamic debug is designed to allow you to dynamically enable/disable
kernel code to obtain additional kernel information. Currently, if
-CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() calls can
-be dynamically enabled per-callsite.
+CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() and
+print_hex_dump_debug()/print_hex_dump_bytes() calls can be dynamically
+enabled per-callsite.
+
+If CONFIG_DYNAMIC_DEBUG is not set, print_hex_dump_debug() is just
+shortcut for print_hex_dump(KERN_DEBUG).
Dynamic debug has even more useful features:
@@ -202,6 +206,9 @@ The flags are:
tInclude thread ID in messages not generated from interrupt context
_No flags are set. (Or'd with others on input)
+For print_hex_dump_debug() and print_hex_dump_bytes(), only 'p' flag
+have meaning, other flags ignored.
+
For display, the flags are preceded by '='
(mnemonic: what the flags are currently equal to).
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 6dd4787..17565ab 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -95,6 +95,16 @@ do {
\
##__VA_ARGS__);\
} while (0)
+#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+do { \
+ DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, prefix_str); \
+ if (unlikely(descriptor.flags _DPRINTK_FLAGS_PRINT)) \
+ print_hex_dump(KERN_DEBUG, prefix_str, \
+ prefix_type, rowsize, groupsize, \
+ buf, len, ascii);\
+} while (0)
+
#else
#include linux/string.h
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 9afc01e..02c95cf 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -321,8 +321,13 @@ extern void hex_dump_to_buffer(const void *buf, size_t len,
extern void print_hex_dump(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
const void *buf, size_t len, bool ascii);
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define print_hex_dump_bytes(prefix_str, prefix_type, buf, len)\
+ dynamic_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true)
+#else
extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
const void *buf, size_t len);
+#endif /* defined(CONFIG_DYNAMIC_DEBUG) */
#else
static inline void print_hex_dump(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
@@ -336,4 +341,16 @@ static inline void print_hex_dump_bytes(const char
*prefix_str, int prefix_type,
#endif
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+ dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)
+#else
+#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+ print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, rowsize,\
+ groupsize, buf, len, ascii)
+#endif /* defined(CONFIG_DYNAMIC_DEBUG) */
+
#endif
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 6540d65..3f0494c 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -227,6 +227,7 @@ void print_hex_dump(const char
Re: [RFC] dynamic_debug: introduce debug_hex_dump()
On Sunday, November 18, 2012 06:38:51 AM Joe Perches wrote:
Another option could be testing __builtin_constant_p(prefix)
You mean something like below? Yes, it will work as well.
Pro: don't need to change existing code
(drop [PATCH 2/2] dynamic_debug: use constant format in print_hex_dump_bytes())
Cons: format in dynamic metadata will be useless
What looks better?
---
#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
groupsize, buf, len, ascii)\
do {\
DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, \
__builtin_constant_p(prefix_str) ? prefix_str : hexdump);\
if (unlikely(descriptor.flags _DPRINTK_FLAGS_PRINT)) \
print_hex_dump(KERN_DEBUG, prefix_str, \
prefix_type, rowsize, groupsize, \
buf, len, ascii);\
} while (0)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] dynamic_debug: introduce debug_hex_dump()
On Sunday, November 18, 2012 05:43:07 PM Vladimir Kondratiev wrote:
On Sunday, November 18, 2012 06:38:51 AM Joe Perches wrote:
Another option could be testing __builtin_constant_p(prefix)
You mean something like below? Yes, it will work as well.
Pro: don't need to change existing code
(drop [PATCH 2/2] dynamic_debug: use constant format in
print_hex_dump_bytes())
Cons: format in dynamic metadata will be useless
What looks better?
---
#define dynamic_hex_dump(prefix_str, prefix_type, rowsize,\
groupsize, buf, len, ascii)\
do { \
DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, \
__builtin_constant_p(prefix_str) ? prefix_str : hexdump);\
if (unlikely(descriptor.flags _DPRINTK_FLAGS_PRINT)) \
print_hex_dump(KERN_DEBUG, prefix_str, \
prefix_type, rowsize, groupsize, \
buf, len, ascii);\
} while (0)
In case this option wins, patch follows. There is no need for
2-nd one to fix existing drivers.
From 42bea6be2b3899eac1ed3f48eb955a4d83960cf5 Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com
Date: Sun, 18 Nov 2012 18:56:29 +0200
Subject: [PATCH] dynamic_debug: dynamic hex dump
Introduce print_hex_dump_debug() that can be dynamically controlled, similar to
pr_debug.
Also, make print_hex_dump_bytes() dynamically controlled
Implement only 'p' flag (_DPRINTK_FLAGS_PRINT) to keep it simple since hex dump
prints
multiple lines and long prefix would impact readability.
To provide line/file etc. information, use pr_debug or similar
before/after print_hex_dump_debug()
Signed-off-by: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com
---
Documentation/dynamic-debug-howto.txt | 15 +--
include/linux/dynamic_debug.h | 11 +++
include/linux/printk.h| 17 +
lib/hexdump.c |4 +++-
4 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/Documentation/dynamic-debug-howto.txt
b/Documentation/dynamic-debug-howto.txt
index 6e16849..72322c6 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -6,8 +6,16 @@ This document describes how to use the dynamic debug (dyndbg)
feature.
Dynamic debug is designed to allow you to dynamically enable/disable
kernel code to obtain additional kernel information. Currently, if
-CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() calls can
-be dynamically enabled per-callsite.
+CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() and
+print_hex_dump_debug()/print_hex_dump_bytes() calls can be dynamically
+enabled per-callsite.
+
+If CONFIG_DYNAMIC_DEBUG is not set, print_hex_dump_debug() is just
+shortcut for print_hex_dump(KERN_DEBUG).
+
+For print_hex_dump_debug()/print_hex_dump_bytes(), format string is
+its 'prefix_str' argument, if it is constant string; or hexdump
+in case 'prefix_str' is build dynamically.
Dynamic debug has even more useful features:
@@ -202,6 +210,9 @@ The flags are:
tInclude thread ID in messages not generated from interrupt context
_No flags are set. (Or'd with others on input)
+For print_hex_dump_debug() and print_hex_dump_bytes(), only 'p' flag
+have meaning, other flags ignored.
+
For display, the flags are preceded by '='
(mnemonic: what the flags are currently equal to).
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 6dd4787..2fe93b2 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -95,6 +95,17 @@ do {
\
##__VA_ARGS__);\
} while (0)
+#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+do { \
+ DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, \
+ __builtin_constant_p(prefix_str) ? prefix_str : hexdump);\
+ if (unlikely(descriptor.flags _DPRINTK_FLAGS_PRINT)) \
+ print_hex_dump(KERN_DEBUG, prefix_str, \
+ prefix_type, rowsize, groupsize, \
+ buf, len, ascii);\
+} while (0)
+
#else
#include linux/string.h
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 9afc01e..02c95cf 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -321,8 +321,13 @@ extern void hex_dump_to_buffer(const void *buf, size_t len,
extern void print_hex_dump(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
const void *buf, size_t len, bool ascii);
+#if defined
Re: [RFC] dynamic_debug: introduce debug_hex_dump()
On Wednesday, November 14, 2012 09:08:52 AM Joe Perches wrote:
> On Wed, 2012-11-14 at 18:18 +0200, Vladimir Kondratiev wrote:
> > And, here it goes (can I call it PATCH?):
> Don't see why not.
>
> > From: Vladimir Kondratiev
>
> []
>
> > Introduce debug_hex_dump() that can be dynamically controlled, similar to
> > pr_debug.
> >
> > Also, make print_hex_dump_bytes() dynamically controlled
>
> print_hex_dump_debug might be a better name than
> debug_hex_dump because it's more similar to the other
> print_hex_dump<_foo> names.
Well, let it be:
>From 5c9a79ea32e300f9a228d659f325e30d450b57c1 Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev
Date: Wed, 14 Nov 2012 19:24:51 +0200
Subject: [PATCH] dynamic_debug: dynamic hex dump
Introduce print_hex_dump_debug() that can be dynamically controlled, similar to
pr_debug.
Also, make print_hex_dump_bytes() dynamically controlled
Implement only 'p' flag (_DPRINTK_FLAGS_PRINT) to keep it simple since hex dump
prints
multiple lines and long prefix would impact readability.
To provide line/file etc. information, use pr_debug or similar
before/after print_hex_dump_debug()
Signed-off-by: Vladimir Kondratiev
---
Documentation/dynamic-debug-howto.txt | 11 +--
include/linux/dynamic_debug.h | 10 ++
include/linux/printk.h| 17 +
3 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/Documentation/dynamic-debug-howto.txt
b/Documentation/dynamic-debug-howto.txt
index 6e16849..b39a771 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -6,8 +6,12 @@ This document describes how to use the dynamic debug (dyndbg)
feature.
Dynamic debug is designed to allow you to dynamically enable/disable
kernel code to obtain additional kernel information. Currently, if
-CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() calls can
-be dynamically enabled per-callsite.
+CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() and
+print_hex_dump_debug()/print_hex_dump_bytes() calls can be dynamically
+enabled per-callsite.
+
+If CONFIG_DYNAMIC_DEBUG is not set, print_hex_dump_debug() is just
+shortcut for print_hex_dump(KERN_DEBUG).
Dynamic debug has even more useful features:
@@ -202,6 +206,9 @@ The flags are:
tInclude thread ID in messages not generated from interrupt context
_No flags are set. (Or'd with others on input)
+For print_hex_dump_debug() and print_hex_dump_bytes(), only 'p' flag
+have meaning, other flags ignored.
+
For display, the flags are preceded by '='
(mnemonic: what the flags are currently equal to).
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 6dd4787..17565ab 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -95,6 +95,16 @@ do {
\
##__VA_ARGS__);\
} while (0)
+#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+do { \
+ DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, prefix_str); \
+ if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \
+ print_hex_dump(KERN_DEBUG, prefix_str, \
+ prefix_type, rowsize, groupsize, \
+ buf, len, ascii);\
+} while (0)
+
#else
#include
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 9afc01e..02c95cf 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -321,8 +321,13 @@ extern void hex_dump_to_buffer(const void *buf, size_t len,
extern void print_hex_dump(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
const void *buf, size_t len, bool ascii);
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define print_hex_dump_bytes(prefix_str, prefix_type, buf, len)\
+ dynamic_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true)
+#else
extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
const void *buf, size_t len);
+#endif /* defined(CONFIG_DYNAMIC_DEBUG) */
#else
static inline void print_hex_dump(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
@@ -336,4 +341,16 @@ static inline void print_hex_dump_bytes(const char
*prefix_str, int prefix_type,
#endif
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+ dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize
Re: [RFC] dynamic_debug: introduce debug_hex_dump()
On Wednesday, November 14, 2012 05:41:08 AM Joe Perches wrote:
> On Wed, 2012-11-14 at 14:17 +0200, Vladimir Kondratiev wrote:
> > Introduce debug_hex_dump() that can be dynamically controlled, similar to
> > pr_debug.
>
> (added Jason Baron, Jim Cromie, GregKH and lkml to cc's)
>
> []
>
> > diff --git a/include/linux/printk.h b/include/linux/printk.h
>
> []
>
> > @@ -220,6 +220,20 @@ extern void dump_stack(void) __cold;
> >
> > no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> >
> > #endifD
> >
> > +#if defined(CONFIG_DYNAMIC_DEBUG)
> > +#define debug_hex_dump(prefix_str, prefix_type, rowsize, \
> > + groupsize, buf, len, ascii) \
> > + dynamic_hex_dump(prefix_str, prefix_type,\
> > + rowsize, groupsize, buf,\
> > + len, ascii)
> > +#else
> > +#define debug_hex_dump(prefix_str, prefix_type, rowsize, \
> > + groupsize, buf, len, ascii) \
> > + print_hex_dump(KERN_DEBUG, prefix_str, \
> > + prefix_type, rowsize, \
> > + groupsize, buf, len, ascii)
> > +#endif
>
> These should be in a different location after print_hex_dump
> is declared. Also for #defines, the indentation doesn't
> need to be so deep.
Ups, correct.
>
> #if defined(CONFIG_DYNAMIC_DEBUG)
> #define debug_hex_dump(prefix_str, prefix_type, rowsize, \
> groupsize, buf, len, ascii) \
> dynamic_hex_dump(prefix_str, prefix_type, \
>rowsize, groupsize, buf, len, ascii)
> #else
> #define debug_hex_dump(prefix_str, prefix_type, rowsize, \
> groupsize, buf, len, ascii) \
> print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, \
> rowsize, groupsize, buf, len, ascii)
> #endif
>
> A better option might be to convert print_hex_dump_bytes()
> to dynamic_debug as that's already KERN_DEBUG. That
> could be simpler overall and it makes existing calls
> become dynamic as well.
And, here it goes (can I call it PATCH?):
>From 755b74861b66435bfe4875b64a53c45bbe172495 Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev
Date: Wed, 14 Nov 2012 18:14:15 +0200
Subject: [PATCH] dynamic_debug: dynamic hex dump
Introduce debug_hex_dump() that can be dynamically controlled, similar to
pr_debug.
Also, make print_hex_dump_bytes() dynamically controlled
Implement only 'p' flag (_DPRINTK_FLAGS_PRINT) to keep it simple since hex dump
prints
multiple lines and long prefix would impact readability.
To provide line/file etc. information, use pr_debug or similar
before/after debug_hex_dump()
Signed-off-by: Vladimir Kondratiev
---
Documentation/dynamic-debug-howto.txt | 11 +--
include/linux/dynamic_debug.h | 10 ++
include/linux/printk.h| 17 +
3 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/Documentation/dynamic-debug-howto.txt
b/Documentation/dynamic-debug-howto.txt
index 6e16849..cec98f8 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -6,8 +6,12 @@ This document describes how to use the dynamic debug (dyndbg)
feature.
Dynamic debug is designed to allow you to dynamically enable/disable
kernel code to obtain additional kernel information. Currently, if
-CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() calls can
-be dynamically enabled per-callsite.
+CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() and
+debug_hex_dump()/print_hex_dump_bytes() calls can be dynamically
+enabled per-callsite.
+
+If CONFIG_DYNAMIC_DEBUG is not set, debug_hex_dump() is just shortcut
+for print_hex_dump(KERN_DEBUG).
Dynamic debug has even more useful features:
@@ -202,6 +206,9 @@ The flags are:
tInclude thread ID in messages not generated from interrupt context
_No flags are set. (Or'd with others on input)
+For debug_hex_dump() and print_hex_dump_bytes(), only 'p' flag have meaning,
+other flags ignored.
+
For display, the flags are preceded by '='
(mnemonic: what the flags are currently equal to).
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 6dd4787..17565ab 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -95,6 +95,16 @@ do {
\
##__VA_ARGS__);\
} while (0)
+#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+do {
Re: [RFC] dynamic_debug: introduce debug_hex_dump()
On Wednesday, November 14, 2012 05:41:08 AM Joe Perches wrote:
On Wed, 2012-11-14 at 14:17 +0200, Vladimir Kondratiev wrote:
Introduce debug_hex_dump() that can be dynamically controlled, similar to
pr_debug.
(added Jason Baron, Jim Cromie, GregKH and lkml to cc's)
[]
diff --git a/include/linux/printk.h b/include/linux/printk.h
[]
@@ -220,6 +220,20 @@ extern void dump_stack(void) __cold;
no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
#endifD
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define debug_hex_dump(prefix_str, prefix_type, rowsize, \
+ groupsize, buf, len, ascii) \
+ dynamic_hex_dump(prefix_str, prefix_type,\
+ rowsize, groupsize, buf,\
+ len, ascii)
+#else
+#define debug_hex_dump(prefix_str, prefix_type, rowsize, \
+ groupsize, buf, len, ascii) \
+ print_hex_dump(KERN_DEBUG, prefix_str, \
+ prefix_type, rowsize, \
+ groupsize, buf, len, ascii)
+#endif
These should be in a different location after print_hex_dump
is declared. Also for #defines, the indentation doesn't
need to be so deep.
Ups, correct.
#if defined(CONFIG_DYNAMIC_DEBUG)
#define debug_hex_dump(prefix_str, prefix_type, rowsize, \
groupsize, buf, len, ascii) \
dynamic_hex_dump(prefix_str, prefix_type, \
rowsize, groupsize, buf, len, ascii)
#else
#define debug_hex_dump(prefix_str, prefix_type, rowsize, \
groupsize, buf, len, ascii) \
print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, \
rowsize, groupsize, buf, len, ascii)
#endif
A better option might be to convert print_hex_dump_bytes()
to dynamic_debug as that's already KERN_DEBUG. That
could be simpler overall and it makes existing calls
become dynamic as well.
And, here it goes (can I call it PATCH?):
From 755b74861b66435bfe4875b64a53c45bbe172495 Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com
Date: Wed, 14 Nov 2012 18:14:15 +0200
Subject: [PATCH] dynamic_debug: dynamic hex dump
Introduce debug_hex_dump() that can be dynamically controlled, similar to
pr_debug.
Also, make print_hex_dump_bytes() dynamically controlled
Implement only 'p' flag (_DPRINTK_FLAGS_PRINT) to keep it simple since hex dump
prints
multiple lines and long prefix would impact readability.
To provide line/file etc. information, use pr_debug or similar
before/after debug_hex_dump()
Signed-off-by: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com
---
Documentation/dynamic-debug-howto.txt | 11 +--
include/linux/dynamic_debug.h | 10 ++
include/linux/printk.h| 17 +
3 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/Documentation/dynamic-debug-howto.txt
b/Documentation/dynamic-debug-howto.txt
index 6e16849..cec98f8 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -6,8 +6,12 @@ This document describes how to use the dynamic debug (dyndbg)
feature.
Dynamic debug is designed to allow you to dynamically enable/disable
kernel code to obtain additional kernel information. Currently, if
-CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() calls can
-be dynamically enabled per-callsite.
+CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() and
+debug_hex_dump()/print_hex_dump_bytes() calls can be dynamically
+enabled per-callsite.
+
+If CONFIG_DYNAMIC_DEBUG is not set, debug_hex_dump() is just shortcut
+for print_hex_dump(KERN_DEBUG).
Dynamic debug has even more useful features:
@@ -202,6 +206,9 @@ The flags are:
tInclude thread ID in messages not generated from interrupt context
_No flags are set. (Or'd with others on input)
+For debug_hex_dump() and print_hex_dump_bytes(), only 'p' flag have meaning,
+other flags ignored.
+
For display, the flags are preceded by '='
(mnemonic: what the flags are currently equal to).
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 6dd4787..17565ab 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -95,6 +95,16 @@ do {
\
##__VA_ARGS__);\
} while (0)
+#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+do { \
+ DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, prefix_str); \
+ if (unlikely(descriptor.flags _DPRINTK_FLAGS_PRINT)) \
+ print_hex_dump(KERN_DEBUG, prefix_str
Re: [RFC] dynamic_debug: introduce debug_hex_dump()
On Wednesday, November 14, 2012 09:08:52 AM Joe Perches wrote:
On Wed, 2012-11-14 at 18:18 +0200, Vladimir Kondratiev wrote:
And, here it goes (can I call it PATCH?):
Don't see why not.
From: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com
[]
Introduce debug_hex_dump() that can be dynamically controlled, similar to
pr_debug.
Also, make print_hex_dump_bytes() dynamically controlled
print_hex_dump_debug might be a better name than
debug_hex_dump because it's more similar to the other
print_hex_dump_foo names.
Well, let it be:
From 5c9a79ea32e300f9a228d659f325e30d450b57c1 Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com
Date: Wed, 14 Nov 2012 19:24:51 +0200
Subject: [PATCH] dynamic_debug: dynamic hex dump
Introduce print_hex_dump_debug() that can be dynamically controlled, similar to
pr_debug.
Also, make print_hex_dump_bytes() dynamically controlled
Implement only 'p' flag (_DPRINTK_FLAGS_PRINT) to keep it simple since hex dump
prints
multiple lines and long prefix would impact readability.
To provide line/file etc. information, use pr_debug or similar
before/after print_hex_dump_debug()
Signed-off-by: Vladimir Kondratiev qca_vkond...@qca.qualcomm.com
---
Documentation/dynamic-debug-howto.txt | 11 +--
include/linux/dynamic_debug.h | 10 ++
include/linux/printk.h| 17 +
3 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/Documentation/dynamic-debug-howto.txt
b/Documentation/dynamic-debug-howto.txt
index 6e16849..b39a771 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -6,8 +6,12 @@ This document describes how to use the dynamic debug (dyndbg)
feature.
Dynamic debug is designed to allow you to dynamically enable/disable
kernel code to obtain additional kernel information. Currently, if
-CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() calls can
-be dynamically enabled per-callsite.
+CONFIG_DYNAMIC_DEBUG is set, then all pr_debug()/dev_dbg() and
+print_hex_dump_debug()/print_hex_dump_bytes() calls can be dynamically
+enabled per-callsite.
+
+If CONFIG_DYNAMIC_DEBUG is not set, print_hex_dump_debug() is just
+shortcut for print_hex_dump(KERN_DEBUG).
Dynamic debug has even more useful features:
@@ -202,6 +206,9 @@ The flags are:
tInclude thread ID in messages not generated from interrupt context
_No flags are set. (Or'd with others on input)
+For print_hex_dump_debug() and print_hex_dump_bytes(), only 'p' flag
+have meaning, other flags ignored.
+
For display, the flags are preceded by '='
(mnemonic: what the flags are currently equal to).
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 6dd4787..17565ab 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -95,6 +95,16 @@ do {
\
##__VA_ARGS__);\
} while (0)
+#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+do { \
+ DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, prefix_str); \
+ if (unlikely(descriptor.flags _DPRINTK_FLAGS_PRINT)) \
+ print_hex_dump(KERN_DEBUG, prefix_str, \
+ prefix_type, rowsize, groupsize, \
+ buf, len, ascii);\
+} while (0)
+
#else
#include linux/string.h
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 9afc01e..02c95cf 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -321,8 +321,13 @@ extern void hex_dump_to_buffer(const void *buf, size_t len,
extern void print_hex_dump(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
const void *buf, size_t len, bool ascii);
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define print_hex_dump_bytes(prefix_str, prefix_type, buf, len)\
+ dynamic_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true)
+#else
extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
const void *buf, size_t len);
+#endif /* defined(CONFIG_DYNAMIC_DEBUG) */
#else
static inline void print_hex_dump(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
@@ -336,4 +341,16 @@ static inline void print_hex_dump_bytes(const char
*prefix_str, int prefix_type,
#endif
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len, ascii)\
+ dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+groupsize, buf, len
Re: wireless 2.6 work
I did posted once; it was long time ago. I am sure I sent it to Dave. I can resend if needed. Basically, I made Dave's stack work on 2.6 kernels; did some changes toward QoS and provided simple utility to imitate low level driver. I was concentrated on interfaces, it is still just skeleton. I did not touched this work since then. I used to do stuff very close to 802.11 stack. Now I am very busy with some different work. I can however consult on any 802.11 standard issues, QoS in particular. Maybe, it is good idea to talk to James Ketrenos as well. Last time I saw him doing good work on .11 stack. Vladimir On Wednesday 23 March 2005 04:15, Jeff Garzik wrote: JG> Luis R. Rodriguez wrote: JG> > Jeff, JG> > JG> > I'm sick off the low activity and slow support on wireless we have. I JG> > know you're busy so I wanted to offer my help in helping around work on JG> > wireless-2.6, now that I have time after work, and before I commit JG> > myself to anything else. It's a bit suicidal, but oh well. Oh yeah and JG> > I'll also start using bitkeeper due to the recent clarifications on the JG> > license of its usage. JG> JG> Great! While I think BitKeeper is useful, you are more than welcome to JG> continue sending patches. JG> JG> To wireless developers, BitKeeper will mainly be of use in sync'ing with JG> the latest wireless-2.6 tree. JG> JG> JG> > I'll willing to review as much patches as I have to and also hopefully JG> > write documentation on writing new wireless drivers. That said, if I can JG> > be of any assistance, where what you like me to start on? JG> > JG> > Here's what's on my agenda so far: JG> > JG> > * Help cleanup new ralink driver, start using ieee802211 and get into wireless-2.6. JG> > * Push prism54's new WPA and WDS support into wireless-2.6 JG> > * Start seeing what I can use off of ieee80211 for prism54, clean it, JG> > and move to wireless-2.6 JG> > * Start incorporating WPA through wpa_supplicant onto as many drivers JG> > * Start standardizing all things a bit, as bitched about and well pointed out JG> > by Dan Williams <[EMAIL PROTECTED]> JG> > * Listen to Jouni, he's the man JG> JG> Well, all this sounds good to me. See also the 'status' post I just JG> made, and the 'note on wireless development process' I am about to write. JG> JG> I'm really hoping someone will look into integrating wireless 802.11 as JG> a "real" protocol, rather than faking ethernet. This work starts with JG> the "p80211" template DaveM provided, and hopefully continues with JG> Vladimir's updates of DaveM's code (did he post those anywhere?). There JG> are also issues such as ARP types that Dan Williams mentioned to me as JG> issues. JG> JG> The "integrate wireless into net stack" work requires a very JG> self-motivated person who is willing to poke into the net stack, and JG> answer their own questions. JG> JG> Jeff JG> JG> JG> JG> JG> pgpa7A3QbhC2k.pgp Description: PGP signature
Re: wireless 2.6 work
I did posted once; it was long time ago. I am sure I sent it to Dave. I can resend if needed. Basically, I made Dave's stack work on 2.6 kernels; did some changes toward QoS and provided simple utility to imitate low level driver. I was concentrated on interfaces, it is still just skeleton. I did not touched this work since then. I used to do stuff very close to 802.11 stack. Now I am very busy with some different work. I can however consult on any 802.11 standard issues, QoS in particular. Maybe, it is good idea to talk to James Ketrenos as well. Last time I saw him doing good work on .11 stack. Vladimir On Wednesday 23 March 2005 04:15, Jeff Garzik wrote: JG Luis R. Rodriguez wrote: JG Jeff, JG JG I'm sick off the low activity and slow support on wireless we have. I JG know you're busy so I wanted to offer my help in helping around work on JG wireless-2.6, now that I have time after work, and before I commit JG myself to anything else. It's a bit suicidal, but oh well. Oh yeah and JG I'll also start using bitkeeper due to the recent clarifications on the JG license of its usage. JG JG Great! While I think BitKeeper is useful, you are more than welcome to JG continue sending patches. JG JG To wireless developers, BitKeeper will mainly be of use in sync'ing with JG the latest wireless-2.6 tree. JG JG JG I'll willing to review as much patches as I have to and also hopefully JG write documentation on writing new wireless drivers. That said, if I can JG be of any assistance, where what you like me to start on? JG JG Here's what's on my agenda so far: JG JG * Help cleanup new ralink driver, start using ieee802211 and get into wireless-2.6. JG * Push prism54's new WPA and WDS support into wireless-2.6 JG * Start seeing what I can use off of ieee80211 for prism54, clean it, JGand move to wireless-2.6 JG * Start incorporating WPA through wpa_supplicant onto as many drivers JG * Start standardizing all things a bit, as bitched about and well pointed out JGby Dan Williams [EMAIL PROTECTED] JG * Listen to Jouni, he's the man JG JG Well, all this sounds good to me. See also the 'status' post I just JG made, and the 'note on wireless development process' I am about to write. JG JG I'm really hoping someone will look into integrating wireless 802.11 as JG a real protocol, rather than faking ethernet. This work starts with JG the p80211 template DaveM provided, and hopefully continues with JG Vladimir's updates of DaveM's code (did he post those anywhere?). There JG are also issues such as ARP types that Dan Williams mentioned to me as JG issues. JG JG The integrate wireless into net stack work requires a very JG self-motivated person who is willing to poke into the net stack, and JG answer their own questions. JG JG Jeff JG JG JG JG JG pgpa7A3QbhC2k.pgp Description: PGP signature
