[Cluster-devel] [PATCH] gfs2: Remove sdp->sd_jheightsize

[Cluster-devel] [PATCH] gfs2: Remove sdp->sd_jheightsize

[Andreas Gruenbacher]

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 <agruenba@redhat.com>
---
 fs/gfs2/bmap.c       | 14 +-------------
 fs/gfs2/incore.h     |  2 --
 fs/gfs2/ops_fstype.c | 19 -------------------
 3 files changed, 1 insertion(+), 34 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 278ed0869c3c..0590e93494f7 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -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))
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 1b6b1e3f5caf..0bbbaa9b05cb 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -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 */
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 3ba3f167641c..c2469833b4fb 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -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;
-- 
2.14.3

[Cluster-devel] [PATCH] gfs2: Remove sdp->sd_jheightsize

[Bob Peterson]

----- Original Message -----
> 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.

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

[Cluster-devel] [PATCH] gfs2: Remove sdp->sd_jheightsize

[Andreas Gruenbacher]

On 13 April 2018 at 15:29, Bob Peterson <rpeterso@redhat.com> wrote:
> ----- Original Message -----
>> 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.
>
> 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.

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

[Cluster-devel] [PATCH] gfs2: Remove sdp->sd_jheightsize

[Bob Peterson]

----- Original Message -----
> 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 <agruenba@redhat.com>
> ---
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