From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Christopherson Date: Tue, 22 Oct 2019 15:52:20 +0000 Subject: Re: [PATCH v2 14/15] KVM: Terminate memslot walks via used_slots Message-Id: <20191022155220.GD2343@linux.intel.com> List-Id: References: <20191022003537.13013-1-sean.j.christopherson@intel.com> <20191022003537.13013-15-sean.j.christopherson@intel.com> <642f73ee-9425-0149-f4f4-f56be9ae5713@redhat.com> <20191022152827.GC2343@linux.intel.com> <625e511f-bd35-3b92-0c6d-550c10fc5827@redhat.com> In-Reply-To: <625e511f-bd35-3b92-0c6d-550c10fc5827@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Paolo Bonzini Cc: James Hogan , Paul Mackerras , Christian Borntraeger , Janosch Frank , Radim =?utf-8?B?S3LEjW3DocWZ?= , Marc Zyngier , David Hildenbrand , Cornelia Huck , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , James Morse , Julien Thierry , Suzuki K Poulose , linux-mips@vger.kernel.org, kvm-ppc@vger.kernel.org, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org On Tue, Oct 22, 2019 at 05:30:58PM +0200, Paolo Bonzini wrote: > On 22/10/19 17:28, Sean Christopherson wrote: > > On Tue, Oct 22, 2019 at 04:04:18PM +0200, Paolo Bonzini wrote: > >> On 22/10/19 02:35, Sean Christopherson wrote: > >>> +static inline int kvm_shift_memslots_forward(struct kvm_memslots *slots, > >>> + struct kvm_memory_slot *new) > >>> +{ > >>> + struct kvm_memory_slot *mslots = slots->memslots; > >>> + int i; > >>> + > >>> + if (WARN_ON_ONCE(slots->id_to_index[new->id] = -1) || > >>> + WARN_ON_ONCE(!slots->used_slots)) > >>> + return -1; > >>> + > >>> + for (i = slots->id_to_index[new->id]; i < slots->used_slots - 1; i++) { > >>> + if (new->base_gfn > mslots[i + 1].base_gfn) > >>> + break; > >>> + > >>> + WARN_ON_ONCE(new->base_gfn = mslots[i + 1].base_gfn); > >>> + > >>> + /* Shift the next memslot forward one and update its index. */ > >>> + mslots[i] = mslots[i + 1]; > >>> + slots->id_to_index[mslots[i].id] = i; > >>> + } > >>> + return i; > >>> +} > >>> + > >>> +static inline int kvm_shift_memslots_back(struct kvm_memslots *slots, > >>> + struct kvm_memory_slot *new, > >>> + int start) > >> > >> This new implementation of the insertion sort loses the comments that > >> were there in the old one. Please keep them as function comments. > > > > I assume you're talking about this blurb in particular? > > > > * The ">=" is needed when creating a slot with base_gfn = 0, > > * so that it moves before all those with base_gfn = npages = 0. > > Yes, well all of the comments. You can also keep them in the caller, as > you prefer. The primary function comment is still there, the only other comment that I dropped was the second half of the above comment: * * On the other hand, if new->npages is zero, the above loop has * already left i pointing to the beginning of the empty part of * mslots, and the ">=" would move the hole backwards in this * case---which is wrong. So skip the loop when deleting a slot. */ Which doesn't carry forward very well. Is there another comment I'm overlooking? Anyways, I'm not at all opposed to adding comments, just want to make sure I'm not forgetting something. If it's ok with you, I'll comment the code and/or functions and reply here to refine them without having to respin the whole series. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EFC55C47E49 for ; Tue, 22 Oct 2019 15:52:27 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 83914218AE for ; Tue, 22 Oct 2019 15:52:27 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 83914218AE Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvmarm-bounces@lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 020F34A960; Tue, 22 Oct 2019 11:52:27 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7GRhUvjdCqKx; Tue, 22 Oct 2019 11:52:25 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 2B7044A95D; Tue, 22 Oct 2019 11:52:25 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 885214A8AF for ; Tue, 22 Oct 2019 11:52:24 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 9XA-S-bUNTK0 for ; Tue, 22 Oct 2019 11:52:23 -0400 (EDT) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 099514A658 for ; Tue, 22 Oct 2019 11:52:22 -0400 (EDT) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Oct 2019 08:52:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,216,1569308400"; d="scan'208";a="196476114" Received: from sjchrist-coffee.jf.intel.com (HELO linux.intel.com) ([10.54.74.41]) by fmsmga008.fm.intel.com with ESMTP; 22 Oct 2019 08:52:20 -0700 Date: Tue, 22 Oct 2019 08:52:20 -0700 From: Sean Christopherson To: Paolo Bonzini Subject: Re: [PATCH v2 14/15] KVM: Terminate memslot walks via used_slots Message-ID: <20191022155220.GD2343@linux.intel.com> References: <20191022003537.13013-1-sean.j.christopherson@intel.com> <20191022003537.13013-15-sean.j.christopherson@intel.com> <642f73ee-9425-0149-f4f4-f56be9ae5713@redhat.com> <20191022152827.GC2343@linux.intel.com> <625e511f-bd35-3b92-0c6d-550c10fc5827@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <625e511f-bd35-3b92-0c6d-550c10fc5827@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) Cc: Cornelia Huck , Wanpeng Li , Janosch Frank , kvm@vger.kernel.org, James Hogan , Joerg Roedel , David Hildenbrand , linux-mips@vger.kernel.org, kvm-ppc@vger.kernel.org, linux-kernel@vger.kernel.org, Paul Mackerras , Christian Borntraeger , linux-arm-kernel@lists.infradead.org, Marc Zyngier , Vitaly Kuznetsov , kvmarm@lists.cs.columbia.edu, Jim Mattson X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu On Tue, Oct 22, 2019 at 05:30:58PM +0200, Paolo Bonzini wrote: > On 22/10/19 17:28, Sean Christopherson wrote: > > On Tue, Oct 22, 2019 at 04:04:18PM +0200, Paolo Bonzini wrote: > >> On 22/10/19 02:35, Sean Christopherson wrote: > >>> +static inline int kvm_shift_memslots_forward(struct kvm_memslots *slots, > >>> + struct kvm_memory_slot *new) > >>> +{ > >>> + struct kvm_memory_slot *mslots = slots->memslots; > >>> + int i; > >>> + > >>> + if (WARN_ON_ONCE(slots->id_to_index[new->id] == -1) || > >>> + WARN_ON_ONCE(!slots->used_slots)) > >>> + return -1; > >>> + > >>> + for (i = slots->id_to_index[new->id]; i < slots->used_slots - 1; i++) { > >>> + if (new->base_gfn > mslots[i + 1].base_gfn) > >>> + break; > >>> + > >>> + WARN_ON_ONCE(new->base_gfn == mslots[i + 1].base_gfn); > >>> + > >>> + /* Shift the next memslot forward one and update its index. */ > >>> + mslots[i] = mslots[i + 1]; > >>> + slots->id_to_index[mslots[i].id] = i; > >>> + } > >>> + return i; > >>> +} > >>> + > >>> +static inline int kvm_shift_memslots_back(struct kvm_memslots *slots, > >>> + struct kvm_memory_slot *new, > >>> + int start) > >> > >> This new implementation of the insertion sort loses the comments that > >> were there in the old one. Please keep them as function comments. > > > > I assume you're talking about this blurb in particular? > > > > * The ">=" is needed when creating a slot with base_gfn == 0, > > * so that it moves before all those with base_gfn == npages == 0. > > Yes, well all of the comments. You can also keep them in the caller, as > you prefer. The primary function comment is still there, the only other comment that I dropped was the second half of the above comment: * * On the other hand, if new->npages is zero, the above loop has * already left i pointing to the beginning of the empty part of * mslots, and the ">=" would move the hole backwards in this * case---which is wrong. So skip the loop when deleting a slot. */ Which doesn't carry forward very well. Is there another comment I'm overlooking? Anyways, I'm not at all opposed to adding comments, just want to make sure I'm not forgetting something. If it's ok with you, I'll comment the code and/or functions and reply here to refine them without having to respin the whole series. _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B69D5CA9EA0 for ; Tue, 22 Oct 2019 15:52:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 90993214B2 for ; Tue, 22 Oct 2019 15:52:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387877AbfJVPwX (ORCPT ); Tue, 22 Oct 2019 11:52:23 -0400 Received: from mga03.intel.com ([134.134.136.65]:53569 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387675AbfJVPwX (ORCPT ); Tue, 22 Oct 2019 11:52:23 -0400 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Oct 2019 08:52:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,216,1569308400"; d="scan'208";a="196476114" Received: from sjchrist-coffee.jf.intel.com (HELO linux.intel.com) ([10.54.74.41]) by fmsmga008.fm.intel.com with ESMTP; 22 Oct 2019 08:52:20 -0700 Date: Tue, 22 Oct 2019 08:52:20 -0700 From: Sean Christopherson To: Paolo Bonzini Cc: James Hogan , Paul Mackerras , Christian Borntraeger , Janosch Frank , Radim =?utf-8?B?S3LEjW3DocWZ?= , Marc Zyngier , David Hildenbrand , Cornelia Huck , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , James Morse , Julien Thierry , Suzuki K Poulose , linux-mips@vger.kernel.org, kvm-ppc@vger.kernel.org, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 14/15] KVM: Terminate memslot walks via used_slots Message-ID: <20191022155220.GD2343@linux.intel.com> References: <20191022003537.13013-1-sean.j.christopherson@intel.com> <20191022003537.13013-15-sean.j.christopherson@intel.com> <642f73ee-9425-0149-f4f4-f56be9ae5713@redhat.com> <20191022152827.GC2343@linux.intel.com> <625e511f-bd35-3b92-0c6d-550c10fc5827@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <625e511f-bd35-3b92-0c6d-550c10fc5827@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-mips-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-mips@vger.kernel.org On Tue, Oct 22, 2019 at 05:30:58PM +0200, Paolo Bonzini wrote: > On 22/10/19 17:28, Sean Christopherson wrote: > > On Tue, Oct 22, 2019 at 04:04:18PM +0200, Paolo Bonzini wrote: > >> On 22/10/19 02:35, Sean Christopherson wrote: > >>> +static inline int kvm_shift_memslots_forward(struct kvm_memslots *slots, > >>> + struct kvm_memory_slot *new) > >>> +{ > >>> + struct kvm_memory_slot *mslots = slots->memslots; > >>> + int i; > >>> + > >>> + if (WARN_ON_ONCE(slots->id_to_index[new->id] == -1) || > >>> + WARN_ON_ONCE(!slots->used_slots)) > >>> + return -1; > >>> + > >>> + for (i = slots->id_to_index[new->id]; i < slots->used_slots - 1; i++) { > >>> + if (new->base_gfn > mslots[i + 1].base_gfn) > >>> + break; > >>> + > >>> + WARN_ON_ONCE(new->base_gfn == mslots[i + 1].base_gfn); > >>> + > >>> + /* Shift the next memslot forward one and update its index. */ > >>> + mslots[i] = mslots[i + 1]; > >>> + slots->id_to_index[mslots[i].id] = i; > >>> + } > >>> + return i; > >>> +} > >>> + > >>> +static inline int kvm_shift_memslots_back(struct kvm_memslots *slots, > >>> + struct kvm_memory_slot *new, > >>> + int start) > >> > >> This new implementation of the insertion sort loses the comments that > >> were there in the old one. Please keep them as function comments. > > > > I assume you're talking about this blurb in particular? > > > > * The ">=" is needed when creating a slot with base_gfn == 0, > > * so that it moves before all those with base_gfn == npages == 0. > > Yes, well all of the comments. You can also keep them in the caller, as > you prefer. The primary function comment is still there, the only other comment that I dropped was the second half of the above comment: * * On the other hand, if new->npages is zero, the above loop has * already left i pointing to the beginning of the empty part of * mslots, and the ">=" would move the hole backwards in this * case---which is wrong. So skip the loop when deleting a slot. */ Which doesn't carry forward very well. Is there another comment I'm overlooking? Anyways, I'm not at all opposed to adding comments, just want to make sure I'm not forgetting something. If it's ok with you, I'll comment the code and/or functions and reply here to refine them without having to respin the whole series. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0ADCCCA9EB9 for ; Tue, 22 Oct 2019 15:52:28 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CE39321925 for ; Tue, 22 Oct 2019 15:52:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="rdCyH5cV" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CE39321925 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=wr+PcDFu/ZqCyIBX/weoRFXAq6oxZ/M6zylErByBtY4=; b=rdCyH5cVoebawc 0W3Uz0M8F1Y6v3ug66T8u3Pe3rON3XVqp3XG+ma2rdW0QXVgpHRgtbfhR3/AHZ6arpukIuUvALWum LZofBD7FbHwoh/XPHXovfyKLhiDy6N4zQWI2QdCqNxkYpjCr583oCd0D+T4E5P1alcNextrh44GVL aia1bhWELfqUmCaySARXQY95lEbpGLT4Re7EJw4+aD5wLsusyrWyViSUhLv4NY14p0tqQD6FRFaSO W8iIwiCyn5oC6shnIUshXcPFYwTrQPQwEy5j9SQv0ApdzvgnC+2runAjwkTdErBk+suuNcKKLZEoH IFPLWUw9Wxm1wOZfhz1g==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1iMwSR-0007cw-60; Tue, 22 Oct 2019 15:52:27 +0000 Received: from mga06.intel.com ([134.134.136.31]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1iMwSN-0007c3-SZ for linux-arm-kernel@lists.infradead.org; Tue, 22 Oct 2019 15:52:25 +0000 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Oct 2019 08:52:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,216,1569308400"; d="scan'208";a="196476114" Received: from sjchrist-coffee.jf.intel.com (HELO linux.intel.com) ([10.54.74.41]) by fmsmga008.fm.intel.com with ESMTP; 22 Oct 2019 08:52:20 -0700 Date: Tue, 22 Oct 2019 08:52:20 -0700 From: Sean Christopherson To: Paolo Bonzini Subject: Re: [PATCH v2 14/15] KVM: Terminate memslot walks via used_slots Message-ID: <20191022155220.GD2343@linux.intel.com> References: <20191022003537.13013-1-sean.j.christopherson@intel.com> <20191022003537.13013-15-sean.j.christopherson@intel.com> <642f73ee-9425-0149-f4f4-f56be9ae5713@redhat.com> <20191022152827.GC2343@linux.intel.com> <625e511f-bd35-3b92-0c6d-550c10fc5827@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <625e511f-bd35-3b92-0c6d-550c10fc5827@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20191022_085223_965688_18B9937E X-CRM114-Status: GOOD ( 16.35 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Cornelia Huck , Wanpeng Li , Janosch Frank , kvm@vger.kernel.org, Radim =?utf-8?B?S3LEjW3DocWZ?= , James Hogan , Joerg Roedel , David Hildenbrand , linux-mips@vger.kernel.org, kvm-ppc@vger.kernel.org, linux-kernel@vger.kernel.org, Paul Mackerras , Christian Borntraeger , James Morse , linux-arm-kernel@lists.infradead.org, Marc Zyngier , Vitaly Kuznetsov , Suzuki K Poulose , kvmarm@lists.cs.columbia.edu, Julien Thierry , Jim Mattson Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Oct 22, 2019 at 05:30:58PM +0200, Paolo Bonzini wrote: > On 22/10/19 17:28, Sean Christopherson wrote: > > On Tue, Oct 22, 2019 at 04:04:18PM +0200, Paolo Bonzini wrote: > >> On 22/10/19 02:35, Sean Christopherson wrote: > >>> +static inline int kvm_shift_memslots_forward(struct kvm_memslots *slots, > >>> + struct kvm_memory_slot *new) > >>> +{ > >>> + struct kvm_memory_slot *mslots = slots->memslots; > >>> + int i; > >>> + > >>> + if (WARN_ON_ONCE(slots->id_to_index[new->id] == -1) || > >>> + WARN_ON_ONCE(!slots->used_slots)) > >>> + return -1; > >>> + > >>> + for (i = slots->id_to_index[new->id]; i < slots->used_slots - 1; i++) { > >>> + if (new->base_gfn > mslots[i + 1].base_gfn) > >>> + break; > >>> + > >>> + WARN_ON_ONCE(new->base_gfn == mslots[i + 1].base_gfn); > >>> + > >>> + /* Shift the next memslot forward one and update its index. */ > >>> + mslots[i] = mslots[i + 1]; > >>> + slots->id_to_index[mslots[i].id] = i; > >>> + } > >>> + return i; > >>> +} > >>> + > >>> +static inline int kvm_shift_memslots_back(struct kvm_memslots *slots, > >>> + struct kvm_memory_slot *new, > >>> + int start) > >> > >> This new implementation of the insertion sort loses the comments that > >> were there in the old one. Please keep them as function comments. > > > > I assume you're talking about this blurb in particular? > > > > * The ">=" is needed when creating a slot with base_gfn == 0, > > * so that it moves before all those with base_gfn == npages == 0. > > Yes, well all of the comments. You can also keep them in the caller, as > you prefer. The primary function comment is still there, the only other comment that I dropped was the second half of the above comment: * * On the other hand, if new->npages is zero, the above loop has * already left i pointing to the beginning of the empty part of * mslots, and the ">=" would move the hole backwards in this * case---which is wrong. So skip the loop when deleting a slot. */ Which doesn't carry forward very well. Is there another comment I'm overlooking? Anyways, I'm not at all opposed to adding comments, just want to make sure I'm not forgetting something. If it's ok with you, I'll comment the code and/or functions and reply here to refine them without having to respin the whole series. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel