cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
From: Bob Peterson <rpeterso@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [gfs2-utils PATCH 03/47] libgfs2: let dir_split_leaf receive a "broken" lindex
Date: Mon, 20 May 2013 12:02:07 -0400 (EDT)	[thread overview]
Message-ID: <1343795838.18040609.1369065727393.JavaMail.root@redhat.com> (raw)
In-Reply-To: <1368633712.2732.100.camel@menhir>

----- Original Message -----
| Hi,
| 
| On Tue, 2013-05-14 at 11:21 -0500, Bob Peterson wrote:
| > For ordinary leaf blocks, the hash table must follow the rules,
| > which means it needs to follow a power-of-two boundary. In other
| > words, it needs to enforce that: start = (lindex & ~(len - 1));
| > But when doing repairs, fsck will need to detect when hash tables
| > violate this rule and fix it. In that case, it may need to pass
| > in an invalid starting offset for a leaf to split. This patch
| > moves the responsibility for checking the starting block to the
| > calling function.
| > 
| > rhbz#902920
| > ---
| >  gfs2/libgfs2/fs_ops.c | 9 ++++++---
| >  1 file changed, 6 insertions(+), 3 deletions(-)
| > 
| > diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c
| > index 89adf32..11ef6b4 100644
| > --- a/gfs2/libgfs2/fs_ops.c
| > +++ b/gfs2/libgfs2/fs_ops.c
| > @@ -957,7 +957,7 @@ void dir_split_leaf(struct gfs2_inode *dip, uint32_t
| > lindex, uint64_t leaf_no,
| >  	len = 1 << (dip->i_di.di_depth - be16_to_cpu(oleaf->lf_depth));
| >  	half_len = len >> 1;
| >  
| > -	start = (lindex & ~(len - 1));
| > +	start = lindex;
| Why not just rename the lindex as start? Otherwise this might be
| confusing to have two names for the same variable,
| 
| Steve.

Hi,

Good idea. I've implemented your change, and the replacement patch is
given below.

Regards,

Bob Peterson
Red Hat File Systems
---
commit 7e1170eec23957b084ca80828eac9fd1c8988062
Author: Bob Peterson <rpeterso@redhat.com>
Date:   Thu Feb 21 09:36:01 2013 -0700

    libgfs2: let dir_split_leaf receive a "broken" lindex
    
    For ordinary leaf blocks, the hash table must follow the rules,
    which means it needs to follow a power-of-two boundary. In other
    words, it needs to enforce that: start = (lindex & ~(len - 1));
    But when doing repairs, fsck will need to detect when hash tables
    violate this rule and fix it. In that case, it may need to pass
    in an invalid starting offset for a leaf to split. This patch
    moves the responsibility for checking the starting block to the
    calling function.

diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c
index 89adf32..d009e2f 100644
--- a/gfs2/libgfs2/fs_ops.c
+++ b/gfs2/libgfs2/fs_ops.c
@@ -925,13 +925,13 @@ void gfs2_put_leaf_nr(struct gfs2_inode *dip, uint32_t inx, uint64_t leaf_out)
 	}
 }
 
-void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex, uint64_t leaf_no,
+void dir_split_leaf(struct gfs2_inode *dip, uint32_t start, uint64_t leaf_no,
 		    struct gfs2_buffer_head *obh)
 {
 	struct gfs2_buffer_head *nbh;
 	struct gfs2_leaf *nleaf, *oleaf;
 	struct gfs2_dirent *dent, *prev = NULL, *next = NULL, *new;
-	uint32_t start, len, half_len, divider;
+	uint32_t len, half_len, divider;
 	uint64_t bn, *lp;
 	uint32_t name_len;
 	int x, moved = FALSE;
@@ -957,8 +957,6 @@ void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex, uint64_t leaf_no,
 	len = 1 << (dip->i_di.di_depth - be16_to_cpu(oleaf->lf_depth));
 	half_len = len >> 1;
 
-	start = (lindex & ~(len - 1));
-
 	lp = calloc(1, half_len * sizeof(uint64_t));
 	if (lp == NULL) {
 		fprintf(stderr, "Out of memory in %s\n", __FUNCTION__);
@@ -1160,7 +1158,7 @@ static int dir_e_add(struct gfs2_inode *dip, const char *filename, int len,
 	struct gfs2_buffer_head *bh, *nbh;
 	struct gfs2_leaf *leaf, *nleaf;
 	struct gfs2_dirent *dent;
-	uint32_t lindex;
+	uint32_t lindex, llen;
 	uint32_t hash;
 	uint64_t leaf_no, bn;
 	int err = 0;
@@ -1182,7 +1180,10 @@ restart:
 		if (dirent_alloc(dip, bh, len, &dent)) {
 
 			if (be16_to_cpu(leaf->lf_depth) < dip->i_di.di_depth) {
-				dir_split_leaf(dip, lindex, leaf_no, bh);
+				llen = 1 << (dip->i_di.di_depth -
+					     be16_to_cpu(leaf->lf_depth));
+				dir_split_leaf(dip, lindex & ~(llen - 1),
+					       leaf_no, bh);
 				brelse(bh);
 				goto restart;
 
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 3147c83..3055355 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -468,7 +468,7 @@ extern void block_map(struct gfs2_inode *ip, uint64_t lblock, int *new,
 extern void gfs2_get_leaf_nr(struct gfs2_inode *dip, uint32_t index,
 			     uint64_t *leaf_out);
 extern void gfs2_put_leaf_nr(struct gfs2_inode *dip, uint32_t inx, uint64_t leaf_out);
-extern void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex,
+extern void dir_split_leaf(struct gfs2_inode *dip, uint32_t start,
 			   uint64_t leaf_no, struct gfs2_buffer_head *obh);
 extern void gfs2_free_block(struct gfs2_sbd *sdp, uint64_t block);
 extern int gfs2_freedi(struct gfs2_sbd *sdp, uint64_t block);



  reply	other threads:[~2013-05-20 16:02 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-14 16:21 [Cluster-devel] [gfs2-utils PATCH 01/47] libgfs2: externalize dir_split_leaf Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 02/47] libgfs2: allow dir_split_leaf to receive a leaf buffer Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 03/47] libgfs2: let dir_split_leaf receive a "broken" lindex Bob Peterson
2013-05-15 16:01   ` Steven Whitehouse
2013-05-20 16:02     ` Bob Peterson [this message]
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 04/47] fsck.gfs2: Move function find_free_blk to util.c Bob Peterson
2013-05-15 16:04   ` Steven Whitehouse
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 05/47] fsck.gfs2: Split out function to make sure lost+found exists Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 06/47] fsck.gfs2: Check for formal inode mismatch when adding to lost+found Bob Peterson
2013-05-15 16:08   ` Steven Whitehouse
2013-05-17 12:47     ` Bob Peterson
2013-05-17 12:55       ` Steven Whitehouse
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 07/47] fsck.gfs2: shorten some debug messages in lost+found Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 08/47] fsck.gfs2: Move basic directory entry checks to separate function Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 09/47] fsck.gfs2: Add formal inode check to basic dirent checks Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 10/47] fsck.gfs2: Add new function to check dir hash tables Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 11/47] fsck.gfs2: Special case '..' when processing bad formal inode number Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 12/47] fsck.gfs2: Move function to read directory hash table to util.c Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 13/47] fsck.gfs2: Misc cleanups Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 14/47] fsck.gfs2: Verify dirent hash values correspond to proper leaf block Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 15/47] fsck.gfs2: re-read hash table if directory height or depth changes Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 16/47] fsck.gfs2: fix leaf blocks, don't try to patch the hash table Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 17/47] fsck.gfs2: check leaf depth when validating leaf blocks Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 18/47] fsck.gfs2: small cleanups Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 19/47] fsck.gfs2: reprocess inodes when blocks are added Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 20/47] fsck.gfs2: Remove redundant leaf depth check Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 21/47] fsck.gfs2: link dinodes that only have extended attribute problems Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 22/47] fsck.gfs2: Add clarifying message to duplicate processing Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 23/47] fsck.gfs2: separate function to calculate metadata block header size Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 24/47] fsck.gfs2: Rework the "undo" functions Bob Peterson
2013-05-16 13:27   ` Steven Whitehouse
2013-05-16 13:49     ` Bob Peterson
2013-05-16 14:02       ` Steven Whitehouse
2013-05-16 15:02         ` Bob Peterson
2013-05-16 15:24           ` Steven Whitehouse
2013-05-20 13:08             ` Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 25/47] fsck.gfs2: Check for interrupt when resolving duplicates Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 26/47] fsck.gfs2: Consistent naming of struct duptree variables Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 27/47] fsck.gfs2: Keep proper counts when duplicates are found Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 28/47] fsck.gfs2: print metadata block reference on data errors Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 29/47] fsck.gfs2: print block count values when fixing them Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 30/47] fsck.gfs2: Do not invalidate metablocks of dinodes with invalid mode Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 31/47] fsck.gfs2: Log when unrecoverable data block errors are encountered Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 32/47] fsck.gfs2: don't remove buffers from the list when errors are found Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 33/47] fsck.gfs2: Don't flag GFS1 non-dinode blocks as duplicates Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 34/47] fsck.gfs2: externalize check_leaf Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 35/47] fsck.gfs2: pass2: check leaf blocks when fixing hash table Bob Peterson
2013-05-14 16:21 ` [Cluster-devel] [gfs2-utils PATCH 36/47] fsck.gfs2: standardize check_metatree return codes Bob Peterson
2013-05-14 16:22 ` [Cluster-devel] [gfs2-utils PATCH 37/47] fsck.gfs2: don't invalidate files with duplicate data block refs Bob Peterson
2013-05-14 16:22 ` [Cluster-devel] [gfs2-utils PATCH 38/47] fsck.gfs2: check for duplicate first references Bob Peterson
2013-05-14 16:22 ` [Cluster-devel] [gfs2-utils PATCH 39/47] fsck.gfs2: When flagging a duplicate reference, show valid or invalid Bob Peterson
2013-05-14 16:22 ` [Cluster-devel] [gfs2-utils PATCH 40/47] fsck.gfs2: major duplicate reference reform Bob Peterson
2013-05-14 16:22 ` [Cluster-devel] [gfs2-utils PATCH 41/47] fsck.gfs2: Remove all bad eattr blocks Bob Peterson
2013-05-14 16:22 ` [Cluster-devel] [gfs2-utils PATCH 42/47] fsck.gfs2: Remove unused variable Bob Peterson
2013-05-14 16:22 ` [Cluster-devel] [gfs2-utils PATCH 43/47] fsck.gfs2: double-check transitions from dinode to data Bob Peterson
2013-05-14 16:22 ` [Cluster-devel] [gfs2-utils PATCH 44/47] fsck.gfs2: Stop "undo" process when error data block is reached Bob Peterson
2013-05-14 16:22 ` [Cluster-devel] [gfs2-utils PATCH 45/47] fsck.gfs2: Don't allocate leaf blocks in pass1 Bob Peterson
2013-05-14 16:22 ` [Cluster-devel] [gfs2-utils PATCH 46/47] fsck.gfs2: take hash table start boundaries into account Bob Peterson
2013-05-14 16:22 ` [Cluster-devel] [gfs2-utils PATCH 47/47] fsck.gfs2: delete all duplicates from unrecoverable damaged dinodes Bob Peterson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1343795838.18040609.1369065727393.JavaMail.root@redhat.com \
    --to=rpeterso@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).