Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 24 Nov 2020 16:15:38 +0000
From:   Wei Liu <wei.liu@kernel.org>
To:     Nuno Das Neves <nunodasneves@linux.microsoft.com>
Cc:     linux-hyperv@vger.kernel.org,
        virtualization@lists.linux-foundation.org,
        linux-kernel@vger.kernel.org, mikelley@microsoft.com,
        viremana@linux.microsoft.com, sunilmut@microsoft.com,
        wei.liu@kernel.org, ligrassi@microsoft.com, kys@microsoft.com
Subject: Re: [RFC PATCH 12/18] virt/mshv: run vp ioctl and isr
Message-ID: <20201124161538.4d6iqbwnxzwhmo4e@liuwe-devbox-debian-v2>
References: <1605918637-12192-1-git-send-email-nunodasneves@linux.microsoft.com>
 <1605918637-12192-13-git-send-email-nunodasneves@linux.microsoft.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1605918637-12192-13-git-send-email-nunodasneves@linux.microsoft.com>
User-Agent: NeoMutt/20180716
Precedence: bulk

On Fri, Nov 20, 2020 at 04:30:31PM -0800, Nuno Das Neves wrote:
[...]
> diff --git a/virt/mshv/mshv_main.c b/virt/mshv/mshv_main.c
> index c9445d2edb37..7ddb66d260ce 100644
> --- a/virt/mshv/mshv_main.c
> +++ b/virt/mshv/mshv_main.c
> @@ -17,6 +17,7 @@
>  #include <linux/mm.h>
>  #include <linux/io.h>
>  #include <linux/cpuhotplug.h>
> +#include <linux/random.h>
>  #include <linux/mshv.h>
>  #include <asm/mshyperv.h>
>  
> @@ -498,6 +499,240 @@ hv_call_set_vp_registers(u32 vp_index,
>  	return -hv_status_to_errno(status);
>  }
>  
> +static void
> +mshv_isr(void)
> +{
[...]
> +
> +	/* Hold this lock for the rest of the isr, because the partition could
> +	 * be released anytime.
> +	 * e.g. the MSHV_RUN_VP thread could wake on another cpu; it could
> +	 * release the partition unless we hold this!
> +	 */
> +	spin_lock_irqsave(&mshv.partitions.lock, flags);
> +

This should be switched to rwlock variant, otherwise vcpus can't run
concurrently.

You will take the read lock and only the ioctl that changes the list
will need to take the write lock.

There may be better and cheaper primitives than rwlock. Not sure if RCU
can be used in this context.

Wei.