Re: [Xen-devel] [PATCH] credit: fix race condition in csched_vcpu flags

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

On Mon, Feb 25, 2013 at 8:20 AM, Igor Pavlikevich <ipavlikevich@xxxxxxxxx> wrote:

When vcpu stops yielding, it's possible to overwrite CSCHED_FLAG_VCPU_PARKED flag after clearing CSCHED_FLAG_VCPU_YIELD.

 332.970218393 -------x d1v1 runstate_continue d1v1 running->running
]332.970248371 -------x d1v1   28005(2:8:5) 2 [ 1 1 ]                  yielding, flags=0x0002
]332.970251968 -------x d1v1                                           clearing 0x0002, flags=0
 332.970252175 -------x d1v1 runstate_continue d1v1 running->running
]332.970282574 -------x d1v1   28005(2:8:5) 2 [ 1 1 ]                  yielding, flags=0x0002
]332.970283068 x------| d32767v0                                       entering csched_acct(), d1v1 flags=0x0002
]332.970285763 x------| d32767v0   28003(2:8:3) 2 [ 1 1 ]              calling vcpu_pause_nosync() for d1v1
]332.970286647 -------x d1v1                                           clearing 0x0002, flags=0
]332.970286771 x------| d32767v0                                       flags |= CSCHED_FLAG_VCPU_PARKED, flags=0x0003
]332.970286990 -------x d1v1   2800e(2:8:e) 2 [ 1 1cacfca ]
]332.970287122 -------x d1v1   2800f(2:8:f) 3 [ 7fff 1cacfca ffffffff ]
]332.970287242 -------x d1v1   2800a(2:8:a) 4 [ 1 1 7fff 7 ]           exiting schedule(), flags=0
 332.970287417 -------x d1v1 runstate_change d1v1 running->offline
 332.970287629 -------x d?v? runstate_change d32767v7 runnable->running
]332.970288397 x------- d32767v0                                       exiting csched_acct(), flags=0x0003
]332.995349690 x------- d32767v0                                       entering csched_acct(), d1v1 flags=0x0000, flag lost.
]332.995352170 x------- d32767v0   28003(2:8:3) 2 [ 1 1 ]              calling vcpu_pause_nosync() for d1v1 again

>From this moment d1v1 has extra +1 in pause_count, so it goes offline right after credit>prv->credits_per_tslice forever.

Good catch -- I guess we don't really test the scheduler "cap" functionality enough to catch this one.

Unfortunately I think the real problem is that the ->flags field should be uniformly using the atomic bit operations.

Can you try the attached patch and see if it works for you?

 -George

 

Attachment: credit-atomic-flags.diff
Description: Binary data

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel