Oops in ufs_fill_super at mount time

Oops in ufs_fill_super at mount time

[Alexey Dobriyan]

Version 2.6.15-43ecb9a33ba8c93ebbda81d48ca05f0d1bbf9056

Actually more or less latest -git is affected too, but
I'm sick of recompiling right now so please wait for bisecting results.

It's random wrt one mount of UFS, but several mount/umounts in a row
reproduce it reliably:

	mount -t ufs -o ro,ufstype=44bsd /dev/hda9 /home/openbsd/usr/

I've tracked this down to line 926 of fs/ufs/super.c:

   921		uspi->s_ntrak = fs32_to_cpu(sb, usb1->fs_ntrak);
   922		uspi->s_nsect = fs32_to_cpu(sb, usb1->fs_nsect);
   923		uspi->s_spc = fs32_to_cpu(sb, usb1->fs_spc);
   924		uspi->s_ipg = fs32_to_cpu(sb, usb1->fs_ipg);
   925		uspi->s_fpg = fs32_to_cpu(sb, usb1->fs_fpg);
   926	===>	uspi->s_cpc = fs32_to_cpu(sb, usb2->fs_cpc);	<=== Oops here
   927		uspi->s_contigsumsize = fs32_to_cpu(sb, usb3->fs_u2.fs_44.fs_contigsumsize);
   928		uspi->s_qbmask = ufs_get_fs_qbmask(sb, usb3);
   929		uspi->s_qfmask = ufs_get_fs_qfmask(sb, usb3);
   930		uspi->s_postblformat = fs32_to_cpu(sb, usb3->fs_postblformat);

(fs/ufs/super.c, 1029), ufs_put_super: ENTER
(fs/ufs/super.c, 545), ufs_fill_super: ENTER
(fs/ufs/super.c, 553), ufs_fill_super: flag 1
(fs/ufs/super.c, 310), ufs_parse_options: ENTER
(fs/ufs/super.c, 593), ufs_fill_super: ufstype=44bsd
(fs/ufs/super.c, 822), ufs_fill_super: another value of block_size or super_block_size 2048, 2048
ufs_print_super_stuff
size of usb:     1380
  magic:         0x11954
  sblkno:        8
  cblkno:        16
  iblkno:        24
  dblkno:        1312
  cgoffset:      16
  ~cgmask:       0xf
  size:          2097144
  dsize:         2063231
  ncg:           26
  bsize:         16384
  fsize:         2048
  frag:          8
  fragshift:     3
  ~fmask:        2047
  fshift:        11
  sbsize:        2048
  spc:           1008
  cpg:           328
  ipg:           20608
  fpg:           82656
  csaddr:        1312
  cssize:        2048
  cgsize:        16384
  fstodb:        2
  contigsumsize: 3
  postblformat:  1
  nrpos:         1
  ndir           29518
  nifree         339567
  nbfree         107711
  nffree         5167

(fs/ufs/super.c, 844), ufs_fill_super: fs is clean
(fs/ufs/super.c, 904), ufs_fill_super: uspi->s_bshift = 14,uspi->s_fshift = 11
before uspi->s_cpc = fs32_to_cpu(sb, usb2->fs_cpc);
Unable to handle kernel paging request at virtual address d734c158
 printing eip:
c019f138
*pde = 0005c067
*pte = 1734c000
Oops: 0000 [#1]
PREEMPT DEBUG_PAGEALLOC
Modules linked in: snd_pcm_oss snd_mixer_oss snd_intel8x0 snd_ac97_codec snd_ac97_bus snd_pcm snd_timer snd soundcore snd_page_alloc ehci_hcd uhci_hcd usbcore
CPU:    0
EIP:    0060:[<c019f138>]    Not tainted VLI
EFLAGS: 00010286   (2.6.15-43ecb9a33ba8c93ebbda81d48ca05f0d1bbf9056)
EIP is at ufs_fill_super+0x1094/0x151e
eax: d734c000   ebx: 00000000   ecx: dd270e4c   edx: c0289c4b
esi: dd378df8   edi: 00000800   ebp: dd394df8   esp: dd270e4c
ds: 007b   es: 007b   ss: 0068
Process mount (pid: 7857, threadinfo=dd270000 task=ddb70b00)
Stack: <0>dd270ec3 00000020 d7b7c374 c0283146 dd270e88 dd270ea4 dd325ef8 00002130
       dd378df8 d730c400 d734c000 d730c000 dd378df8 00000020 c0283142 00001000
       dd394df8 dd394df8 d730be5c 00000000 d7309000 c014a1ce 39616468 c0142000
Call Trace:
 [<c014a1ce>] get_sb_bdev+0xbf/0xfa
 [<c0142000>] kmem_cache_alloc+0x59/0x5c
 [<c015afb3>] alloc_vfsmnt+0x97/0xbe
 [<c015afb3>] alloc_vfsmnt+0x97/0xbe
 [<c019fc31>] ufs_get_sb+0xe/0x11
 [<c019e0a4>] ufs_fill_super+0x0/0x151e
 [<c014a340>] do_kern_mount+0x3b/0x9d
 [<c015c22a>] do_new_mount+0x61/0x90
 [<c015c7d4>] do_mount+0x161/0x179
 [<c012e8d3>] __alloc_pages+0x47/0x256
 [<c015cab5>] sys_mount+0x6f/0xad
 [<c01025fd>] syscall_call+0x7/0xb
Code: 91 bc 00 00 00 83 b8 80 00 00 00 00 89 d1 74 02 0f c9 8b 74 24 30 89 8e cc 00 00 00 68 4b 9c 28 c0 e8 ec 25 f7 ff 58 8b 44 24 28 <8b> 90 58 01 00 00 8b 85 68 01 00 00 83 b8 80 00 00 00 00 89 d1

Re: Oops in ufs_fill_super at mount time

[Linus Torvalds]

On Fri, 13 Jan 2006, Alexey Dobriyan wrote:
>
> Version 2.6.15-43ecb9a33ba8c93ebbda81d48ca05f0d1bbf9056
> 
> Actually more or less latest -git is affected too, but
> I'm sick of recompiling right now so please wait for bisecting results.

Hmm.. I don't see any recent changes that could affect this. Not after 
2.6.15, but in fact not even after 2.6.14.

Your oops is also interesting in another way...

> Unable to handle kernel paging request at virtual address d734c158
>  printing eip:
> c019f138
> *pde = 0005c067
> *pte = 1734c000
> Oops: 0000 [#1]
> PREEMPT DEBUG_PAGEALLOC

This is a free'd page fault, so it's due to DEBUG_PAGEALLOC rather than a 
wild pointer.

Is that something new for you? Maybe the bug is older, but you've enabled 
PAGEALLOC only recently? Also, out of interest, have you enabled slab 
debugging?

That said, the whole ubh_get_usb_second() and ubh_get_usb_third() thing is 
pretty damn scary. There's no testing of the values passed in at all and 
comparing them to the allocated buffer heads. But from what I can tell, 
ubh_bread_uspi() will zero out any unallocated bh's, and it certainly 
_looks_ like the calculations to calculate "usb2" should fit within the 
sectors that were read..

Very strange.

			Linus

Re: Oops in ufs_fill_super at mount time

[Alexey Dobriyan]

On Thu, Jan 12, 2006 at 05:14:25PM -0800, Linus Torvalds wrote:
> On Fri, 13 Jan 2006, Alexey Dobriyan wrote:
> >
> > Version 2.6.15-43ecb9a33ba8c93ebbda81d48ca05f0d1bbf9056
> >
> > Actually more or less latest -git is affected too, but
> > I'm sick of recompiling right now so please wait for bisecting results.
>
> Hmm.. I don't see any recent changes that could affect this. Not after
> 2.6.15, but in fact not even after 2.6.14.
>
> Your oops is also interesting in another way...
>
> > Unable to handle kernel paging request at virtual address d734c158
> >  printing eip:
> > c019f138
> > *pde = 0005c067
> > *pte = 1734c000
> > Oops: 0000 [#1]
> > PREEMPT DEBUG_PAGEALLOC
>
> This is a free'd page fault, so it's due to DEBUG_PAGEALLOC rather than a
> wild pointer.

That's true. Turning it off makes mounting reliable again.

> Is that something new for you? Maybe the bug is older, but you've enabled
> PAGEALLOC only recently?

Yup. In response to hangs re disk activity.

> Also, out of interest, have you enabled slab
> debugging?

Yup.

> That said, the whole ubh_get_usb_second() and ubh_get_usb_third() thing is
> pretty damn scary. There's no testing of the values passed in at all and
> comparing them to the allocated buffer heads. But from what I can tell,
> ubh_bread_uspi() will zero out any unallocated bh's, and it certainly
> _looks_ like the calculations to calculate "usb2" should fit within the
> sectors that were read..
>
> Very strange.

Re: Oops in ufs_fill_super at mount time

[Linus Torvalds]

On Fri, 13 Jan 2006, Alexey Dobriyan wrote:

> On Thu, Jan 12, 2006 at 05:14:25PM -0800, Linus Torvalds wrote:
> >
> > This is a free'd page fault, so it's due to DEBUG_PAGEALLOC rather than a
> > wild pointer.
> 
> That's true. Turning it off makes mounting reliable again.
> 
> > Is that something new for you? Maybe the bug is older, but you've enabled
> > PAGEALLOC only recently?
> 
> Yup. In response to hangs re disk activity.

Ok, That explains why it started happening for you only _now_, but not why 
it happens in the first place.

Can you test if the patch that Evgeniy sent out fixes it for you even with 
PAGEALLOC debugging enabled?

Evgeniy - That is one ugly macro, can you (or Alexey, for that matter: 
somebody who can test it) turn it into an inline function or something to 
make it half-way readable? I realize that means changing the arguments too 
(right now that horrid macro accesses "uspi" directly - uggghhh).

If somebody maintains - or is interested in doing so - UFS, please speak 
up, we don't have anybody listed in the MAINTAINERS file, and when I look 
through the history, all I see is updates due to secondary issues (ie 
somebody did generic cleanups and just happened to touch UFS as part of 
that, rather than working _directly_ on UFS issues).

		Linus

Re: Oops in ufs_fill_super at mount time

[Alexey Dobriyan]

On Fri, Jan 13, 2006 at 07:45:12AM -0800, Linus Torvalds wrote:
> On Fri, 13 Jan 2006, Alexey Dobriyan wrote:
>
> > On Thu, Jan 12, 2006 at 05:14:25PM -0800, Linus Torvalds wrote:
> > >
> > > This is a free'd page fault, so it's due to DEBUG_PAGEALLOC rather than a
> > > wild pointer.
> >
> > That's true. Turning it off makes mounting reliable again.
> >
> > > Is that something new for you? Maybe the bug is older, but you've enabled
> > > PAGEALLOC only recently?
> >
> > Yup. In response to hangs re disk activity.
>
> Ok, That explains why it started happening for you only _now_, but not why
> it happens in the first place.
>
> Can you test if the patch that Evgeniy sent out fixes it for you even with
> PAGEALLOC debugging enabled?

It does the trick! And you saved me from going before 2.6.0. ;-)

[PATCH 2.6.15] ufs cleanup

[Evgeniy]

On Fri, Jan 13, 2006 at 07:45:12AM -0800, Linus Torvalds wrote:
> Evgeniy - That is one ugly macro, can you (or Alexey, for that matter: 
> somebody who can test it) turn it into an inline function or something to 
> make it half-way readable? I realize that means changing the arguments too 
> (right now that horrid macro accesses "uspi" directly - uggghhh).

You mean something like this?

This patch converts ubh_get_usb_* to inline functions.

Signed-off-by: Evgeniy Dushistov <dushistov@mail.ru>

---


diff -uprN -X linux-2.6.15-vanilla/Documentation/dontdiff linux-2.6.15-vanilla/fs/ufs/balloc.c linux-2.6.15/fs/ufs/balloc.c
--- linux-2.6.15-vanilla/fs/ufs/balloc.c	2006-01-03 06:21:10.000000000 +0300
+++ linux-2.6.15/fs/ufs/balloc.c	2006-01-13 21:32:50.308214250 +0300
@@ -48,7 +48,7 @@ void ufs_free_fragments (struct inode * 
 	
 	sb = inode->i_sb;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 	
 	UFSD(("ENTER, fragment %u, count %u\n", fragment, count))
 	
@@ -142,7 +142,7 @@ void ufs_free_blocks (struct inode * ino
 	
 	sb = inode->i_sb;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 
 	UFSD(("ENTER, fragment %u, count %u\n", fragment, count))
 	
@@ -246,7 +246,7 @@ unsigned ufs_new_fragments (struct inode
 	
 	sb = inode->i_sb;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 	*err = -ENOSPC;
 
 	lock_super (sb);
@@ -406,7 +406,7 @@ ufs_add_fragments (struct inode * inode,
 	
 	sb = inode->i_sb;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first (USPI_UBH);
+	usb1 = ubh_get_usb_first (uspi);
 	count = newcount - oldcount;
 	
 	cgno = ufs_dtog(fragment);
@@ -489,7 +489,7 @@ static unsigned ufs_alloc_fragments (str
 
 	sb = inode->i_sb;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 	oldcg = cgno;
 	
 	/*
@@ -605,7 +605,7 @@ static unsigned ufs_alloccg_block (struc
 
 	sb = inode->i_sb;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 	ucg = ubh_get_ucg(UCPI_UBH);
 
 	if (goal == 0) {
@@ -662,7 +662,7 @@ static unsigned ufs_bitmap_search (struc
 	UFSD(("ENTER, cg %u, goal %u, count %u\n", ucpi->c_cgx, goal, count))
 
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first (USPI_UBH);
+	usb1 = ubh_get_usb_first (uspi);
 	ucg = ubh_get_ucg(UCPI_UBH);
 
 	if (goal)
diff -uprN -X linux-2.6.15-vanilla/Documentation/dontdiff linux-2.6.15-vanilla/fs/ufs/ialloc.c linux-2.6.15/fs/ufs/ialloc.c
--- linux-2.6.15-vanilla/fs/ufs/ialloc.c	2006-01-03 06:21:10.000000000 +0300
+++ linux-2.6.15/fs/ufs/ialloc.c	2006-01-13 21:32:50.328215500 +0300
@@ -72,7 +72,7 @@ void ufs_free_inode (struct inode * inod
 
 	sb = inode->i_sb;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 	
 	ino = inode->i_ino;
 
@@ -167,7 +167,7 @@ struct inode * ufs_new_inode(struct inod
 	ufsi = UFS_I(inode);
 	sbi = UFS_SB(sb);
 	uspi = sbi->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 
 	lock_super (sb);
 
diff -uprN -X linux-2.6.15-vanilla/Documentation/dontdiff linux-2.6.15-vanilla/fs/ufs/super.c linux-2.6.15/fs/ufs/super.c
--- linux-2.6.15-vanilla/fs/ufs/super.c	2006-01-03 06:21:10.000000000 +0300
+++ linux-2.6.15/fs/ufs/super.c	2006-01-13 21:32:50.336216000 +0300
@@ -221,7 +221,7 @@ void ufs_error (struct super_block * sb,
 	va_list args;
 
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 	
 	if (!(sb->s_flags & MS_RDONLY)) {
 		usb1->fs_clean = UFS_FSBAD;
@@ -253,7 +253,7 @@ void ufs_panic (struct super_block * sb,
 	va_list args;
 	
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 	
 	if (!(sb->s_flags & MS_RDONLY)) {
 		usb1->fs_clean = UFS_FSBAD;
@@ -735,9 +735,9 @@ again:	
             goto failed;
 
 	
-	usb1 = ubh_get_usb_first(USPI_UBH);
-	usb2 = ubh_get_usb_second(USPI_UBH);
-	usb3 = ubh_get_usb_third(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
+	usb2 = ubh_get_usb_second(uspi);
+	usb3 = ubh_get_usb_third(uspi);
 	usb  = (struct ufs_super_block *)
 		((struct ufs_buffer_head *)uspi)->bh[0]->b_data ;
 
@@ -1006,8 +1006,8 @@ static void ufs_write_super (struct supe
 	UFSD(("ENTER\n"))
 	flags = UFS_SB(sb)->s_flags;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
-	usb3 = ubh_get_usb_third(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
+	usb3 = ubh_get_usb_third(uspi);
 
 	if (!(sb->s_flags & MS_RDONLY)) {
 		usb1->fs_time = cpu_to_fs32(sb, get_seconds());
@@ -1049,8 +1049,8 @@ static int ufs_remount (struct super_blo
 	
 	uspi = UFS_SB(sb)->s_uspi;
 	flags = UFS_SB(sb)->s_flags;
-	usb1 = ubh_get_usb_first(USPI_UBH);
-	usb3 = ubh_get_usb_third(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
+	usb3 = ubh_get_usb_third(uspi);
 	
 	/*
 	 * Allow the "check" option to be passed as a remount option.
@@ -1124,7 +1124,7 @@ static int ufs_statfs (struct super_bloc
 	lock_kernel();
 
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first (USPI_UBH);
+	usb1 = ubh_get_usb_first (uspi);
 	usb  = (struct ufs_super_block *)
 		((struct ufs_buffer_head *)uspi)->bh[0]->b_data ;
 	
diff -uprN -X linux-2.6.15-vanilla/Documentation/dontdiff linux-2.6.15-vanilla/fs/ufs/util.h linux-2.6.15/fs/ufs/util.h
--- linux-2.6.15-vanilla/fs/ufs/util.h	2006-01-13 21:28:46.724991250 +0300
+++ linux-2.6.15/fs/ufs/util.h	2006-01-13 21:32:50.344216500 +0300
@@ -249,18 +249,30 @@ extern void _ubh_memcpyubh_(struct ufs_s
 
 
 /*
- * macros to get important structures from ufs_buffer_head
+ * inline functions to get important structures from ufs_sb_private_info
  */
-#define ubh_get_usb_first(ubh) \
-	((struct ufs_super_block_first *)((ubh)->bh[0]->b_data))
+static inline struct ufs_super_block_first *
+ubh_get_usb_first(struct ufs_sb_private_info *uspi)
+{
+	char *res=uspi->s_ubh.bh[0]->b_data;
+	return (struct ufs_super_block_first *)res;
+}
 
-#define ubh_get_usb_second(ubh) \
-	((struct ufs_super_block_second *)((ubh)->\
-					   bh[UFS_SECTOR_SIZE >> uspi->s_fshift]->b_data + (UFS_SECTOR_SIZE & ~uspi->s_fmask)))
-
-#define ubh_get_usb_third(ubh) \
-	((struct ufs_super_block_third *)((ubh)-> \
-	bh[UFS_SECTOR_SIZE*2 >> uspi->s_fshift]->b_data + (UFS_SECTOR_SIZE*2 & ~uspi->s_fmask)))
+static inline struct ufs_super_block_second *
+ubh_get_usb_second(struct ufs_sb_private_info *uspi)
+{
+	char *res=uspi->s_ubh.bh[UFS_SECTOR_SIZE >> uspi->s_fshift]->b_data + 
+		(UFS_SECTOR_SIZE & ~uspi->s_fmask);
+	return (struct ufs_super_block_second *)res;
+}
+
+static inline struct ufs_super_block_third *
+ubh_get_usb_third(struct ufs_sb_private_info *uspi)
+{
+	char *res=uspi->s_ubh.bh[UFS_SECTOR_SIZE*2 >> uspi->s_fshift]->b_data + 
+		(UFS_SECTOR_SIZE*2 & ~uspi->s_fmask);	
+	return (struct ufs_super_block_third *)res;
+}
 
 #define ubh_get_ucg(ubh) \
 	((struct ufs_cylinder_group *)((ubh)->bh[0]->b_data))

-- 
/Evgeniy

Re: [PATCH 2.6.15] ufs cleanup

[Linus Torvalds]

On Fri, 13 Jan 2006, Evgeniy wrote:
> +static inline struct ufs_super_block_second *
> +ubh_get_usb_second(struct ufs_sb_private_info *uspi)
> +{
> +	char *res=uspi->s_ubh.bh[UFS_SECTOR_SIZE >> uspi->s_fshift]->b_data + 
> +		(UFS_SECTOR_SIZE & ~uspi->s_fmask);
> +	return (struct ufs_super_block_second *)res;
> +}

I was thinking of something even more abstracted:

	static inline void *get_usb_offset(struct ufs_sb_private_info *uspi,
		unsigned int offset)
	{
		unsigned int index;

		index = offset >> uspi->s_fshift;
		offset &= ~uspi->s_fmask;
		return uspi->s_ubh.bh[index]->b_data + offset;
	}

and then just doing

	#define ubs_get_usb_first(uspi) \
		((struct ufs_super_block_first *)get_usb_offset(uspi, 0))

	#define ubh_get_usb_second(uspi) \
		((struct ufs_super_block_second *)get_usb_offset(uspi, UFS_SECTOR_SIZE))

	#define ubh_get_usb_third(uspi) \
		((struct ufs_super_block_third *)get_usb_offset(uspi, 2*UFS_SECTOR_SIZE))

or something similar. Which seems a hell of a lot more readable to me, and 
assuming it passes testing (ie I didn't screw up), I think it's more 
likely to stay correct in the future and just generally be maintainable.

Hmm?

		Linus

Re: [PATCH 2.6.15] ufs cleanup v. 2

[Evgeniy]

On Fri, Jan 13, 2006 at 11:51:28AM -0800, Linus Torvalds wrote:
> I was thinking of something even more abstracted:
> 
> 	static inline void *get_usb_offset(struct ufs_sb_private_info *uspi,
> 		unsigned int offset)
> 	{
> 		unsigned int index;
> 
> 		index = offset >> uspi->s_fshift;
> 		offset &= ~uspi->s_fmask;
> 		return uspi->s_ubh.bh[index]->b_data + offset;
> 	}
> Hmm?

Yes, this is much better then my suggestion.

Here is update of ufs cleanup patch,
it also includes
* fix compilation warnings which appears if debug mode turn on
* remove unnecessary duplication of code to support UFS2

I tested it on ufs1 and ufs2 file-systems.

Signed-off-by: Evgeniy Dushistov <dushistov@mail.ru>

---

diff -uprN -X linux-2.6.15-vanilla/Documentation/dontdiff linux-2.6.15-vanilla/fs/ufs/balloc.c linux-2.6.15/fs/ufs/balloc.c
--- linux-2.6.15-vanilla/fs/ufs/balloc.c	2006-01-03 06:21:10.000000000 +0300
+++ linux-2.6.15/fs/ufs/balloc.c	2006-01-14 09:50:06.768125250 +0300
@@ -48,7 +48,7 @@ void ufs_free_fragments (struct inode * 
 	
 	sb = inode->i_sb;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 	
 	UFSD(("ENTER, fragment %u, count %u\n", fragment, count))
 	
@@ -80,8 +80,9 @@ void ufs_free_fragments (struct inode * 
 	for (i = bit; i < end_bit; i++) {
 		if (ubh_isclr (UCPI_UBH, ucpi->c_freeoff, i))
 			ubh_setbit (UCPI_UBH, ucpi->c_freeoff, i);
-		else ufs_error (sb, "ufs_free_fragments",
-			"bit already cleared for fragment %u", i);
+		else 
+			ufs_error (sb, "ufs_free_fragments",
+				   "bit already cleared for fragment %u", i);
 	}
 	
 	DQUOT_FREE_BLOCK (inode, count);
@@ -142,7 +143,7 @@ void ufs_free_blocks (struct inode * ino
 	
 	sb = inode->i_sb;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 
 	UFSD(("ENTER, fragment %u, count %u\n", fragment, count))
 	
@@ -246,7 +247,7 @@ unsigned ufs_new_fragments (struct inode
 	
 	sb = inode->i_sb;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 	*err = -ENOSPC;
 
 	lock_super (sb);
@@ -406,7 +407,7 @@ ufs_add_fragments (struct inode * inode,
 	
 	sb = inode->i_sb;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first (USPI_UBH);
+	usb1 = ubh_get_usb_first (uspi);
 	count = newcount - oldcount;
 	
 	cgno = ufs_dtog(fragment);
@@ -489,7 +490,7 @@ static unsigned ufs_alloc_fragments (str
 
 	sb = inode->i_sb;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 	oldcg = cgno;
 	
 	/*
@@ -605,7 +606,7 @@ static unsigned ufs_alloccg_block (struc
 
 	sb = inode->i_sb;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 	ucg = ubh_get_ucg(UCPI_UBH);
 
 	if (goal == 0) {
@@ -662,7 +663,7 @@ static unsigned ufs_bitmap_search (struc
 	UFSD(("ENTER, cg %u, goal %u, count %u\n", ucpi->c_cgx, goal, count))
 
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first (USPI_UBH);
+	usb1 = ubh_get_usb_first (uspi);
 	ucg = ubh_get_ucg(UCPI_UBH);
 
 	if (goal)
diff -uprN -X linux-2.6.15-vanilla/Documentation/dontdiff linux-2.6.15-vanilla/fs/ufs/ialloc.c linux-2.6.15/fs/ufs/ialloc.c
--- linux-2.6.15-vanilla/fs/ufs/ialloc.c	2006-01-03 06:21:10.000000000 +0300
+++ linux-2.6.15/fs/ufs/ialloc.c	2006-01-14 09:50:06.924135000 +0300
@@ -72,7 +72,7 @@ void ufs_free_inode (struct inode * inod
 
 	sb = inode->i_sb;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 	
 	ino = inode->i_ino;
 
@@ -167,7 +167,7 @@ struct inode * ufs_new_inode(struct inod
 	ufsi = UFS_I(inode);
 	sbi = UFS_SB(sb);
 	uspi = sbi->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 
 	lock_super (sb);
 
diff -uprN -X linux-2.6.15-vanilla/Documentation/dontdiff linux-2.6.15-vanilla/fs/ufs/inode.c linux-2.6.15/fs/ufs/inode.c
--- linux-2.6.15-vanilla/fs/ufs/inode.c	2006-01-03 06:21:10.000000000 +0300
+++ linux-2.6.15/fs/ufs/inode.c	2006-01-14 09:50:06.964137500 +0300
@@ -61,7 +61,7 @@ static int ufs_block_to_path(struct inod
 	int n = 0;
 
 
-	UFSD(("ptrs=uspi->s_apb = %d,double_blocks=%d \n",ptrs,double_blocks));
+	UFSD(("ptrs=uspi->s_apb = %d,double_blocks=%ld \n",ptrs,double_blocks));
 	if (i_block < 0) {
 		ufs_warning(inode->i_sb, "ufs_block_to_path", "block < 0");
 	} else if (i_block < direct_blocks) {
@@ -104,7 +104,7 @@ u64  ufs_frag_map(struct inode *inode, s
 	unsigned flags = UFS_SB(sb)->s_flags;
 	u64 temp = 0L;
 
-	UFSD((": frag = %lu  depth = %d\n",frag,depth));
+	UFSD((": frag = %llu  depth = %d\n", (unsigned long long)frag, depth));
 	UFSD((": uspi->s_fpbshift = %d ,uspi->s_apbmask = %x, mask=%llx\n",uspi->s_fpbshift,uspi->s_apbmask,mask));
 
 	if (depth == 0)
@@ -365,9 +365,10 @@ repeat:
 		sync_dirty_buffer(bh);
 	inode->i_ctime = CURRENT_TIME_SEC;
 	mark_inode_dirty(inode);
+	UFSD(("result %u\n", tmp + blockoff));
 out:
 	brelse (bh);
-	UFSD(("EXIT, result %u\n", tmp + blockoff))
+	UFSD(("EXIT\n"));
 	return result;
 }
 
@@ -386,7 +387,7 @@ static int ufs_getfrag_block (struct ino
 	
 	if (!create) {
 		phys64 = ufs_frag_map(inode, fragment);
-		UFSD(("phys64 = %lu \n",phys64));
+		UFSD(("phys64 = %llu \n",phys64));
 		if (phys64)
 			map_bh(bh_result, sb, phys64);
 		return 0;
@@ -401,7 +402,7 @@ static int ufs_getfrag_block (struct ino
 
 	lock_kernel();
 
-	UFSD(("ENTER, ino %lu, fragment %u\n", inode->i_ino, fragment))
+	UFSD(("ENTER, ino %lu, fragment %llu\n", inode->i_ino, (unsigned long long)fragment))
 	if (fragment < 0)
 		goto abort_negative;
 	if (fragment >
diff -uprN -X linux-2.6.15-vanilla/Documentation/dontdiff linux-2.6.15-vanilla/fs/ufs/super.c linux-2.6.15/fs/ufs/super.c
--- linux-2.6.15-vanilla/fs/ufs/super.c	2006-01-03 06:21:10.000000000 +0300
+++ linux-2.6.15/fs/ufs/super.c	2006-01-14 09:50:07.036142000 +0300
@@ -221,7 +221,7 @@ void ufs_error (struct super_block * sb,
 	va_list args;
 
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 	
 	if (!(sb->s_flags & MS_RDONLY)) {
 		usb1->fs_clean = UFS_FSBAD;
@@ -253,7 +253,7 @@ void ufs_panic (struct super_block * sb,
 	va_list args;
 	
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
 	
 	if (!(sb->s_flags & MS_RDONLY)) {
 		usb1->fs_clean = UFS_FSBAD;
@@ -420,21 +420,18 @@ static int ufs_read_cylinder_structures 
 		if (i + uspi->s_fpb > blks)
 			size = (blks - i) * uspi->s_fsize;
 
-		if ((flags & UFS_TYPE_MASK) == UFS_TYPE_UFS2) {
+		if ((flags & UFS_TYPE_MASK) == UFS_TYPE_UFS2) 
 			ubh = ubh_bread(sb,
 				fs64_to_cpu(sb, usb->fs_u11.fs_u2.fs_csaddr) + i, size);
-			if (!ubh)
-				goto failed;
-			ubh_ubhcpymem (space, ubh, size);
-			sbi->s_csp[ufs_fragstoblks(i)]=(struct ufs_csum *)space;
-		}
-		else {
+		else 
 			ubh = ubh_bread(sb, uspi->s_csaddr + i, size);
-			if (!ubh)
-				goto failed;
-			ubh_ubhcpymem(space, ubh, size);
-			sbi->s_csp[ufs_fragstoblks(i)]=(struct ufs_csum *)space;
-		}
+		
+		if (!ubh)
+			goto failed;
+
+		ubh_ubhcpymem (space, ubh, size);
+		sbi->s_csp[ufs_fragstoblks(i)]=(struct ufs_csum *)space;
+
 		space += size;
 		ubh_brelse (ubh);
 		ubh = NULL;
@@ -539,6 +536,7 @@ static int ufs_fill_super(struct super_b
 	struct inode *inode;
 	unsigned block_size, super_block_size;
 	unsigned flags;
+	unsigned super_block_offset;
 
 	uspi = NULL;
 	ubh = NULL;
@@ -586,10 +584,11 @@ static int ufs_fill_super(struct super_b
 	if (!uspi)
 		goto failed;
 
+	super_block_offset=UFS_SBLOCK;
+
 	/* Keep 2Gig file limit. Some UFS variants need to override 
 	   this but as I don't know which I'll let those in the know loosen
 	   the rules */
-	   
 	switch (sbi->s_mount_opt & UFS_MOUNT_UFSTYPE) {
 	case UFS_MOUNT_UFSTYPE_44BSD:
 		UFSD(("ufstype=44bsd\n"))
@@ -601,7 +600,8 @@ static int ufs_fill_super(struct super_b
 		flags |= UFS_DE_44BSD | UFS_UID_44BSD | UFS_ST_44BSD | UFS_CG_44BSD;
 		break;
 	case UFS_MOUNT_UFSTYPE_UFS2:
-		UFSD(("ufstype=ufs2\n"))
+		UFSD(("ufstype=ufs2\n"));
+		super_block_offset=SBLOCK_UFS2;
 		uspi->s_fsize = block_size = 512;
 		uspi->s_fmask = ~(512 - 1);
 		uspi->s_fshift = 9;
@@ -725,19 +725,16 @@ again:	
 	/*
 	 * read ufs super block from device
 	 */
-	if ( (flags & UFS_TYPE_MASK) == UFS_TYPE_UFS2) {
-		ubh = ubh_bread_uspi(uspi, sb, uspi->s_sbbase + SBLOCK_UFS2/block_size, super_block_size);
-	}
-	else {
-		ubh = ubh_bread_uspi(uspi, sb, uspi->s_sbbase + UFS_SBLOCK/block_size, super_block_size);
-	}
+
+	ubh = ubh_bread_uspi(uspi, sb, uspi->s_sbbase + super_block_offset/block_size, super_block_size);
+	
 	if (!ubh) 
             goto failed;
 
 	
-	usb1 = ubh_get_usb_first(USPI_UBH);
-	usb2 = ubh_get_usb_second(USPI_UBH);
-	usb3 = ubh_get_usb_third(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
+	usb2 = ubh_get_usb_second(uspi);
+	usb3 = ubh_get_usb_third(uspi);
 	usb  = (struct ufs_super_block *)
 		((struct ufs_buffer_head *)uspi)->bh[0]->b_data ;
 
@@ -1006,8 +1003,8 @@ static void ufs_write_super (struct supe
 	UFSD(("ENTER\n"))
 	flags = UFS_SB(sb)->s_flags;
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first(USPI_UBH);
-	usb3 = ubh_get_usb_third(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
+	usb3 = ubh_get_usb_third(uspi);
 
 	if (!(sb->s_flags & MS_RDONLY)) {
 		usb1->fs_time = cpu_to_fs32(sb, get_seconds());
@@ -1049,8 +1046,8 @@ static int ufs_remount (struct super_blo
 	
 	uspi = UFS_SB(sb)->s_uspi;
 	flags = UFS_SB(sb)->s_flags;
-	usb1 = ubh_get_usb_first(USPI_UBH);
-	usb3 = ubh_get_usb_third(USPI_UBH);
+	usb1 = ubh_get_usb_first(uspi);
+	usb3 = ubh_get_usb_third(uspi);
 	
 	/*
 	 * Allow the "check" option to be passed as a remount option.
@@ -1124,7 +1121,7 @@ static int ufs_statfs (struct super_bloc
 	lock_kernel();
 
 	uspi = UFS_SB(sb)->s_uspi;
-	usb1 = ubh_get_usb_first (USPI_UBH);
+	usb1 = ubh_get_usb_first (uspi);
 	usb  = (struct ufs_super_block *)
 		((struct ufs_buffer_head *)uspi)->bh[0]->b_data ;
 	
diff -uprN -X linux-2.6.15-vanilla/Documentation/dontdiff linux-2.6.15-vanilla/fs/ufs/util.h linux-2.6.15/fs/ufs/util.h
--- linux-2.6.15-vanilla/fs/ufs/util.h	2006-01-14 09:47:20.429729750 +0300
+++ linux-2.6.15/fs/ufs/util.h	2006-01-14 09:59:08.261966500 +0300
@@ -249,18 +249,28 @@ extern void _ubh_memcpyubh_(struct ufs_s
 
 
 /*
- * macros to get important structures from ufs_buffer_head
+ * macros and inline function to get important structures from ufs_sb_private_info
  */
-#define ubh_get_usb_first(ubh) \
-	((struct ufs_super_block_first *)((ubh)->bh[0]->b_data))
 
-#define ubh_get_usb_second(ubh) \
-	((struct ufs_super_block_second *)((ubh)->\
-					   bh[UFS_SECTOR_SIZE >> uspi->s_fshift]->b_data + (UFS_SECTOR_SIZE & ~uspi->s_fmask)))
-
-#define ubh_get_usb_third(ubh) \
-	((struct ufs_super_block_third *)((ubh)-> \
-	bh[UFS_SECTOR_SIZE*2 >> uspi->s_fshift]->b_data + (UFS_SECTOR_SIZE*2 & ~uspi->s_fmask)))
+static inline void *get_usb_offset(struct ufs_sb_private_info *uspi,
+				   unsigned int offset)
+{
+	unsigned int index;
+	
+	index = offset >> uspi->s_fshift;
+	offset &= ~uspi->s_fmask;
+	return uspi->s_ubh.bh[index]->b_data + offset;
+}
+
+#define ubh_get_usb_first(uspi) \
+	((struct ufs_super_block_first *)get_usb_offset((uspi), 0))
+
+#define ubh_get_usb_second(uspi) \
+	((struct ufs_super_block_second *)get_usb_offset((uspi), UFS_SECTOR_SIZE))
+
+#define ubh_get_usb_third(uspi)	\
+	((struct ufs_super_block_third *)get_usb_offset((uspi), 2*UFS_SECTOR_SIZE))
+
 
 #define ubh_get_ucg(ubh) \
 	((struct ufs_cylinder_group *)((ubh)->bh[0]->b_data))

-- 
/Evgeniy

[PATCH 2.6.15] Re: Oops in ufs_fill_super at mount time

[Evgeniy]

On Fri, Jan 13, 2006 at 03:54:50AM +0300, Alexey Dobriyan wrote:
> Version 2.6.15-43ecb9a33ba8c93ebbda81d48ca05f0d1bbf9056
> 
> Actually more or less latest -git is affected too, but
> I'm sick of recompiling right now so please wait for bisecting results.
> 
> It's random wrt one mount of UFS, but several mount/umounts in a row
> reproduce it reliably:
> 

I suppose problem in a couple of brackets in fs/ufs/utils.h,
instead of 512th byte of buffer, usb2 point to nth structure of
type ufs_super_block_second.

This patch fix problem for me, 
can you test it?

Signed-off-by: Evgeniy Dushistov <dushistov@mail.ru>

---

--- linux-2.6.15/fs/ufs/util.h.orig	2006-01-13 17:33:49.123946250 +0300
+++ linux-2.6.15/fs/ufs/util.h	2006-01-13 17:35:50.127508500 +0300
@@ -255,8 +255,8 @@ extern void _ubh_memcpyubh_(struct ufs_s
 	((struct ufs_super_block_first *)((ubh)->bh[0]->b_data))
 
 #define ubh_get_usb_second(ubh) \
-	((struct ufs_super_block_second *)(ubh)-> \
-	bh[UFS_SECTOR_SIZE >> uspi->s_fshift]->b_data + (UFS_SECTOR_SIZE & ~uspi->s_fmask))
+	((struct ufs_super_block_second *)((ubh)->\
+					   bh[UFS_SECTOR_SIZE >> uspi->s_fshift]->b_data + (UFS_SECTOR_SIZE & ~uspi->s_fmask)))
 
 #define ubh_get_usb_third(ubh) \
 	((struct ufs_super_block_third *)((ubh)-> \

-- 
/Evgeniy