diff mbox series

[25/30] vboxsf: Convert vboxsf_read_folio() to use a folio

Message ID 20240420025029.2166544-26-willy@infradead.org (mailing list archive)
State New, archived
Headers show
Series Remove PG_error flag | expand

Commit Message

Matthew Wilcox April 20, 2024, 2:50 a.m. UTC
Remove conversion to a page and use folio APIs throughout.  This includes
a removal of setting the error flag as nobody checks the error flag on
vboxsf folios.  This does not include large folio support as we would
have to map each page individually.

Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/vboxsf/file.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

Comments

Hans de Goede April 22, 2024, 10:59 a.m. UTC | #1
Hi Matthew,

On 4/20/24 4:50 AM, Matthew Wilcox (Oracle) wrote:
> Remove conversion to a page and use folio APIs throughout.  This includes
> a removal of setting the error flag as nobody checks the error flag on
> vboxsf folios.  This does not include large folio support as we would
> have to map each page individually.
> 
> Cc: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/vboxsf/file.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/vboxsf/file.c b/fs/vboxsf/file.c
> index 118dedef8ebe..e149158b105d 100644
> --- a/fs/vboxsf/file.c
> +++ b/fs/vboxsf/file.c
> @@ -228,26 +228,19 @@ const struct inode_operations vboxsf_reg_iops = {
>  
>  static int vboxsf_read_folio(struct file *file, struct folio *folio)
>  {
> -	struct page *page = &folio->page;
>  	struct vboxsf_handle *sf_handle = file->private_data;
> -	loff_t off = page_offset(page);
> +	loff_t off = folio_pos(folio);
>  	u32 nread = PAGE_SIZE;
>  	u8 *buf;
>  	int err;
>  
> -	buf = kmap(page);
> +	buf = kmap_local_folio(folio, 0);
>  
>  	err = vboxsf_read(sf_handle->root, sf_handle->handle, off, &nread, buf);
> -	if (err == 0) {
> -		memset(&buf[nread], 0, PAGE_SIZE - nread);
> -		flush_dcache_page(page);
> -		SetPageUptodate(page);
> -	} else {
> -		SetPageError(page);
> -	}
> +	buf = folio_zero_tail(folio, nread, buf);
>  
> -	kunmap(page);
> -	unlock_page(page);
> +	kunmap_local(buf);
> +	folio_end_read(folio, err == 0);
>  	return err;
>  }
>  

Thanks you for the patch.

I have this a test spin, but I got all 0 content for the kernel's README when trying
to read that from a directory shared through vboxsf.

I came up with the following fix for this, which I assume is the correct fix,
but please check:

--- a/fs/vboxsf/file.c
+++ b/fs/vboxsf/file.c
@@ -237,7 +237,7 @@ static int vboxsf_read_folio(struct file *file, struct folio *folio)
 	buf = kmap_local_folio(folio, 0);
 
 	err = vboxsf_read(sf_handle->root, sf_handle->handle, off, &nread, buf);
-	buf = folio_zero_tail(folio, nread, buf);
+	buf = folio_zero_tail(folio, nread, buf + nread);
 
 	kunmap_local(buf);
 	folio_end_read(folio, err == 0);


With this fix squashed in, this looks good to me and you can add:

Tested-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Feel free to merge this together with the other folio patches from this series.

Regards,

Hans






> @@ -295,7 +288,6 @@ static int vboxsf_writepage(struct page *page, struct writeback_control *wbc)
>  	kref_put(&sf_handle->refcount, vboxsf_handle_release);
>  
>  	if (err == 0) {
> -		ClearPageError(page);
>  		/* mtime changed */
>  		sf_i->force_restat = 1;
>  	} else {
diff mbox series

Patch

diff --git a/fs/vboxsf/file.c b/fs/vboxsf/file.c
index 118dedef8ebe..e149158b105d 100644
--- a/fs/vboxsf/file.c
+++ b/fs/vboxsf/file.c
@@ -228,26 +228,19 @@  const struct inode_operations vboxsf_reg_iops = {
 
 static int vboxsf_read_folio(struct file *file, struct folio *folio)
 {
-	struct page *page = &folio->page;
 	struct vboxsf_handle *sf_handle = file->private_data;
-	loff_t off = page_offset(page);
+	loff_t off = folio_pos(folio);
 	u32 nread = PAGE_SIZE;
 	u8 *buf;
 	int err;
 
-	buf = kmap(page);
+	buf = kmap_local_folio(folio, 0);
 
 	err = vboxsf_read(sf_handle->root, sf_handle->handle, off, &nread, buf);
-	if (err == 0) {
-		memset(&buf[nread], 0, PAGE_SIZE - nread);
-		flush_dcache_page(page);
-		SetPageUptodate(page);
-	} else {
-		SetPageError(page);
-	}
+	buf = folio_zero_tail(folio, nread, buf);
 
-	kunmap(page);
-	unlock_page(page);
+	kunmap_local(buf);
+	folio_end_read(folio, err == 0);
 	return err;
 }
 
@@ -295,7 +288,6 @@  static int vboxsf_writepage(struct page *page, struct writeback_control *wbc)
 	kref_put(&sf_handle->refcount, vboxsf_handle_release);
 
 	if (err == 0) {
-		ClearPageError(page);
 		/* mtime changed */
 		sf_i->force_restat = 1;
 	} else {