[email protected] (Eric Van Hensbergen) wrote:
>
> Subject: [PATCH] v9fs: add readpage support
>
> v9fs mmap support was originally removed from v9fs at Al Viro's request,
> but recently there have been requests from folks who want readpage
> functionality (primarily to enable execution of files mounted via 9P).
> This patch adds readpage support (but not writepage which contained most of
> the objectionable code). It passes FSX (and other regressions) so it
> should be relatively safe.
>
> +
> +static int v9fs_vfs_readpage(struct file *filp, struct page *page)
> +{
> + char *buffer = NULL;
> + int retval = -EIO;
> + loff_t offset = page_offset(page);
> + int count = PAGE_CACHE_SIZE;
> + struct inode *inode = filp->f_dentry->d_inode;
> + struct v9fs_session_info *v9ses = v9fs_inode2v9ses(inode);
> + int rsize = v9ses->maxdata - V9FS_IOHDRSZ;
> + struct v9fs_fid *v9f = filp->private_data;
> + struct v9fs_fcall *fcall = NULL;
> + int fid = v9f->fid;
> + int total = 0;
> + int result = 0;
> +
> + buffer = kmap(page);
> + do {
> + if (count < rsize)
> + rsize = count;
> +
> + result = v9fs_t_read(v9ses, fid, offset, rsize, &fcall);
> +
> + if (result < 0) {
> + printk(KERN_ERR "v9fs_t_read returned %d\n",
> + result);
> +
> + kfree(fcall);
> + goto UnmapAndUnlock;
> + } else
> + offset += result;
> +
> + memcpy(buffer, fcall->params.rread.data, result);
> +
> + count -= result;
> + buffer += result;
> + total += result;
> +
> + kfree(fcall);
Minor thing: from my reading of v9fs_mux_rpc() there's potential for a
double-kfree here. Either v9fs_mux_rpc() needs to be changed to
unambiguously zero out *rcall (or, better, v9fs_t_read does it) or you need
to zero fcall on each go around the loop.
> + if (result < rsize)
> + break;
> + } while (count);
> +
> + memset(buffer, 0, count);
> + flush_dcache_page(page);
> + SetPageUptodate(page);
if (result < rsize), is the page really up to date?
> + retval = 0;
> +
> + UnmapAndUnlock:
> + kunmap(page);
eww, do you really indent labels like that?
> diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
> index 6852f0e..feddc5c 100644
> --- a/fs/9p/vfs_file.c
> +++ b/fs/9p/vfs_file.c
> @@ -289,6 +289,8 @@ v9fs_file_write(struct file *filp, const
> total += result;
> } while (count);
>
> + invalidate_inode_pages2(inode->i_mapping);
> +
> return total;
> }
That's a really scary function you have there. Can you explain the
thinking behind its use here? What are we trying to achieve?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
