diff mbox series

[RFC,2/2] KVM: guest_memfd: use filemap_grab_folios in write

Message ID 20250110154659.95464-3-kalyazin@amazon.com (mailing list archive)
State New
Headers show
Series mm: filemap: add filemap_grab_folios | expand

Commit Message

Nikita Kalyazin Jan. 10, 2025, 3:46 p.m. UTC 
The write syscall on guest_memfd makes use of filemap_grab_folios to
grab folios in batches.  This speeds up population by 8.3% due to the
reduction in locking and tree walking when adding folios to the
pagecache.

Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
---
 virt/kvm/guest_memfd.c | 176 +++++++++++++++++++++++++++++++++--------
 1 file changed, 143 insertions(+), 33 deletions(-)

Comments

Mike Day Jan. 10, 2025, 9:08 p.m. UTC | #1 
On 1/10/25 09:46, Nikita Kalyazin wrote:
> The write syscall on guest_memfd makes use of filemap_grab_folios to
> grab folios in batches.  This speeds up population by 8.3% due to the
> reduction in locking and tree walking when adding folios to the
> pagecache.
> 
> Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
> ---
>   virt/kvm/guest_memfd.c | 176 +++++++++++++++++++++++++++++++++--------
>   1 file changed, 143 insertions(+), 33 deletions(-)
> 
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index e80566ef56e9..ccfadc3a7389 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -102,17 +102,134 @@ static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index)
>   	return filemap_grab_folio(inode->i_mapping, index);
>   }
>   
> +/*
> + * Returns locked folios on success.  The caller is responsible for
> + * setting the up-to-date flag before the memory is mapped into the guest.
> + * There is no backing storage for the memory, so the folios will remain
> + * up-to-date until they're removed.
> + *
> + * Ignore accessed, referenced, and dirty flags.  The memory is
> + * unevictable and there is no storage to write back to.
> + */
> +static int kvm_gmem_get_folios(struct inode *inode, pgoff_t index,
> +			       struct folio **folios, int num)
> +{
> +	return filemap_grab_folios(inode->i_mapping, index, folios, num);
> +}
> +
>   #if defined(CONFIG_KVM_GENERIC_PRIVATE_MEM) && !defined(CONFIG_KVM_AMD_SEV)
> +static int kvm_kmem_gmem_write_inner(struct inode *inode, pgoff_t index,
> +				     const void __user *buf,
> +                                     struct folio **folios, int num)
> +{
> +	int ret, i, num_grabbed, num_written;
> +
> +	num_grabbed = kvm_gmem_get_folios(inode, index, folios, num);
> +	if (num_grabbed < 0)
> +		return num_grabbed;
> +
> +	for (i = 0; i < num_grabbed; i++) {
> +		struct folio *folio = folios[i];
> +		void *vaddr;
> +
> +		if (folio_test_hwpoison(folio)) {
> +			folio_unlock(folio);
> +			folio_put(folio);
> +			ret = -EFAULT;
> +			break;
> +		}
> +
> +		if (folio_test_uptodate(folio)) {
> +			folio_unlock(folio);
> +			folio_put(folio);
> +			ret = -ENOSPC;
> +			break;
> +		}
> +
> +		folio_unlock(folio);
> +
> +		vaddr = kmap_local_folio(folio, 0);
> +		ret = copy_from_user(vaddr, buf + (i << PAGE_SHIFT), PAGE_SIZE);
> +		if (ret)
> +			ret = -EINVAL;
> +		kunmap_local(vaddr);
> +
> +		if (ret) {
> +			folio_put(folio);
> +			break;
> +		} else {
> +			kvm_gmem_mark_prepared(folio);
> +			folio_put(folio);
> +		}
> +	}
> +
> +	num_written = i;
> +
> +	for (i = num_written; i < num_grabbed; i++) {
> +		folio_unlock(folios[i]);
> +		folio_put(folios[i]);
> +	}
> +
> +	return num_written ?: ret;
> +}
> +
> +static struct folio *kvm_kmem_gmem_write_folio(struct inode *inode, pgoff_t index,
> +					       const char __user *buf)
> 

This could probably be rewritten as:

	struct folio *p_folio;
	int ret;

	ret = kvm_kmem_gmem_write_inner(inode, index, buf, &p_folio, 1);
	
	if (ret == 1)
		return p_folio;
	else
		return ERR_PTR(ret);

Would remove a few lines of duplicated code and use only one prototype.

Mike

+{
> +	struct folio *folio;
> +	void *vaddr;
> +	int ret = 0;
> +
> +	folio = kvm_gmem_get_folio(inode, index);
> +	if (IS_ERR(folio))
> +		return ERR_PTR(-EFAULT);
> +
> +	if (folio_test_hwpoison(folio)) {
> +		ret = -EFAULT;
> +		goto out_unlock_put;
> +	}
> +
> +	if (folio_test_uptodate(folio)) {
> +		ret = -ENOSPC;
> +		goto out_unlock_put;
> +	}
> +
> +	folio_unlock(folio);
> +
> +	vaddr = kmap_local_folio(folio, 0);
> +	ret = copy_from_user(vaddr, buf, PAGE_SIZE);
> +	if (ret)
> +		ret = -EINVAL;
> +	kunmap_local(vaddr);
> +
> +	if (ret) {
> +		folio_put(folio);
> +		kvm_gmem_mark_prepared(folio);
> +		goto out_err;
> +	}
> +
> +	folio_put(folio);
> +
> +	return folio;
> +
> +out_unlock_put:
> +	folio_unlock(folio);
> +	folio_put(folio);
> +out_err:
> +	return ERR_PTR(ret);
> +}
> +
>   static ssize_t kvm_kmem_gmem_write(struct file *file, const char __user *buf,
>   				   size_t count, loff_t *offset)
>   {
> +	struct inode *inode = file_inode(file);
> +	int ret = 0, batch_size = FILEMAP_GET_FOLIOS_BATCH_SIZE;
>   	pgoff_t start, end, index;
> -	ssize_t ret = 0;
>   
>   	if (!PAGE_ALIGNED(*offset) || !PAGE_ALIGNED(count))
>   		return -EINVAL;
>   
> -	if (*offset + count > i_size_read(file_inode(file)))
> +	if (*offset + count > i_size_read(inode))
>   		return -EINVAL;
>   
>   	if (!buf)
> @@ -123,9 +240,8 @@ static ssize_t kvm_kmem_gmem_write(struct file *file, const char __user *buf,
>   
>   	filemap_invalidate_lock(file->f_mapping);
>   
> -	for (index = start; index < end; ) {
> -		struct folio *folio;
> -		void *vaddr;
> +	for (index = start; index + batch_size - 1 < end; ) {
> +		struct folio *folios[FILEMAP_GET_FOLIOS_BATCH_SIZE] = { NULL };
>   		pgoff_t buf_offset = (index - start) << PAGE_SHIFT;
>   
>   		if (signal_pending(current)) {
> @@ -133,46 +249,40 @@ static ssize_t kvm_kmem_gmem_write(struct file *file, const char __user *buf,
>   			goto out;
>   		}
>   
> -		folio = kvm_gmem_get_folio(file_inode(file), index);
> -		if (IS_ERR(folio)) {
> -			ret = -EFAULT;
> +		ret = kvm_kmem_gmem_write_inner(inode, index, buf + buf_offset, folios, batch_size);
> +		if (ret < 0)
>   			goto out;
> -		}
>   
> -		if (folio_test_hwpoison(folio)) {
> -			folio_unlock(folio);
> -			folio_put(folio);
> -			ret = -EFAULT;
> +		index += ret;
> +		if (ret < batch_size)
> +			break;
> +	}
> +
> +	for (; index < end; index++) {
> +		struct folio *folio;
> +		pgoff_t buf_offset = (index - start) << PAGE_SHIFT;
> +
> +		if (signal_pending(current)) {
> +			ret = -EINTR;
>   			goto out;
>   		}
>   
> -		if (folio_test_uptodate(folio)) {
> -			folio_unlock(folio);
> -			folio_put(folio);
> -			ret = -ENOSPC;
> +		folio = kvm_kmem_gmem_write_folio(inode, index,
> +						  buf + buf_offset);
> +		if (IS_ERR(folio)) {
> +			ret = PTR_ERR(folio);
>   			goto out;
>   		}
> -
> -		folio_unlock(folio);
> -
> -		vaddr = kmap_local_folio(folio, 0);
> -		ret = copy_from_user(vaddr, buf + buf_offset, PAGE_SIZE);
> -		if (ret)
> -			ret = -EINVAL;
> -		kunmap_local(vaddr);
> -
> -		kvm_gmem_mark_prepared(folio);
> -		folio_put(folio);
> -
> -		index = folio_next_index(folio);
> -		*offset += PAGE_SIZE;
>   	}
>   
>   out:
>   	filemap_invalidate_unlock(file->f_mapping);
> +	if (index > start) {
> +		*offset += (index - start) << PAGE_SHIFT;
> +		return (index - start) << PAGE_SHIFT;
> +	}
>   
> -	return ret && start == (*offset >> PAGE_SHIFT) ?
> -		ret : *offset - (start << PAGE_SHIFT);
> +	return ret;
>   }
>   #endif
>
diff mbox series

Patch

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index e80566ef56e9..ccfadc3a7389 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -102,17 +102,134 @@  static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index)
 	return filemap_grab_folio(inode->i_mapping, index);
 }
 
+/*
+ * Returns locked folios on success.  The caller is responsible for
+ * setting the up-to-date flag before the memory is mapped into the guest.
+ * There is no backing storage for the memory, so the folios will remain
+ * up-to-date until they're removed.
+ *
+ * Ignore accessed, referenced, and dirty flags.  The memory is
+ * unevictable and there is no storage to write back to.
+ */
+static int kvm_gmem_get_folios(struct inode *inode, pgoff_t index,
+			       struct folio **folios, int num)
+{
+	return filemap_grab_folios(inode->i_mapping, index, folios, num);
+}
+
 #if defined(CONFIG_KVM_GENERIC_PRIVATE_MEM) && !defined(CONFIG_KVM_AMD_SEV)
+static int kvm_kmem_gmem_write_inner(struct inode *inode, pgoff_t index,
+				     const void __user *buf,
+                                     struct folio **folios, int num)
+{
+	int ret, i, num_grabbed, num_written;
+
+	num_grabbed = kvm_gmem_get_folios(inode, index, folios, num);
+	if (num_grabbed < 0)
+		return num_grabbed;
+
+	for (i = 0; i < num_grabbed; i++) {
+		struct folio *folio = folios[i];
+		void *vaddr;
+
+		if (folio_test_hwpoison(folio)) {
+			folio_unlock(folio);
+			folio_put(folio);
+			ret = -EFAULT;
+			break;
+		}
+
+		if (folio_test_uptodate(folio)) {
+			folio_unlock(folio);
+			folio_put(folio);
+			ret = -ENOSPC;
+			break;
+		}
+
+		folio_unlock(folio);
+
+		vaddr = kmap_local_folio(folio, 0);
+		ret = copy_from_user(vaddr, buf + (i << PAGE_SHIFT), PAGE_SIZE);
+		if (ret)
+			ret = -EINVAL;
+		kunmap_local(vaddr);
+
+		if (ret) {
+			folio_put(folio);
+			break;
+		} else {
+			kvm_gmem_mark_prepared(folio);
+			folio_put(folio);
+		}
+	}
+
+	num_written = i;
+
+	for (i = num_written; i < num_grabbed; i++) {
+		folio_unlock(folios[i]);
+		folio_put(folios[i]);
+	}
+
+	return num_written ?: ret;
+}
+
+static struct folio *kvm_kmem_gmem_write_folio(struct inode *inode, pgoff_t index,
+					       const char __user *buf)
+{
+	struct folio *folio;
+	void *vaddr;
+	int ret = 0;
+
+	folio = kvm_gmem_get_folio(inode, index);
+	if (IS_ERR(folio))
+		return ERR_PTR(-EFAULT);
+
+	if (folio_test_hwpoison(folio)) {
+		ret = -EFAULT;
+		goto out_unlock_put;
+	}
+
+	if (folio_test_uptodate(folio)) {
+		ret = -ENOSPC;
+		goto out_unlock_put;
+	}
+
+	folio_unlock(folio);
+
+	vaddr = kmap_local_folio(folio, 0);
+	ret = copy_from_user(vaddr, buf, PAGE_SIZE);
+	if (ret)
+		ret = -EINVAL;
+	kunmap_local(vaddr);
+
+	if (ret) {
+		folio_put(folio);
+		kvm_gmem_mark_prepared(folio);
+		goto out_err;
+	}
+
+	folio_put(folio);
+
+	return folio;
+
+out_unlock_put:
+	folio_unlock(folio);
+	folio_put(folio);
+out_err:
+	return ERR_PTR(ret);
+}
+
 static ssize_t kvm_kmem_gmem_write(struct file *file, const char __user *buf,
 				   size_t count, loff_t *offset)
 {
+	struct inode *inode = file_inode(file);
+	int ret = 0, batch_size = FILEMAP_GET_FOLIOS_BATCH_SIZE;
 	pgoff_t start, end, index;
-	ssize_t ret = 0;
 
 	if (!PAGE_ALIGNED(*offset) || !PAGE_ALIGNED(count))
 		return -EINVAL;
 
-	if (*offset + count > i_size_read(file_inode(file)))
+	if (*offset + count > i_size_read(inode))
 		return -EINVAL;
 
 	if (!buf)
@@ -123,9 +240,8 @@  static ssize_t kvm_kmem_gmem_write(struct file *file, const char __user *buf,
 
 	filemap_invalidate_lock(file->f_mapping);
 
-	for (index = start; index < end; ) {
-		struct folio *folio;
-		void *vaddr;
+	for (index = start; index + batch_size - 1 < end; ) {
+		struct folio *folios[FILEMAP_GET_FOLIOS_BATCH_SIZE] = { NULL };
 		pgoff_t buf_offset = (index - start) << PAGE_SHIFT;
 
 		if (signal_pending(current)) {
@@ -133,46 +249,40 @@  static ssize_t kvm_kmem_gmem_write(struct file *file, const char __user *buf,
 			goto out;
 		}
 
-		folio = kvm_gmem_get_folio(file_inode(file), index);
-		if (IS_ERR(folio)) {
-			ret = -EFAULT;
+		ret = kvm_kmem_gmem_write_inner(inode, index, buf + buf_offset, folios, batch_size);
+		if (ret < 0)
 			goto out;
-		}
 
-		if (folio_test_hwpoison(folio)) {
-			folio_unlock(folio);
-			folio_put(folio);
-			ret = -EFAULT;
+		index += ret;
+		if (ret < batch_size)
+			break;
+	}
+
+	for (; index < end; index++) {
+		struct folio *folio;
+		pgoff_t buf_offset = (index - start) << PAGE_SHIFT;
+
+		if (signal_pending(current)) {
+			ret = -EINTR;
 			goto out;
 		}
 
-		if (folio_test_uptodate(folio)) {
-			folio_unlock(folio);
-			folio_put(folio);
-			ret = -ENOSPC;
+		folio = kvm_kmem_gmem_write_folio(inode, index,
+						  buf + buf_offset);
+		if (IS_ERR(folio)) {
+			ret = PTR_ERR(folio);
 			goto out;
 		}
-
-		folio_unlock(folio);
-
-		vaddr = kmap_local_folio(folio, 0);
-		ret = copy_from_user(vaddr, buf + buf_offset, PAGE_SIZE);
-		if (ret)
-			ret = -EINVAL;
-		kunmap_local(vaddr);
-
-		kvm_gmem_mark_prepared(folio);
-		folio_put(folio);
-
-		index = folio_next_index(folio);
-		*offset += PAGE_SIZE;
 	}
 
 out:
 	filemap_invalidate_unlock(file->f_mapping);
+	if (index > start) {
+		*offset += (index - start) << PAGE_SHIFT;
+		return (index - start) << PAGE_SHIFT;
+	}
 
-	return ret && start == (*offset >> PAGE_SHIFT) ?
-		ret : *offset - (start << PAGE_SHIFT);
+	return ret;
 }
 #endif