From:   "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
Subject: Re: [PATCH v5.5 23/30] KVM: Resolve memslot ID via a hash table
 instead of via a static array
To:     Sean Christopherson <seanjc@google.com>
Cc:     James Morse <james.morse@arm.com>,
        Alexandru Elisei <alexandru.elisei@arm.com>,
        Suzuki K Poulose <suzuki.poulose@arm.com>,
        Atish Patra <atish.patra@wdc.com>,
        David Hildenbrand <david@redhat.com>,
        Cornelia Huck <cohuck@redhat.com>,
        Claudio Imbrenda <imbrenda@linux.ibm.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>,
        linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu,
        linux-mips@vger.kernel.org, kvm@vger.kernel.org,
        kvm-ppc@vger.kernel.org, kvm-riscv@lists.infradead.org,
        linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
        Ben Gardon <bgardon@google.com>, Marc Zyngier <maz@kernel.org>,
        Huacai Chen <chenhuacai@kernel.org>,
        Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>,
        Paul Mackerras <paulus@ozlabs.org>,
        Anup Patel <anup.patel@wdc.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Christian Borntraeger <borntraeger@de.ibm.com>,
        Janosch Frank <frankja@linux.ibm.com>,
        Paolo Bonzini <pbonzini@redhat.com>
References: <20211104002531.1176691-1-seanjc@google.com>
 <20211104002531.1176691-24-seanjc@google.com>
Message-ID: <f0b364ed-bf9e-5de9-0449-6d7ba3682405@oracle.com>
Date:   Fri, 12 Nov 2021 00:51:47 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
In-Reply-To: <20211104002531.1176691-24-seanjc@google.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?V3ZKWlBYWjdNL2g1Y1ZUWnZPTUxXQ0tuR3REK1cvOExEYzlJTCtkMTNJeWdR?=
 =?utf-8?B?TlNheFpQWitaUHMrYWd2Z3QxWkdIbWcxUUxUNzFYWHFKdmtMcUFLQ2Fxcytp?=
 =?utf-8?B?QW1WQXBVakRvYzJRU0pMWWVjTlljYU13YTlpV0xrZE9ZT2UxMTJKSXpDQWIw?=
 =?utf-8?B?NExwbm1DWHo1anh2OExxako2Z3BCYnNObnh3U2VOcEdWUmY2Mk5MVE1TUlpj?=
 =?utf-8?B?S2tZVC9WcUNMUzlGSktUMUJ0NC9CUEwxSTFVak1ZcHRnUGQ3Ky92N2FoQlQz?=
 =?utf-8?B?cFlKemlHSHMvK2toYk1hVG1SekxqdExhVGlnbzFmMHVsS253L3M3VGN2V05G?=
 =?utf-8?B?SEd3dDlXTVFRVlpGR0J3am5LWkp4QURhV0I3M1lnUHZnOG8yNytmQnVGbnFs?=
 =?utf-8?B?eE42c0FOdDRGRzFIbFJDY1FiRlVJU01ZcThEYjlPeTdncWk5M2E0ekkvdlNp?=
 =?utf-8?B?NUZ5N1FVSWFBS1piT0FhMTUxYWJnSkhlYzhEaVJqa29JeVlIWWlWa2UwM2Vq?=
 =?utf-8?B?M0YrbGFFc2pHK0NJUk9jendHWWpUTk1PU0k3cFh4SXRDczR5dWZtcHVNcEty?=
 =?utf-8?B?N0JFWjZuZjRnczRxbjNsL2x0Z1JGakhkQVVPRGRkMG92ZDZ4VTFXd3d4MGVL?=
 =?utf-8?B?ZExkdEQwUmJZdW5ML0pzTWFsdHdqL0RrRnpRV0piZHZmYWNPWVRyZlNqVGp4?=
 =?utf-8?B?aiswL1FqdkxBN0tQeXBybmtCN1R3K2hxeHZJTjBOejkyVTJ1cUtBd29KRmxM?=
 =?utf-8?B?czloUGpSUXFPU1FiWEExQlZJSjBuRTNGTUwrV1oralE1WFRnUzVkZjdJM21W?=
 =?utf-8?B?RGRyeFptaWdGK0tmeko2RW9qTnorN21wNTFieWtVUytINTArc0FneVNhYlpZ?=
 =?utf-8?B?aFZyelIyMEozTTl4NWpwenhjWTh2emtBdEpEb3JJYkJnK3JQYkdWTE1qSUdE?=
 =?utf-8?B?ZjBTbFd6VFlzOG5BNnFHdVRhbjQ2ZzRBMjAyMDlTbC9jbEtNL1VKU2cwSWRZ?=
 =?utf-8?B?MkJaWnRKWld6WHFxOTJBRFNjYWpocVlSajNrWDYxUjhjMGtTRkdwRTdmM0RE?=
 =?utf-8?B?Yys5bFBSbnp3eGszSW94YlFNMk8xWjVrR08zUXN3ZHU1dFFtTkRNN3NnSjlV?=
 =?utf-8?B?dkdJcUFmK2VtS2M3V1ZUaDI5VFhOdGFKaTFacU82TXF0TXVVQWJJTkxUT3RW?=
 =?utf-8?B?SG04Nll2Vmpsb3NvSCtDZ1NpRWF2bmhPM3MyazhvN0RCRTNHQ1dlbTUxTzFu?=
 =?utf-8?B?ZFVNZ3hsbTE1cVV0ako0ekdHZWZvTy9WQ0JmaC9ldHNUSkR3cERoNWZ6SXhy?=
 =?utf-8?B?d05NS1ZHV2tOejFwcGV2U1B4RzRrcUg3TUxKOHMvRXYxRkVBTFBBRjlKeEZs?=
 =?utf-8?B?d3JocTJFR29KRGhrdDRkTSszbjVBczBpa010TCtpZk1aTkxTWXJqN0hvOGlh?=
 =?utf-8?B?Z2xLUDdQb05NVGlia0VsdnhQYlNMR2YxV25PWFlJWk0wdjJDUzY4dHlqa2tD?=
 =?utf-8?B?NVM0NnF5ckhoM1BVMnZKVnIvaFlqMHNjUk5BT2RZV1pjaGpYUDIraG10NGhw?=
 =?utf-8?B?QTlMSUwwYk9ud1B6MlJ0RmNRUTRhV2FwR3V1RWJiaHRHa1hxRkpwdExaS2l5?=
 =?utf-8?B?c20rZ2prOUhxeHB5bWVlamE0ekFoMi9IeCs4SUNIVERyUW54T3JnSDFRMnZp?=
 =?utf-8?B?eEt1QklFbzd0anpFc0Q0WFdxRTJweXhENEkrU0JvZnB5NTdISytjVGgzUklW?=
 =?utf-8?B?SVl0OU1LY3RFN3NCaUlRRDZENnNnc2lRUGRWaXl0cENJcFNIVG5RODdrNmw5?=
 =?utf-8?B?b1gxMUJYbkhzTml5Rm0yQU5zOTBlVEQrdVlNUUh3U3ZucDRzNEJrVGd4VE5o?=
 =?utf-8?B?SDBnKy9CcDY0dy96bmVzMXNGSVo2SVYvelR2N1JPRlRYb1c4WjQyMGR6WU4r?=
 =?utf-8?B?MitVanBKUGRZeFEzaFo0dHpuT3p5T3Y4OHBqdGozRGdST2JRWmp1NDZoRVdJ?=
 =?utf-8?B?MXVhQytGbHo5ZVArckthSVZOOXlxaTRDdG8xSVhTQ201QXp2WmY0bjZ2bDNm?=
 =?utf-8?B?WDVvM2h2T3FUeFZmRFd0MnhGMTAvMTVqRE9iZTl2Sy9oaytFa3lvRkUyMHpJ?=
 =?utf-8?B?RmQySDZJc0pkWVB4Nm9uMWZ2QW1RYWdkNnRTT2hTT0FoRVVOS1h3SjRPYVZ0?=
 =?utf-8?B?QzlEdWptQmtFYWNGQlhSK2lDVUlhVW43T0FBUkprZENRNW5RQlVTZy8xZUVp?=
 =?utf-8?B?M0J0NGI1ckdydXpZNFIzaGNjM0xBPT0=?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e30843d1-daba-4a47-4b8b-08d9a56e3ee9
X-MS-Exchange-CrossTenant-AuthSource: CH2PR10MB4008.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Nov 2021 23:51:57.1817
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: azHd9qj+0UtomEB0BWYqo11FcyS5MZnu9lMA8tXIJkhL2+eIjlyMNI3sAVX8THhszztWCIlpvLqeHZRSGh84H5axOusSWNy7AuuA6zuZins=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB4938
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10165 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 mlxscore=0
 suspectscore=0 bulkscore=0 spamscore=0 phishscore=0 malwarescore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2110150000 definitions=main-2111110121
X-Proofpoint-GUID: hVGcF-rOAkFRn2UikYu4Yw-pb0p-cly9
X-Proofpoint-ORIG-GUID: hVGcF-rOAkFRn2UikYu4Yw-pb0p-cly9
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 04.11.2021 01:25, Sean Christopherson wrote:
> From: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> 
> Memslot ID to the corresponding memslot mappings are currently kept as
> indices in static id_to_index array.
> The size of this array depends on the maximum allowed memslot count
> (regardless of the number of memslots actually in use).
> 
> This has become especially problematic recently, when memslot count cap was
> removed, so the maximum count is now full 32k memslots - the maximum
> allowed by the current KVM API.
> 
> Keeping these IDs in a hash table (instead of an array) avoids this
> problem.
> 
> Resolving a memslot ID to the actual memslot (instead of its index) will
> also enable transitioning away from an array-based implementation of the
> whole memslots structure in a later commit.
> 
> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   include/linux/kvm_host.h | 16 +++----
>   virt/kvm/kvm_main.c      | 96 +++++++++++++++++++++++++++++++---------
>   2 files changed, 84 insertions(+), 28 deletions(-)
> 
(..)
> @@ -1259,17 +1257,49 @@ static int kvm_alloc_dirty_bitmap(struct kvm_memory_slot *memslot)
>   	return 0;
>   }
>   
> +static void kvm_replace_memslot(struct kvm_memslots *slots,
> +				struct kvm_memory_slot *old,
> +				struct kvm_memory_slot *new)
> +{
> +	/*
> +	 * Remove the old memslot from the hash list, copying the node data
> +	 * would corrupt the list.
> +	 */
> +	if (old) {
> +		hash_del(&old->id_node);
> +
> +		if (!new)
> +			return;
> +	}
> +
> +	/* Copy the source *data*, not the pointer, to the destination. */
> +	if (old)
> +		*new = *old;

This way of writing it (that, is re-checking whether "old" is not-NULL)
suggests that it could have been set to NULL inside the previous block
(since the last check), which isn't true.

Thanks,
Maciej