[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] crash in csched_load_balance after xl vcpu-pin


  • To: Dario Faggioli <dfaggioli@xxxxxxxx>, Olaf Hering <olaf@xxxxxxxxx>
  • From: George Dunlap <george.dunlap@xxxxxxxxxx>
  • Date: Wed, 11 Apr 2018 11:37:52 +0100
  • Autocrypt: addr=george.dunlap@xxxxxxxxxx; prefer-encrypt=mutual; keydata= xsFNBFPqG+MBEACwPYTQpHepyshcufo0dVmqxDo917iWPslB8lauFxVf4WZtGvQSsKStHJSj 92Qkxp4CH2DwudI8qpVbnWCXsZxodDWac9c3PordLwz5/XL41LevEoM3NWRm5TNgJ3ckPA+J K5OfSK04QtmwSHFP3G/SXDJpGs+oDJgASta2AOl9vPV+t3xG6xyfa2NMGn9wmEvvVMD44Z7R W3RhZPn/NEZ5gaJhIUMgTChGwwWDOX0YPY19vcy5fT4bTIxvoZsLOkLSGoZb/jHIzkAAznug Q7PPeZJ1kXpbW9EHHaUHiCD9C87dMyty0N3TmWfp0VvBCaw32yFtM9jUgB7UVneoZUMUKeHA fgIXhJ7I7JFmw3J0PjGLxCLHf2Q5JOD8jeEXpdxugqF7B/fWYYmyIgwKutiGZeoPhl9c/7RE Bf6f9Qv4AtQoJwtLw6+5pDXsTD5q/GwhPjt7ohF7aQZTMMHhZuS52/izKhDzIufl6uiqUBge 0lqG+/ViLKwCkxHDREuSUTtfjRc9/AoAt2V2HOfgKORSCjFC1eI0+8UMxlfdq2z1AAchinU0 eSkRpX2An3CPEjgGFmu2Je4a/R/Kd6nGU8AFaE8ta0oq5BSFDRYdcKchw4TSxetkG6iUtqOO ZFS7VAdF00eqFJNQpi6IUQryhnrOByw+zSobqlOPUO7XC5fjnwARAQABzSRHZW9yZ2UgVy4g RHVubGFwIDxkdW5sYXBnQHVtaWNoLmVkdT7CwYAEEwEKACoCGwMFCwkIBwMFFQoJCAsFFgID AQACHgECF4ACGQEFAlpk2IEFCQo9I54ACgkQpjY8MQWQtG1A1BAAnc0oX3+M/jyv4j/ESJTO U2JhuWUWV6NFuzU10pUmMqpgQtiVEVU2QbCvTcZS1U/S6bqAUoiWQreDMSSgGH3a3BmRNi8n HKtarJqyK81aERM2HrjYkC1ZlRYG+jS8oWzzQrCQiTwn3eFLJrHjqowTbwahoiMw/nJ+OrZO /VXLfNeaxA5GF6emwgbpshwaUtESQ/MC5hFAFmUBZKAxp9CXG2ZhTP6ROV4fwhpnHaz8z+BT NQz8YwA4gkmFJbDUA9I0Cm9D/EZscrCGMeaVvcyldbMhWS+aH8nbqv6brhgbJEQS22eKCZDD J/ng5ea25QnS0fqu3bMrH39tDqeh7rVnt8Yu/YgOwc3XmgzmAhIDyzSinYEWJ1FkOVpIbGl9 uR6seRsfJmUK84KCScjkBhMKTOixWgNEQ/zTcLUsfTh6KQdLTn083Q5aFxWOIal2hiy9UyqR VQydowXy4Xx58rqvZjuYzdGDdAUlZ+D2O3Jp28ez5SikA/ZaaoGI9S1VWvQsQdzNfD2D+xfL qfd9yv7gko9eTJzv5zFr2MedtRb/nCrMTnvLkwNX4abB5+19JGneeRU4jy7yDYAhUXcI/waS /hHioT9MOjMh+DoLCgeZJYaOcgQdORY/IclLiLq4yFnG+4Ocft8igp79dbYYHkAkmC9te/2x Kq9nEd0Hg288EO/OwE0EVFq6vQEIAO2idItaUEplEemV2Q9mBA8YmtgckdLmaE0uzdDWL9To 1PL+qdNe7tBXKOfkKI7v32fe0nB4aecRlQJOZMWQRQ0+KLyXdJyHkq9221sHzcxsdcGs7X3c 17ep9zASq+wIYqAdZvr7pN9a3nVHZ4W7bzezuNDAvn4EpOf/o0RsWNyDlT6KECs1DuzOdRqD oOMJfYmtx9hMzqBoTdr6U20/KgnC/dmWWcJAUZXaAFp+3NYRCkk7k939VaUpoY519CeLrymd Vdke66KCiWBQXMkgtMGvGk5gLQLy4H3KXvpXoDrYKgysy7jeOccxI8owoiOdtbfM8TTDyWPR Ygjzb9LApA8AEQEAAcLBZQQYAQoADwUCVFq6vQIbDAUJAeEzgAAKCRCmNjwxBZC0bWknD/97 Tkh3PMAcvMZINmJefBdYYspmwTWZSR9USsy68oWzDsXKNDNTqBC781lR/7PSqhqaSOmSnty3 FNblaBYKfMV3OOWgrP0H8Voqp4IgH3yOOkQLVITIwulqbbxQtmCsJ3xkhZm6CA0EKbc9VM/j FX3aCAfOJf52vlY1gXjYOvVjrdrRrBXEjs8E5f6EsrQKDrWCKNx/9qRfmtsQeKHTsgpINkpZ s11ClX/sM/RCR9/BgB/K08QQZYsWD6lgZh1KxLXRzKRunba0L+jpcRsoQFUMj/ofrfnHAdl0 q2upzISM/wR8aer+kekMo+y00schmYJYu5JAAzbjQQuhCAg0UTBGPaNwteL2l3c9Ps8on1nl mq9TnbYwGLAxJzXSb3BATgz7dygpsBBNS5WhUNQgIJvcZJbLggEIqjZGs8o7/+dt4klwxCYL FVlsWYSwEjX0UYHVLMS/F7FcXbCMUeoN/4krmRyv7YICE/VDQSDPcSKedzWvQM8T+5uY5pFJ NiIaa6asFndP50GiKbFtD6xAM+rbnwT7Io+iPtvD/3ddMXQs58IVMzgNA/hcdOX/qlx6Jqk/ hYQQsl4HoQsx/GyrNiwiPErTx32QNeXxoGYm6kwxt7F5qK7AN5tyYNkEyoxYrv8bl9VjAve8 hpECyf4O1mOGC/dIuBCDk8gxL5Pbo3jl98LBZQQYAQoADwIbDAUCVlNqsQUJA9njdAAKCRCm NjwxBZC0bbJMEACigmtpL2lzS47DXydApr1X8SYCHIPc39OjvmErjP05lKUZjmesmhlM5eKO gPb/fzeJ0wXB4J8OyseIJ0D/XwyLLQeM8d/HUFFMBWr+HE7jIukAUXeQ6GRwR+MBYGK/KmR9 JHbMAUz8f3G087Ma12BfpNWayndlFwR3rvdV4lvlyx6cl0EaFhbzPu/N07HG5MTk0evtphgZ 7wuG1oAtO+DGA6orHEicor6nBAQNZzPyjqo40dBxTs+amx7UndMRPSL1dD57eJwbbvBeNa8I w8wT7oNy2/C21VWmSy5XzMzcUTgmjmQz6DSNJPz2dMK4Y/LtcVFTfSZTmlBIkfoc9Vay2EB9 3z2EmjZwGT7n/DRu9QDtLbXyeVTBuLTaP3D+q5AyR1/5Z4T0LhwNvxeND5yO+YNAwqocZwL+ OcctpSZUBpAuU4Ju/9JKMX57GlnbjB8YGahoBJsQZx4CZyw0MXlkCk5cR0EPjY9iI2CEA5lO QueOSbo0hf1ZJwCx724lx0WSwL8ngd8wZTYMNc8GngaU61kmzfcuCklhokTxQdK7Efme5ccv A1txzgGewx9mDhPgNcJweasBnyL0N3wya2RMAzm04gCio8y4FKQepwQpKCNKAYZIU4juAPxn nb6cbBGiMGO1NDuxG+qvl1cMElnq+cuhSUlZdr2sE9JRfa0gucLBZQQYAQoADwIbDAUCWHQN VAUJBfqGFwAKCRCmNjwxBZC0bbgCD/oC6mWUrxQKWPDvFE9+fzm8UKqKP7aciz+gvWUN3o4i 4sRFNyvAEOW/QY2zwM1pN07BFZ3Z+8AVxpgR6h7RQzDJYSPZ5k5WWCJzJEQs2sPI5rfYJGK8 um7mlsSvf2xcLK/1Aj07BmWDjR6glDDRY+iMmSSdHe6Te6tiQPPS6Woj8AE3qf5lBsdvcEln nrkSwzNeVKRQQROUOskVw4WmCsNJjZtKmrVpgId3df/5HWG7Bi4nPwA8IFOt6O72lJlkORFy DF5P7ML7Pc5LbEFimzETPBxTJzVu1UoOQb/THB+qxhKMXXudSf/5sdMhwvOwItIcc5pib/v6 7gWK48bAzoOTgNYzmDCVC/roeLLU2SpEQIlIR0eAaWImgt8VEtre3Gch33e41DtbUli54DX0 dRdhqQaDM1T1q77VyDoZcs+SpGX9Ic9mxl+BN+6vtGIUVgaOG5pF85aQlRfCD6IlFQgiZtiR XeRpeIYG27RUw5kIljW+VxPMdBUvZpUXEazqjoPvBKybg0oKFfMXrMj4vHo6J0FD3ZEToGnP dANspUCZRewRozjp7ZWIu7QfGasfJNQ8c1IDiAFl3rV+dAGXXdmrDcX6w2q5lqoFz+8npK2I ehKCA94U+J/RLywUiaLuHnXt40WvQ98kHm7uTsy36iWqqawPqzmn8m5ruynVHmmcXsLBZQQY AQoADwIbDAUCWmTXMwUJB+tP9gAKCRCmNjwxBZC0bb+2D/9hjn1k5WcRHlu19WGuH6q0Kgm1 LRT7PnnSz904igHNElMB5a7wRjw5kdNwU3sRm2nnmHeOJH8kYj2Hn1QgX5SqQsysWTHWOEse GeoXydx9zZZkt3oQJM+9NV1VjK0bOXwqhiQyEUWz5/9l467FS/k4FJ5CHNRumvhLa0l2HEEu 5pxq463HQZHDt4YE/9Y74eXOnYCB4nrYxQD/GSXEZvWryEWreDoaFqzq1TKtzHhFgQG7yFUE epxLRUUtYsEpT6Rks2l4LCqG3hVD0URFIiTyuxJx3VC2Ta4LH3hxQtiaIpuXqq2D4z63h6vC x2wxfZc/WRHGbr4NAlB81l35Q/UHyMocVuYLj0llF0rwU4AjiKZ5qWNSEdvEpL43fTvZYxQh DCjQTKbb38omu5P4kOf1HT7s+kmQKRtiLBlqHzK17D4K/180ADw7a3gnmr5RumcZP3NGSSZA 6jP5vNqQpNu4gqrPFWNQKQcW8HBiYFgq6SoLQQWbRxJDHvTRYJ2ms7oCe870gh4D1wFFqTLe yXiVqjddENGNaP8ZlCDw6EU82N8Bn5LXKjR1GWo2UK3CjrkHpTt3YYZvrhS2MO2EYEcWjyu6 LALF/lS6z6LKeQZ+t9AdQUcILlrx9IxqXv6GvAoBLJY1jjGBq+/kRPrWXpoaQn7FXWGfMqU+ NkY9enyrlw==
  • Cc: "xen-devel@xxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxx>
  • Delivery-date: Wed, 11 Apr 2018 10:37:58 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Openpgp: preference=signencrypt

On 04/10/2018 11:59 PM, Dario Faggioli wrote:
> [Adding Andrew, not because I expect anything, but just because we've 
>  chatted about this issue on IRC :-) ]
> 
> On Tue, 2018-04-10 at 22:37 +0200, Olaf Hering wrote:
>> On Tue, Apr 10, Dario Faggioli wrote:
>>
>>     BUG_ON(__vcpu_on_runq(CSCHED_VCPU(vc)));
>>
>> (XEN) Xen BUG at sched_credit.c:876
>> (XEN) ----[ Xen-4.11.20180410T125709.50f8ba84a5-
>> 3.bug1087289_411  x86_64  debug=y   Not tainted ]----
>> (XEN) CPU:    118
>> (XEN) RIP:    e008:[<ffff82d080229ab4>]
>> sched_credit.c#csched_vcpu_migrate+0x27/0x51
>> ...
>> (XEN) Xen call trace:
>> (XEN)    [<ffff82d080229ab4>]
>> sched_credit.c#csched_vcpu_migrate+0x27/0x51
>> (XEN)    [<ffff82d080236348>] schedule.c#vcpu_move_locked+0xbb/0xc2
>> (XEN)    [<ffff82d08023764c>] schedule.c#vcpu_migrate+0x226/0x25b
>> (XEN)    [<ffff82d08023935f>] context_saved+0x8d/0x94
>> (XEN)    [<ffff82d08027797d>] context_switch+0xe66/0xeb0
>> (XEN)    [<ffff82d080236943>] schedule.c#schedule+0x5f4/0x627
>> (XEN)    [<ffff82d080239f15>] softirq.c#__do_softirq+0x85/0x90
>> (XEN)    [<ffff82d080239f6a>] do_softirq+0x13/0x15
>> (XEN)    [<ffff82d08031f5db>] vmx_asm_do_vmentry+0x2b/0x30
>>
> Hey... unless I've really put there a totally bogous BUG_ON(), this
> looks interesting and potentially useful.
> 
> It says that the vcpu which is being context switched out, and on which
> we are calling vcpu_migrate() on, because we found it to be
> VPF_migrating, is actually in the runqueue already when we actually get
> to execute vcpu_migrate()->vcpu_move_locked().
> 
> Mmm... let's see.
> 
>  CPU A                                  CPU B
>  .                                      .
>  schedule(current == v)                 vcpu_set_affinity(v)
>   prev = current     // == v             .
>   schedule_lock(CPU A)                   .
>    csched_schedule()                     schedule_lock(CPU A)
>    if (runnable(v))  //YES               x
>     runq_insert(v)                       x
>    return next != v                      x
>   schedule_unlock(CPU A)                 x // takes the lock
>   context_switch(prev,next)              set_bit(v, VPF_migrating)  [*]
>    context_saved(prev) // still == v     .
>     v->is_running = 0                    schedule_unlock(CPU A)
>     SMP_MB                               .
>     if (test_bit(v, VPF_migrating)) // YES!!
>      vcpu_migrate(v)                     .
>       for {                              .
>        schedule_lock(CPU A)              .
>        SCHED_OP(v, pick_cpu)             .
>         set_bit(v, CSCHED_MIGRATING)     .
>         return CPU C                     .
>        pick_called = 1                   .
>        schedule_unlock(CPU A)            .
>        schedule_lock(CPU A + CPU C)      .
>        if (pick_called && ...) // YES    .
>         break                            .
>       }                                  .
>       // v->is_running is 0              .
>       //!test_and_clear(v, VPF_migrating)) is false!!
>       clear_bit(v, VPF_migrating)        .
>       vcpu_move_locked(v, CPU C)         .
>       BUG_ON(__vcpu_on_runq(v))          .
> 
> [*] after this point, and until someone manages to call vcpu_sleep(),  
>       v sits in CPU A's runqueue with the VPF_migrating pause flag set
> 
> So, basically, the race is between context_saved() and
> vcpu_set_affinity(). Basically, vcpu_set_affinity() sets the
> VPF_migrating pause flags on a vcpu in a runqueue, with the intent of
> letting either a vcpu_sleep_nosync() or a reschedule remove it from
> there, but context_saved() manage to see the flag, before the removal
> can happen.
> 
> And I think this explains also the original BUG at sched_credit.c:1694
> (it's just a bit more involved).

Yep, that looks correct.  I had considered some sort of race between
set_affinity() and context_switch(), but just never noticed that it
could omit to take the vcpu off the runqueue.

> As it can be seen above (and also in the code comment in ) there is a
> barrier (which further testify that this is indeed a tricky passage),
> but I guess it is not that effective! :-/
> 
> TBH, I have actually never fully understood what that comment really
> meant, what the barrier was protecting, and how... e.g., isn't it
> missing its paired one? In fact, there's another comment, clearly
> related, right in vcpu_set_affinity(). But again I'm a bit at loss at
> properly figuring out what the big idea is.

I think the idea is to make sure that the change to v->is_running
happens before whatever happens to come next (i.e., that the compiler
doesn't reorder the write as part of its normal optimization
activities).  As it happens nothing that comes next looks like it really
needs such ordering (particularly as you can't reorder things over a
function call, AFAIUI), but it's good to have those in place in case
anybody *does* add that sort of thing.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.