Acked, and also a comment towards the end (so please keep reading beyond the Ack!).
--- On Fri, 30/11/12, Vyacheslav Dubeyko <slava@xxxxxxxxxxx> wrote:
> Hi,
>
> The patch adds in hfs_btree_write() returning of -EIO in the
> case of failing of b-tree's node searching.
>
> With the best regards,
> Vyacheslav Dubeyko.
> --
> From: Vyacheslav Dubeyko <slava@xxxxxxxxxxx>
> Subject: [PATCH] hfsplus: rework processing of
> hfs_btree_write() returned error
>
> The patch adds in hfs_btree_write() returning of -EIO in the
> case of failing of b-tree's node searching. It is added also
> logic of processing errors of hfs_btree_write() in
> hfsplus_system_write_inode() with message about b-tree
> writing failure.
>
> Signed-off-by: Vyacheslav Dubeyko <slava@xxxxxxxxxxx>
Acked-by: Hin-Tak Leung <htl10@xxxxxxxxxxxxxxxxxxxxx>
> ---
> fs/hfsplus/btree.c | 5
> +++--
> fs/hfsplus/hfsplus_fs.h | 2 +-
> fs/hfsplus/super.c
> | 12 ++++++++++--
> 3 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
> index 21023d9..685d07d 100644
> --- a/fs/hfsplus/btree.c
> +++ b/fs/hfsplus/btree.c
> @@ -159,7 +159,7 @@ void hfs_btree_close(struct hfs_btree
> *tree)
> kfree(tree);
> }
>
> -void hfs_btree_write(struct hfs_btree *tree)
> +int hfs_btree_write(struct hfs_btree *tree)
> {
> struct hfs_btree_header_rec *head;
> struct hfs_bnode *node;
> @@ -168,7 +168,7 @@ void hfs_btree_write(struct hfs_btree
> *tree)
> node = hfs_bnode_find(tree, 0);
> if (IS_ERR(node))
> /* panic? */
> - return;
> + return -EIO;
> /* Load the header */
> page = node->page[0];
> head = (struct hfs_btree_header_rec
> *)(kmap(page) +
> @@ -186,6 +186,7 @@ void hfs_btree_write(struct hfs_btree
> *tree)
> kunmap(page);
> set_page_dirty(page);
> hfs_bnode_put(node);
> + return 0;
> }
>
> static struct hfs_bnode *hfs_bmap_new_bmap(struct hfs_bnode
> *prev, u32 idx)
> diff --git a/fs/hfsplus/hfsplus_fs.h
> b/fs/hfsplus/hfsplus_fs.h
> index c571de2..a6da86b 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -335,7 +335,7 @@ int hfsplus_block_free(struct
> super_block *, u32, u32);
> /* btree.c */
> struct hfs_btree *hfs_btree_open(struct super_block *,
> u32);
> void hfs_btree_close(struct hfs_btree *);
> -void hfs_btree_write(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 *);
>
> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> index 811a84d..3033b73 100644
> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -99,6 +99,7 @@ static int
> hfsplus_system_write_inode(struct inode *inode)
> struct hfsplus_vh *vhdr =
> sbi->s_vhdr;
> struct hfsplus_fork_raw *fork;
> struct hfs_btree *tree = NULL;
> + int err;
>
> switch (inode->i_ino) {
> case HFSPLUS_EXT_CNID:
> @@ -127,8 +128,15 @@ static int
> hfsplus_system_write_inode(struct inode *inode)
>
> hfsplus_mark_mdb_dirty(inode->i_sb);
> }
> hfsplus_inode_write_fork(inode, fork);
> - if (tree)
> -
> hfs_btree_write(tree);
> + if (tree) {
> + err =
> hfs_btree_write(tree);
> + if (err) {
> +
> printk(KERN_ERR "hfs: unable to write b-tree\n");
> +
> dprint(DBG_INODE, "hfsplus_system_write_inode: %lu\n",
> +
> inode->i_ino);
> +
> return err;
> + }
> + }
> return 0;
> }
>
There is a small issue which crossed my mind when I first noticed with:
"hfsplus: rework processing errors in hfsplus_free_extents()"
It is a printk() followed by dprint() with additional information, on error. Because dprint() needs a compile-time change (or at least, echo'ing into debugfs if switched to dynamic-debug). This means the additional information would not be seen the first time the error occurred. I am wondering whether the additional info should be included in the printk - in a short-hand form, maybe?
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
