>From aa2ce7440f16002266dc8464f749992d0c8ac0e5 Mon Sep 17 00:00:00 2001
From: Liu, Jinsong <[email protected]>
Date: Tue, 12 Jun 2012 23:11:16 +0800
Subject: [PATCH] xen/mce: schedule a workqueue to avoid sleep in atomic context
copy_to_user might sleep and print a stack trace if it is executed
in an atomic spinlock context.
This patch schedule a workqueue for IRQ handler to poll the data,
and use mutex instead of spinlock, so copy_to_user sleep in atomic
context would not occur.
Reported-by: Konrad Rzeszutek Wilk <[email protected]>
Suggested-by: Konrad Rzeszutek Wilk <[email protected]>
Signed-off-by: Liu, Jinsong <[email protected]>
---
drivers/xen/mcelog.c | 18 +++++++++++-------
1 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/xen/mcelog.c b/drivers/xen/mcelog.c
index 72e87d2..804aa3c 100644
--- a/drivers/xen/mcelog.c
+++ b/drivers/xen/mcelog.c
@@ -55,7 +55,7 @@ static struct mc_info g_mi;
static struct mcinfo_logical_cpu *g_physinfo;
static uint32_t ncpus;
-static DEFINE_SPINLOCK(mcelog_lock);
+static DEFINE_MUTEX(mcelog_lock);
static struct xen_mce_log xen_mcelog = {
.signature = XEN_MCE_LOG_SIGNATURE,
@@ -106,7 +106,7 @@ static ssize_t xen_mce_chrdev_read(struct file *filp, char __user *ubuf,
unsigned num;
int i, err;
- spin_lock(&mcelog_lock);
+ mutex_lock(&mcelog_lock);
num = xen_mcelog.next;
@@ -130,7 +130,7 @@ static ssize_t xen_mce_chrdev_read(struct file *filp, char __user *ubuf,
err = -EFAULT;
out:
- spin_unlock(&mcelog_lock);
+ mutex_unlock(&mcelog_lock);
return err ? err : buf - ubuf;
}
@@ -310,12 +310,11 @@ static int mc_queue_handle(uint32_t flags)
}
/* virq handler for machine check error info*/
-static irqreturn_t xen_mce_interrupt(int irq, void *dev_id)
+static void xen_mce_work_fn(struct work_struct *work)
{
int err;
- unsigned long tmp;
- spin_lock_irqsave(&mcelog_lock, tmp);
+ mutex_lock(&mcelog_lock);
/* urgent mc_info */
err = mc_queue_handle(XEN_MC_URGENT);
@@ -330,8 +329,13 @@ static irqreturn_t xen_mce_interrupt(int irq, void *dev_id)
pr_err(XEN_MCELOG
"Failed to handle nonurgent mc_info queue.\n");
- spin_unlock_irqrestore(&mcelog_lock, tmp);
+ mutex_unlock(&mcelog_lock);
+}
+static DECLARE_WORK(xen_mce_work, xen_mce_work_fn);
+static irqreturn_t xen_mce_interrupt(int irq, void *dev_id)
+{
+ schedule_work(&xen_mce_work);
return IRQ_HANDLED;
}
--
1.7.1
On Tue, Jun 12, 2012 at 07:51:03AM +0000, Liu, Jinsong wrote:
> >From aa2ce7440f16002266dc8464f749992d0c8ac0e5 Mon Sep 17 00:00:00 2001
> From: Liu, Jinsong <[email protected]>
> Date: Tue, 12 Jun 2012 23:11:16 +0800
> Subject: [PATCH] xen/mce: schedule a workqueue to avoid sleep in atomic context
>
> copy_to_user might sleep and print a stack trace if it is executed
> in an atomic spinlock context.
>
> This patch schedule a workqueue for IRQ handler to poll the data,
> and use mutex instead of spinlock, so copy_to_user sleep in atomic
> context would not occur.
Ah much better. Usually one also includes the report of what the
stack trace was. So I've added that in.
>
> Reported-by: Konrad Rzeszutek Wilk <[email protected]>
> Suggested-by: Konrad Rzeszutek Wilk <[email protected]>
> Signed-off-by: Liu, Jinsong <[email protected]>
> ---
> drivers/xen/mcelog.c | 18 +++++++++++-------
> 1 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/xen/mcelog.c b/drivers/xen/mcelog.c
> index 72e87d2..804aa3c 100644
> --- a/drivers/xen/mcelog.c
> +++ b/drivers/xen/mcelog.c
> @@ -55,7 +55,7 @@ static struct mc_info g_mi;
> static struct mcinfo_logical_cpu *g_physinfo;
> static uint32_t ncpus;
>
> -static DEFINE_SPINLOCK(mcelog_lock);
> +static DEFINE_MUTEX(mcelog_lock);
>
> static struct xen_mce_log xen_mcelog = {
> .signature = XEN_MCE_LOG_SIGNATURE,
> @@ -106,7 +106,7 @@ static ssize_t xen_mce_chrdev_read(struct file *filp, char __user *ubuf,
> unsigned num;
> int i, err;
>
> - spin_lock(&mcelog_lock);
> + mutex_lock(&mcelog_lock);
>
> num = xen_mcelog.next;
>
> @@ -130,7 +130,7 @@ static ssize_t xen_mce_chrdev_read(struct file *filp, char __user *ubuf,
> err = -EFAULT;
>
> out:
> - spin_unlock(&mcelog_lock);
> + mutex_unlock(&mcelog_lock);
>
> return err ? err : buf - ubuf;
> }
> @@ -310,12 +310,11 @@ static int mc_queue_handle(uint32_t flags)
> }
>
> /* virq handler for machine check error info*/
> -static irqreturn_t xen_mce_interrupt(int irq, void *dev_id)
> +static void xen_mce_work_fn(struct work_struct *work)
> {
> int err;
> - unsigned long tmp;
>
> - spin_lock_irqsave(&mcelog_lock, tmp);
> + mutex_lock(&mcelog_lock);
>
> /* urgent mc_info */
> err = mc_queue_handle(XEN_MC_URGENT);
> @@ -330,8 +329,13 @@ static irqreturn_t xen_mce_interrupt(int irq, void *dev_id)
> pr_err(XEN_MCELOG
> "Failed to handle nonurgent mc_info queue.\n");
>
> - spin_unlock_irqrestore(&mcelog_lock, tmp);
> + mutex_unlock(&mcelog_lock);
> +}
> +static DECLARE_WORK(xen_mce_work, xen_mce_work_fn);
>
> +static irqreturn_t xen_mce_interrupt(int irq, void *dev_id)
> +{
> + schedule_work(&xen_mce_work);
> return IRQ_HANDLED;
> }
>
> --
> 1.7.1
> _______________________________________________
> Xen-devel mailing list
> [email protected]
> http://lists.xen.org/xen-devel
On Tue, Jun 12, 2012 at 08:40:15AM -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Jun 12, 2012 at 07:51:03AM +0000, Liu, Jinsong wrote:
> > >From aa2ce7440f16002266dc8464f749992d0c8ac0e5 Mon Sep 17 00:00:00 2001
> > From: Liu, Jinsong <[email protected]>
> > Date: Tue, 12 Jun 2012 23:11:16 +0800
> > Subject: [PATCH] xen/mce: schedule a workqueue to avoid sleep in atomic context
> >
> > copy_to_user might sleep and print a stack trace if it is executed
> > in an atomic spinlock context.
> >
> > This patch schedule a workqueue for IRQ handler to poll the data,
> > and use mutex instead of spinlock, so copy_to_user sleep in atomic
> > context would not occur.
>
> Ah much better. Usually one also includes the report of what the
> stack trace was. So I've added that in.
So another bug which is that mcelog is spinning at 100% CPU (and only
under Xen).
It seems to be doing:
ppoll([{fd=4, events=POLLIN}, {fd=3, events=POLLIN}], 2, NULL, [], 8) = 1 ([{fd=3, revents=POLLIN}])
read(3, "", 2816) = 0
ppoll([{fd=4, events=POLLIN}, {fd=3, events=POLLIN}], 2, NULL, [], 8) = 1 ([{fd=3, revents=POLLIN}])
read(3, "", 2816)
constantly.
Konrad Rzeszutek Wilk wrote:
> On Tue, Jun 12, 2012 at 08:40:15AM -0400, Konrad Rzeszutek Wilk wrote:
>> On Tue, Jun 12, 2012 at 07:51:03AM +0000, Liu, Jinsong wrote:
>>>> From aa2ce7440f16002266dc8464f749992d0c8ac0e5 Mon Sep 17 00:00:00
>>>> 2001
>>> From: Liu, Jinsong <[email protected]>
>>> Date: Tue, 12 Jun 2012 23:11:16 +0800
>>> Subject: [PATCH] xen/mce: schedule a workqueue to avoid sleep in
>>> atomic context
>>>
>>> copy_to_user might sleep and print a stack trace if it is executed
>>> in an atomic spinlock context.
>>>
>>> This patch schedule a workqueue for IRQ handler to poll the data,
>>> and use mutex instead of spinlock, so copy_to_user sleep in atomic
>>> context would not occur.
>>
>> Ah much better. Usually one also includes the report of what the
>> stack trace was. So I've added that in.
>
> So another bug which is that mcelog is spinning at 100% CPU (and only
> under Xen).
>
> It seems to be doing:
>
> ppoll([{fd=4, events=POLLIN}, {fd=3, events=POLLIN}], 2, NULL, [], 8)
> = 1 ([{fd=3, revents=POLLIN}]) read(3, "", 2816)
> = 0
> ppoll([{fd=4, events=POLLIN}, {fd=3, events=POLLIN}], 2, NULL, [], 8)
> = 1 ([{fd=3, revents=POLLIN}]) read(3, "", 2816)
>
> constantly.
I will debug it. I have try at my platform, but fail to reproduce it. (You still use the config you send me last time, right?)
Would you tell me your step?
Thanks,
Jinsong-