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@xxxxxxxxxx>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
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 {
