linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] vt/console: try harder to print output when panicing
@ 2010-06-23  3:12 Dave Airlie
  2010-06-23 15:43 ` Jesse Barnes
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Dave Airlie @ 2010-06-23  3:12 UTC (permalink / raw)
  To: linux-fbdev; +Cc: dri-devel, linux-kernel, Jesse Barnes

From: Jesse Barnes <jbarnes@virtuousgeek.org>

Jesse's initial patch commit said:

"At panic time (i.e. when oops_in_progress is set) we should try a bit
harder to update the screen and make sure output gets to the VT, since
some drivers are capable of flipping back to it.

So make sure we try to unblank and update the display if called from a
panic context."

I've enhanced this to add a flag to the vc that console layer can set
to indicate they want this behaviour to occur. This also adds support
to fbcon for that flag and adds an fb flag for drivers to indicate
they want to use the support. It enables this for KMS drivers.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/char/vt.c                       |   13 +++++++++----
 drivers/gpu/drm/i915/intel_fb.c         |    4 +---
 drivers/gpu/drm/nouveau/nouveau_fbcon.c |    1 +
 drivers/gpu/drm/radeon/radeon_fb.c      |    2 +-
 drivers/video/console/fbcon.c           |    4 +++-
 include/linux/console_struct.h          |    1 +
 include/linux/fb.h                      |    4 ++++
 include/linux/vt_kern.h                 |    7 +++++++
 8 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/char/vt.c b/drivers/char/vt.c
index 7cdb6ee..6e04c9e 100644
--- a/drivers/char/vt.c
+++ b/drivers/char/vt.c
@@ -698,7 +698,10 @@ void redraw_screen(struct vc_data *vc, int is_switch)
 			update_attr(vc);
 			clear_buffer_attributes(vc);
 		}
-		if (update && vc->vc_mode != KD_GRAPHICS)
+
+		/* Forcibly update if we're panicing */
+		if ((update && vc->vc_mode != KD_GRAPHICS) ||
+		    vt_force_oops_output(vc))
 			do_update_region(vc, vc->vc_origin, vc->vc_screenbuf_size / 2);
 	}
 	set_cursor(vc);
@@ -736,6 +739,7 @@ static void visual_init(struct vc_data *vc, int num, int init)
 	vc->vc_hi_font_mask = 0;
 	vc->vc_complement_mask = 0;
 	vc->vc_can_do_color = 0;
+	vc->vc_panic_force_write = false;
 	vc->vc_sw->con_init(vc, init);
 	if (!vc->vc_complement_mask)
 		vc->vc_complement_mask = vc->vc_can_do_color ? 0x7700 : 0x0800;
@@ -2498,7 +2502,7 @@ static void vt_console_print(struct console *co, const char *b, unsigned count)
 		goto quit;
 	}
 
-	if (vc->vc_mode != KD_TEXT)
+	if (vc->vc_mode != KD_TEXT && !vt_force_oops_output(vc))
 		goto quit;
 
 	/* undraw cursor first */
@@ -3703,7 +3707,8 @@ void do_unblank_screen(int leaving_gfx)
 		return;
 	}
 	vc = vc_cons[fg_console].d;
-	if (vc->vc_mode != KD_TEXT)
+	/* Try to unblank in oops case too */
+	if (vc->vc_mode != KD_TEXT && !vt_force_oops_output(vc))
 		return; /* but leave console_blanked != 0 */
 
 	if (blankinterval) {
@@ -3712,7 +3717,7 @@ void do_unblank_screen(int leaving_gfx)
 	}
 
 	console_blanked = 0;
-	if (vc->vc_sw->con_blank(vc, 0, leaving_gfx))
+	if (vc->vc_sw->con_blank(vc, 0, leaving_gfx) || vt_force_oops_output(vc))
 		/* Low-level driver cannot restore -> do it ourselves */
 		update_screen(vc);
 	if (console_blank_hook)
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index c3c5052..bd5d87a 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -128,7 +128,7 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
 
 	strcpy(info->fix.id, "inteldrmfb");
 
-	info->flags = FBINFO_DEFAULT;
+	info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
 	info->fbops = &intelfb_ops;
 
 	/* setup aperture base/size for vesafb takeover */
@@ -146,8 +146,6 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
 	info->fix.smem_start = dev->mode_config.fb_base + obj_priv->gtt_offset;
 	info->fix.smem_len = size;
 
-	info->flags = FBINFO_DEFAULT;
-
 	info->screen_base = ioremap_wc(dev->agp->base + obj_priv->gtt_offset,
 				       size);
 	if (!info->screen_base) {
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index c9a4a0d..9b2d3b7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -250,6 +250,7 @@ nouveau_fbcon_create(struct nouveau_fbdev *nfbdev,
 		info->flags = FBINFO_DEFAULT | FBINFO_HWACCEL_COPYAREA |
 			      FBINFO_HWACCEL_FILLRECT |
 			      FBINFO_HWACCEL_IMAGEBLIT;
+	info->flags |= FBINFO_CAN_FORCE_OUTPUT;
 	info->fbops = &nouveau_fbcon_ops;
 	info->fix.smem_start = dev->mode_config.fb_base + nvbo->bo.offset -
 			       dev_priv->vm_vram_base;
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index dc1634b..dbf8696 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -224,7 +224,7 @@ static int radeonfb_create(struct radeon_fbdev *rfbdev,
 
 	drm_fb_helper_fill_fix(info, fb->pitch, fb->depth);
 
-	info->flags = FBINFO_DEFAULT;
+	info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
 	info->fbops = &radeonfb_ops;
 
 	tmp = radeon_bo_gpu_offset(rbo) - rdev->mc.vram_start;
diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index b0a3fa0..7eadb35 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -283,7 +283,8 @@ static inline int fbcon_is_inactive(struct vc_data *vc, struct fb_info *info)
 	struct fbcon_ops *ops = info->fbcon_par;
 
 	return (info->state != FBINFO_STATE_RUNNING ||
-		vc->vc_mode != KD_TEXT || ops->graphics);
+		vc->vc_mode != KD_TEXT || ops->graphics) &&
+		!vt_force_oops_output(vc);
 }
 
 static inline int get_color(struct vc_data *vc, struct fb_info *info,
@@ -1073,6 +1074,7 @@ static void fbcon_init(struct vc_data *vc, int init)
 	if (p->userfont)
 		charcnt = FNTCHARCNT(p->fontdata);
 
+	vc->vc_panic_force_write = !!(info->flags & FBINFO_CAN_FORCE_OUTPUT);
 	vc->vc_can_do_color = (fb_get_color_depth(&info->var, &info->fix)!=1);
 	vc->vc_complement_mask = vc->vc_can_do_color ? 0x7700 : 0x0800;
 	if (charcnt = 256) {
diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h
index 38fe59d..d7d9acd 100644
--- a/include/linux/console_struct.h
+++ b/include/linux/console_struct.h
@@ -105,6 +105,7 @@ struct vc_data {
 	struct vc_data **vc_display_fg;		/* [!] Ptr to var holding fg console for this display */
 	unsigned long	vc_uni_pagedir;
 	unsigned long	*vc_uni_pagedir_loc;  /* [!] Location of uni_pagedir variable for this console */
+	bool vc_panic_force_write; /* when oops/panic this VC can accept forced output/blanking */
 	/* additional information is in vt_kern.h */
 };
 
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 8e5a9df..25f4950 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -812,6 +812,10 @@ struct fb_tile_ops {
  */
 #define FBINFO_BE_MATH  0x100000
 
+/* report to the VT layer that this fb driver can accept forced console
+   output like oopses */
+#define FBINFO_CAN_FORCE_OUTPUT     0x200000
+
 struct fb_info {
 	int node;
 	int flags;
diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h
index 7f56db4..56cce34 100644
--- a/include/linux/vt_kern.h
+++ b/include/linux/vt_kern.h
@@ -100,6 +100,13 @@ extern int unbind_con_driver(const struct consw *csw, int first, int last,
 			     int deflt);
 int vty_init(const struct file_operations *console_fops);
 
+static inline bool vt_force_oops_output(struct vc_data *vc)
+{
+	if (oops_in_progress && vc->vc_panic_force_write)
+		return true;
+	return false;
+}
+
 /*
  * vc_screen.c shares this temporary buffer with the console write code so that
  * we can easily avoid touching user space while holding the console spinlock.
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] vt/console: try harder to print output when panicing
  2010-06-23  3:12 [PATCH] vt/console: try harder to print output when panicing Dave Airlie
@ 2010-06-23 15:43 ` Jesse Barnes
  2010-06-23 15:57 ` James Simmons
  2010-06-23 19:56 ` Andrew Morton
  2 siblings, 0 replies; 7+ messages in thread
From: Jesse Barnes @ 2010-06-23 15:43 UTC (permalink / raw)
  To: Dave Airlie; +Cc: linux-fbdev, dri-devel, linux-kernel

On Wed, 23 Jun 2010 13:12:59 +1000
Dave Airlie <airlied@gmail.com> wrote:

> From: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> Jesse's initial patch commit said:
> 
> "At panic time (i.e. when oops_in_progress is set) we should try a bit
> harder to update the screen and make sure output gets to the VT, since
> some drivers are capable of flipping back to it.
> 
> So make sure we try to unblank and update the display if called from a
> panic context."
> 
> I've enhanced this to add a flag to the vc that console layer can set
> to indicate they want this behaviour to occur. This also adds support
> to fbcon for that flag and adds an fb flag for drivers to indicate
> they want to use the support. It enables this for KMS drivers.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---

Yeah this looks a little nicer, thanks.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] vt/console: try harder to print output when panicing
  2010-06-23  3:12 [PATCH] vt/console: try harder to print output when panicing Dave Airlie
  2010-06-23 15:43 ` Jesse Barnes
@ 2010-06-23 15:57 ` James Simmons
  2010-06-23 19:56 ` Andrew Morton
  2 siblings, 0 replies; 7+ messages in thread
From: James Simmons @ 2010-06-23 15:57 UTC (permalink / raw)
  To: Dave Airlie; +Cc: linux-fbdev, dri-devel, linux-kernel, Jesse Barnes


> From: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> Jesse's initial patch commit said:
> 
> "At panic time (i.e. when oops_in_progress is set) we should try a bit
> harder to update the screen and make sure output gets to the VT, since
> some drivers are capable of flipping back to it.
> 
> So make sure we try to unblank and update the display if called from a
> panic context."
> 
> I've enhanced this to add a flag to the vc that console layer can set
> to indicate they want this behaviour to occur. This also adds support
> to fbcon for that flag and adds an fb flag for drivers to indicate
> they want to use the support. It enables this for KMS drivers.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>

I really like the fact that the console driver has a flag to turn this on 
or off.

Signed-off-by: James Simmons <jsimmons@infradead.org>

> ---
>  drivers/char/vt.c                       |   13 +++++++++----
>  drivers/gpu/drm/i915/intel_fb.c         |    4 +---
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c |    1 +
>  drivers/gpu/drm/radeon/radeon_fb.c      |    2 +-
>  drivers/video/console/fbcon.c           |    4 +++-
>  include/linux/console_struct.h          |    1 +
>  include/linux/fb.h                      |    4 ++++
>  include/linux/vt_kern.h                 |    7 +++++++
>  8 files changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/char/vt.c b/drivers/char/vt.c
> index 7cdb6ee..6e04c9e 100644
> --- a/drivers/char/vt.c
> +++ b/drivers/char/vt.c
> @@ -698,7 +698,10 @@ void redraw_screen(struct vc_data *vc, int is_switch)
>  			update_attr(vc);
>  			clear_buffer_attributes(vc);
>  		}
> -		if (update && vc->vc_mode != KD_GRAPHICS)
> +
> +		/* Forcibly update if we're panicing */
> +		if ((update && vc->vc_mode != KD_GRAPHICS) ||
> +		    vt_force_oops_output(vc))
>  			do_update_region(vc, vc->vc_origin, vc->vc_screenbuf_size / 2);
>  	}
>  	set_cursor(vc);
> @@ -736,6 +739,7 @@ static void visual_init(struct vc_data *vc, int num, int init)
>  	vc->vc_hi_font_mask = 0;
>  	vc->vc_complement_mask = 0;
>  	vc->vc_can_do_color = 0;
> +	vc->vc_panic_force_write = false;
>  	vc->vc_sw->con_init(vc, init);
>  	if (!vc->vc_complement_mask)
>  		vc->vc_complement_mask = vc->vc_can_do_color ? 0x7700 : 0x0800;
> @@ -2498,7 +2502,7 @@ static void vt_console_print(struct console *co, const char *b, unsigned count)
>  		goto quit;
>  	}
>  
> -	if (vc->vc_mode != KD_TEXT)
> +	if (vc->vc_mode != KD_TEXT && !vt_force_oops_output(vc))
>  		goto quit;
>  
>  	/* undraw cursor first */
> @@ -3703,7 +3707,8 @@ void do_unblank_screen(int leaving_gfx)
>  		return;
>  	}
>  	vc = vc_cons[fg_console].d;
> -	if (vc->vc_mode != KD_TEXT)
> +	/* Try to unblank in oops case too */
> +	if (vc->vc_mode != KD_TEXT && !vt_force_oops_output(vc))
>  		return; /* but leave console_blanked != 0 */
>  
>  	if (blankinterval) {
> @@ -3712,7 +3717,7 @@ void do_unblank_screen(int leaving_gfx)
>  	}
>  
>  	console_blanked = 0;
> -	if (vc->vc_sw->con_blank(vc, 0, leaving_gfx))
> +	if (vc->vc_sw->con_blank(vc, 0, leaving_gfx) || vt_force_oops_output(vc))
>  		/* Low-level driver cannot restore -> do it ourselves */
>  		update_screen(vc);
>  	if (console_blank_hook)
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index c3c5052..bd5d87a 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -128,7 +128,7 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
>  
>  	strcpy(info->fix.id, "inteldrmfb");
>  
> -	info->flags = FBINFO_DEFAULT;
> +	info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
>  	info->fbops = &intelfb_ops;
>  
>  	/* setup aperture base/size for vesafb takeover */
> @@ -146,8 +146,6 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
>  	info->fix.smem_start = dev->mode_config.fb_base + obj_priv->gtt_offset;
>  	info->fix.smem_len = size;
>  
> -	info->flags = FBINFO_DEFAULT;
> -
>  	info->screen_base = ioremap_wc(dev->agp->base + obj_priv->gtt_offset,
>  				       size);
>  	if (!info->screen_base) {
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> index c9a4a0d..9b2d3b7 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> @@ -250,6 +250,7 @@ nouveau_fbcon_create(struct nouveau_fbdev *nfbdev,
>  		info->flags = FBINFO_DEFAULT | FBINFO_HWACCEL_COPYAREA |
>  			      FBINFO_HWACCEL_FILLRECT |
>  			      FBINFO_HWACCEL_IMAGEBLIT;
> +	info->flags |= FBINFO_CAN_FORCE_OUTPUT;
>  	info->fbops = &nouveau_fbcon_ops;
>  	info->fix.smem_start = dev->mode_config.fb_base + nvbo->bo.offset -
>  			       dev_priv->vm_vram_base;
> diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
> index dc1634b..dbf8696 100644
> --- a/drivers/gpu/drm/radeon/radeon_fb.c
> +++ b/drivers/gpu/drm/radeon/radeon_fb.c
> @@ -224,7 +224,7 @@ static int radeonfb_create(struct radeon_fbdev *rfbdev,
>  
>  	drm_fb_helper_fill_fix(info, fb->pitch, fb->depth);
>  
> -	info->flags = FBINFO_DEFAULT;
> +	info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
>  	info->fbops = &radeonfb_ops;
>  
>  	tmp = radeon_bo_gpu_offset(rbo) - rdev->mc.vram_start;
> diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> index b0a3fa0..7eadb35 100644
> --- a/drivers/video/console/fbcon.c
> +++ b/drivers/video/console/fbcon.c
> @@ -283,7 +283,8 @@ static inline int fbcon_is_inactive(struct vc_data *vc, struct fb_info *info)
>  	struct fbcon_ops *ops = info->fbcon_par;
>  
>  	return (info->state != FBINFO_STATE_RUNNING ||
> -		vc->vc_mode != KD_TEXT || ops->graphics);
> +		vc->vc_mode != KD_TEXT || ops->graphics) &&
> +		!vt_force_oops_output(vc);
>  }
>  
>  static inline int get_color(struct vc_data *vc, struct fb_info *info,
> @@ -1073,6 +1074,7 @@ static void fbcon_init(struct vc_data *vc, int init)
>  	if (p->userfont)
>  		charcnt = FNTCHARCNT(p->fontdata);
>  
> +	vc->vc_panic_force_write = !!(info->flags & FBINFO_CAN_FORCE_OUTPUT);
>  	vc->vc_can_do_color = (fb_get_color_depth(&info->var, &info->fix)!=1);
>  	vc->vc_complement_mask = vc->vc_can_do_color ? 0x7700 : 0x0800;
>  	if (charcnt = 256) {
> diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h
> index 38fe59d..d7d9acd 100644
> --- a/include/linux/console_struct.h
> +++ b/include/linux/console_struct.h
> @@ -105,6 +105,7 @@ struct vc_data {
>  	struct vc_data **vc_display_fg;		/* [!] Ptr to var holding fg console for this display */
>  	unsigned long	vc_uni_pagedir;
>  	unsigned long	*vc_uni_pagedir_loc;  /* [!] Location of uni_pagedir variable for this console */
> +	bool vc_panic_force_write; /* when oops/panic this VC can accept forced output/blanking */
>  	/* additional information is in vt_kern.h */
>  };
>  
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 8e5a9df..25f4950 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -812,6 +812,10 @@ struct fb_tile_ops {
>   */
>  #define FBINFO_BE_MATH  0x100000
>  
> +/* report to the VT layer that this fb driver can accept forced console
> +   output like oopses */
> +#define FBINFO_CAN_FORCE_OUTPUT     0x200000
> +
>  struct fb_info {
>  	int node;
>  	int flags;
> diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h
> index 7f56db4..56cce34 100644
> --- a/include/linux/vt_kern.h
> +++ b/include/linux/vt_kern.h
> @@ -100,6 +100,13 @@ extern int unbind_con_driver(const struct consw *csw, int first, int last,
>  			     int deflt);
>  int vty_init(const struct file_operations *console_fops);
>  
> +static inline bool vt_force_oops_output(struct vc_data *vc)
> +{
> +	if (oops_in_progress && vc->vc_panic_force_write)
> +		return true;
> +	return false;
> +}
> +
>  /*
>   * vc_screen.c shares this temporary buffer with the console write code so that
>   * we can easily avoid touching user space while holding the console spinlock.
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] vt/console: try harder to print output when panicing
  2010-06-23  3:12 [PATCH] vt/console: try harder to print output when panicing Dave Airlie
  2010-06-23 15:43 ` Jesse Barnes
  2010-06-23 15:57 ` James Simmons
@ 2010-06-23 19:56 ` Andrew Morton
  2010-06-23 20:05   ` Jesse Barnes
  2 siblings, 1 reply; 7+ messages in thread
From: Andrew Morton @ 2010-06-23 19:56 UTC (permalink / raw)
  To: Dave Airlie; +Cc: linux-fbdev, dri-devel, linux-kernel, Jesse Barnes

On Wed, 23 Jun 2010 13:12:59 +1000
Dave Airlie <airlied@gmail.com> wrote:

> Jesse's initial patch commit said:
> 
> "At panic time (i.e. when oops_in_progress is set) we should try a bit
> harder to update the screen and make sure output gets to the VT, since
> some drivers are capable of flipping back to it.
> 
> So make sure we try to unblank and update the display if called from a
> panic context."
> 
> I've enhanced this to add a flag to the vc that console layer can set
> to indicate they want this behaviour to occur. This also adds support
> to fbcon for that flag and adds an fb flag for drivers to indicate
> they want to use the support. It enables this for KMS drivers.

Interesting.  Getting real oops traces from machines running X will
make Rusty happy, and that's what we're all here for.

How well does this all work?  How reliable is it?  What's the success rate?



What's the downside here?  After all, not all oopses are catastrophic -
sometimes the machine will go blurt and keep running so the user can
take a look in the logs then perform an orderly reboot, etc.  As I
understand it, those non-catastrophic oopses will now flip the machine
from X and into the vt display, yes?  Can the user get it back to X
mode?

Worse, there's also a risk that doing all this new work within the
oopsing context will screw the machine up, so the user will be
_deprived_ of an oops report which he otherwise would have been able to
get from the logs.  This is particularly the case when it's the DRI
stuff which oopsed, which is not exactly an uncommon occurrence lately ;)

Oh well, the best way to tell is to ship-it-and-see.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] vt/console: try harder to print output when panicing
  2010-06-23 19:56 ` Andrew Morton
@ 2010-06-23 20:05   ` Jesse Barnes
  2010-06-23 20:36     ` Andrew Morton
  0 siblings, 1 reply; 7+ messages in thread
From: Jesse Barnes @ 2010-06-23 20:05 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Dave Airlie, linux-fbdev, dri-devel, linux-kernel

On Wed, 23 Jun 2010 12:56:05 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Wed, 23 Jun 2010 13:12:59 +1000
> Dave Airlie <airlied@gmail.com> wrote:
> 
> > Jesse's initial patch commit said:
> > 
> > "At panic time (i.e. when oops_in_progress is set) we should try a bit
> > harder to update the screen and make sure output gets to the VT, since
> > some drivers are capable of flipping back to it.
> > 
> > So make sure we try to unblank and update the display if called from a
> > panic context."
> > 
> > I've enhanced this to add a flag to the vc that console layer can set
> > to indicate they want this behaviour to occur. This also adds support
> > to fbcon for that flag and adds an fb flag for drivers to indicate
> > they want to use the support. It enables this for KMS drivers.
> 
> Interesting.  Getting real oops traces from machines running X will
> make Rusty happy, and that's what we're all here for.
> 
> How well does this all work?  How reliable is it?  What's the success rate?
> 
> 
> 
> What's the downside here?  After all, not all oopses are catastrophic -
> sometimes the machine will go blurt and keep running so the user can
> take a look in the logs then perform an orderly reboot, etc.  As I
> understand it, those non-catastrophic oopses will now flip the machine
> from X and into the vt display, yes?  Can the user get it back to X
> mode?

No, we'll only flip from the panic notifier chain.  However if
oops_in_progress is set (as it is whenever bust_spinlocks(1) is called,
i.e. from panic() and oops_begin()) we'll try to print into the fbcon
buffer.  In the oops case this should still be safe since the buffer is
pinned, at least in the KMS world.

> Worse, there's also a risk that doing all this new work within the
> oopsing context will screw the machine up, so the user will be
> _deprived_ of an oops report which he otherwise would have been able to
> get from the logs.  This is particularly the case when it's the DRI
> stuff which oopsed, which is not exactly an uncommon occurrence lately ;)
> 
> Oh well, the best way to tell is to ship-it-and-see.

To avoid the oops part (which as I said should still be safe) we could
add a new panic_in_progress flag, that would make sure things were no
worse than they are currently.

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] vt/console: try harder to print output when panicing
  2010-06-23 20:05   ` Jesse Barnes
@ 2010-06-23 20:36     ` Andrew Morton
  2010-06-23 20:40       ` Jesse Barnes
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Morton @ 2010-06-23 20:36 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: Dave Airlie, linux-fbdev, dri-devel, linux-kernel

On Wed, 23 Jun 2010 13:05:47 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Wed, 23 Jun 2010 12:56:05 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Wed, 23 Jun 2010 13:12:59 +1000
> > Dave Airlie <airlied@gmail.com> wrote:
> > 
> > > Jesse's initial patch commit said:
> > > 
> > > "At panic time (i.e. when oops_in_progress is set) we should try a bit
> > > harder to update the screen and make sure output gets to the VT, since
> > > some drivers are capable of flipping back to it.
> > > 
> > > So make sure we try to unblank and update the display if called from a
> > > panic context."
> > > 
> > > I've enhanced this to add a flag to the vc that console layer can set
> > > to indicate they want this behaviour to occur. This also adds support
> > > to fbcon for that flag and adds an fb flag for drivers to indicate
> > > they want to use the support. It enables this for KMS drivers.
> > 
> > Interesting.  Getting real oops traces from machines running X will
> > make Rusty happy, and that's what we're all here for.
> > 
> > How well does this all work?  How reliable is it?  What's the success rate?
> > 
> > 
> > 
> > What's the downside here?  After all, not all oopses are catastrophic -
> > sometimes the machine will go blurt and keep running so the user can
> > take a look in the logs then perform an orderly reboot, etc.  As I
> > understand it, those non-catastrophic oopses will now flip the machine
> > from X and into the vt display, yes?  Can the user get it back to X
> > mode?
> 
> No, we'll only flip from the panic notifier chain.

So we still don't get to see the output from BUGs and random oopses?  I
don't think panics are all that common.

So am I correct in believing that if a user is getting invisible-oopses
then he can set /proc/sys/kernel/panic_on_oops, and then the oops
should be visible?

(Shouldn't all this stuff be explained in the changlog?)



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] vt/console: try harder to print output when panicing
  2010-06-23 20:36     ` Andrew Morton
@ 2010-06-23 20:40       ` Jesse Barnes
  0 siblings, 0 replies; 7+ messages in thread
From: Jesse Barnes @ 2010-06-23 20:40 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Dave Airlie, linux-fbdev, dri-devel, linux-kernel

On Wed, 23 Jun 2010 13:36:37 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Wed, 23 Jun 2010 13:05:47 -0700
> Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > On Wed, 23 Jun 2010 12:56:05 -0700
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > > On Wed, 23 Jun 2010 13:12:59 +1000
> > > Dave Airlie <airlied@gmail.com> wrote:
> > > 
> > > > Jesse's initial patch commit said:
> > > > 
> > > > "At panic time (i.e. when oops_in_progress is set) we should try a bit
> > > > harder to update the screen and make sure output gets to the VT, since
> > > > some drivers are capable of flipping back to it.
> > > > 
> > > > So make sure we try to unblank and update the display if called from a
> > > > panic context."
> > > > 
> > > > I've enhanced this to add a flag to the vc that console layer can set
> > > > to indicate they want this behaviour to occur. This also adds support
> > > > to fbcon for that flag and adds an fb flag for drivers to indicate
> > > > they want to use the support. It enables this for KMS drivers.
> > > 
> > > Interesting.  Getting real oops traces from machines running X will
> > > make Rusty happy, and that's what we're all here for.
> > > 
> > > How well does this all work?  How reliable is it?  What's the success rate?
> > > 
> > > 
> > > 
> > > What's the downside here?  After all, not all oopses are catastrophic -
> > > sometimes the machine will go blurt and keep running so the user can
> > > take a look in the logs then perform an orderly reboot, etc.  As I
> > > understand it, those non-catastrophic oopses will now flip the machine
> > > from X and into the vt display, yes?  Can the user get it back to X
> > > mode?
> > 
> > No, we'll only flip from the panic notifier chain.
> 
> So we still don't get to see the output from BUGs and random oopses?  I
> don't think panics are all that common.

No, BUG() ends up in panic iirc by doing an bad pointer ref which
should fault and panic. Random oopses won't show up though (but they're
not supposed to be fatal and can be seen in the log).

> So am I correct in believing that if a user is getting invisible-oopses
> then he can set /proc/sys/kernel/panic_on_oops, and then the oops
> should be visible?

Yep.

> (Shouldn't all this stuff be explained in the changlog?)

Maybe?  Linux oops/panic handling is pretty ad-hoc, a separate document
would probably be best.

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2010-06-23 20:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-23  3:12 [PATCH] vt/console: try harder to print output when panicing Dave Airlie
2010-06-23 15:43 ` Jesse Barnes
2010-06-23 15:57 ` James Simmons
2010-06-23 19:56 ` Andrew Morton
2010-06-23 20:05   ` Jesse Barnes
2010-06-23 20:36     ` Andrew Morton
2010-06-23 20:40       ` Jesse Barnes

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).