Re: [PATCH] 9p: add write-cache support to loose cache mode (take 2)

[Josef Sipek <jsipek@fsl.cs.sunysb.edu>]

On Thu, Feb 15, 2007 at 08:44:19AM -0600, Eric Van Hensbergen wrote:
...
> diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
> index bed48fa..d43d6fb 100644
> --- a/fs/9p/vfs_addr.c
> +++ b/fs/9p/vfs_addr.c
> @@ -6,6 +6,9 @@
>   *  Copyright (C) 2005 by Eric Van Hensbergen <ericvh@gmail.com>
>   *  Copyright (C) 2002 by Ron Minnich <rminnich@lanl.gov>
>   *
> + *  Some operations based on code from fs/cifs/file.c
> + *  Copyright (C) International Business Machines  Corp., 2002,2003
> + *
>   *  This program is free software; you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License version 2
>   *  as published by the Free Software Foundation.
> @@ -41,14 +44,14 @@
>  #include "fid.h"
>  
>  /**
> - * v9fs_vfs_readpage - read an entire page in from 9P
> + * v9fs_vfs_readpage_worker - read an entire page in from 9P
>   *
>   * @file: file being read
>   * @page: structure to page
>   *
>   */
>  
> -static int v9fs_vfs_readpage(struct file *filp, struct page *page)
> +static int v9fs_vfs_readpage_worker(struct file *filp, struct page *page)
>  {
>  	char *buffer = NULL;
>  	int retval = -EIO;
> @@ -63,23 +66,21 @@ static int v9fs_vfs_readpage(struct file *filp, struct page *page)
>  	int total = 0;
>  	int result = 0;
>  
> -	dprintk(DEBUG_VFS, "\n");
> -
> +	page_cache_get(page);
>  	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);
> +			printk(KERN_ERR "v9fs_t_read returned %d\n", result);
>  
>  			kfree(fcall);
> -			goto UnmapAndUnlock;
> -		} else
> +			goto io_error;
> +		} else {
>  			offset += result;
> +		}

There are a bunch of places where you have unneeded braces.

>  
>  		memcpy(buffer, fcall->params.rread.data, result);
>  
> @@ -98,12 +99,178 @@ static int v9fs_vfs_readpage(struct file *filp, struct page *page)
>  	SetPageUptodate(page);
>  	retval = 0;
>  
> -UnmapAndUnlock:
> +      io_error:

Indentation...

> +	kunmap(page);
> +	page_cache_release(page);
> +	return retval;
> +}
> +
> +/**
> + * v9fs_prepare_write - prepare for mmap or page-cache I/O
> + *
> + * @file: file being read
> + * @page: structure to page
> + * @from: starting offset
> + * @to: ending offset
> + *
> + */
> +
> +int v9fs_prepare_write(struct file *file, struct page *page,
> +		       unsigned from, unsigned to)
> +{
> +	if(!PageUptodate(page)) {
> +		if (to - from != PAGE_CACHE_SIZE) {
> +			void *kaddr = kmap_atomic(page, KM_USER0);
> +			memset(kaddr, 0, from);
> +			memset(kaddr + to, 0, PAGE_CACHE_SIZE - to);
> +			flush_dcache_page(page);
> +			kunmap_atomic(kaddr, KM_USER0);
> +		}
> +		if ((file->f_flags & O_ACCMODE) != O_WRONLY) {
> +			v9fs_vfs_readpage_worker(file, page);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * v9fs_commit_write - prepare for mmap or page-cache I/O
> + *
> + * @file: file being read
> + * @page: structure to page
> + * @offset: starting offset
> + * @to: ending offset
> + *
> + * TODO: Perhaps I should just do the write here?
> + */
> +
> +int v9fs_commit_write(struct file *file, struct page *page,
> +		       unsigned offset, unsigned to)
> +{
> +        struct inode *inode = page->mapping->host;
> +        loff_t pos = ((loff_t)page->index << PAGE_CACHE_SHIFT) + to;

whitespace

> +
> +	dprintk(DEBUG_VFS, "\n");
> +
> +	if(!PageUptodate(page))
> +		SetPageUptodate(page);
> +
> +        /*
> +	 * No need to use i_size_read() here, the i_size
> +	 * cannot change under us because we hold the i_mutex.
> +	 *
> +	 */
> +        if (pos > inode->i_size) {
> +                i_size_write(inode, pos);
> +		/* need to adjust inode on remote */
> +	}
> +        set_page_dirty(page);
> +	dprintk(DEBUG_CURRENT, "inode->size: %llx\n",inode->i_size);
> +
> +	return 0;
> +}
...

Josef "Jeff" Sipek.

-- 
Computer Science is no more about computers than astronomy is about
telescopes.
		- Edsger Dijkstra