[Ocfs2-devel] [PATCH 1/1] Ocfs2: Add missing ocfs2_journal_acces_*() for couple of funcs in dir.c
tristan
tristan.ye at oracle.com
Tue Aug 31 18:25:15 PDT 2010
Hi Mark,
Could you please take a glance at this patch:-)
Tristan.
Tristan Ye wrote:
> To correctly journal the metadata in ocfs2, it's known for us to call
> ocfs2_journal_access_*() and ocfs2_journal_dirty()to mark buffer dirty,
> they are expected to exist in a pair.
>
> Whereas several funcs for dx-dirs manipulation were forgeting to call
> appropriate ocfs2_journal_access*() to correctly journal the dirty
> metadata, which may cause a BUG in jbd2, reporting a NULL pointer
> gets ASSERTED in the journal buffer head.
>
> Currently, we found three functions being hurt in dir.c, all serving
> for dx-dirs:
>
> - ocfs2_dx_dir_transfer_leaf()
> - ocfs2_remove_block_from_free_list()
> - ocfs2_recalc_free_list()
>
> Signed-off-by: Tristan Ye <tristan.ye at oracle.com>
> ---
> fs/ocfs2/dir.c | 109 +++++++++++++++++++++++++++++++++++++++++++++-----------
> 1 files changed, 88 insertions(+), 21 deletions(-)
>
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index f04ebcf..fe89136 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -1556,41 +1556,67 @@ out:
> return ret;
> }
>
> -static void ocfs2_remove_block_from_free_list(struct inode *dir,
> - handle_t *handle,
> - struct ocfs2_dir_lookup_result *lookup)
> +static int ocfs2_remove_block_from_free_list(struct inode *dir,
> + handle_t *handle,
> + struct ocfs2_dir_lookup_result *lookup)
> {
> + int status = 0;
> struct ocfs2_dir_block_trailer *trailer, *prev;
> struct ocfs2_dx_root_block *dx_root;
> struct buffer_head *bh;
> + ocfs2_journal_access_func access = ocfs2_journal_access_dr;
>
> trailer = ocfs2_trailer_from_bh(lookup->dl_leaf_bh, dir->i_sb);
>
> if (ocfs2_free_list_at_root(lookup)) {
> bh = lookup->dl_dx_root_bh;
> + status = access(handle, INODE_CACHE(dir), bh,
> + OCFS2_JOURNAL_ACCESS_WRITE);
> + if (status < 0) {
> + mlog_errno(status);
> + return status;
> + }
> dx_root = (struct ocfs2_dx_root_block *)bh->b_data;
> dx_root->dr_free_blk = trailer->db_free_next;
> } else {
> + access = ocfs2_journal_access_db;
> bh = lookup->dl_prev_leaf_bh;
> + status = access(handle, INODE_CACHE(dir), bh,
> + OCFS2_JOURNAL_ACCESS_WRITE);
> + if (status < 0) {
> + mlog_errno(status);
> + return status;
> + }
> prev = ocfs2_trailer_from_bh(bh, dir->i_sb);
> prev->db_free_next = trailer->db_free_next;
> }
>
> + ocfs2_journal_dirty(handle, bh);
> +
> + status = ocfs2_journal_access_db(handle, INODE_CACHE(dir),
> + lookup->dl_leaf_bh,
> + OCFS2_JOURNAL_ACCESS_WRITE);
> + if (status < 0) {
> + mlog_errno(status);
> + return status;
> + }
> +
> trailer->db_free_rec_len = cpu_to_le16(0);
> trailer->db_free_next = cpu_to_le64(0);
>
> - ocfs2_journal_dirty(handle, bh);
> ocfs2_journal_dirty(handle, lookup->dl_leaf_bh);
> +
> + return status;
> }
>
> /*
> * This expects that a journal write has been reserved on
> * lookup->dl_prev_leaf_bh or lookup->dl_dx_root_bh
> */
> -static void ocfs2_recalc_free_list(struct inode *dir, handle_t *handle,
> - struct ocfs2_dir_lookup_result *lookup)
> +static int ocfs2_recalc_free_list(struct inode *dir, handle_t *handle,
> + struct ocfs2_dir_lookup_result *lookup)
> {
> - int max_rec_len;
> + int max_rec_len, status = 0;
> struct ocfs2_dir_block_trailer *trailer;
>
> /* Walk dl_leaf_bh to figure out what the new free rec_len is. */
> @@ -1601,12 +1627,24 @@ static void ocfs2_recalc_free_list(struct inode *dir, handle_t *handle,
> * from the free list. In this case, we just want to update
> * the rec len accounting.
> */
> + status = ocfs2_journal_access_db(handle, INODE_CACHE(dir),
> + lookup->dl_leaf_bh,
> + OCFS2_JOURNAL_ACCESS_WRITE);
> + if (status < 0) {
> + mlog_errno(status);
> + return status;
> + }
> +
> trailer = ocfs2_trailer_from_bh(lookup->dl_leaf_bh, dir->i_sb);
> trailer->db_free_rec_len = cpu_to_le16(max_rec_len);
> ocfs2_journal_dirty(handle, lookup->dl_leaf_bh);
> } else {
> - ocfs2_remove_block_from_free_list(dir, handle, lookup);
> + status = ocfs2_remove_block_from_free_list(dir, handle, lookup);
> + if (status < 0)
> + mlog_errno(status);
> }
> +
> + return status;
> }
>
> /* we don't always have a dentry for what we want to add, so people
> @@ -1748,8 +1786,14 @@ int __ocfs2_add_entry(handle_t *handle,
> de->name_len = namelen;
> memcpy(de->name, name, namelen);
>
> - if (ocfs2_dir_indexed(dir))
> - ocfs2_recalc_free_list(dir, handle, lookup);
> + if (ocfs2_dir_indexed(dir)) {
> + retval = ocfs2_recalc_free_list(dir, handle,
> + lookup);
> + if (retval < 0) {
> + mlog_errno(retval);
> + goto bail;
> + }
> + }
>
> dir->i_version++;
> ocfs2_journal_dirty(handle, insert_bh);
> @@ -3736,14 +3780,14 @@ static int ocfs2_dx_dir_find_leaf_split(struct ocfs2_dx_leaf *dx_leaf,
> * of minor_hash, we can optimize - an item at block offset X within
> * the original cluster, will be at offset X within the new cluster.
> */
> -static void ocfs2_dx_dir_transfer_leaf(struct inode *dir, u32 split_hash,
> - handle_t *handle,
> - struct ocfs2_dx_leaf *tmp_dx_leaf,
> - struct buffer_head **orig_dx_leaves,
> - struct buffer_head **new_dx_leaves,
> - int num_dx_leaves)
> +static int ocfs2_dx_dir_transfer_leaf(struct inode *dir, u32 split_hash,
> + handle_t *handle,
> + struct ocfs2_dx_leaf *tmp_dx_leaf,
> + struct buffer_head **orig_dx_leaves,
> + struct buffer_head **new_dx_leaves,
> + int num_dx_leaves)
> {
> - int i, j, num_used;
> + int i, j, num_used, status = 0;
> u32 major_hash;
> struct ocfs2_dx_leaf *orig_dx_leaf, *new_dx_leaf;
> struct ocfs2_dx_entry_list *orig_list, *new_list, *tmp_list;
> @@ -3763,6 +3807,14 @@ static void ocfs2_dx_dir_transfer_leaf(struct inode *dir, u32 split_hash,
> tmp_list->de_num_used = cpu_to_le16(0);
> memset(&tmp_list->de_entries, 0, sizeof(*dx_entry)*num_used);
>
> + status = ocfs2_journal_access_dl(handle, INODE_CACHE(dir),
> + new_dx_leaves[i],
> + OCFS2_JOURNAL_ACCESS_WRITE);
> + if (status < 0) {
> + mlog_errno(status);
> + return status;
> + }
> +
> for (j = 0; j < num_used; j++) {
> dx_entry = &orig_list->de_entries[j];
> major_hash = le32_to_cpu(dx_entry->dx_major_hash);
> @@ -3773,11 +3825,23 @@ static void ocfs2_dx_dir_transfer_leaf(struct inode *dir, u32 split_hash,
> ocfs2_dx_dir_leaf_insert_tail(tmp_dx_leaf,
> dx_entry);
> }
> +
> + ocfs2_journal_dirty(handle, new_dx_leaves[i]);
> +
> + status = ocfs2_journal_access_dl(handle, INODE_CACHE(dir),
> + orig_dx_leaves[i],
> + OCFS2_JOURNAL_ACCESS_WRITE);
> + if (status < 0) {
> + mlog_errno(status);
> + return status;
> + }
> +
> memcpy(orig_dx_leaf, tmp_dx_leaf, dir->i_sb->s_blocksize);
>
> ocfs2_journal_dirty(handle, orig_dx_leaves[i]);
> - ocfs2_journal_dirty(handle, new_dx_leaves[i]);
> }
> +
> + return status;
> }
>
> static int ocfs2_dx_dir_rebalance_credits(struct ocfs2_super *osb,
> @@ -3950,8 +4014,11 @@ static int ocfs2_dx_dir_rebalance(struct ocfs2_super *osb, struct inode *dir,
> goto out_commit;
> }
>
> - ocfs2_dx_dir_transfer_leaf(dir, split_hash, handle, tmp_dx_leaf,
> - orig_dx_leaves, new_dx_leaves, num_dx_leaves);
> + ret = ocfs2_dx_dir_transfer_leaf(dir, split_hash, handle, tmp_dx_leaf,
> + orig_dx_leaves, new_dx_leaves,
> + num_dx_leaves);
> + if (ret)
> + mlog_errno(ret);
>
> out_commit:
> if (ret < 0 && did_quota)
>
More information about the Ocfs2-devel
mailing list