* Oops in ufs_fill_super at mount time @ 2006-01-13 0:54 Alexey Dobriyan 2006-01-13 1:14 ` Linus Torvalds 2006-01-13 15:12 ` [PATCH 2.6.15] Re: Oops in ufs_fill_super at mount time Evgeniy 0 siblings, 2 replies; 9+ messages in thread From: Alexey Dobriyan @ 2006-01-13 0:54 UTC (permalink / raw) To: linux-kernel, linux-fsdevel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Oops in ufs_fill_super at mount time 2006-01-13 0:54 Oops in ufs_fill_super at mount time Alexey Dobriyan @ 2006-01-13 1:14 ` Linus Torvalds 2006-01-13 10:21 ` Alexey Dobriyan 2006-01-13 15:12 ` [PATCH 2.6.15] Re: Oops in ufs_fill_super at mount time Evgeniy 1 sibling, 1 reply; 9+ messages in thread From: Linus Torvalds @ 2006-01-13 1:14 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: linux-kernel, linux-fsdevel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Oops in ufs_fill_super at mount time 2006-01-13 1:14 ` Linus Torvalds @ 2006-01-13 10:21 ` Alexey Dobriyan 2006-01-13 15:45 ` Linus Torvalds 0 siblings, 1 reply; 9+ messages in thread From: Alexey Dobriyan @ 2006-01-13 10:21 UTC (permalink / raw) To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Oops in ufs_fill_super at mount time 2006-01-13 10:21 ` Alexey Dobriyan @ 2006-01-13 15:45 ` Linus Torvalds 2006-01-13 16:36 ` Alexey Dobriyan 2006-01-13 19:05 ` [PATCH 2.6.15] ufs cleanup Evgeniy 0 siblings, 2 replies; 9+ messages in thread From: Linus Torvalds @ 2006-01-13 15:45 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: Linux Kernel Mailing List, linux-fsdevel, Evgeniy 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Oops in ufs_fill_super at mount time 2006-01-13 15:45 ` Linus Torvalds @ 2006-01-13 16:36 ` Alexey Dobriyan 2006-01-13 19:05 ` [PATCH 2.6.15] ufs cleanup Evgeniy 1 sibling, 0 replies; 9+ messages in thread From: Alexey Dobriyan @ 2006-01-13 16:36 UTC (permalink / raw) To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel, Evgeniy 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. ;-) ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2.6.15] ufs cleanup 2006-01-13 15:45 ` Linus Torvalds 2006-01-13 16:36 ` Alexey Dobriyan @ 2006-01-13 19:05 ` Evgeniy 2006-01-13 19:51 ` Linus Torvalds 1 sibling, 1 reply; 9+ messages in thread From: Evgeniy @ 2006-01-13 19:05 UTC (permalink / raw) To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.15] ufs cleanup 2006-01-13 19:05 ` [PATCH 2.6.15] ufs cleanup Evgeniy @ 2006-01-13 19:51 ` Linus Torvalds 2006-01-14 8:42 ` [PATCH 2.6.15] ufs cleanup v. 2 Evgeniy 0 siblings, 1 reply; 9+ messages in thread From: Linus Torvalds @ 2006-01-13 19:51 UTC (permalink / raw) To: Evgeniy; +Cc: linux-kernel, linux-fsdevel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.15] ufs cleanup v. 2 2006-01-13 19:51 ` Linus Torvalds @ 2006-01-14 8:42 ` Evgeniy 0 siblings, 0 replies; 9+ messages in thread From: Evgeniy @ 2006-01-14 8:42 UTC (permalink / raw) To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2.6.15] Re: Oops in ufs_fill_super at mount time 2006-01-13 0:54 Oops in ufs_fill_super at mount time Alexey Dobriyan 2006-01-13 1:14 ` Linus Torvalds @ 2006-01-13 15:12 ` Evgeniy 1 sibling, 0 replies; 9+ messages in thread From: Evgeniy @ 2006-01-13 15:12 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: linux-kernel, linux-fsdevel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2006-01-14 8:55 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2006-01-13 0:54 Oops in ufs_fill_super at mount time Alexey Dobriyan 2006-01-13 1:14 ` Linus Torvalds 2006-01-13 10:21 ` Alexey Dobriyan 2006-01-13 15:45 ` Linus Torvalds 2006-01-13 16:36 ` Alexey Dobriyan 2006-01-13 19:05 ` [PATCH 2.6.15] ufs cleanup Evgeniy 2006-01-13 19:51 ` Linus Torvalds 2006-01-14 8:42 ` [PATCH 2.6.15] ufs cleanup v. 2 Evgeniy 2006-01-13 15:12 ` [PATCH 2.6.15] Re: Oops in ufs_fill_super at mount time Evgeniy
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).