Re: [RFC PATCH 18/21] KVM: x86: Split huge boundary leafs before private to shared conversion

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Mon, 2025-05-12 at 10:25 +0800, Yan Zhao wrote:
> > FALLOC_FL_PUNCH_HOLE demotion failure doesn't look like it is addressed in
> > this
> Hmm, FALLOC_FL_PUNCH_HOLE demotion failure is handled in patch 19.

Oh, right you are.

> 
> > series. I noticed that mmu notifier failures are allowed to be handled by
> > blocking until success is possible, in most cases. KVM just doesn't need to
> > because it can't fail. We could think about doing retries for
> > FALLOC_FL_PUNCH_HOLE, while checking for signals. Or adding a ENOMEM error
> > code
> > to fallocate.
> In patch 19, FALLOC_FL_PUNCH_HOLE could return -ENOMEM.

Yes. It is not in the man pages, but looking this morning I see other fallocate
handlers are already returning it.

> 
> Returning -ENOMEM may be inevitable as we can't endlessly retry. So for
> simplicity, there's no retry in this series.

Ok, seems good.

> 
> 
> Besides that, do you think whether we need to conduct splittings before any
> unmap is invoked?
> 
> As in the patch log:
> "
> The downside of this approach is that although
> kvm_split_boundary_leafs()        
> is invoked before kvm_unmap_gfn_range() for each GFN range, the
> entire           
> conversion range may consist of several GFN ranges. If an out-of-
> memory          
> error occurs during the splitting of a GFN range, some previous GFN
> ranges       
> may have been successfully split and zapped, even though their
> page              
> attributes remain unchanged due to the splitting failure. This may not be
> a      
> big problem as the user can retry the ioctl to split and zap the
> full            
> range.
> "

If we ended up plumbing the zapping errors all the way through the MMU, it
probably would be simpler to do it during the unmap. Of course callers would
have to be aware the range may be half unmapped on error. I think the way you
have it is nice for not churning the MMU though.

But for the case of having to retry the split and walking the mirror EPT range
again, it's a rare case and not worth optimizing for. Let's not consider it
much.