[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] xen/sched: fix sched_move_domain() for domain without vcpus
- To: Juergen Gross <jgross@xxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- Date: Mon, 6 Sep 2021 12:18:37 +0100
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=PRyRSKeSK8xNzfD/CkdS3fjOL8eQZf6W+On4hkPJgyY=; b=EVZATBHR2QTHRMBDr6eciyk5kLKhOS+R3maEnPpRg+DhwVcU5VV6TZ+1e2D2ntozFc7PbEKPN7IlK1JFghYz9MZGXC6rHLDhA33J9RTC7XwLkexUYqbINpZ9qKQ+55+jSUBjyuB226eGRBM0G9s4ln9ZInYdqkX2C5uio+xHzd/ND3O1iXkaAeRjy8Ywt6jBYtlnzshvMf9iR1qyoOwDUziLuauTjbLdpN/vM5qjm5W4q3jASWD5MasqEihSZS0hVI8BmsKw4AiH+vmIQ2e7U7lF2W3uVi2uOgB8EUaSwLz/x1p1TB4kWDAOeIz8XONpyRuefMJt8fNLJ9PMBF8YZA==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nfjvxeuYzk9xRximLHfT1uCp3BqMPTwM+t1MFhP7VM4LKf4cuYzKWsvXDDp/fsVWsJIYWSzMREH96vwZxb2kNTkh/srzEdU37ZlVkYtFL6KJJ91uCBlZFCg/RS0PuZHaa0OEyf0sz0Rn8UZ3qJbGqan6nBpROCVkmCjMmD+KYUJJBJnRDjQGpoWhB7QGVcQ+TJ54OwTHe51aZ15TjlJRaixe4tPX1KffAVPScdFAyjhpG95GuWPrl0npPB2ZbF05ovnITDtrPF3Z/d0HkfjGp95Trs3AE/B9OtTFajeBVadT9Jky6lvUZjcJscmUNUWbZMacHJCqI7xX10/+cEktRQ==
- Authentication-results: esa6.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
- Cc: George Dunlap <george.dunlap@xxxxxxxxxx>, Dario Faggioli <dfaggioli@xxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>
- Delivery-date: Mon, 06 Sep 2021 11:18:53 +0000
- Ironport-hdrordr: A9a23:1Ym+2KACDVsh0T/lHeglsceALOsnbusQ8zAXPh9KJyC9I/b2qy nxppgmPH/P6Ar4WBkb6La90dq7MA3hHPlOkPYs1NaZLXXbUQ6TTb2KgrGSuAEIdxeOktK1kJ 0QDpSWa+eAfmSS7/yKmDVQeuxIqLLsndHK9IWuvUuFDzsaEp2Ihz0JejpzeXcGITWua6BJc6 Z0qvA33QZJLh8sH7WG7zQ+Lqf+juyOsKijTQ8NBhYh5gXLpTS06ITiGxzd+hsFSTtAzZor7G CAymXCl+SemsD+7iWZ+37Y7pxQltek4txfBPaUgsxQDjn3kA6naKloRrXHljEop+OE7kosjb D30lkdFvU2z0mUUnC+oBPr1QWl+DEy60X6wVvdunfnqdyRfkNxN+NxwaZiNjfJ4Uspu99xlI hR2XiCipZRBRTc2Azg+tnhTXhR5wiJiEtntdRWo21UUIMYZrMUh5cY5llpHJAJGz+/wJw7Ed NpENrX6J9tABCnhkjizy1SKeGXLzMO9k/seDlFhiXV6UkXoJlB9Tpc+CRF9U1wra7USPF/lq /52+pT5elzpmJ/V9MKOA47e7rCNoX6e2OFDIujGyWTKEg5AQO7l3fW2sR52Aj4Qu1F8HMN8K 6xGW+w81RCIH7TNQ==
- Ironport-sdr: 9UqJlYe1eqL2Dv9Z2t16LuKYX0yFqSP+7scu9rBlrvCeY3EYfNfMIda9FVfG+YfjFUVBp8VDMt vRRFR3Ealss/X9PCPiDpJs5iKxkEH0ezMAuCDw+Pd5URBhkeYSUpYHKUGS9qiwYYeCWn1JfU06 Pqkfh+6wHTzL5tEOgjBpraWtQxEhPpRJjfaRzz2fdHigUEdyJIJxkLlvVX20ptI9Eqn3XHv70S kw98ZgxjuxdL4RSVnysg/0MZ1TyKl7zOnJ26kJo548hV9m2rbM0fuM8dL9ko3P/CEu8IqOAGxx iZ3K+XuMJbIKlZrgJOI2cLn3
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 06/09/2021 12:14, Andrew Cooper wrote:
> On 06/09/2021 12:00, Juergen Gross wrote:
>> In case a domain is created with a cpupool other than Pool-0 specified
>> it will be moved to that cpupool before any vcpus are allocated.
>>
>> This will lead to a NULL pointer dereference in sched_move_domain().
>>
>> Fix that by tolerating vcpus not being allocated yet.
>>
>> Fixes: 70fadc41635b9b6 ("xen/cpupool: support moving domain between cpupools
>> with different granularity")
>> Reported-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
>> Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>> ---
>> xen/common/sched/core.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
>> index 8d178baf3d..79c9100680 100644
>> --- a/xen/common/sched/core.c
>> +++ b/xen/common/sched/core.c
>> @@ -671,6 +671,10 @@ int sched_move_domain(struct domain *d, struct cpupool
>> *c)
>>
>> for ( unit_idx = 0; unit_idx < n_units; unit_idx++ )
>> {
>> + /* Special case for move at domain creation time. */
>> + if ( !d->vcpu[unit_idx * gran] )
>> + break;
>> +
>> unit = sched_alloc_unit_mem();
>> if ( unit )
>> {
> I think the logic would be clearer if you wrap the entire for loop in if
> ( d->max_vcpus ).
And of course, this is wrong. Turns out the domain_has_vcpus()
predicate still hasn't been committed, but d->vcpu[0] is the correct
internal.
~Andrew
> This loop is only allocating units in the new
> scheduler for existing vcpus, so there's no point entering the loop at
> all during domain creation.
>
> Also, this removes a non-speculatively-guarded d->vcpu[] deference.
>
> ~Andrew
>
>
|
