Re: [osd-dev] [PATCH] exofs: exofs_file_fsync correctness

[Boaz Harrosh <bharrosh@panasas.com>]

On 06/01/2010 06:30 PM, Boaz Harrosh wrote:
> 
> As per Christoph advise: no need to call filemap_write_and_wait().
> In exofs all metadata is at the inode so just writing the pages
> and inode is all is needed. fsync implies this must be done
> synchronously.
> 
> FIXME: remove the sb_sync and fix that sb_update better.
> 
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> ---
>  fs/exofs/file.c |   21 ++++++++++++---------
>  1 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/exofs/file.c b/fs/exofs/file.c
> index f9bfe2b..54bb17d 100644
> --- a/fs/exofs/file.c
> +++ b/fs/exofs/file.c
> @@ -30,9 +30,6 @@
>   * along with exofs; if not, write to the Free Software
>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>   */
> -
> -#include <linux/buffer_head.h>
> -
>  #include "exofs.h"
>  
>  static int exofs_release_file(struct inode *inode, struct file *filp)
> @@ -40,19 +37,25 @@ static int exofs_release_file(struct inode *inode, struct file *filp)
>  	return 0;
>  }
>  
> +/* exofs_file_fsync - flush the inode to disk
> + *
> + *   @datasync is not used. All metadata is written in one place regardless.
> + *   the writeout is synchronous
> + */
>  static int exofs_file_fsync(struct file *filp, int datasync)
>  {
>  	int ret;
>  	struct address_space *mapping = filp->f_mapping;
>  	struct inode *inode = mapping->host;
>  	struct super_block *sb;
> +	struct writeback_control wbc = {
> +		.sync_mode = WB_SYNC_ALL,
> +		.nr_to_write = LONG_MAX,
> +		.range_start = 0,
> +		.range_end = LLONG_MAX,

Christoph what do you say should I do

+		.range_end = i_size_read(inode);
here to cap the sync up to current size?

Boaz
> +	};
>  
> -	ret = filemap_write_and_wait(mapping);
> -	if (ret)
> -		return ret;
> -
> -	/* sync the inode attributes */
> -	ret = write_inode_now(inode, 1);
> +	ret = sync_inode(inode, &wbc);
>  
>  	/* This is a good place to write the sb */
>  	/* TODO: Sechedule an sb-sync on create */