Yan Zhao <yan.y.zhao@xxxxxxxxx> writes:
> On Wed, May 14, 2025 at 04:41:41PM -0700, Ackerley Tng wrote:
>> +static enum shareability kvm_gmem_shareability_get(struct inode *inode,
>> + pgoff_t index)
>> +{
>> + struct maple_tree *mt;
>> + void *entry;
>> +
>> + mt = &kvm_gmem_private(inode)->shareability;
>> + entry = mtree_load(mt, index);
>> + WARN(!entry,
>> + "Shareability should always be defined for all indices in inode.");
>> +
>> + return xa_to_value(entry);
>> +}
>> +
> Hi Ackerley,
>
> Not sure if it's a known issue. Just want to let you know in case you're unaware.
>
Thanks for informing me, and thanks for the analysis :)
> During a test to repeatedly launching/destroying TDs, I encountered a warning
> from kvm_gmem_shareability_get() (see the attached log at the bottom).
> The reproducing rate is 1 in every 20-100 times of launching TD.
>
> After some analysis, I found that the warning was produced by
> kvm_gmem_shareability_get() when it's called from kvm_gmem_is_private(), which
> is not protected by any locks.
>
> I can get rid of the warning by either fix 1 or fix 2 below.
> (I prefer fix 1 though :))
>
> fix 1:
>
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index e78fbebf4f53..136d46c5b2ab 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -2024,7 +2024,7 @@ static struct inode *kvm_gmem_inode_make_secure_inode(const char *name,
>
> #ifdef CONFIG_KVM_GMEM_SHARED_MEM
> if (flags & GUEST_MEMFD_FLAG_SUPPORT_SHARED) {
> - mt_init(&private->shareability);
> + mt_init_flags(&private->shareability, MT_FLAGS_USE_RCU);
>
> err = kvm_gmem_shareability_setup(private, size, flags);
> if (err)
>
Not sure about the version of the conversion patch series that you're
using, in the version I'm preparing, I'm using
filemap_invalidate_lock_shared() to guard shareability
reads. filemap_invalidate_lock() is held during shareability updates, so
I think this issue should be fixed.
Please let me know if you're still seeing this issue in the next series
(coming soon). Thank you!
>
> fix 2:
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index e78fbebf4f53..9a4518104d56 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -171,7 +171,9 @@ static enum shareability kvm_gmem_shareability_get(struct inode *inode,
> void *entry;
>
> mt = &kvm_gmem_private(inode)->shareability;
> + mtree_lock(mt);
> entry = mtree_load(mt, index);
> + mtree_unlock(mt);
> WARN(!entry,
> "Shareability should always be defined for all indices in inode.");
>
>
> Thanks
> Yan
>
>
> [...snip...]
>
