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

Ping²: [PATCH v2] x86/PoD: move increment of entry count


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 24 Feb 2022 14:04:05 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.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:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=2WuumZucJz4sXSydEPYHjq1IVRJ/Q3Li9fOh+beuZHg=; b=BpvU5Ljpc/dynrO6aXtZtPaAOQURbAdubGUvJ+o/ryUqt4UgNiBEWORk4UjVXYWQb6pEvdaPdzCejEvrtngUyk8PmA4CbcUtxdanr8s1S3YQ6ISVwqY2SWPRnV2GvmHH4/KjC1bYgBBM+lteNM8nt8d8q5ZGlReaQo/3YYqqY3iTA2gq40VG/RXrGswhXDJDxhjgUS0LMCLWnaH0s224B5KPPx6AMHHoFs0uBA0Y47W5YOoXx2VSXg+swTMzf6fHjlxTWSLEv7syhzHbWMCvVdVwaFUDR4wD89aCyf8wC+G+pTkVRY5DXaRochAUxE10EKPdPE3E+MlwO752s1FYlA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=KVYYBms1Hg+0If4Sp+qhFJlma5lC8uhAytHqQKHbOib1OPmg1JBqQrVoR9+BfhczkJjIgXX00Yfk8R/gBgmXWrr8187RljMzATPg35PasyV5zIboI4WAOub91uFIQXGuIhOly9nMNgDqJ0xxgAlgVqvk8S+8ZAi/h8uthfkDlg6dm8+BJ4gibjm/oOOYS4b+PT2j59myWGacnlV03ThHwuML1Fenesb9jNYCTrsyv/8gES9WDtdMdfHxBffj3BwzdB7bcC5Wb8a9ohcXwXm+oSESdimC/McYP9XNnu5G3qnT/afaphHQhG87WhLo4uDBkpHFqdPVCuTyE0PMO26oLA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 24 Feb 2022 13:04:16 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 27.01.2022 16:04, Jan Beulich wrote:
> On 04.01.2022 11:57, Jan Beulich wrote:
>> When not holding the PoD lock across the entire region covering P2M
>> update and stats update, the entry count should indicate too large a
>> value in preference to a too small one, to avoid functions bailing early
>> when they find the count is zero. Hence increments should happen ahead
>> of P2M updates, while decrements should happen only after. Deal with the
>> one place where this hasn't been the case yet.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> v2: Add comments.
> 
> Ping?

Similarly here: Another 4 weeks have passed ...

Thanks for feedback either way, Jan

>> ---
>> While it might be possible to hold the PoD lock over the entire
>> operation, I didn't want to chance introducing a lock order violation on
>> a perhaps rarely taken code path.
>>
>> --- a/xen/arch/x86/mm/p2m-pod.c
>> +++ b/xen/arch/x86/mm/p2m-pod.c
>> @@ -1342,19 +1342,22 @@ mark_populate_on_demand(struct domain *d
>>          }
>>      }
>>  
>> +    /*
>> +     * Without holding the PoD lock across the entire operation, bump the
>> +     * entry count up front assuming success of p2m_set_entry(), undoing the
>> +     * bump as necessary upon failure.  Bumping only upon success would risk
>> +     * code elsewhere observing entry count being zero despite there 
>> actually
>> +     * still being PoD entries.
>> +     */
>> +    pod_lock(p2m);
>> +    p2m->pod.entry_count += (1UL << order) - pod_count;
>> +    pod_unlock(p2m);
>> +
>>      /* Now, actually do the two-way mapping */
>>      rc = p2m_set_entry(p2m, gfn, INVALID_MFN, order,
>>                         p2m_populate_on_demand, p2m->default_access);
>>      if ( rc == 0 )
>> -    {
>> -        pod_lock(p2m);
>> -        p2m->pod.entry_count += 1UL << order;
>> -        p2m->pod.entry_count -= pod_count;
>> -        BUG_ON(p2m->pod.entry_count < 0);
>> -        pod_unlock(p2m);
>> -
>>          ioreq_request_mapcache_invalidate(d);
>> -    }
>>      else if ( order )
>>      {
>>          /*
>> @@ -1366,6 +1369,14 @@ mark_populate_on_demand(struct domain *d
>>                 d, gfn_l, order, rc);
>>          domain_crash(d);
>>      }
>> +    else if ( !pod_count )
>> +    {
>> +        /* Undo earlier increment; see comment above. */
>> +        pod_lock(p2m);
>> +        BUG_ON(!p2m->pod.entry_count);
>> +        --p2m->pod.entry_count;
>> +        pod_unlock(p2m);
>> +    }
>>  
>>  out:
>>      gfn_unlock(p2m, gfn, order);
>>
>>
> 




 


Rackspace

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