------------------------------
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
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
