Re: [PATCH] hfsplus: fix "unused node is not erased" error

[Vyacheslav Dubeyko <slava@dubeyko.com>]

On Tue, 2014-05-20 at 19:44 +0200, Sergei Antonov wrote:

[snip]
>  
> -int hfsplus_file_extend(struct inode *inode)
> +int hfsplus_file_extend(struct inode *inode, bool zeroout)
>  {
>  	struct super_block *sb = inode->i_sb;
>  	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
> @@ -463,6 +463,12 @@ int hfsplus_file_extend(struct inode *inode)
>  		}
>  	}
>  
> +	if (zeroout) {
> +		res = sb_issue_zeroout(sb, start, len, GFP_NOFS);

As I can see, sb_issue_zeroout() initiate request for write. But
previously the hfsplus_file_extend() operated by page cache only during
file extending. From one point of view, we can fail during operation of
file extending but, anyway, we will zero out blocks by means of writing.
>From another point of view, prepared pages are returned as tree's nodes
for filling by some data and, finally, it will be written on volume as a
result of node creation.

So, I think that it makes sense to zero out namely prepared pages but
not to initiate request for write via sb_issue_zeroout().

> +		if (res)
> +			goto out;
> +	}
> +
>  	hfs_dbg(EXTENT, "extend %lu: %u,%u\n", inode->i_ino, start, len);
>  
>  	if (hip->alloc_blocks <= hip->first_blocks) {
> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> index 83dc292..3c872b2 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -417,6 +417,7 @@ void hfs_bnode_free(struct hfs_bnode *);
>  struct hfs_bnode *hfs_bnode_create(struct hfs_btree *, u32);
>  void hfs_bnode_get(struct hfs_bnode *);
>  void hfs_bnode_put(struct hfs_bnode *);
> +bool hfs_bnode_need_zeroout(struct hfs_btree *);
>  
>  /* brec.c */
>  u16 hfs_brec_lenoff(struct hfs_bnode *, u16, u16 *);
> @@ -463,7 +464,7 @@ int hfsplus_ext_write_extent(struct inode *);
>  int hfsplus_get_block(struct inode *, sector_t, struct buffer_head *, int);
>  int hfsplus_free_fork(struct super_block *, u32,
>  		struct hfsplus_fork_raw *, int);
> -int hfsplus_file_extend(struct inode *);
> +int hfsplus_file_extend(struct inode *, bool zeroout);

I think that it doesn't make sense to keep name of second argument here.

Thanks,
Vyacheslav Dubeyko.