2019-11-06 03:09:01

by Dmitry Safonov

[permalink] [raw]
Subject: [PATCH 18/50] microblaze: Add loglvl to microblaze_unwind_inner()

Currently, the log-level of show_stack() depends on a platform
realization. It creates situations where the headers are printed with
lower log level or higher than the stacktrace (depending on
a platform or user).

Furthermore, it forces the logic decision from user to an architecture
side. In result, some users as sysrq/kdb/etc are doing tricks with
temporary rising console_loglevel while printing their messages.
And in result it not only may print unwanted messages from other CPUs,
but also omit printing at all in the unlucky case where the printk()
was deferred.

Introducing log-level parameter and KERN_UNSUPPRESSED [1] seems
an easier approach than introducing more printk buffers.
Also, it will consolidate printings with headers.

Add log level argument to microblaze_unwind_inner() as a preparation for
introducing show_stack_loglvl().

Cc: Michal Simek <[email protected]>
[1]: https://lore.kernel.org/lkml/[email protected]/T/#u
Signed-off-by: Dmitry Safonov <[email protected]>
---
arch/microblaze/kernel/unwind.c | 35 ++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/arch/microblaze/kernel/unwind.c b/arch/microblaze/kernel/unwind.c
index 4241cdd28ee7..9a7343feadf5 100644
--- a/arch/microblaze/kernel/unwind.c
+++ b/arch/microblaze/kernel/unwind.c
@@ -162,16 +162,18 @@ static void microblaze_unwind_inner(struct task_struct *task,
*/
#ifdef CONFIG_MMU
static inline void unwind_trap(struct task_struct *task, unsigned long pc,
- unsigned long fp, struct stack_trace *trace)
+ unsigned long fp, struct stack_trace *trace,
+ const char *loglvl)
{
/* To be implemented */
}
#else
static inline void unwind_trap(struct task_struct *task, unsigned long pc,
- unsigned long fp, struct stack_trace *trace)
+ unsigned long fp, struct stack_trace *trace,
+ const char *loglvl)
{
const struct pt_regs *regs = (const struct pt_regs *) fp;
- microblaze_unwind_inner(task, regs->pc, regs->r1, regs->r15, trace);
+ microblaze_unwind_inner(task, regs->pc, regs->r1, regs->r15, trace, loglvl);
}
#endif

@@ -184,11 +186,13 @@ static inline void unwind_trap(struct task_struct *task, unsigned long pc,
* the caller's return address.
* @trace : Where to store stack backtrace (PC values).
* NULL == print backtrace to kernel log
+ * @loglvl : Used for printk log level if (trace == NULL).
*/
static void microblaze_unwind_inner(struct task_struct *task,
unsigned long pc, unsigned long fp,
unsigned long leaf_return,
- struct stack_trace *trace)
+ struct stack_trace *trace,
+ const char *loglvl)
{
int ofs = 0;

@@ -214,11 +218,11 @@ static void microblaze_unwind_inner(struct task_struct *task,
const struct pt_regs *regs =
(const struct pt_regs *) fp;
#endif
- pr_info("HW EXCEPTION\n");
+ printk("%sHW EXCEPTION\n", loglvl);
#ifndef CONFIG_MMU
microblaze_unwind_inner(task, regs->r17 - 4,
fp + EX_HANDLER_STACK_SIZ,
- regs->r15, trace);
+ regs->r15, trace, loglvl);
#endif
return;
}
@@ -228,8 +232,8 @@ static void microblaze_unwind_inner(struct task_struct *task,
if ((return_to >= handler->start_addr)
&& (return_to <= handler->end_addr)) {
if (!trace)
- pr_info("%s\n", handler->trap_name);
- unwind_trap(task, pc, fp, trace);
+ printk("%s%s\n", loglvl, handler->trap_name);
+ unwind_trap(task, pc, fp, trace, loglvl);
return;
}
}
@@ -248,13 +252,13 @@ static void microblaze_unwind_inner(struct task_struct *task,
} else {
/* Have we reached userland? */
if (unlikely(pc == task_pt_regs(task)->pc)) {
- pr_info("[<%p>] PID %lu [%s]\n",
- (void *) pc,
+ printk("%s[<%p>] PID %lu [%s]\n",
+ loglvl, (void *) pc,
(unsigned long) task->pid,
task->comm);
break;
} else
- print_ip_sym(KERN_INFO, pc);
+ print_ip_sym(loglvl, pc);
}

/* Stop when we reach anything not part of the kernel */
@@ -285,11 +289,13 @@ static void microblaze_unwind_inner(struct task_struct *task,
*/
void microblaze_unwind(struct task_struct *task, struct stack_trace *trace)
{
+ const char *loglvl = KERN_INFO;
+
if (task) {
if (task == current) {
const struct pt_regs *regs = task_pt_regs(task);
microblaze_unwind_inner(task, regs->pc, regs->r1,
- regs->r15, trace);
+ regs->r15, trace, loglvl);
} else {
struct thread_info *thread_info =
(struct thread_info *)(task->stack);
@@ -299,7 +305,8 @@ void microblaze_unwind(struct task_struct *task, struct stack_trace *trace)
microblaze_unwind_inner(task,
(unsigned long) &_switch_to,
cpu_context->r1,
- cpu_context->r15, trace);
+ cpu_context->r15,
+ trace, loglvl);
}
} else {
unsigned long pc, fp;
@@ -314,7 +321,7 @@ void microblaze_unwind(struct task_struct *task, struct stack_trace *trace)
);

/* Since we are not a leaf function, use leaf_return = 0 */
- microblaze_unwind_inner(current, pc, fp, 0, trace);
+ microblaze_unwind_inner(current, pc, fp, 0, trace, loglvl);
}
}

--
2.23.0


2019-11-07 09:01:19

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH 18/50] microblaze: Add loglvl to microblaze_unwind_inner()

On 06. 11. 19 4:05, Dmitry Safonov wrote:
> Currently, the log-level of show_stack() depends on a platform
> realization. It creates situations where the headers are printed with
> lower log level or higher than the stacktrace (depending on
> a platform or user).
>
> Furthermore, it forces the logic decision from user to an architecture
> side. In result, some users as sysrq/kdb/etc are doing tricks with
> temporary rising console_loglevel while printing their messages.
> And in result it not only may print unwanted messages from other CPUs,
> but also omit printing at all in the unlucky case where the printk()
> was deferred.
>
> Introducing log-level parameter and KERN_UNSUPPRESSED [1] seems
> an easier approach than introducing more printk buffers.
> Also, it will consolidate printings with headers.
>
> Add log level argument to microblaze_unwind_inner() as a preparation for
> introducing show_stack_loglvl().
>
> Cc: Michal Simek <[email protected]>
> [1]: https://lore.kernel.org/lkml/[email protected]/T/#u
> Signed-off-by: Dmitry Safonov <[email protected]>
> ---
> arch/microblaze/kernel/unwind.c | 35 ++++++++++++++++++++-------------
> 1 file changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/arch/microblaze/kernel/unwind.c b/arch/microblaze/kernel/unwind.c
> index 4241cdd28ee7..9a7343feadf5 100644
> --- a/arch/microblaze/kernel/unwind.c
> +++ b/arch/microblaze/kernel/unwind.c
> @@ -162,16 +162,18 @@ static void microblaze_unwind_inner(struct task_struct *task,
> */
> #ifdef CONFIG_MMU
> static inline void unwind_trap(struct task_struct *task, unsigned long pc,
> - unsigned long fp, struct stack_trace *trace)
> + unsigned long fp, struct stack_trace *trace,
> + const char *loglvl)
> {
> /* To be implemented */
> }
> #else
> static inline void unwind_trap(struct task_struct *task, unsigned long pc,
> - unsigned long fp, struct stack_trace *trace)
> + unsigned long fp, struct stack_trace *trace,
> + const char *loglvl)
> {
> const struct pt_regs *regs = (const struct pt_regs *) fp;
> - microblaze_unwind_inner(task, regs->pc, regs->r1, regs->r15, trace);
> + microblaze_unwind_inner(task, regs->pc, regs->r1, regs->r15, trace, loglvl);
> }
> #endif
>
> @@ -184,11 +186,13 @@ static inline void unwind_trap(struct task_struct *task, unsigned long pc,
> * the caller's return address.
> * @trace : Where to store stack backtrace (PC values).
> * NULL == print backtrace to kernel log
> + * @loglvl : Used for printk log level if (trace == NULL).
> */
> static void microblaze_unwind_inner(struct task_struct *task,
> unsigned long pc, unsigned long fp,
> unsigned long leaf_return,
> - struct stack_trace *trace)
> + struct stack_trace *trace,
> + const char *loglvl)
> {
> int ofs = 0;
>
> @@ -214,11 +218,11 @@ static void microblaze_unwind_inner(struct task_struct *task,
> const struct pt_regs *regs =
> (const struct pt_regs *) fp;
> #endif
> - pr_info("HW EXCEPTION\n");
> + printk("%sHW EXCEPTION\n", loglvl);
> #ifndef CONFIG_MMU
> microblaze_unwind_inner(task, regs->r17 - 4,
> fp + EX_HANDLER_STACK_SIZ,
> - regs->r15, trace);
> + regs->r15, trace, loglvl);
> #endif
> return;
> }
> @@ -228,8 +232,8 @@ static void microblaze_unwind_inner(struct task_struct *task,
> if ((return_to >= handler->start_addr)
> && (return_to <= handler->end_addr)) {
> if (!trace)
> - pr_info("%s\n", handler->trap_name);
> - unwind_trap(task, pc, fp, trace);
> + printk("%s%s\n", loglvl, handler->trap_name);
> + unwind_trap(task, pc, fp, trace, loglvl);
> return;
> }
> }
> @@ -248,13 +252,13 @@ static void microblaze_unwind_inner(struct task_struct *task,
> } else {
> /* Have we reached userland? */
> if (unlikely(pc == task_pt_regs(task)->pc)) {
> - pr_info("[<%p>] PID %lu [%s]\n",
> - (void *) pc,
> + printk("%s[<%p>] PID %lu [%s]\n",
> + loglvl, (void *) pc,
> (unsigned long) task->pid,
> task->comm);
> break;
> } else
> - print_ip_sym(KERN_INFO, pc);
> + print_ip_sym(loglvl, pc);
> }
>
> /* Stop when we reach anything not part of the kernel */
> @@ -285,11 +289,13 @@ static void microblaze_unwind_inner(struct task_struct *task,
> */
> void microblaze_unwind(struct task_struct *task, struct stack_trace *trace)
> {
> + const char *loglvl = KERN_INFO;
> +
> if (task) {
> if (task == current) {
> const struct pt_regs *regs = task_pt_regs(task);
> microblaze_unwind_inner(task, regs->pc, regs->r1,
> - regs->r15, trace);
> + regs->r15, trace, loglvl);
> } else {
> struct thread_info *thread_info =
> (struct thread_info *)(task->stack);
> @@ -299,7 +305,8 @@ void microblaze_unwind(struct task_struct *task, struct stack_trace *trace)
> microblaze_unwind_inner(task,
> (unsigned long) &_switch_to,
> cpu_context->r1,
> - cpu_context->r15, trace);
> + cpu_context->r15,
> + trace, loglvl);
> }
> } else {
> unsigned long pc, fp;
> @@ -314,7 +321,7 @@ void microblaze_unwind(struct task_struct *task, struct stack_trace *trace)
> );
>
> /* Since we are not a leaf function, use leaf_return = 0 */
> - microblaze_unwind_inner(current, pc, fp, 0, trace);
> + microblaze_unwind_inner(current, pc, fp, 0, trace, loglvl);
> }
> }
>
>

I can't see any reaction from bots but you forget to update function
declaration there which is causing build issue.

here is c&p patch.
Other then this it looks good to me.

diff --git a/arch/microblaze/kernel/unwind.c
b/arch/microblaze/kernel/unwind.c
index 74dded96c10a..778a761af0a7 100644
--- a/arch/microblaze/kernel/unwind.c
+++ b/arch/microblaze/kernel/unwind.c
@@ -154,7 +154,8 @@ static int lookup_prev_stack_frame(unsigned long fp,
unsigned long pc,
static void microblaze_unwind_inner(struct task_struct *task,
unsigned long pc, unsigned long fp,
unsigned long leaf_return,
- struct stack_trace *trace);
+ struct stack_trace *trace,
+ const char *loglvl);

/**
* unwind_trap - Unwind through a system trap, that stored previous state

Thanks,
Michal




--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs



Attachments:
signature.asc (201.00 B)
OpenPGP digital signature

2019-11-08 07:54:11

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 18/50] microblaze: Add loglvl to microblaze_unwind_inner()

Hi Dmitry,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.4-rc6 next-20191107]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Dmitry-Safonov/kallsyms-printk-Add-loglvl-to-print_ip_sym/20191108-124037
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 847120f859cc45e074204f4cf33c8df069306eb2
config: microblaze-nommu_defconfig (attached as .config)
compiler: microblaze-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=microblaze

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

arch/microblaze/kernel/unwind.c: In function 'unwind_trap':
>> arch/microblaze/kernel/unwind.c:176:2: error: too many arguments to function 'microblaze_unwind_inner'
microblaze_unwind_inner(task, regs->pc, regs->r1, regs->r15, trace, loglvl);
^~~~~~~~~~~~~~~~~~~~~~~
arch/microblaze/kernel/unwind.c:154:13: note: declared here
static void microblaze_unwind_inner(struct task_struct *task,
^~~~~~~~~~~~~~~~~~~~~~~
arch/microblaze/kernel/unwind.c: At top level:
>> arch/microblaze/kernel/unwind.c:191:13: error: conflicting types for 'microblaze_unwind_inner'
static void microblaze_unwind_inner(struct task_struct *task,
^~~~~~~~~~~~~~~~~~~~~~~
arch/microblaze/kernel/unwind.c:154:13: note: previous declaration of 'microblaze_unwind_inner' was here
static void microblaze_unwind_inner(struct task_struct *task,
^~~~~~~~~~~~~~~~~~~~~~~
>> arch/microblaze/kernel/unwind.c:154:13: warning: 'microblaze_unwind_inner' used but never defined

vim +/microblaze_unwind_inner +176 arch/microblaze/kernel/unwind.c

153
> 154 static void microblaze_unwind_inner(struct task_struct *task,
155 unsigned long pc, unsigned long fp,
156 unsigned long leaf_return,
157 struct stack_trace *trace);
158
159 /**
160 * unwind_trap - Unwind through a system trap, that stored previous state
161 * on the stack.
162 */
163 #ifdef CONFIG_MMU
164 static inline void unwind_trap(struct task_struct *task, unsigned long pc,
165 unsigned long fp, struct stack_trace *trace,
166 const char *loglvl)
167 {
168 /* To be implemented */
169 }
170 #else
171 static inline void unwind_trap(struct task_struct *task, unsigned long pc,
172 unsigned long fp, struct stack_trace *trace,
173 const char *loglvl)
174 {
175 const struct pt_regs *regs = (const struct pt_regs *) fp;
> 176 microblaze_unwind_inner(task, regs->pc, regs->r1, regs->r15, trace, loglvl);
177 }
178 #endif
179
180 /**
181 * microblaze_unwind_inner - Unwind the stack from the specified point
182 * @task : Task whose stack we are to unwind (may be NULL)
183 * @pc : Program counter from which we start unwinding
184 * @fp : Frame (stack) pointer from which we start unwinding
185 * @leaf_return : Value of r15 at pc. If the function is a leaf, this is
186 * the caller's return address.
187 * @trace : Where to store stack backtrace (PC values).
188 * NULL == print backtrace to kernel log
189 * @loglvl : Used for printk log level if (trace == NULL).
190 */
> 191 static void microblaze_unwind_inner(struct task_struct *task,
192 unsigned long pc, unsigned long fp,
193 unsigned long leaf_return,
194 struct stack_trace *trace,
195 const char *loglvl)
196 {
197 int ofs = 0;
198
199 pr_debug(" Unwinding with PC=%p, FP=%p\n", (void *)pc, (void *)fp);
200 if (!pc || !fp || (pc & 3) || (fp & 3)) {
201 pr_debug(" Invalid state for unwind, aborting\n");
202 return;
203 }
204 for (; pc != 0;) {
205 unsigned long next_fp, next_pc = 0;
206 unsigned long return_to = pc + 2 * sizeof(unsigned long);
207 const struct trap_handler_info *handler =
208 &microblaze_trap_handlers;
209
210 /* Is previous function the HW exception handler? */
211 if ((return_to >= (unsigned long)&_hw_exception_handler)
212 &&(return_to < (unsigned long)&ex_handler_unhandled)) {
213 /*
214 * HW exception handler doesn't save all registers,
215 * so we open-code a special case of unwind_trap()
216 */
217 #ifndef CONFIG_MMU
218 const struct pt_regs *regs =
219 (const struct pt_regs *) fp;
220 #endif
221 printk("%sHW EXCEPTION\n", loglvl);
222 #ifndef CONFIG_MMU
223 microblaze_unwind_inner(task, regs->r17 - 4,
224 fp + EX_HANDLER_STACK_SIZ,
225 regs->r15, trace, loglvl);
226 #endif
227 return;
228 }
229
230 /* Is previous function a trap handler? */
231 for (; handler->start_addr; ++handler) {
232 if ((return_to >= handler->start_addr)
233 && (return_to <= handler->end_addr)) {
234 if (!trace)
235 printk("%s%s\n", loglvl, handler->trap_name);
236 unwind_trap(task, pc, fp, trace, loglvl);
237 return;
238 }
239 }
240 pc -= ofs;
241
242 if (trace) {
243 #ifdef CONFIG_STACKTRACE
244 if (trace->skip > 0)
245 trace->skip--;
246 else
247 trace->entries[trace->nr_entries++] = pc;
248
249 if (trace->nr_entries >= trace->max_entries)
250 break;
251 #endif
252 } else {
253 /* Have we reached userland? */
254 if (unlikely(pc == task_pt_regs(task)->pc)) {
255 printk("%s[<%p>] PID %lu [%s]\n",
256 loglvl, (void *) pc,
257 (unsigned long) task->pid,
258 task->comm);
259 break;
260 } else
261 print_ip_sym(loglvl, pc);
262 }
263
264 /* Stop when we reach anything not part of the kernel */
265 if (!kernel_text_address(pc))
266 break;
267
268 if (lookup_prev_stack_frame(fp, pc, leaf_return, &next_fp,
269 &next_pc) == 0) {
270 ofs = sizeof(unsigned long);
271 pc = next_pc & ~3;
272 fp = next_fp;
273 leaf_return = 0;
274 } else {
275 pr_debug(" Failed to find previous stack frame\n");
276 break;
277 }
278
279 pr_debug(" Next PC=%p, next FP=%p\n",
280 (void *)next_pc, (void *)next_fp);
281 }
282 }
283

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation


Attachments:
(No filename) (7.05 kB)
.config.gz (13.29 kB)
Download all attachments