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

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

[Hin-Tak Leung]

------------------------------
On Wed, May 21, 2014 5:40 PM BST Vyacheslav Dubeyko wrote:

>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.
>
>

I disagree - I think 
If you are expanding the function prototype, you could also give the first argument some meaningful name while you are at it? 

Hin-Tak

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

[Vyacheslav Dubeyko]

On Wed, 2014-05-21 at 21:12 +0100, Hin-Tak Leung wrote:

[snip]
> >> 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.
> >
> >
> 
> I disagree - I think 
> If you are expanding the function prototype, you could also give the first argument some meaningful name while you are at it? 
> 

To be honest, I don't quite follow your remark. I mean really simple
thing here. It makes sense to have here the prototype with names for
both arguments:

int hfsplus_file_extend(struct inode *inode, bool zeroout);

or without names of arguments:

int hfsplus_file_extend(struct inode *, bool);

Mixed declaration looks really weird for my taste. Moreover, most of
prototypes are declared without names of arguments.

With the best regards,
Vyacheslav Dubeyko.

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

[Andrew Morton]

On Thu, 22 May 2014 10:29:45 +0400 Vyacheslav Dubeyko <slava@dubeyko.com> wrote:

> On Wed, 2014-05-21 at 21:12 +0100, Hin-Tak Leung wrote:
> 
> [snip]
> > >> 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.
> > >
> > >
> > 
> > I disagree - I think 
> > If you are expanding the function prototype, you could also give the first argument some meaningful name while you are at it? 
> > 
> 
> To be honest, I don't quite follow your remark. I mean really simple
> thing here. It makes sense to have here the prototype with names for
> both arguments:
> 
> int hfsplus_file_extend(struct inode *inode, bool zeroout);
> 
> or without names of arguments:
> 
> int hfsplus_file_extend(struct inode *, bool);
> 
> Mixed declaration looks really weird for my taste. Moreover, most of
> prototypes are declared without names of arguments.
> 

Yes, mixed is weird.

I personally prefer that the names be included - if you ever actually
have to *read* one of these declarations it can be quite maddening when
they are omitted.

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

[Sergei Antonov]

On Thu, 22 May 2014, Andrew Morton wrote:

> On Thu, 22 May 2014 10:29:45 +0400 Vyacheslav Dubeyko <slava@dubeyko.com> wrote:
>
>> On Wed, 2014-05-21 at 21:12 +0100, Hin-Tak Leung wrote:
>>
>> [snip]
>>>>> 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.
>>>>
>>>>
>>>
>>> I disagree - I think
>>> If you are expanding the function prototype, you could also give the first argument some meaningful name while you are at it?
>>>
>>
>> To be honest, I don't quite follow your remark. I mean really simple
>> thing here. It makes sense to have here the prototype with names for
>> both arguments:
>>
>> int hfsplus_file_extend(struct inode *inode, bool zeroout);
>>
>> or without names of arguments:
>>
>> int hfsplus_file_extend(struct inode *, bool);
>>
>> Mixed declaration looks really weird for my taste. Moreover, most of
>> prototypes are declared without names of arguments.
>>
>
> Yes, mixed is weird.
>
> I personally prefer that the names be included - if you ever actually
> have to *read* one of these declarations it can be quite maddening when
> they are omitted.

There were several other mixed declarations, so I decided to fix the whole
hfsplus_fs.h file. The patch follows. It is supposed to be applied after
the patch being discussed in this thread.

---
Subject: [PATCH] hfsplus: coding style fix for declarations in hfsplus_fs.h

Some function declarations in hfsplus_fs.h were with argument names, some
without, and some were mixed. This patch adds argument names everywhere, sorts
functions in order they appear in .c files, and moves hfs_part_find() to a
proper section.

Auto-formatting was done with:
cfunctions *.c | indent -linux | sed "s| \* | \*|"

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Vyacheslav Dubeyko <slava@dubeyko.com>
Cc: Hin-Tak Leung <htl10@users.sourceforge.net>
Signed-off-by: Sergei Antonov <saproj@gmail.com>
---
  fs/hfsplus/hfsplus_fs.h | 208 +++++++++++++++++++++++++-----------------------
  1 file changed, 109 insertions(+), 99 deletions(-)

diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 3c872b2..f917fe4 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -369,129 +369,139 @@ typedef int (*search_strategy_t)(struct hfs_bnode *,
  /* attributes.c */
  int __init hfsplus_create_attr_tree_cache(void);
  void hfsplus_destroy_attr_tree_cache(void);
+int hfsplus_attr_bin_cmp_key(const hfsplus_btree_key *k1,
+			     const hfsplus_btree_key *k2);
+int hfsplus_attr_build_key(struct super_block *sb, hfsplus_btree_key *key,
+			   u32 cnid, const char *name);
+void hfsplus_attr_build_key_uni(hfsplus_btree_key *key, u32 cnid,
+				struct hfsplus_attr_unistr *name);
  hfsplus_attr_entry *hfsplus_alloc_attr_entry(void);
-void hfsplus_destroy_attr_entry(hfsplus_attr_entry *entry_p);
-int hfsplus_attr_bin_cmp_key(const hfsplus_btree_key *,
-		const hfsplus_btree_key *);
-int hfsplus_attr_build_key(struct super_block *, hfsplus_btree_key *,
-			u32, const char *);
-void hfsplus_attr_build_key_uni(hfsplus_btree_key *key,
-					u32 cnid,
-					struct hfsplus_attr_unistr *name);
-int hfsplus_find_attr(struct super_block *, u32,
-			const char *, struct hfs_find_data *);
+void hfsplus_destroy_attr_entry(hfsplus_attr_entry *entry);
+int hfsplus_find_attr(struct super_block *sb, u32 cnid, const char *name,
+		      struct hfs_find_data *fd);
  int hfsplus_attr_exists(struct inode *inode, const char *name);
-int hfsplus_create_attr(struct inode *, const char *, const void *, size_t);
-int hfsplus_delete_attr(struct inode *, const char *);
+int hfsplus_create_attr(struct inode *inode, const char *name,
+			const void *value, size_t size);
+int hfsplus_delete_attr(struct inode *inode, const char *name);
  int hfsplus_delete_all_attrs(struct inode *dir, u32 cnid);

  /* bitmap.c */
-int hfsplus_block_allocate(struct super_block *, u32, u32, u32 *);
-int hfsplus_block_free(struct super_block *, u32, u32);
+int hfsplus_block_allocate(struct super_block *sb, u32 size, u32 offset,
+			   u32 *max);
+int hfsplus_block_free(struct super_block *sb, u32 offset, u32 count);

  /* btree.c */
-u32 hfsplus_calc_btree_clump_size(u32, u32, u64, int);
-struct hfs_btree *hfs_btree_open(struct super_block *, u32);
-void hfs_btree_close(struct hfs_btree *);
-int hfs_btree_write(struct hfs_btree *);
-struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *);
-void hfs_bmap_free(struct hfs_bnode *);
+u32 hfsplus_calc_btree_clump_size(u32 block_size, u32 node_size, u64 sectors,
+				  int file_id);
+struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id);
+void hfs_btree_close(struct hfs_btree *tree);
+int hfs_btree_write(struct hfs_btree *tree);
+struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree);
+void hfs_bmap_free(struct hfs_bnode *node);

  /* bnode.c */
-void hfs_bnode_read(struct hfs_bnode *, void *, int, int);
-u16 hfs_bnode_read_u16(struct hfs_bnode *, int);
-u8 hfs_bnode_read_u8(struct hfs_bnode *, int);
-void hfs_bnode_read_key(struct hfs_bnode *, void *, int);
-void hfs_bnode_write(struct hfs_bnode *, void *, int, int);
-void hfs_bnode_write_u16(struct hfs_bnode *, int, u16);
-void hfs_bnode_clear(struct hfs_bnode *, int, int);
-void hfs_bnode_copy(struct hfs_bnode *, int,
-		    struct hfs_bnode *, int, int);
-void hfs_bnode_move(struct hfs_bnode *, int, int, int);
-void hfs_bnode_dump(struct hfs_bnode *);
-void hfs_bnode_unlink(struct hfs_bnode *);
-struct hfs_bnode *hfs_bnode_findhash(struct hfs_btree *, u32);
-struct hfs_bnode *hfs_bnode_find(struct hfs_btree *, u32);
-void hfs_bnode_unhash(struct hfs_bnode *);
-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 *);
+void hfs_bnode_read(struct hfs_bnode *node, void *buf, int off, int len);
+u16 hfs_bnode_read_u16(struct hfs_bnode *node, int off);
+u8 hfs_bnode_read_u8(struct hfs_bnode *node, int off);
+void hfs_bnode_read_key(struct hfs_bnode *node, void *key, int off);
+void hfs_bnode_write(struct hfs_bnode *node, void *buf, int off, int len);
+void hfs_bnode_write_u16(struct hfs_bnode *node, int off, u16 data);
+void hfs_bnode_clear(struct hfs_bnode *node, int off, int len);
+void hfs_bnode_copy(struct hfs_bnode *dst_node, int dst,
+		    struct hfs_bnode *src_node, int src, int len);
+void hfs_bnode_move(struct hfs_bnode *node, int dst, int src, int len);
+void hfs_bnode_dump(struct hfs_bnode *node);
+void hfs_bnode_unlink(struct hfs_bnode *node);
+struct hfs_bnode *hfs_bnode_findhash(struct hfs_btree *tree, u32 cnid);
+void hfs_bnode_unhash(struct hfs_bnode *node);
+struct hfs_bnode *hfs_bnode_find(struct hfs_btree *tree, u32 num);
+void hfs_bnode_free(struct hfs_bnode *node);
+struct hfs_bnode *hfs_bnode_create(struct hfs_btree *tree, u32 num);
+void hfs_bnode_get(struct hfs_bnode *node);
+void hfs_bnode_put(struct hfs_bnode *node);
+bool hfs_bnode_need_zeroout(struct hfs_btree *tree);

  /* brec.c */
-u16 hfs_brec_lenoff(struct hfs_bnode *, u16, u16 *);
-u16 hfs_brec_keylen(struct hfs_bnode *, u16);
-int hfs_brec_insert(struct hfs_find_data *, void *, int);
-int hfs_brec_remove(struct hfs_find_data *);
+u16 hfs_brec_lenoff(struct hfs_bnode *node, u16 rec, u16 *off);
+u16 hfs_brec_keylen(struct hfs_bnode *node, u16 rec);
+int hfs_brec_insert(struct hfs_find_data *fd, void *entry, int entry_len);
+int hfs_brec_remove(struct hfs_find_data *fd);

  /* bfind.c */
-int hfs_find_init(struct hfs_btree *, struct hfs_find_data *);
-void hfs_find_exit(struct hfs_find_data *);
-int hfs_find_1st_rec_by_cnid(struct hfs_bnode *,
-				struct hfs_find_data *,
-				int *, int *, int *);
-int hfs_find_rec_by_key(struct hfs_bnode *,
-				struct hfs_find_data *,
-				int *, int *, int *);
-int __hfs_brec_find(struct hfs_bnode *, struct hfs_find_data *,
-				search_strategy_t);
-int hfs_brec_find(struct hfs_find_data *, search_strategy_t);
-int hfs_brec_read(struct hfs_find_data *, void *, int);
-int hfs_brec_goto(struct hfs_find_data *, int);
+int hfs_find_init(struct hfs_btree *tree, struct hfs_find_data *fd);
+void hfs_find_exit(struct hfs_find_data *fd);
+int hfs_find_1st_rec_by_cnid(struct hfs_bnode *bnode, struct hfs_find_data *fd,
+			     int *begin, int *end, int *cur_rec);
+int hfs_find_rec_by_key(struct hfs_bnode *bnode, struct hfs_find_data *fd,
+			int *begin, int *end, int *cur_rec);
+int __hfs_brec_find(struct hfs_bnode *bnode, struct hfs_find_data *fd,
+		    search_strategy_t rec_found);
+int hfs_brec_find(struct hfs_find_data *fd, search_strategy_t do_key_compare);
+int hfs_brec_read(struct hfs_find_data *fd, void *rec, int rec_len);
+int hfs_brec_goto(struct hfs_find_data *fd, int cnt);

  /* catalog.c */
-int hfsplus_cat_case_cmp_key(const hfsplus_btree_key *,
-		const hfsplus_btree_key *);
-int hfsplus_cat_bin_cmp_key(const hfsplus_btree_key *,
-		const hfsplus_btree_key *);
-void hfsplus_cat_build_key(struct super_block *sb,
-		hfsplus_btree_key *, u32, struct qstr *);
-int hfsplus_find_cat(struct super_block *, u32, struct hfs_find_data *);
-int hfsplus_create_cat(u32, struct inode *, struct qstr *, struct inode *);
-int hfsplus_delete_cat(u32, struct inode *, struct qstr *);
-int hfsplus_rename_cat(u32, struct inode *, struct qstr *,
-		       struct inode *, struct qstr *);
+int hfsplus_cat_case_cmp_key(const hfsplus_btree_key *k1,
+			     const hfsplus_btree_key *k2);
+int hfsplus_cat_bin_cmp_key(const hfsplus_btree_key *k1,
+			    const hfsplus_btree_key *k2);
+void hfsplus_cat_build_key(struct super_block *sb, hfsplus_btree_key *key,
+			   u32 parent, struct qstr *str);
  void hfsplus_cat_set_perms(struct inode *inode, struct hfsplus_perm *perms);
+int hfsplus_find_cat(struct super_block *sb, u32 cnid,
+		     struct hfs_find_data *fd);
+int hfsplus_create_cat(u32 cnid, struct inode *dir, struct qstr *str,
+		       struct inode *inode);
+int hfsplus_delete_cat(u32 cnid, struct inode *dir, struct qstr *str);
+int hfsplus_rename_cat(u32 cnid, struct inode *src_dir, struct qstr *src_name,
+		       struct inode *dst_dir, struct qstr *dst_name);

  /* dir.c */
  extern const struct inode_operations hfsplus_dir_inode_operations;
  extern const struct file_operations hfsplus_dir_operations;

  /* extents.c */
-int hfsplus_ext_cmp_key(const hfsplus_btree_key *, const hfsplus_btree_key *);
-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 *, bool zeroout);
-void hfsplus_file_truncate(struct inode *);
+int hfsplus_ext_cmp_key(const hfsplus_btree_key *k1,
+			const hfsplus_btree_key *k2);
+int hfsplus_ext_write_extent(struct inode *inode);
+int hfsplus_get_block(struct inode *inode, sector_t iblock,
+		      struct buffer_head *bh_result, int create);
+int hfsplus_free_fork(struct super_block *sb, u32 cnid,
+		      struct hfsplus_fork_raw *fork, int type);
+int hfsplus_file_extend(struct inode *inode, bool zeroout);
+void hfsplus_file_truncate(struct inode *inode);

  /* inode.c */
  extern const struct address_space_operations hfsplus_aops;
  extern const struct address_space_operations hfsplus_btree_aops;
  extern const struct dentry_operations hfsplus_dentry_operations;

-void hfsplus_inode_read_fork(struct inode *, struct hfsplus_fork_raw *);
-void hfsplus_inode_write_fork(struct inode *, struct hfsplus_fork_raw *);
-int hfsplus_cat_read_inode(struct inode *, struct hfs_find_data *);
-int hfsplus_cat_write_inode(struct inode *);
-struct inode *hfsplus_new_inode(struct super_block *, umode_t);
-void hfsplus_delete_inode(struct inode *);
  int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
  		       int datasync);
+struct inode *hfsplus_new_inode(struct super_block *sb, umode_t mode);
+void hfsplus_delete_inode(struct inode *inode);
+void hfsplus_inode_read_fork(struct inode *inode,
+			     struct hfsplus_fork_raw *fork);
+void hfsplus_inode_write_fork(struct inode *inode,
+			      struct hfsplus_fork_raw *fork);
+int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd);
+int hfsplus_cat_write_inode(struct inode *inode);

  /* ioctl.c */
  long hfsplus_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);

  /* options.c */
-int hfsplus_parse_options(char *, struct hfsplus_sb_info *);
+void hfsplus_fill_defaults(struct hfsplus_sb_info *opts);
  int hfsplus_parse_options_remount(char *input, int *force);
-void hfsplus_fill_defaults(struct hfsplus_sb_info *);
-int hfsplus_show_options(struct seq_file *, struct dentry *);
+int hfsplus_parse_options(char *input, struct hfsplus_sb_info *sbi);
+int hfsplus_show_options(struct seq_file *seq, struct dentry *root);
+
+/* part_tbl.c */
+int hfs_part_find(struct super_block *sb, sector_t *part_start,
+		  sector_t *part_size);

  /* super.c */
-struct inode *hfsplus_iget(struct super_block *, unsigned long);
+struct inode *hfsplus_iget(struct super_block *sb, unsigned long ino);
  void hfsplus_mark_mdb_dirty(struct super_block *sb);

  /* tables.c */
@@ -500,23 +510,23 @@ extern u16 hfsplus_decompose_table[];
  extern u16 hfsplus_compose_table[];

  /* unicode.c */
-int hfsplus_strcasecmp(const struct hfsplus_unistr *,
-		const struct hfsplus_unistr *);
-int hfsplus_strcmp(const struct hfsplus_unistr *,
-		const struct hfsplus_unistr *);
-int hfsplus_uni2asc(struct super_block *,
-		const struct hfsplus_unistr *, char *, int *);
-int hfsplus_asc2uni(struct super_block *,
-		struct hfsplus_unistr *, int, const char *, int);
+int hfsplus_strcasecmp(const struct hfsplus_unistr *s1,
+		       const struct hfsplus_unistr *s2);
+int hfsplus_strcmp(const struct hfsplus_unistr *s1,
+		   const struct hfsplus_unistr *s2);
+int hfsplus_uni2asc(struct super_block *sb, const struct hfsplus_unistr *ustr,
+		    char *astr, int *len_p);
+int hfsplus_asc2uni(struct super_block *sb, struct hfsplus_unistr *ustr,
+		    int max_unistr_len, const char *astr, int len);
  int hfsplus_hash_dentry(const struct dentry *dentry, struct qstr *str);
-int hfsplus_compare_dentry(const struct dentry *parent, const struct dentry *dentry,
-		unsigned int len, const char *str, const struct qstr *name);
+int hfsplus_compare_dentry(const struct dentry *parent,
+			   const struct dentry *dentry, unsigned int len,
+			   const char *str, const struct qstr *name);

  /* wrapper.c */
-int hfsplus_read_wrapper(struct super_block *);
-int hfs_part_find(struct super_block *, sector_t *, sector_t *);
-int hfsplus_submit_bio(struct super_block *sb, sector_t sector,
-		void *buf, void **data, int rw);
+int hfsplus_submit_bio(struct super_block *sb, sector_t sector, void *buf,
+		       void **data, int rw);
+int hfsplus_read_wrapper(struct super_block *sb);

  /* time macros */
  #define __hfsp_mt2ut(t)		(be32_to_cpu(t) - 2082844800U)
-- 
1.9.0

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

[Vyacheslav Dubeyko]

On Thu, 2014-05-22 at 14:48 +0200, Sergei Antonov wrote:

[snip]
>   fs/hfsplus/hfsplus_fs.h | 208 +++++++++++++++++++++++++-----------------------
>   1 file changed, 109 insertions(+), 99 deletions(-)
> 
> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> index 3c872b2..f917fe4 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -369,129 +369,139 @@ typedef int (*search_strategy_t)(struct hfs_bnode *,
>   /* attributes.c */
>   int __init hfsplus_create_attr_tree_cache(void);
>   void hfsplus_destroy_attr_tree_cache(void);
> +int hfsplus_attr_bin_cmp_key(const hfsplus_btree_key *k1,
> +			     const hfsplus_btree_key *k2);
> +int hfsplus_attr_build_key(struct super_block *sb, hfsplus_btree_key *key,
> +			   u32 cnid, const char *name);
> +void hfsplus_attr_build_key_uni(hfsplus_btree_key *key, u32 cnid,
> +				struct hfsplus_attr_unistr *name);

As I remember, this function was deleted recently. Could you check it?

Thanks,
Vyacheslav Dubeyko.

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

[Sergei Antonov]

On 22 May 2014 15:13, Vyacheslav Dubeyko <slava@dubeyko.com> wrote:
> On Thu, 2014-05-22 at 14:48 +0200, Sergei Antonov wrote:
>
> [snip]
>>   fs/hfsplus/hfsplus_fs.h | 208 +++++++++++++++++++++++++-----------------------
>>   1 file changed, 109 insertions(+), 99 deletions(-)
>>
>> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
>> index 3c872b2..f917fe4 100644
>> --- a/fs/hfsplus/hfsplus_fs.h
>> +++ b/fs/hfsplus/hfsplus_fs.h
>> @@ -369,129 +369,139 @@ typedef int (*search_strategy_t)(struct hfs_bnode *,
>>   /* attributes.c */
>>   int __init hfsplus_create_attr_tree_cache(void);
>>   void hfsplus_destroy_attr_tree_cache(void);
>> +int hfsplus_attr_bin_cmp_key(const hfsplus_btree_key *k1,
>> +                          const hfsplus_btree_key *k2);
>> +int hfsplus_attr_build_key(struct super_block *sb, hfsplus_btree_key *key,
>> +                        u32 cnid, const char *name);
>> +void hfsplus_attr_build_key_uni(hfsplus_btree_key *key, u32 cnid,
>> +                             struct hfsplus_attr_unistr *name);
>
> As I remember, this function was deleted recently. Could you check it?

This function exists in the Linus's tree. Should I have done a patch
against linux-next?

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

[Vyacheslav Dubeyko]

On Thu, 2014-05-22 at 15:25 +0200, Sergei Antonov wrote:
> On 22 May 2014 15:13, Vyacheslav Dubeyko <slava@dubeyko.com> wrote:
> > On Thu, 2014-05-22 at 14:48 +0200, Sergei Antonov wrote:
> >
> > [snip]
> >>   fs/hfsplus/hfsplus_fs.h | 208 +++++++++++++++++++++++++-----------------------
> >>   1 file changed, 109 insertions(+), 99 deletions(-)
> >>
> >> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> >> index 3c872b2..f917fe4 100644
> >> --- a/fs/hfsplus/hfsplus_fs.h
> >> +++ b/fs/hfsplus/hfsplus_fs.h
> >> @@ -369,129 +369,139 @@ typedef int (*search_strategy_t)(struct hfs_bnode *,
> >>   /* attributes.c */
> >>   int __init hfsplus_create_attr_tree_cache(void);
> >>   void hfsplus_destroy_attr_tree_cache(void);
> >> +int hfsplus_attr_bin_cmp_key(const hfsplus_btree_key *k1,
> >> +                          const hfsplus_btree_key *k2);
> >> +int hfsplus_attr_build_key(struct super_block *sb, hfsplus_btree_key *key,
> >> +                        u32 cnid, const char *name);
> >> +void hfsplus_attr_build_key_uni(hfsplus_btree_key *key, u32 cnid,
> >> +                             struct hfsplus_attr_unistr *name);
> >
> > As I remember, this function was deleted recently. Could you check it?
> 
> This function exists in the Linus's tree. Should I have done a patch
> against linux-next?

Yes, this change exists in linux-next yet.

Thanks,
Vyacheslav Dubeyko.

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

[Sergei Antonov]

On Thu, 22 May 2014, Vyacheslav Dubeyko wrote:

> On Thu, 2014-05-22 at 15:25 +0200, Sergei Antonov wrote:
>> On 22 May 2014 15:13, Vyacheslav Dubeyko <slava@dubeyko.com> wrote:
>>> On Thu, 2014-05-22 at 14:48 +0200, Sergei Antonov wrote:
>>>
>>> [snip]
>>>>   fs/hfsplus/hfsplus_fs.h | 208 +++++++++++++++++++++++++-----------------------
>>>>   1 file changed, 109 insertions(+), 99 deletions(-)
>>>>
>>>> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
>>>> index 3c872b2..f917fe4 100644
>>>> --- a/fs/hfsplus/hfsplus_fs.h
>>>> +++ b/fs/hfsplus/hfsplus_fs.h
>>>> @@ -369,129 +369,139 @@ typedef int (*search_strategy_t)(struct hfs_bnode *,
>>>>   /* attributes.c */
>>>>   int __init hfsplus_create_attr_tree_cache(void);
>>>>   void hfsplus_destroy_attr_tree_cache(void);
>>>> +int hfsplus_attr_bin_cmp_key(const hfsplus_btree_key *k1,
>>>> +                          const hfsplus_btree_key *k2);
>>>> +int hfsplus_attr_build_key(struct super_block *sb, hfsplus_btree_key *key,
>>>> +                        u32 cnid, const char *name);
>>>> +void hfsplus_attr_build_key_uni(hfsplus_btree_key *key, u32 cnid,
>>>> +                             struct hfsplus_attr_unistr *name);
>>>
>>> As I remember, this function was deleted recently. Could you check it?
>>
>> This function exists in the Linus's tree. Should I have done a patch
>> against linux-next?
>
> Yes, this change exists in linux-next yet.

Thanks for telling. The patch against linux-next follows.

---
Subject: [next][PATCH] hfsplus: coding style fix for declarations in hfsplus_fs.h

Some function declarations in hfsplus_fs.h were with argument names, some
without, and some were mixed. This patch adds argument names everywhere, sorts
function in order they go in .c files, and moves hfs_part_find() to a proper
section.

Auto-formatting and sorting was done with:
cfunctions *.c | indent -linux | sed "s| \* | \*|"

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Vyacheslav Dubeyko <slava@dubeyko.com>
Cc: Hin-Tak Leung <htl10@users.sourceforge.net>
Signed-off-by: Sergei Antonov <saproj@gmail.com>
---
  fs/hfsplus/hfsplus_fs.h | 205 +++++++++++++++++++++++++-----------------------
  1 file changed, 108 insertions(+), 97 deletions(-)

diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 8b35648..0861b35 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -369,128 +369,139 @@ typedef int (*search_strategy_t)(struct hfs_bnode *,
  /* attributes.c */
  int __init hfsplus_create_attr_tree_cache(void);
  void hfsplus_destroy_attr_tree_cache(void);
+int hfsplus_attr_bin_cmp_key(const hfsplus_btree_key *k1,
+			     const hfsplus_btree_key *k2);
+int hfsplus_attr_build_key(struct super_block *sb, hfsplus_btree_key *key,
+			   u32 cnid, const char *name);
  hfsplus_attr_entry *hfsplus_alloc_attr_entry(void);
-void hfsplus_destroy_attr_entry(hfsplus_attr_entry *entry_p);
-int hfsplus_attr_bin_cmp_key(const hfsplus_btree_key *,
-		const hfsplus_btree_key *);
-int hfsplus_attr_build_key(struct super_block *, hfsplus_btree_key *,
-			u32, const char *);
-int hfsplus_find_attr(struct super_block *, u32,
-			const char *, struct hfs_find_data *);
+void hfsplus_destroy_attr_entry(hfsplus_attr_entry *entry);
+int hfsplus_find_attr(struct super_block *sb, u32 cnid, const char *name,
+		      struct hfs_find_data *fd);
  int hfsplus_attr_exists(struct inode *inode, const char *name);
-int hfsplus_create_attr(struct inode *, const char *, const void *, size_t);
-int hfsplus_delete_attr(struct inode *, const char *);
+int hfsplus_create_attr(struct inode *inode, const char *name,
+			const void *value, size_t size);
+int hfsplus_delete_attr(struct inode *inode, const char *name);
  int hfsplus_delete_all_attrs(struct inode *dir, u32 cnid);

  /* bitmap.c */
-int hfsplus_block_allocate(struct super_block *, u32, u32, u32 *);
-int hfsplus_block_free(struct super_block *, u32, u32);
+int hfsplus_block_allocate(struct super_block *sb, u32 size, u32 offset,
+			   u32 *max);
+int hfsplus_block_free(struct super_block *sb, u32 offset, u32 count);

  /* btree.c */
-u32 hfsplus_calc_btree_clump_size(u32, u32, u64, int);
-struct hfs_btree *hfs_btree_open(struct super_block *, u32);
-void hfs_btree_close(struct hfs_btree *);
-int hfs_btree_write(struct hfs_btree *);
-struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *);
-void hfs_bmap_free(struct hfs_bnode *);
+u32 hfsplus_calc_btree_clump_size(u32 block_size, u32 node_size, u64 sectors,
+				  int file_id);
+struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id);
+void hfs_btree_close(struct hfs_btree *tree);
+int hfs_btree_write(struct hfs_btree *tree);
+struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree);
+void hfs_bmap_free(struct hfs_bnode *node);

  /* bnode.c */
-void hfs_bnode_read(struct hfs_bnode *, void *, int, int);
-u16 hfs_bnode_read_u16(struct hfs_bnode *, int);
-u8 hfs_bnode_read_u8(struct hfs_bnode *, int);
-void hfs_bnode_read_key(struct hfs_bnode *, void *, int);
-void hfs_bnode_write(struct hfs_bnode *, void *, int, int);
-void hfs_bnode_write_u16(struct hfs_bnode *, int, u16);
-void hfs_bnode_clear(struct hfs_bnode *, int, int);
-void hfs_bnode_copy(struct hfs_bnode *, int,
-		    struct hfs_bnode *, int, int);
-void hfs_bnode_move(struct hfs_bnode *, int, int, int);
-void hfs_bnode_dump(struct hfs_bnode *);
-void hfs_bnode_unlink(struct hfs_bnode *);
-struct hfs_bnode *hfs_bnode_findhash(struct hfs_btree *, u32);
-struct hfs_bnode *hfs_bnode_find(struct hfs_btree *, u32);
-void hfs_bnode_unhash(struct hfs_bnode *);
-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 *);
+void hfs_bnode_read(struct hfs_bnode *node, void *buf, int off, int len);
+u16 hfs_bnode_read_u16(struct hfs_bnode *node, int off);
+u8 hfs_bnode_read_u8(struct hfs_bnode *node, int off);
+void hfs_bnode_read_key(struct hfs_bnode *node, void *key, int off);
+void hfs_bnode_write(struct hfs_bnode *node, void *buf, int off, int len);
+void hfs_bnode_write_u16(struct hfs_bnode *node, int off, u16 data);
+void hfs_bnode_clear(struct hfs_bnode *node, int off, int len);
+void hfs_bnode_copy(struct hfs_bnode *dst_node, int dst,
+		    struct hfs_bnode *src_node, int src, int len);
+void hfs_bnode_move(struct hfs_bnode *node, int dst, int src, int len);
+void hfs_bnode_dump(struct hfs_bnode *node);
+void hfs_bnode_unlink(struct hfs_bnode *node);
+struct hfs_bnode *hfs_bnode_findhash(struct hfs_btree *tree, u32 cnid);
+void hfs_bnode_unhash(struct hfs_bnode *node);
+struct hfs_bnode *hfs_bnode_find(struct hfs_btree *tree, u32 num);
+void hfs_bnode_free(struct hfs_bnode *node);
+struct hfs_bnode *hfs_bnode_create(struct hfs_btree *tree, u32 num);
+void hfs_bnode_get(struct hfs_bnode *node);
+void hfs_bnode_put(struct hfs_bnode *node);
+bool hfs_bnode_need_zeroout(struct hfs_btree *tree);

  /* brec.c */
-u16 hfs_brec_lenoff(struct hfs_bnode *, u16, u16 *);
-u16 hfs_brec_keylen(struct hfs_bnode *, u16);
-int hfs_brec_insert(struct hfs_find_data *, void *, int);
-int hfs_brec_remove(struct hfs_find_data *);
+u16 hfs_brec_lenoff(struct hfs_bnode *node, u16 rec, u16 *off);
+u16 hfs_brec_keylen(struct hfs_bnode *node, u16 rec);
+int hfs_brec_insert(struct hfs_find_data *fd, void *entry, int entry_len);
+int hfs_brec_remove(struct hfs_find_data *fd);

  /* bfind.c */
-int hfs_find_init(struct hfs_btree *, struct hfs_find_data *);
-void hfs_find_exit(struct hfs_find_data *);
-int hfs_find_1st_rec_by_cnid(struct hfs_bnode *,
-				struct hfs_find_data *,
-				int *, int *, int *);
-int hfs_find_rec_by_key(struct hfs_bnode *,
-				struct hfs_find_data *,
-				int *, int *, int *);
-int __hfs_brec_find(struct hfs_bnode *, struct hfs_find_data *,
-				search_strategy_t);
-int hfs_brec_find(struct hfs_find_data *, search_strategy_t);
-int hfs_brec_read(struct hfs_find_data *, void *, int);
-int hfs_brec_goto(struct hfs_find_data *, int);
+int hfs_find_init(struct hfs_btree *tree, struct hfs_find_data *fd);
+void hfs_find_exit(struct hfs_find_data *fd);
+int hfs_find_1st_rec_by_cnid(struct hfs_bnode *bnode, struct hfs_find_data *fd,
+			     int *begin, int *end, int *cur_rec);
+int hfs_find_rec_by_key(struct hfs_bnode *bnode, struct hfs_find_data *fd,
+			int *begin, int *end, int *cur_rec);
+int __hfs_brec_find(struct hfs_bnode *bnode, struct hfs_find_data *fd,
+		    search_strategy_t rec_found);
+int hfs_brec_find(struct hfs_find_data *fd, search_strategy_t do_key_compare);
+int hfs_brec_read(struct hfs_find_data *fd, void *rec, int rec_len);
+int hfs_brec_goto(struct hfs_find_data *fd, int cnt);

  /* catalog.c */
-int hfsplus_cat_case_cmp_key(const hfsplus_btree_key *,
-		const hfsplus_btree_key *);
-int hfsplus_cat_bin_cmp_key(const hfsplus_btree_key *,
-		const hfsplus_btree_key *);
-int hfsplus_cat_build_key(struct super_block *sb,
-		hfsplus_btree_key *, u32, struct qstr *);
+int hfsplus_cat_case_cmp_key(const hfsplus_btree_key *k1,
+			     const hfsplus_btree_key *k2);
+int hfsplus_cat_bin_cmp_key(const hfsplus_btree_key *k1,
+			    const hfsplus_btree_key *k2);
+int hfsplus_cat_build_key(struct super_block *sb, hfsplus_btree_key *key,
+			  u32 parent, struct qstr *str);
  void hfsplus_cat_build_key_with_cnid(struct super_block *sb,
-		hfsplus_btree_key *, u32);
-int hfsplus_find_cat(struct super_block *, u32, struct hfs_find_data *);
-int hfsplus_create_cat(u32, struct inode *, struct qstr *, struct inode *);
-int hfsplus_delete_cat(u32, struct inode *, struct qstr *);
-int hfsplus_rename_cat(u32, struct inode *, struct qstr *,
-		       struct inode *, struct qstr *);
+				     hfsplus_btree_key *key, u32 parent);
  void hfsplus_cat_set_perms(struct inode *inode, struct hfsplus_perm *perms);
+int hfsplus_find_cat(struct super_block *sb, u32 cnid,
+		     struct hfs_find_data *fd);
+int hfsplus_create_cat(u32 cnid, struct inode *dir, struct qstr *str,
+		       struct inode *inode);
+int hfsplus_delete_cat(u32 cnid, struct inode *dir, struct qstr *str);
+int hfsplus_rename_cat(u32 cnid, struct inode *src_dir, struct qstr *src_name,
+		       struct inode *dst_dir, struct qstr *dst_name);

  /* dir.c */
  extern const struct inode_operations hfsplus_dir_inode_operations;
  extern const struct file_operations hfsplus_dir_operations;

  /* extents.c */
-int hfsplus_ext_cmp_key(const hfsplus_btree_key *, const hfsplus_btree_key *);
-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 *, bool zeroout);
-void hfsplus_file_truncate(struct inode *);
+int hfsplus_ext_cmp_key(const hfsplus_btree_key *k1,
+			const hfsplus_btree_key *k2);
+int hfsplus_ext_write_extent(struct inode *inode);
+int hfsplus_get_block(struct inode *inode, sector_t iblock,
+		      struct buffer_head *bh_result, int create);
+int hfsplus_free_fork(struct super_block *sb, u32 cnid,
+		      struct hfsplus_fork_raw *fork, int type);
+int hfsplus_file_extend(struct inode *inode, bool zeroout);
+void hfsplus_file_truncate(struct inode *inode);

  /* inode.c */
  extern const struct address_space_operations hfsplus_aops;
  extern const struct address_space_operations hfsplus_btree_aops;
  extern const struct dentry_operations hfsplus_dentry_operations;

-void hfsplus_inode_read_fork(struct inode *, struct hfsplus_fork_raw *);
-void hfsplus_inode_write_fork(struct inode *, struct hfsplus_fork_raw *);
-int hfsplus_cat_read_inode(struct inode *, struct hfs_find_data *);
-int hfsplus_cat_write_inode(struct inode *);
-struct inode *hfsplus_new_inode(struct super_block *, umode_t);
-void hfsplus_delete_inode(struct inode *);
  int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
  		       int datasync);
+struct inode *hfsplus_new_inode(struct super_block *sb, umode_t mode);
+void hfsplus_delete_inode(struct inode *inode);
+void hfsplus_inode_read_fork(struct inode *inode,
+			     struct hfsplus_fork_raw *fork);
+void hfsplus_inode_write_fork(struct inode *inode,
+			      struct hfsplus_fork_raw *fork);
+int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd);
+int hfsplus_cat_write_inode(struct inode *inode);

  /* ioctl.c */
  long hfsplus_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);

  /* options.c */
-int hfsplus_parse_options(char *, struct hfsplus_sb_info *);
+void hfsplus_fill_defaults(struct hfsplus_sb_info *opts);
  int hfsplus_parse_options_remount(char *input, int *force);
-void hfsplus_fill_defaults(struct hfsplus_sb_info *);
-int hfsplus_show_options(struct seq_file *, struct dentry *);
+int hfsplus_parse_options(char *input, struct hfsplus_sb_info *sbi);
+int hfsplus_show_options(struct seq_file *seq, struct dentry *root);
+
+/* part_tbl.c */
+int hfs_part_find(struct super_block *sb, sector_t *part_start,
+		  sector_t *part_size);

  /* super.c */
-struct inode *hfsplus_iget(struct super_block *, unsigned long);
+struct inode *hfsplus_iget(struct super_block *sb, unsigned long ino);
  void hfsplus_mark_mdb_dirty(struct super_block *sb);

  /* tables.c */
@@ -499,23 +510,23 @@ extern u16 hfsplus_decompose_table[];
  extern u16 hfsplus_compose_table[];

  /* unicode.c */
-int hfsplus_strcasecmp(const struct hfsplus_unistr *,
-		const struct hfsplus_unistr *);
-int hfsplus_strcmp(const struct hfsplus_unistr *,
-		const struct hfsplus_unistr *);
-int hfsplus_uni2asc(struct super_block *,
-		const struct hfsplus_unistr *, char *, int *);
-int hfsplus_asc2uni(struct super_block *,
-		struct hfsplus_unistr *, int, const char *, int);
+int hfsplus_strcasecmp(const struct hfsplus_unistr *s1,
+		       const struct hfsplus_unistr *s2);
+int hfsplus_strcmp(const struct hfsplus_unistr *s1,
+		   const struct hfsplus_unistr *s2);
+int hfsplus_uni2asc(struct super_block *sb, const struct hfsplus_unistr *ustr,
+		    char *astr, int *len_p);
+int hfsplus_asc2uni(struct super_block *sb, struct hfsplus_unistr *ustr,
+		    int max_unistr_len, const char *astr, int len);
  int hfsplus_hash_dentry(const struct dentry *dentry, struct qstr *str);
-int hfsplus_compare_dentry(const struct dentry *parent, const struct dentry *dentry,
-		unsigned int len, const char *str, const struct qstr *name);
+int hfsplus_compare_dentry(const struct dentry *parent,
+			   const struct dentry *dentry, unsigned int len,
+			   const char *str, const struct qstr *name);

  /* wrapper.c */
-int hfsplus_read_wrapper(struct super_block *);
-int hfs_part_find(struct super_block *, sector_t *, sector_t *);
-int hfsplus_submit_bio(struct super_block *sb, sector_t sector,
-		void *buf, void **data, int rw);
+int hfsplus_submit_bio(struct super_block *sb, sector_t sector, void *buf,
+		       void **data, int rw);
+int hfsplus_read_wrapper(struct super_block *sb);

  /* time macros */
  #define __hfsp_mt2ut(t)		(be32_to_cpu(t) - 2082844800U)
-- 
1.9.0

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

[Sergei Antonov]

On 23 May 2014 08:21, Vyacheslav Dubeyko <slava@dubeyko.com> wrote:
> On Thu, 2014-05-22 at 15:18 +0200, Sergei Antonov wrote:
>> On 22 May 2014 15:07, Vyacheslav Dubeyko <slava@dubeyko.com> wrote:
>> > On Wed, 2014-05-21 at 20:15 +0200, Sergei Antonov wrote:
>> >> On 21 May 2014 18:40, Vyacheslav Dubeyko <slava@dubeyko.com> wrote:
>> >> > 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.
>> >>
>> >> Which is not bad. Those blocks are free space.
>> >>
>> >
>> > For me personally, proper place for sb_issue_zeroout() can be in
>> > hfs_bmap_alloc() method
>> > (http://lxr.free-electrons.com/source/fs/hfsplus/btree.c#L364):
>> >
>> >
>> >         while (!tree->free_nodes) {
>> >                 struct inode *inode = tree->inode;
>> >                 struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
>> >                 u32 count;
>> >                 int res;
>> >
>> >                 res = hfsplus_file_extend(inode);
>> >                 if (res)
>> >                         return ERR_PTR(res);
>> >
>> >                 /* here can be added sb_issue_zeroout() call */
>> >
>> >                 hip->phys_size = inode->i_size =
>> >                         (loff_t)hip->alloc_blocks <<
>> >                                 HFSPLUS_SB(tree->sb)->alloc_blksz_shift;
>> >                 hip->fs_blocks =
>> >                         hip->alloc_blocks << HFSPLUS_SB(tree->sb)->fs_shift;
>> >                 inode_set_bytes(inode, inode->i_size);
>> >                 count = inode->i_size >> tree->node_size_shift;
>> >                 tree->free_nodes = count - tree->node_count;
>> >                 tree->node_count = count;
>> >         }
>> >
>> > First of all, here we know that trying to extend file was successful.
>> > And, secondly, hfs_bmap_alloc() method is dedicated b-tree case only.
>> > I think that modification of hfsplus_file_extend() is not very good
>> > idea. The hfs_bmap_alloc() method is more clear solution, from my
>> > viewpoint.
>>
>> hfs_bmap_alloc() does not know about volume blocks. It is on a higher
>> level of abstraction. Try, as an experiment, to write a call to
>> sb_issue_zeroout() passing correct arguments from hfs_bmap_alloc().
>>
>
> The hfs_bmap_alloc() has pointer on hfsplus_inode_info structure of
> btree's inode. The hfsplus_inode_info structure contains
> hip->first_extents, hip->first_blocks, hip->cached_extents,
> hip->cached_blocks and so on fields. Finally, hfsplus_file_extend()
> method stores allocated extent in hip->first_extents or in
> hip->cached_extents.

Yo-ho! What a ride!

> So, I don't see anything impossible in calling
> sb_issue_zeroout() with correct arguments from hfs_bmap_alloc().

And you seem to like riding.

> Maybe
> only to call sb_issue_zeroout() from hfsplus_file_extend() is more easy
> way. But I think that placement this logic in hfs_bmap_alloc() is more
> correct way, from architecture point of view.

It is nasty from an architectural point of view for reasons provided
earlier. hfsplus_file_extend() is volume blocks level and that's what
one needs for sb_issue_*.

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

[Vyacheslav Dubeyko]

On Thu, 2014-05-22 at 15:18 +0200, Sergei Antonov wrote:
> On 22 May 2014 15:07, Vyacheslav Dubeyko <slava@dubeyko.com> wrote:
> > On Wed, 2014-05-21 at 20:15 +0200, Sergei Antonov wrote:
> >> On 21 May 2014 18:40, Vyacheslav Dubeyko <slava@dubeyko.com> wrote:
> >> > 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.
> >>
> >> Which is not bad. Those blocks are free space.
> >>
> >
> > For me personally, proper place for sb_issue_zeroout() can be in
> > hfs_bmap_alloc() method
> > (http://lxr.free-electrons.com/source/fs/hfsplus/btree.c#L364):
> >
> >
> >         while (!tree->free_nodes) {
> >                 struct inode *inode = tree->inode;
> >                 struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
> >                 u32 count;
> >                 int res;
> >
> >                 res = hfsplus_file_extend(inode);
> >                 if (res)
> >                         return ERR_PTR(res);
> >
> >                 /* here can be added sb_issue_zeroout() call */
> >
> >                 hip->phys_size = inode->i_size =
> >                         (loff_t)hip->alloc_blocks <<
> >                                 HFSPLUS_SB(tree->sb)->alloc_blksz_shift;
> >                 hip->fs_blocks =
> >                         hip->alloc_blocks << HFSPLUS_SB(tree->sb)->fs_shift;
> >                 inode_set_bytes(inode, inode->i_size);
> >                 count = inode->i_size >> tree->node_size_shift;
> >                 tree->free_nodes = count - tree->node_count;
> >                 tree->node_count = count;
> >         }
> >
> > First of all, here we know that trying to extend file was successful.
> > And, secondly, hfs_bmap_alloc() method is dedicated b-tree case only.
> > I think that modification of hfsplus_file_extend() is not very good
> > idea. The hfs_bmap_alloc() method is more clear solution, from my
> > viewpoint.
> 
> hfs_bmap_alloc() does not know about volume blocks. It is on a higher
> level of abstraction. Try, as an experiment, to write a call to
> sb_issue_zeroout() passing correct arguments from hfs_bmap_alloc().
> 

The hfs_bmap_alloc() has pointer on hfsplus_inode_info structure of
btree's inode. The hfsplus_inode_info structure contains
hip->first_extents, hip->first_blocks, hip->cached_extents,
hip->cached_blocks and so on fields. Finally, hfsplus_file_extend()
method stores allocated extent in hip->first_extents or in
hip->cached_extents. So, I don't see anything impossible in calling
sb_issue_zeroout() with correct arguments from hfs_bmap_alloc(). Maybe
only to call sb_issue_zeroout() from hfsplus_file_extend() is more easy
way. But I think that placement this logic in hfs_bmap_alloc() is more
correct way, from architecture point of view.

Thanks,
Vyacheslav Dubeyko.

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

[Sergei Antonov]

On 22 May 2014 15:07, Vyacheslav Dubeyko <slava@dubeyko.com> wrote:
> On Wed, 2014-05-21 at 20:15 +0200, Sergei Antonov wrote:
>> On 21 May 2014 18:40, Vyacheslav Dubeyko <slava@dubeyko.com> wrote:
>> > 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.
>>
>> Which is not bad. Those blocks are free space.
>>
>
> For me personally, proper place for sb_issue_zeroout() can be in
> hfs_bmap_alloc() method
> (http://lxr.free-electrons.com/source/fs/hfsplus/btree.c#L364):
>
>
>         while (!tree->free_nodes) {
>                 struct inode *inode = tree->inode;
>                 struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
>                 u32 count;
>                 int res;
>
>                 res = hfsplus_file_extend(inode);
>                 if (res)
>                         return ERR_PTR(res);
>
>                 /* here can be added sb_issue_zeroout() call */
>
>                 hip->phys_size = inode->i_size =
>                         (loff_t)hip->alloc_blocks <<
>                                 HFSPLUS_SB(tree->sb)->alloc_blksz_shift;
>                 hip->fs_blocks =
>                         hip->alloc_blocks << HFSPLUS_SB(tree->sb)->fs_shift;
>                 inode_set_bytes(inode, inode->i_size);
>                 count = inode->i_size >> tree->node_size_shift;
>                 tree->free_nodes = count - tree->node_count;
>                 tree->node_count = count;
>         }
>
> First of all, here we know that trying to extend file was successful.
> And, secondly, hfs_bmap_alloc() method is dedicated b-tree case only.
> I think that modification of hfsplus_file_extend() is not very good
> idea. The hfs_bmap_alloc() method is more clear solution, from my
> viewpoint.

hfs_bmap_alloc() does not know about volume blocks. It is on a higher
level of abstraction. Try, as an experiment, to write a call to
sb_issue_zeroout() passing correct arguments from hfs_bmap_alloc().

>> > 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.
>>
>> A result of node creation is only 1 node, but catalog file is expanded
>> in clumps. Normally a clump is at least several megabytes. So the task
>> is to zero these megabytes on disk before (or immediately after) the
>> new extent is added to the catalog.
>>
>> > So, I think that it makes sense to zero out namely prepared pages but
>> > not to initiate request for write via sb_issue_zeroout().
>>
>> You mean mapping pages, do memset(,0,) and flushing them? Slower,
>> memory consuming, complicated.
>>
>
> I worried here about consistency between block state and memory page
> state during a new node allocation. But as I can see
> __hfs_bnode_create() zero out memory page during node creation
> (http://lxr.free-electrons.com/source/fs/hfsplus/bnode.c#L421). So, all
> should be OK.
>
> Thanks,
> Vyacheslav Dubeyko.
>
>

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

[Vyacheslav Dubeyko]

On Wed, 2014-05-21 at 20:15 +0200, Sergei Antonov wrote:
> On 21 May 2014 18:40, Vyacheslav Dubeyko <slava@dubeyko.com> wrote:
> > 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.
> 
> Which is not bad. Those blocks are free space.
> 

For me personally, proper place for sb_issue_zeroout() can be in
hfs_bmap_alloc() method
(http://lxr.free-electrons.com/source/fs/hfsplus/btree.c#L364):


        while (!tree->free_nodes) {
                struct inode *inode = tree->inode;
                struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
                u32 count;
                int res;

                res = hfsplus_file_extend(inode);
                if (res)
                        return ERR_PTR(res);

                /* here can be added sb_issue_zeroout() call */

                hip->phys_size = inode->i_size =
                        (loff_t)hip->alloc_blocks <<
                                HFSPLUS_SB(tree->sb)->alloc_blksz_shift;
                hip->fs_blocks =
                        hip->alloc_blocks << HFSPLUS_SB(tree->sb)->fs_shift;
                inode_set_bytes(inode, inode->i_size);
                count = inode->i_size >> tree->node_size_shift;
                tree->free_nodes = count - tree->node_count;
                tree->node_count = count;
        }

First of all, here we know that trying to extend file was successful.
And, secondly, hfs_bmap_alloc() method is dedicated b-tree case only.
I think that modification of hfsplus_file_extend() is not very good
idea. The hfs_bmap_alloc() method is more clear solution, from my
viewpoint.

> > 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.
> 
> A result of node creation is only 1 node, but catalog file is expanded
> in clumps. Normally a clump is at least several megabytes. So the task
> is to zero these megabytes on disk before (or immediately after) the
> new extent is added to the catalog.
> 
> > So, I think that it makes sense to zero out namely prepared pages but
> > not to initiate request for write via sb_issue_zeroout().
> 
> You mean mapping pages, do memset(,0,) and flushing them? Slower,
> memory consuming, complicated.
> 

I worried here about consistency between block state and memory page
state during a new node allocation. But as I can see
__hfs_bnode_create() zero out memory page during node creation
(http://lxr.free-electrons.com/source/fs/hfsplus/bnode.c#L421). So, all
should be OK.

Thanks,
Vyacheslav Dubeyko.

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

[Sergei Antonov]

On 21 May 2014 18:40, Vyacheslav Dubeyko <slava@dubeyko.com> wrote:
> 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.

Which is not bad. Those blocks are free space.

> 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.

A result of node creation is only 1 node, but catalog file is expanded
in clumps. Normally a clump is at least several megabytes. So the task
is to zero these megabytes on disk before (or immediately after) the
new extent is added to the catalog.

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

You mean mapping pages, do memset(,0,) and flushing them? Slower,
memory consuming, complicated.

>> +             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.

Yes, for consistency with other prototypes.

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

[Vyacheslav Dubeyko]

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.

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

[Sergei Antonov]

Zero newly allocated extents in the catalog tree if volume attributes tell
us to. Not doing so we risk getting the "unused node is not erased" error.
See kHFSUnusedNodeFix flag in Apple's source code for reference.

There was a previous commit clearing the node when it is freed:
  commit 899bed05e9f6bbb21776f9ebd88f5631987f987a
  Author: Vyacheslav Dubeyko <slava@dubeyko.com>
  Date:   Wed Feb 27 17:03:06 2013 -0800
  hfsplus: fix issue with unzeroed unused b-tree nodes
It did not handle newly allocated extents (this patch fixes it). And it zeroed
nodes in all trees unconditionally which is an overkill. This patch adds a
condition and also switches to 'tree->node_size' as a simpler method of getting
the length to zero.

Cc: Anton Altaparmakov <aia21@cam.ac.uk>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Vyacheslav Dubeyko <slava@dubeyko.com>
Cc: Hin-Tak Leung <htl10@users.sourceforge.net>
Cc: Kyle Laracey <kalaracey@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Sergei Antonov <saproj@gmail.com>
---
 fs/hfsplus/bnode.c       | 17 +++++++++++++++--
 fs/hfsplus/btree.c       |  2 +-
 fs/hfsplus/extents.c     | 10 ++++++++--
 fs/hfsplus/hfsplus_fs.h  |  3 ++-
 fs/hfsplus/hfsplus_raw.h |  1 +
 fs/hfsplus/xattr.c       |  2 +-
 6 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
index 11c8602..15a844f 100644
--- a/fs/hfsplus/bnode.c
+++ b/fs/hfsplus/bnode.c
@@ -648,8 +648,8 @@ void hfs_bnode_put(struct hfs_bnode *node)
 		if (test_bit(HFS_BNODE_DELETED, &node->flags)) {
 			hfs_bnode_unhash(node);
 			spin_unlock(&tree->hash_lock);
-			hfs_bnode_clear(node, 0,
-				PAGE_CACHE_SIZE * tree->pages_per_bnode);
+			if (hfs_bnode_need_zeroout(tree))
+				hfs_bnode_clear(node, 0, tree->node_size);
 			hfs_bmap_free(node);
 			hfs_bnode_free(node);
 			return;
@@ -658,3 +658,16 @@ void hfs_bnode_put(struct hfs_bnode *node)
 	}
 }
 
+/*
+ * Unused nodes have to be zeroed if this is the catalog tree and
+ * a corresponding flag in the volume header is set.
+ */
+bool hfs_bnode_need_zeroout(struct hfs_btree *tree)
+{
+	struct super_block *sb = tree->inode->i_sb;
+	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
+	const u32 volume_attr = be32_to_cpu(sbi->s_vhdr->attributes);
+
+	return tree->cnid == HFSPLUS_CAT_CNID &&
+		volume_attr & HFSPLUS_VOL_UNUSED_NODE_FIX;
+}
diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
index 0fcec8b..3345c75 100644
--- a/fs/hfsplus/btree.c
+++ b/fs/hfsplus/btree.c
@@ -358,7 +358,7 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
 		u32 count;
 		int res;
 
-		res = hfsplus_file_extend(inode);
+		res = hfsplus_file_extend(inode, hfs_bnode_need_zeroout(tree));
 		if (res)
 			return ERR_PTR(res);
 		hip->phys_size = inode->i_size =
diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
index a7aafb35..a09fcb6 100644
--- a/fs/hfsplus/extents.c
+++ b/fs/hfsplus/extents.c
@@ -235,7 +235,7 @@ int hfsplus_get_block(struct inode *inode, sector_t iblock,
 		if (iblock > hip->fs_blocks || !create)
 			return -EIO;
 		if (ablock >= hip->alloc_blocks) {
-			res = hfsplus_file_extend(inode);
+			res = hfsplus_file_extend(inode, false);
 			if (res)
 				return res;
 		}
@@ -425,7 +425,7 @@ int hfsplus_free_fork(struct super_block *sb, u32 cnid,
 	return res;
 }
 
-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);
+		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);
 void hfsplus_file_truncate(struct inode *);
 
 /* inode.c */
diff --git a/fs/hfsplus/hfsplus_raw.h b/fs/hfsplus/hfsplus_raw.h
index 5a12682..8298d09 100644
--- a/fs/hfsplus/hfsplus_raw.h
+++ b/fs/hfsplus/hfsplus_raw.h
@@ -144,6 +144,7 @@ struct hfsplus_vh {
 #define HFSPLUS_VOL_NODEID_REUSED	(1 << 12)
 #define HFSPLUS_VOL_JOURNALED		(1 << 13)
 #define HFSPLUS_VOL_SOFTLOCK		(1 << 15)
+#define HFSPLUS_VOL_UNUSED_NODE_FIX	(1 << 31)
 
 /* HFS+ BTree node descriptor */
 struct hfs_bnode_desc {
diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
index 4e27edc..1ed05d0 100644
--- a/fs/hfsplus/xattr.c
+++ b/fs/hfsplus/xattr.c
@@ -195,7 +195,7 @@ check_attr_tree_state_again:
 	}
 
 	while (hip->alloc_blocks < hip->clump_blocks) {
-		err = hfsplus_file_extend(attr_file);
+		err = hfsplus_file_extend(attr_file, false);
 		if (unlikely(err)) {
 			pr_err("failed to extend attributes file\n");
 			goto end_attr_file_creation;
-- 
1.9.0