Bug 741226 - GFS2: mkfs.gfs2 creates a bad fs with -j 10 -b 512
Summary: GFS2: mkfs.gfs2 creates a bad fs with -j 10 -b 512
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Fedora
Classification: Fedora
Component: gfs2-utils
Version: rawhide
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: ---
Assignee: Andrew Price
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 745126 745161
TreeView+ depends on / blocked
 
Reported: 2011-09-26 10:34 UTC by Andrew Price
Modified: 2011-10-11 14:31 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 745126 745161 (view as bug list)
Environment:
Last Closed: 2011-10-05 13:59:38 UTC


Attachments (Terms of Use)
Full mkfs and fsck output log (13.39 KB, text/x-log)
2011-09-26 10:34 UTC, Andrew Price
no flags Details
Patch, first attempt (1.13 KB, patch)
2011-10-04 23:35 UTC, Andrew Price
no flags Details | Diff

Description Andrew Price 2011-09-26 10:34:34 UTC
Created attachment 524881 [details]
Full mkfs and fsck output log

Description of problem:

With upstream mkfs.gfs2, creating a gfs2 with a block size of 512 and 10 journals creates a bad fs which requires a couple of fsck runs to fix.

How reproducible:

Every time

Steps to Reproduce:
1. mkfs.gfs2 -t foo:bar -p lock_nolock -j 10 -b 512 /dev/dm-3
2. fsck.gfs2 -v /dev/dm-3
  
Actual results:

fsck.gfs2 initially complains:

File system journal "journal10" is missing: pass1 will try to recreate it.
File system journal "journal11" is missing: pass1 will try to recreate it.
(Full output attached)

And has to be run twice to fully fix the fs

Expected results:

Either a clean fs or some kind of "Can't create an fs with these options" message.

Comment 1 Andrew Price 2011-09-26 14:37:27 UTC
fsck.gfs2 is obviously expecting 12 journals when there should only be 10. It calculates the number of expected journals with:
sdp->md.journals = (sdp->md.pinode->i_di.di_entries - 2) / 3;
So here's the per_node info from the new fs:

$ sudo gfs2_edit -p per_node /dev/dm-3
Block #2666845    (0x28b15d) of 7995392 (0x7a0000) (disk inode)
--------------- Per-node Dir -------------------
Dinode:
  mh_magic              0x01161970(hex)
  mh_type               4                   0x4
  mh_format             400                 0x190
  no_formal_ino         14                  0xe
  no_addr               2666845             0x28b15d
  di_mode               040700(decimal)
  di_uid                0                   0x0
  di_gid                0                   0x0
  di_nlink              2                   0x2
  di_size               256                 0x100
  di_blocks             16                  0x10
  di_atime              1317042686          0x4e8079fe
  di_mtime              1317042686          0x4e8079fe
  di_ctime              1317042686          0x4e8079fe
  di_major              0                   0x0
  di_minor              0                   0x0
  di_goal_meta          2685626             0x28faba
  di_goal_data          2666845             0x28b15d
  di_flags              0x00000203(hex)
  di_payload_format     0                   0x0
  di_height             0                   0x0
  di_depth              5                   0x5
  di_entries            38                  0x26
  di_eattr              0                   0x0
This directory contains 15 indirect blocks
Indirect blocks:
0 => 0x28f290 / 2683536
Directory block: lf_depth:5, lf_entries:2,fmt:1200 next=0x0 (2 dirents).
     1. (1). 2683533 (0x28f28d): File    statfs_change8
     2. (2). 2685628 (0x28fabc): File    quota_change9

1 => 0x28f28f / 2683535
Directory block: lf_depth:5, lf_entries:7,fmt:1200 next=0x0 (7 dirents).
     1. (1). 2666845 (0x28b15d): Dir     .
     2. (2). 2666847 (0x28b15f): File    statfs_change0
     3. (3). 2668934 (0x28b986): File    quota_change1
     4. (4). 2671018 (0x28c1aa): File    inum_range2
     5. (5). 2675190 (0x28d1f6): File    statfs_change4
     6. (6). 2677277 (0x28da1d): File    quota_change5
     7. (7). 2679360 (0x28e240): File    inum_range6

2 => 0x28f28e / 2683534
Directory block: lf_depth:4, lf_entries:0,fmt:1200 next=0x0 (1 dirents).
     1. (1). 2666845 (0x28b15d): Dir     .

3 => 0x28d1f7 / 2675191
Directory block: lf_depth:3, lf_entries:0,fmt:1200 next=0x0 (1 dirents).
     1. (1). 2666845 (0x28b15d): Dir     .

4 => 0x28f292 / 2683538
Directory block: lf_depth:3, lf_entries:0,fmt:1200 next=0x0 (0 dirents).

5 => 0x28f293 / 2683539
Directory block: lf_depth:4, lf_entries:0,fmt:1200 next=0x0 (0 dirents).

6 => 0x28f294 / 2683540
Directory block: lf_depth:5, lf_entries:2,fmt:1200 next=0x0 (2 dirents).
     1. (1). 2683537 (0x28f291): File    quota_change8
     2. (2). 2685627 (0x28fabb): File    statfs_change9

7 => 0x28b987 / 2668935
Directory block: lf_depth:5, lf_entries:6,fmt:1200 next=0x0 (6 dirents).
     1. (1). 2675192 (0x28d1f8): File    quota_change4
     2. (2). 2666848 (0x28b160): File    quota_change0
     3. (3). 2668933 (0x28b985): File    statfs_change1
     4. (4). 2677276 (0x28da1c): File    statfs_change5
     5. (5). 2681445 (0x28ea65): File    inum_range7
     6. (6). 2673103 (0x28c9cf): File    inum_range3

8 => 0x28fab9 / 2685625
Directory block: lf_depth:4, lf_entries:0,fmt:1200 next=0x0 (0 dirents).

9 => 0x28faba / 2685626
Directory block: lf_depth:5, lf_entries:7,fmt:1200 next=0x0 (7 dirents).
     1. (1). 387 (0x183): Dir     ..
     2. (2). 2671020 (0x28c1ac): File    quota_change2
     3. (3). 2668931 (0x28b983): File    inum_range1
     4. (4). 2673104 (0x28c9d0): File    statfs_change3
     5. (5). 2677275 (0x28da1b): File    inum_range5
     6. (6). 2679362 (0x28e242): File    quota_change6
     7. (7). 2681446 (0x28ea66): File    statfs_change7

10 => 0x28fab8 / 2685624
Directory block: lf_depth:5, lf_entries:1,fmt:1200 next=0x0 (1 dirents).
     1. (1). 2685623 (0x28fab7): File    inum_range9

11 => 0x28c9d2 / 2673106
Directory block: lf_depth:3, lf_entries:0,fmt:1200 next=0x0 (1 dirents).
     1. (1). 387 (0x183): Dir     ..

12 => 0x28f28b / 2683531
Directory block: lf_depth:3, lf_entries:0,fmt:1200 next=0x0 (0 dirents).

13 => 0x28f28c / 2683532
Directory block: lf_depth:4, lf_entries:7,fmt:1200 next=0x0 (7 dirents).
     1. (1). 2673105 (0x28c9d1): File    quota_change3
     2. (2). 2666846 (0x28b15e): File    inum_range0
     3. (3). 2671019 (0x28c1ab): File    statfs_change2
     4. (4). 2675189 (0x28d1f5): File    inum_range4
     5. (5). 2679361 (0x28e241): File    statfs_change6
     6. (6). 2681447 (0x28ea67): File    quota_change7
     7. (7). 2683530 (0x28f28a): File    inum_range8

14 => 0x28b984 / 2668932
Directory block: lf_depth:4, lf_entries:0,fmt:1200 next=0x0 (1 dirents).
     1. (1). 2673105 (0x28c9d1): File    quota_change3


------------------------------------------------------

Now the question is, is mkfs.gfs2 writing the per_node data wrongly or is fsck.gfs2 wrongly relying on di_entries being (3*journal_count)+2?

Comment 2 Robert Peterson 2011-09-26 15:41:24 UTC
I believe the di_entries value of 38 is wrong and the calculation
of (3*journal_count)+2 is correct.  For every journal, there
should be three system files: quota_changeX, inum_range_X and
statfs_changeX.  Plus two for "." and "..".

So 3*10 journals = 30 + 2 = 32.  So di_entries should be 32, not 38.
In fact, if you total up all the individual leaf block's
lf_entries, you get 2 + 7 + 2 + 6 + 7 + 1 + 7 = 32.
So how did it reach 38?

Comment 3 Andrew Price 2011-10-04 23:35:34 UTC
Created attachment 526345 [details]
Patch, first attempt

This one-liner seems to fix the problem, but does it look sane to you guys? In particular, is my understanding correct that it's a sentinel dirent which is being created and causing di_entries to be incremented? The directory adding/allocation/leaf splitting code wasn't easy to get my head around :)

Comment 4 Robert Peterson 2011-10-05 13:25:40 UTC
This makes perfect sense, and it meshes well with my earlier
suspicions.  Here's an excerpt from a recent irc chat session:

<bob> Without having looked at the code: I rather suspect that some function, like dir_split_leaf is deleting "." or ".." or making them a sentinel, without decrementing di_entries
<bob> AndyP: ^

Comment 5 Andrew Price 2011-10-05 13:59:38 UTC
Ok cool, it's reassuring that your suspicion was correct. I'll go ahead and push this one.


Note You need to log in before you can comment on or make changes to this bug.