gfs2: Remove sdp->sd_jheightsize

PATCH New Andreas Gruenbacher 2 participants 4 messages Apr 13, 2018 – Apr 16, 2018 GFS2 file system

Share & tools

Andreas Gruenbacher

author maintainer

2018-04-13 13:01 UTC #0 lore raw

PATCH 1/1 gfs2: Remove sdp->sd_jheightsize

GFS2 keeps two arrarys in the superblock that define the maximum size of an inode depending on the inode's height: sdp->sd_heightsize defines the heights in units of sb->s_blocksize; sdp->sd_jheightsize defines them in units of sb->s_blocksize - sizeof(struct gfs2_meta_header). These arrays are used to determine when additional layers of indirect blocks are needed. The second array is used for directories which have an additional gfs2_meta_header at the beginning of each block.

Distinguishing between these two cases makes no sense: the height required for representing N blocks will come out the same no matter if the calculation is done in gross (sb->s_blocksize) or net (sb->s_blocksize - sizeof(struct gfs2_meta_header)) units.

Stuffed directories don't have an additional gfs2_meta_header, but the stuffed case is handled separately for both files and directories, anyway.

Remove the unncessary sdp->sd_jheightsize array.

Signed-off-by Andreas Gruenbacher <[email protected]>

---
 fs/gfs2/bmap.c       | 14 +-------------
 fs/gfs2/incore.h     |  2 --
 fs/gfs2/ops_fstype.c | 19 -------------------
 3 files changed, 1 insertion(+), 34 deletions(-)

fs/gfs2/bmap.c +1 ~~-13~~

index 278ed0869c3c..0590e93494f7 100644
@@ -700,8 +700,6 @@ int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
 	struct metapath mp = { .mp_aheight = 1, };
-	unsigned int factor = sdp->sd_sb.sb_bsize;
-	const u64 *arr = sdp->sd_heightsize;
 	__be64 *ptr;
 	sector_t lblock;
 	sector_t lend;

@@ -737,22 +735,12 @@ int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
 	iomap->flags = IOMAP_F_MERGED;
 	bmap_lock(ip, flags & IOMAP_WRITE);
 
-	/*
-	 * Directory data blocks have a struct gfs2_meta_header header, so the
-	 * remaining size is smaller than the filesystem block size.  Logical
-	 * block numbers for directories are in units of this remaining size!
-	 */
-	if (gfs2_is_dir(ip)) {
-		factor = sdp->sd_jbsize;
-		arr = sdp->sd_jheightsize;
-	}
-
 	ret = gfs2_meta_inode_buffer(ip, &mp.mp_bh[0]);
 	if (ret)
 		goto out_release;
 
 	height = ip->i_height;
-	while ((lblock + 1) * factor > arr[height])
+	while ((lblock + 1) * sdp->sd_sb.sb_bsize > sdp->sd_heightsize[height])
 		height++;
 	find_metapath(sdp, lblock, &mp, height);
 	if (height > ip->i_height || gfs2_is_stuffed(ip))

fs/gfs2/incore.h +0 -2

index 1b6b1e3f5caf..0bbbaa9b05cb 100644
@@ -696,8 +696,6 @@ struct gfs2_sbd {
 	u32 sd_max_dirres;	/* Max blocks needed to add a directory entry */
 	u32 sd_max_height;	/* Max height of a file's metadata tree */
 	u64 sd_heightsize[GFS2_MAX_META_HEIGHT + 1];
-	u32 sd_max_jheight; /* Max height of journaled file's meta tree */
-	u64 sd_jheightsize[GFS2_MAX_META_HEIGHT + 1];
 	u32 sd_max_dents_per_leaf; /* Max number of dirents in a leaf block */
 
 	struct gfs2_args sd_args;	/* Mount arguments */

fs/gfs2/ops_fstype.c +0 ~~-19~~

index 3ba3f167641c..c2469833b4fb 100644
@@ -335,25 +335,6 @@ static int gfs2_read_sb(struct gfs2_sbd *sdp, int silent)
 	sdp->sd_heightsize[x] = ~0;
 	gfs2_assert(sdp, sdp->sd_max_height <= GFS2_MAX_META_HEIGHT);
 
-	sdp->sd_jheightsize[0] = sdp->sd_sb.sb_bsize -
-				 sizeof(struct gfs2_dinode);
-	sdp->sd_jheightsize[1] = sdp->sd_jbsize * sdp->sd_diptrs;
-	for (x = 2;; x++) {
-		u64 space, d;
-		u32 m;
-
-		space = sdp->sd_jheightsize[x - 1] * sdp->sd_inptrs;
-		d = space;
-		m = do_div(d, sdp->sd_inptrs);
-
-		if (d != sdp->sd_jheightsize[x - 1] || m)
-			break;
-		sdp->sd_jheightsize[x] = space;
-	}
-	sdp->sd_max_jheight = x;
-	sdp->sd_jheightsize[x] = ~0;
-	gfs2_assert(sdp, sdp->sd_max_jheight <= GFS2_MAX_META_HEIGHT);
-
 	sdp->sd_max_dents_per_leaf = (sdp->sd_sb.sb_bsize -
 				      sizeof(struct gfs2_leaf)) /
 				     GFS2_MIN_DIRENT_SIZE;

Signature · 1 line

2.14.3

Bob Peterson

2018-04-13 13:29 UTC #1 lore raw

PATCH 1/1 gfs2: Remove sdp->sd_jheightsize

----- Original Message -----

Quoted 8 lines

GFS2 keeps two arrarys in the superblock that define the maximum size of an inode depending on the inode's height: sdp->sd_heightsize defines the heights in units of sb->s_blocksize; sdp->sd_jheightsize defines them in units of sb->s_blocksize - sizeof(struct gfs2_meta_header). These arrays are used to determine when additional layers of indirect blocks are needed. The second array is used for directories which have an additional gfs2_meta_header at the beginning of each block.

Quoted 4 lines

Distinguishing between these two cases makes no sense: the height required for representing N blocks will come out the same no matter if the calculation is done in gross (sb->s_blocksize) or net (sb->s_blocksize - sizeof(struct gfs2_meta_header)) units.

Hm. Granted it's early morning for me and the caffeine still hasn't made it to my brain, but I'm not convinced the above statement is true.

I know in almost all cases it's a stupid thing to do, but if the block size is 512 bytes:

regular file: 512/8 bytes per pointer = 64 indirect pointers per block regular file: max is: 64 * 512 = 32768 bytes before it goes to height 2

directory:    488/8 bytes per pointer = 61 indirect pointers per block
directory:    max is: 61 * 512 = 31232, but the number of pointers in a

directory is always power of 2.

So for a directory, if the exhash algorithm doubles the hash table size from 2048 pointers to 4096 pointers, the table needs to go from 16K to 32K bytes. Thus, the height needs to go from 1 to 2, whereas if the same thing was done to a file, the 16K would still fit inside a dinode of height 1. Right?

Bob Peterson
Red Hat File Systems

Andreas Gruenbacher

author maintainer

2018-04-13 13:51 UTC #2 lore raw

PATCH 1/1 gfs2: Remove sdp->sd_jheightsize

Quoted 20 lines · Bob Peterson

----- Original Message -----
Quoted 8 lines
GFS2 keeps two arrarys in the superblock that define the maximum size of an inode depending on the inode's height: sdp->sd_heightsize defines the heights in units of sb->s_blocksize; sdp->sd_jheightsize defines them in units of sb->s_blocksize - sizeof(struct gfs2_meta_header). These arrays are used to determine when additional layers of indirect blocks are needed. The second array is used for directories which have an additional gfs2_meta_header at the beginning of each block.
Quoted 4 lines
Distinguishing between these two cases makes no sense: the height required for representing N blocks will come out the same no matter if the calculation is done in gross (sb->s_blocksize) or net (sb->s_blocksize - sizeof(struct gfs2_meta_header)) units.
Hm. Granted it's early morning for me and the caffeine still hasn't made it to my brain, but I'm not convinced the above statement is true.
I know in almost all cases it's a stupid thing to do, but if the block size is 512 bytes:

Quoted 6 lines

regular file: 512/8 bytes per pointer = 64 indirect pointers per block regular file: max is: 64 * 512 = 32768 bytes before it goes to height 2
directory:    488/8 bytes per pointer = 61 indirect pointers per block
directory:    max is: 61 * 512 = 31232, but the number of pointers in a
directory is always power of 2.

Directories don't have additional metadata in their indirect blocks. In your example, they would always contain 64 pointers. It's only the data blocks that are different, and what the directory code puts in there doesn't matter to the block allocation code.

Andreas

Bob Peterson

2018-04-16 16:39 UTC #3 lore raw

PATCH 1/1 gfs2: Remove sdp->sd_jheightsize

In reply to Andreas Gruenbacher

----- Original Message -----

Quoted 13 lines

GFS2 keeps two arrarys in the superblock that define the maximum size of an inode depending on the inode's height: sdp->sd_heightsize defines the heights in units of sb->s_blocksize; sdp->sd_jheightsize defines them in units of sb->s_blocksize - sizeof(struct gfs2_meta_header). These arrays are used to determine when additional layers of indirect blocks are needed. The second array is used for directories which have an additional gfs2_meta_header at the beginning of each block.
Distinguishing between these two cases makes no sense: the height required for representing N blocks will come out the same no matter if the calculation is done in gross (sb->s_blocksize) or net (sb->s_blocksize - sizeof(struct gfs2_meta_header)) units.

Quoted 8 lines

Stuffed directories don't have an additional gfs2_meta_header, but the stuffed case is handled separately for both files and directories, anyway.
Remove the unncessary sdp->sd_jheightsize array.
Signed-off-by Andreas Gruenbacher <[email protected]>
---

Hi,

Thanks. This is now pushed to the for-next branch of the linux-gfs2 tree:
https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?h=for-next&id=9a38662ba4e2682f3f3e9f3ce02a243b837aa8c6

Regards,

Bob Peterson
Red Hat File Systems