[PATCH] linux/netback: save interrupt state in add_to_net_schedule_list_tail

[PATCH] linux/netback: save interrupt state in add_to_net_schedule_list_tail

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 2574 bytes --]

From: Ian Campbell <ijc@hellion.org.uk>
Subject: xen: netback: save interrupt state in add_to_net_schedule_list_tail

add_to_net_schedule_list_tail is called from both hard interrupt context
(add_to_net_schedule_list_tail) and soft interrupt/process context
(netif_schedule_work) so use the interrupt state saving spinlock
variants.

Fixes:
        ------------[ cut here ]------------
        WARNING: at kernel/lockdep.c:2323 trace_hardirqs_on_caller+0xef/0x1a0()
        Hardware name: PowerEdge 860
        Modules linked in: rtc_cmos rtc_core rtc_lib
        Pid: 16, comm: xenwatch Not tainted 2.6.32.18-x86_32p-xen0-00850-ge6b9b2c #98
        Call Trace:
         [<c103951c>] warn_slowpath_common+0x6c/0xc0
         [<c1039585>] warn_slowpath_null+0x15/0x20
         [<c105f60f>] trace_hardirqs_on_caller+0xef/0x1a0
         [<c105f6cb>] trace_hardirqs_on+0xb/0x10
         [<c136cc72>] _spin_unlock_irq+0x22/0x40
         [<c11ab9ef>] add_to_net_schedule_list_tail+0x5f/0xb0
         [<c11aba6b>] netif_be_int+0x2b/0x120
         [<c106dd8e>] handle_IRQ_event+0x2e/0xe0
         [<c106f98e>] handle_level_irq+0x6e/0xf0
         [<c1197cdf>] __xen_evtchn_do_upcall+0x16f/0x190
         [<c11981b8>] xen_evtchn_do_upcall+0x28/0x40
         [<c100b487>] xen_do_upcall+0x7/0xc
         [<c119bcf9>] xs_talkv+0x59/0x1a0
         [<c119bf6a>] xs_single+0x3a/0x50
         [<c119c6f9>] xenbus_read+0x39/0x60
         [<c11adf77>] frontend_changed+0x3e7/0x6a0
         [<c119d35a>] xenbus_otherend_changed+0x8a/0xa0
         [<c119d572>] frontend_changed+0x12/0x20
         [<c119b9dc>] xenwatch_thread+0x7c/0x140
         [<c104ea74>] kthread+0x74/0x80
         [<c100b433>] kernel_thread_helper+0x7/0x10
        ---[ end trace 48d73949a8e0909a ]---
        
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -821,16 +821,18 @@ static void remove_from_net_schedule_lis
 
 static void add_to_net_schedule_list_tail(netif_t *netif)
 {
+	unsigned long flags;
+
 	if (__on_net_schedule_list(netif))
 		return;
 
-	spin_lock_irq(&net_schedule_list_lock);
+	spin_lock_irqsave(&net_schedule_list_lock, flags);
 	if (!__on_net_schedule_list(netif) &&
 	    likely(netif_schedulable(netif))) {
 		list_add_tail(&netif->list, &net_schedule_list);
 		netif_get(netif);
 	}
-	spin_unlock_irq(&net_schedule_list_lock);
+	spin_unlock_irqrestore(&net_schedule_list_lock, flags);
 }
 
 /*




[-- Attachment #2: xen-netback-add-sched-list-irqs.patch --]
[-- Type: text/plain, Size: 2568 bytes --]

From: Ian Campbell <ijc@hellion.org.uk>
Subject: xen: netback: save interrupt state in add_to_net_schedule_list_tail

add_to_net_schedule_list_tail is called from both hard interrupt context
(add_to_net_schedule_list_tail) and soft interrupt/process context
(netif_schedule_work) so use the interrupt state saving spinlock
variants.

Fixes:
        ------------[ cut here ]------------
        WARNING: at kernel/lockdep.c:2323 trace_hardirqs_on_caller+0xef/0x1a0()
        Hardware name: PowerEdge 860
        Modules linked in: rtc_cmos rtc_core rtc_lib
        Pid: 16, comm: xenwatch Not tainted 2.6.32.18-x86_32p-xen0-00850-ge6b9b2c #98
        Call Trace:
         [<c103951c>] warn_slowpath_common+0x6c/0xc0
         [<c1039585>] warn_slowpath_null+0x15/0x20
         [<c105f60f>] trace_hardirqs_on_caller+0xef/0x1a0
         [<c105f6cb>] trace_hardirqs_on+0xb/0x10
         [<c136cc72>] _spin_unlock_irq+0x22/0x40
         [<c11ab9ef>] add_to_net_schedule_list_tail+0x5f/0xb0
         [<c11aba6b>] netif_be_int+0x2b/0x120
         [<c106dd8e>] handle_IRQ_event+0x2e/0xe0
         [<c106f98e>] handle_level_irq+0x6e/0xf0
         [<c1197cdf>] __xen_evtchn_do_upcall+0x16f/0x190
         [<c11981b8>] xen_evtchn_do_upcall+0x28/0x40
         [<c100b487>] xen_do_upcall+0x7/0xc
         [<c119bcf9>] xs_talkv+0x59/0x1a0
         [<c119bf6a>] xs_single+0x3a/0x50
         [<c119c6f9>] xenbus_read+0x39/0x60
         [<c11adf77>] frontend_changed+0x3e7/0x6a0
         [<c119d35a>] xenbus_otherend_changed+0x8a/0xa0
         [<c119d572>] frontend_changed+0x12/0x20
         [<c119b9dc>] xenwatch_thread+0x7c/0x140
         [<c104ea74>] kthread+0x74/0x80
         [<c100b433>] kernel_thread_helper+0x7/0x10
        ---[ end trace 48d73949a8e0909a ]---
        
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -821,16 +821,18 @@ static void remove_from_net_schedule_lis
 
 static void add_to_net_schedule_list_tail(netif_t *netif)
 {
+	unsigned long flags;
+
 	if (__on_net_schedule_list(netif))
 		return;
 
-	spin_lock_irq(&net_schedule_list_lock);
+	spin_lock_irqsave(&net_schedule_list_lock, flags);
 	if (!__on_net_schedule_list(netif) &&
 	    likely(netif_schedulable(netif))) {
 		list_add_tail(&netif->list, &net_schedule_list);
 		netif_get(netif);
 	}
-	spin_unlock_irq(&net_schedule_list_lock);
+	spin_unlock_irqrestore(&net_schedule_list_lock, flags);
 }
 
 /*

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] linux/netback: save interrupt state in add_to_net_schedule_list_tail

[Ian Campbell]

Hi Jan, 

Which tree are you intending for this (and the other kernel patches you
sent recently) to be applied to?

Ian.

On Mon, 2010-09-13 at 14:56 +0100, Jan Beulich wrote:
> From: Ian Campbell <ijc@hellion.org.uk>
> Subject: xen: netback: save interrupt state in add_to_net_schedule_list_tail
> 
> add_to_net_schedule_list_tail is called from both hard interrupt context
> (add_to_net_schedule_list_tail) and soft interrupt/process context
> (netif_schedule_work) so use the interrupt state saving spinlock
> variants.
> 
> Fixes:
>         ------------[ cut here ]------------
>         WARNING: at kernel/lockdep.c:2323 trace_hardirqs_on_caller+0xef/0x1a0()
>         Hardware name: PowerEdge 860
>         Modules linked in: rtc_cmos rtc_core rtc_lib
>         Pid: 16, comm: xenwatch Not tainted 2.6.32.18-x86_32p-xen0-00850-ge6b9b2c #98
>         Call Trace:
>          [<c103951c>] warn_slowpath_common+0x6c/0xc0
>          [<c1039585>] warn_slowpath_null+0x15/0x20
>          [<c105f60f>] trace_hardirqs_on_caller+0xef/0x1a0
>          [<c105f6cb>] trace_hardirqs_on+0xb/0x10
>          [<c136cc72>] _spin_unlock_irq+0x22/0x40
>          [<c11ab9ef>] add_to_net_schedule_list_tail+0x5f/0xb0
>          [<c11aba6b>] netif_be_int+0x2b/0x120
>          [<c106dd8e>] handle_IRQ_event+0x2e/0xe0
>          [<c106f98e>] handle_level_irq+0x6e/0xf0
>          [<c1197cdf>] __xen_evtchn_do_upcall+0x16f/0x190
>          [<c11981b8>] xen_evtchn_do_upcall+0x28/0x40
>          [<c100b487>] xen_do_upcall+0x7/0xc
>          [<c119bcf9>] xs_talkv+0x59/0x1a0
>          [<c119bf6a>] xs_single+0x3a/0x50
>          [<c119c6f9>] xenbus_read+0x39/0x60
>          [<c11adf77>] frontend_changed+0x3e7/0x6a0
>          [<c119d35a>] xenbus_otherend_changed+0x8a/0xa0
>          [<c119d572>] frontend_changed+0x12/0x20
>          [<c119b9dc>] xenwatch_thread+0x7c/0x140
>          [<c104ea74>] kthread+0x74/0x80
>          [<c100b433>] kernel_thread_helper+0x7/0x10
>         ---[ end trace 48d73949a8e0909a ]---
>         
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> --- a/drivers/xen/netback/netback.c
> +++ b/drivers/xen/netback/netback.c
> @@ -821,16 +821,18 @@ static void remove_from_net_schedule_lis
>  
>  static void add_to_net_schedule_list_tail(netif_t *netif)
>  {
> +	unsigned long flags;
> +
>  	if (__on_net_schedule_list(netif))
>  		return;
>  
> -	spin_lock_irq(&net_schedule_list_lock);
> +	spin_lock_irqsave(&net_schedule_list_lock, flags);
>  	if (!__on_net_schedule_list(netif) &&
>  	    likely(netif_schedulable(netif))) {
>  		list_add_tail(&netif->list, &net_schedule_list);
>  		netif_get(netif);
>  	}
> -	spin_unlock_irq(&net_schedule_list_lock);
> +	spin_unlock_irqrestore(&net_schedule_list_lock, flags);
>  }
>  
>  /*
> 
> 
> 

Re: [PATCH] linux/netback: save interrupt state in add_to_net_schedule_list_tail

[Jan Beulich]

>>> On 13.09.10 at 16:14, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> Which tree are you intending for this (and the other kernel patches you
> sent recently) to be applied to?

Oh, right, for this one it isn't obvious from the paths. They're all for
the 2.6.18 one, in an attempt to at least avoid carrying patches of
our own where they can easily be made apply to what we derive
our tree from.

Jan

Re: [PATCH] linux/netback: save interrupt state in add_to_net_schedule_list_tail

[Ian Campbell]

On Mon, 2010-09-13 at 15:38 +0100, Jan Beulich wrote:
> >>> On 13.09.10 at 16:14, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> > Which tree are you intending for this (and the other kernel patches you
> > sent recently) to be applied to?
> 
> Oh, right, for this one it isn't obvious from the paths. They're all for
> the 2.6.18 one, in an attempt to at least avoid carrying patches of
> our own where they can easily be made apply to what we derive
> our tree from.

Ah, I hadn't realised anyone still cared about updating the 2.6.18-xen
tree so it didn't occur to me.

Ian.

Re: [PATCH] linux/netback: save interrupt state in add_to_net_schedule_list_tail

[Jan Beulich]

>>> On 14.09.10 at 11:12, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> On Mon, 2010-09-13 at 15:38 +0100, Jan Beulich wrote:
>> >>> On 13.09.10 at 16:14, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
>> > Which tree are you intending for this (and the other kernel patches you
>> > sent recently) to be applied to?
>> 
>> Oh, right, for this one it isn't obvious from the paths. They're all for
>> the 2.6.18 one, in an attempt to at least avoid carrying patches of
>> our own where they can easily be made apply to what we derive
>> our tree from.
> 
> Ah, I hadn't realised anyone still cared about updating the 2.6.18-xen
> tree so it didn't occur to me.

How would we (and even you) not care? There's now newer tree
available that can serve as an input (in some cases we can pull
fixes from the pv-ops tree, but that's not the normal case). Even
your XCP tree (indirectly) depends on the 2.6.18 one, as you
derive it from ours (and hence we can't reasonably use it as a
replacement source). With pv-ops continuing to be (somewhat?)
experimental, I have always been wishing we could get to a point
where we'd have a more modern baseline tree, but I don't really
have any hope for such.

Nor can I foresee when the pv-ops tree will be reliable enough
and sufficiently functionally complete (without hacks that in
some cases I think are worse than those in the 2.6.18 tree) to
be used as the basis of an enterprise Dom0 (which is the
criteria that could make us finally do the long hoped for switch).

Jan

Re: [PATCH] linux/netback: save interrupt state in add_to_net_schedule_list_tail

[Ian Campbell]

On Tue, 2010-09-14 at 10:26 +0100, Jan Beulich wrote:
> >>> On 14.09.10 at 11:12, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> > On Mon, 2010-09-13 at 15:38 +0100, Jan Beulich wrote:
> >> >>> On 13.09.10 at 16:14, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> >> > Which tree are you intending for this (and the other kernel patches you
> >> > sent recently) to be applied to?
> >> 
> >> Oh, right, for this one it isn't obvious from the paths. They're all for
> >> the 2.6.18 one, in an attempt to at least avoid carrying patches of
> >> our own where they can easily be made apply to what we derive
> >> our tree from.
> > 
> > Ah, I hadn't realised anyone still cared about updating the 2.6.18-xen
> > tree so it didn't occur to me.
> 
> How would we (and even you) not care? There's now newer tree
> available that can serve as an input (in some cases we can pull
> fixes from the pv-ops tree, but that's not the normal case). Even
> your XCP tree (indirectly) depends on the 2.6.18 one, as you
> derive it from ours (and hence we can't reasonably use it as a
> replacement source).

Yes, I hadn't considered this until now, I see the dilemma.

I don't think any one really wants to perpetuate the classic-Xen port
any further by creating a new upstream linux-2.6.32-xen.hg (although I
suppose we could if there was demand) so stashing patches like this in
the linux-2.6.18-xen.hg tree makes sense.

>  With pv-ops continuing to be (somewhat?)
> experimental, I have always been wishing we could get to a point
> where we'd have a more modern baseline tree, but I don't really
> have any hope for such.
> 
> Nor can I foresee when the pv-ops tree will be reliable enough
> and sufficiently functionally complete (without hacks that in
> some cases I think are worse than those in the 2.6.18 tree) to
> be used as the basis of an enterprise Dom0 (which is the
> criteria that could make us finally do the long hoped for switch).

I guess there is a certain amount of chicken and egg there and I also
suppose nobody really wants to be the guinea pig ;-)

FWIW I hope that XCP can make the switch before too long and that
XenServer will be able to follow, perhaps in the next major release.

Ian.

Re: xen.org kernel trees

[Pasi Kärkkäinen]

On Tue, Sep 14, 2010 at 11:00:51AM +0100, Ian Campbell wrote:
> On Tue, 2010-09-14 at 10:26 +0100, Jan Beulich wrote:
> > >>> On 14.09.10 at 11:12, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> > > On Mon, 2010-09-13 at 15:38 +0100, Jan Beulich wrote:
> > >> >>> On 13.09.10 at 16:14, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> > >> > Which tree are you intending for this (and the other kernel patches you
> > >> > sent recently) to be applied to?
> > >> 
> > >> Oh, right, for this one it isn't obvious from the paths. They're all for
> > >> the 2.6.18 one, in an attempt to at least avoid carrying patches of
> > >> our own where they can easily be made apply to what we derive
> > >> our tree from.
> > > 
> > > Ah, I hadn't realised anyone still cared about updating the 2.6.18-xen
> > > tree so it didn't occur to me.
> > 
> > How would we (and even you) not care? There's now newer tree
> > available that can serve as an input (in some cases we can pull
> > fixes from the pv-ops tree, but that's not the normal case). Even
> > your XCP tree (indirectly) depends on the 2.6.18 one, as you
> > derive it from ours (and hence we can't reasonably use it as a
> > replacement source).
> 
> Yes, I hadn't considered this until now, I see the dilemma.
> 
> I don't think any one really wants to perpetuate the classic-Xen port
> any further by creating a new upstream linux-2.6.32-xen.hg (although I
> suppose we could if there was demand) so stashing patches like this in
> the linux-2.6.18-xen.hg tree makes sense.
> 

Since both Novell/SLES and Citrix/XenServer/XCP currently use
the classic Xen patches (Xenlinux) it sounds like a good idea
to have a common upstream tree? 

I think it should be less effort than porting back and forward
to/from linux-2.6.18-xen.hg ? Am I correct?


> >  With pv-ops continuing to be (somewhat?)
> > experimental, I have always been wishing we could get to a point
> > where we'd have a more modern baseline tree, but I don't really
> > have any hope for such.
> > 
> > Nor can I foresee when the pv-ops tree will be reliable enough
> > and sufficiently functionally complete (without hacks that in
> > some cases I think are worse than those in the 2.6.18 tree) to
> > be used as the basis of an enterprise Dom0 (which is the
> > criteria that could make us finally do the long hoped for switch).
> 
> I guess there is a certain amount of chicken and egg there and I also
> suppose nobody really wants to be the guinea pig ;-)
> 
> FWIW I hope that XCP can make the switch before too long and that
> XenServer will be able to follow, perhaps in the next major release.
> 

Yeah, pvops tree obviously should be the main xen.org kernel tree for 
all new development and testing efforts.

-- Pasi

Re: Re: [PATCH] linux/netback: save interrupt state in add_to_net_schedule_list_tail

[Jeremy Fitzhardinge]

 On 09/14/2010 02:26 AM, Jan Beulich wrote:
> Nor can I foresee when the pv-ops tree will be reliable enough
> and sufficiently functionally complete (without hacks that in
> some cases I think are worse than those in the 2.6.18 tree) to
> be used as the basis of an enterprise Dom0 (which is the
> criteria that could make us finally do the long hoped for switch).
>

I'd be interested to know what you have in mind here.

    J

Re: Re: [PATCH] linux/netback: save interrupt state in add_to_net_schedule_list_tail

[Jan Beulich]

 >>> On 15.09.10 at 22:58, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> On 09/14/2010 02:26 AM, Jan Beulich wrote:
>> Nor can I foresee when the pv-ops tree will be reliable enough
>> and sufficiently functionally complete (without hacks that in
>> some cases I think are worse than those in the 2.6.18 tree) to
>> be used as the basis of an enterprise Dom0 (which is the
>> criteria that could make us finally do the long hoped for switch).
>>
> 
> I'd be interested to know what you have in mind here.

One missing bit of functionality that I can think of right away are
the GNTST_eagain retry loops needed for Xen's paging (and
perhaps also memory sharing). I looked for them maybe a week
ago, and didn't find any of them - did I perhaps just look in the
wrong branches (see below)?

Others coming to mind without much looking around would be
pv-scsi, pv-usb, and the accelerator parts of netfront/netback.

A second aspect is the multitude of branches all in different
feature and maintenance states. A stable 2.6.32 tree as some
sort of main branch is nice, but insufficient for the purpose of
integration when our main tree tracks upstream very closely
(switching usually around -rc2). Having otoh several dozen
branches (some in an apparently forgotten about state) with
features and fixes scattered around isn't very helpful either.

As to the hacks - from what I saw I'm not certain I can create a
non-Xen kernel from any of the branches that would be (close
to) binary identical with one created from the respective upstream
(or stable) kernel. I'm not even certain it would build. But then
again I'm judging only from looking at it, not from really having
tried.

Especially the ACPI stuff introduced Xen-specific (parts of) files
outside of drivers/xen/ that I thought would be a no-go in the
pv-ops tree.

Finally, I'm continuing to see way too many "does not work" mails
on xen-devel.

Jan