NetBSD Problem Report #46282
From cheusov@tut.by Fri Mar 30 10:08:48 2012
Return-Path: <cheusov@tut.by>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
by www.NetBSD.org (Postfix) with ESMTP id 6CF0263BBEC
for <gnats-bugs@gnats.netbsd.org>; Fri, 30 Mar 2012 10:08:48 +0000 (UTC)
Message-Id: <s93k422h0fi.fsf@work.imb.invention.com>
Date: Fri, 30 Mar 2012 13:01:53 +0300
From: cheusov@tut.by
To: gnats-bugs@gnats.NetBSD.org
Subject: 6.0_BETA crash: msdosfs_bmap -> pcbmap -> bread -> bio_doread
X-Send-Pr-Version: 3.95
>Number: 46282
>Category: kern
>Synopsis: 6.0_BETA crash: msdosfs_bmap -> pcbmap -> bread -> bio_doread
>Confidential: no
>Severity: critical
>Priority: high
>Responsible: hannken
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Fri Mar 30 10:10:00 +0000 2012
>Closed-Date: Thu Dec 20 11:48:38 +0000 2012
>Last-Modified: Thu Dec 20 11:48:38 +0000 2012
>Originator: Aleksey Cheusov
>Release: NetBSD 6.0_BETA
>Organization:
>Environment:
System: NetBSD work.imb.invention.com 6.0_BETA NetBSD 6.0_BETA (GENERIC) #4: Mon Mar 26 15:56:02 FET 2012 cheusov@work.imb.invention.com:/srv/obj-current/sys/arch/i386/compile/GENERIC i386
Architecture: i386
Machine: i386
>Description:
Stacktrace of the crash is below. It happened at the time
of writing to msdosfs filesystem.
#0 0xc05c5be8 in maybe_dump (howto=260) at /srv/src_netbsd6/sys/arch/i386/i386/machdep.c:878
#1 cpu_reboot (howto=260, bootstr=0x0) at /srv/src_netbsd6/sys/arch/i386/i386/machdep.c:899
#2 0xc07c320a in vpanic (fmt=0xc0c33178 "bio_doread: no such buf", ap=0xdb9187e8 "\240 ") at /srv/src_netbsd6/sys/kern/subr_prf.c:308
#3 0xc07c32af in panic (fmt=0xc0c33178 "bio_doread: no such buf") at /srv/src_netbsd6/sys/kern/subr_prf.c:205
#4 0xc08d354a in bio_doread (vp=0xca0d3b00, blkno=<optimized out>, size=65536, async=0, cred=<optimized out>) at /srv/src_netbsd6/sys/kern/vfs_bio.c:667
#5 0xc08d366c in bread (vp=0xca0d3b00, blkno=8352, size=65536, cred=0xffffffff, flags=0, bpp=0xdb918890) at /srv/src_netbsd6/sys/kern/vfs_bio.c:722
#6 0xc05fab22 in pcbmap (dep=0xc56591f0, findcn=15, bnp=0xdb9188fc, cnp=0x0, sp=0x0) at /srv/src_netbsd6/sys/fs/msdosfs/msdosfs_fat.c:257
#7 0xc05ff02b in msdosfs_bmap (v=0xdb918948) at /srv/src_netbsd6/sys/fs/msdosfs/msdosfs_vnops.c:1662
#8 0xc0907862 in VOP_BMAP (vp=0xc75a5794, bn=14, vpp=0xdb9189ec, bnp=0xdb9189dc, runp=0xdb9189f0) at /srv/src_netbsd6/sys/kern/vnode_if.c:1166
#9 0xc0341a72 in genfs_do_io (vp=0xc75a5794, off=<optimized out>, kva=3655794688, len=16384, flags=9, rw=UIO_WRITE, iodone=0xc08c7b4d <uvm_aio_biodone>) at /srv/src_netbsd6/sys/miscfs/genfs/genfs_io.c:1396
#10 0xc034489c in genfs_gop_write (vp=0xc75a5794, pgs=0xdb918b7c, npages=4, flags=9) at /srv/src_netbsd6/sys/miscfs/genfs/genfs_io.c:1288
#11 0xc0343fa7 in genfs_do_putpages (vp=0xc75a5794, startoff=458752, endoff=462848, origflags=9, busypg=0x0) at /srv/src_netbsd6/sys/miscfs/genfs/genfs_io.c:1172
#12 0xc034483b in genfs_putpages (v=0xdb918c38) at /srv/src_netbsd6/sys/miscfs/genfs/genfs_io.c:806
#13 0xc0907c36 in VOP_PUTPAGES (vp=0xc75a5794, offlo=458752, offhi=462848, flags=9) at /srv/src_netbsd6/sys/kern/vnode_if.c:1428
#14 0xc08c8a76 in uvmpd_scan_queue () at /srv/src_netbsd6/sys/uvm/uvm_pdaemon.c:784
#15 uvmpd_scan () at /srv/src_netbsd6/sys/uvm/uvm_pdaemon.c:946
#16 uvm_pageout (arg=0xc3826800) at /srv/src_netbsd6/sys/uvm/uvm_pdaemon.c:303
#17 0xc0100321 in ?? ()
>How-To-Repeat:
>Fix:
no idea
>Release-Note:
>Audit-Trail:
From: "J. Hannken-Illjes" <hannken@eis.cs.tu-bs.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/46282: 6.0_BETA crash: msdosfs_bmap -> pcbmap -> bread -> bio_doread
Date: Fri, 30 Mar 2012 16:15:14 +0200
It is the pagedaemon that runs this VOP_PUTPAGES() and msdosfs_bmap/pcbmap
doesn't handle this case where getblk() returns NULL and bread() will panic.
How did you get this panic (dmesg, mount -v, file system sizes, commands)?
--
Juergen Hannken-Illjes - hannken@eis.cs.tu-bs.de - TU Braunschweig (Germany)
From: "J. Hannken-Illjes" <hannken@eis.cs.tu-bs.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/46282: 6.0_BETA crash: msdosfs_bmap -> pcbmap -> bread -> bio_doread
Date: Sat, 31 Mar 2012 09:51:41 +0200
--Apple-Mail-2-538299323
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
charset=us-ascii
Please try the attached patch. It replaces bread() with
getblk() / VOP_STRATEGY() and returns an error if getblk() fails.
--
Juergen Hannken-Illjes - hannken@eis.cs.tu-bs.de - TU Braunschweig (Germany)
--Apple-Mail-2-538299323
Content-Disposition: attachment;
filename=msdosfs_fat.c.diff
Content-Type: application/octet-stream;
name="msdosfs_fat.c.diff"
Content-Transfer-Encoding: 7bit
Index: sys/fs/msdosfs/msdosfs_fat.c
===================================================================
RCS file: /cvsroot/src/sys/fs/msdosfs/msdosfs_fat.c,v
retrieving revision 1.19
diff -p -u -4 -r1.19 msdosfs_fat.c
--- sys/fs/msdosfs/msdosfs_fat.c 26 Jan 2010 20:25:52 -0000 1.19
+++ sys/fs/msdosfs/msdosfs_fat.c 31 Mar 2012 07:48:29 -0000
@@ -253,13 +253,28 @@ pcbmap(struct denode *dep, u_long findcn
fatblock(pmp, byteoffset, &bn, &bsize, &bo);
if (bn != bp_bn) {
if (bp)
brelse(bp, 0);
- error = bread(pmp->pm_devvp, de_bn2kb(pmp, bn), bsize,
- NOCRED, 0, &bp);
- if (error) {
- brelse(bp, 0);
- return (error);
+ bp = getblk(pmp->pm_devvp, de_bn2kb(pmp, bn), bsize,
+ 0, 0);
+ if (bp == NULL) {
+ /*
+ * getblk() above returns NULL only iff we are
+ * pagedaemon. See the implementation of getblk
+ * for detail.
+ */
+ return ENOMEM;
+ }
+ if (!ISSET(bp->b_oflags, (BO_DONE | BO_DELWRI))) {
+ SET(bp->b_flags, B_READ);
+ BIO_SETPRIO(bp, BPRIO_TIMECRITICAL);
+ VOP_STRATEGY(pmp->pm_devvp, bp);
+ curlwp->l_ru.ru_inblock++;
+ error = biowait(bp);
+ if (error) {
+ brelse(bp, 0);
+ return error;
+ }
}
bp_bn = bn;
}
prevcn = cn;
--Apple-Mail-2-538299323--
From: Chuck Silvers <chuq@chuq.com>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/46282: 6.0_BETA crash: msdosfs_bmap -> pcbmap -> bread ->
bio_doread
Date: Sat, 31 Mar 2012 14:25:26 -0700
On Sat, Mar 31, 2012 at 07:55:04AM +0000, J. Hannken-Illjes wrote:
> Please try the attached patch. It replaces bread() with
> getblk() / VOP_STRATEGY() and returns an error if getblk() fails.
I don't think that expanding bread() like this is the way to go.
several other file systems (nilfs, ntfs, maybe others) also have this problem,
we don't want a bunch of copies of this all over the place.
it would be better to make bread() and bio_doread() handle this case.
there's a wrinkle with that though. currently bread() always returns
a buffer via bpp even if it returns an error, and the caller is expected
to brelse() that buffer. if getblk() returns NULL then we have no buffer
for the caller to brelse(). the best thing to do would be to change things
so that if bread() returns an error, bread() takes care of brelse()ing
the buffer (if any) instead of the caller doing it. that means changing
all ~140 callers, but it's a pretty straightforward change.
-Chuck
From: "J. Hannken-Illjes" <hannken@eis.cs.tu-bs.de>
To: gnats-bugs@NetBSD.org
Cc: Chuck Silvers <chuq@chuq.com>
Subject: Re: kern/46282: 6.0_BETA crash: msdosfs_bmap -> pcbmap -> bread -> bio_doread
Date: Sun, 1 Apr 2012 11:33:01 +0200
I agree, from a quick look adosfs, efs and nilfs are affected too.
To make pullups easier we should probably do it in two steps, first
this patch to msdosfs, pullup and then try changing bread.
--
Juergen Hannken-Illjes - hannken@eis.cs.tu-bs.de - TU Braunschweig =
(Germany)
On Mar 31, 2012, at 11:30 PM, Chuck Silvers wrote:
> The following reply was made to PR kern/46282; it has been noted by =
GNATS.
>=20
> From: Chuck Silvers <chuq@chuq.com>
> To: gnats-bugs@NetBSD.org
> Cc:=20
> Subject: Re: kern/46282: 6.0_BETA crash: msdosfs_bmap -> pcbmap -> =
bread ->
> bio_doread
> Date: Sat, 31 Mar 2012 14:25:26 -0700
>=20
> On Sat, Mar 31, 2012 at 07:55:04AM +0000, J. Hannken-Illjes wrote:
>> Please try the attached patch. It replaces bread() with
>> getblk() / VOP_STRATEGY() and returns an error if getblk() fails.
>=20
> I don't think that expanding bread() like this is the way to go.
> several other file systems (nilfs, ntfs, maybe others) also have this =
problem,
> we don't want a bunch of copies of this all over the place.
> it would be better to make bread() and bio_doread() handle this case.
>=20
> there's a wrinkle with that though. currently bread() always returns
> a buffer via bpp even if it returns an error, and the caller is =
expected
> to brelse() that buffer. if getblk() returns NULL then we have no =
buffer
> for the caller to brelse(). the best thing to do would be to change =
things
> so that if bread() returns an error, bread() takes care of brelse()ing
> the buffer (if any) instead of the caller doing it. that means =
changing
> all ~140 callers, but it's a pretty straightforward change.
>=20
> -Chuck
>=20
Responsible-Changed-From-To: kern-bug-people->hannken
Responsible-Changed-By: hannken@NetBSD.org
Responsible-Changed-When: Sun, 08 Apr 2012 17:30:04 +0000
Responsible-Changed-Why:
Take.
From: Aleksey Cheusov <cheusov@tut.by>
To: gnats-bugs@netbsd.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: kern/46282: 6.0_BETA crash: msdosfs_bmap -> pcbmap -> bread -> bio_doread
Date: Mon, 9 Apr 2012 10:19:05 +0300
> =A0Please try the attached patch. It replaces bread() with
> =A0getblk() / VOP_STRATEGY() and returns an error if getblk() fails.
I applied this patch and wrote about 30Gb to msdos filesystem.
It seems it works fine now.
From: "Juergen Hannken-Illjes" <hannken@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/46282 CVS commit: src/sys/fs/msdosfs
Date: Mon, 9 Apr 2012 11:10:07 +0000
Module Name: src
Committed By: hannken
Date: Mon Apr 9 11:10:07 UTC 2012
Modified Files:
src/sys/fs/msdosfs: msdosfs_fat.c
Log Message:
pcbmap(): We cannot use bread() here as for the pagedaemon getblk()
may fail leading to a panic in bread().
Replace bread() with getblk() / VOP_STRATEGY() and return
an error if getblk() fails.
Fixes PR#46282: 6.0_BETA crash: msdosfs_bmap -> pcbmap -> bread -> bio_doread
This is an interim solution for easy pullup. The final solution
is be to change bread() to not return a buffer on error. As
we have to change all callers of bread() this will not qualify
for a pullup.
To generate a diff of this commit:
cvs rdiff -u -r1.19 -r1.20 src/sys/fs/msdosfs/msdosfs_fat.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Jeff Rizzo" <riz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/46282 CVS commit: [netbsd-6] src/sys/fs/msdosfs
Date: Mon, 16 Apr 2012 15:37:13 +0000
Module Name: src
Committed By: riz
Date: Mon Apr 16 15:37:12 UTC 2012
Modified Files:
src/sys/fs/msdosfs [netbsd-6]: msdosfs_fat.c
Log Message:
Pull up following revision(s) (requested by hannken in ticket #183):
sys/fs/msdosfs/msdosfs_fat.c: revision 1.20
pcbmap(): We cannot use bread() here as for the pagedaemon getblk()
may fail leading to a panic in bread().
Replace bread() with getblk() / VOP_STRATEGY() and return
an error if getblk() fails.
Fixes PR#46282: 6.0_BETA crash: msdosfs_bmap -> pcbmap -> bread ->
bio_doread
This is an interim solution for easy pullup. The final solution
is be to change bread() to not return a buffer on error. As
we have to change all callers of bread() this will not qualify
for a pullup.
To generate a diff of this commit:
cvs rdiff -u -r1.19 -r1.19.18.1 src/sys/fs/msdosfs/msdosfs_fat.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: Aleksey Cheusov <cheusov@tut.by>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: PR/46282 CVS commit: [netbsd-6] src/sys/fs/msdosfs
Date: Thu, 26 Apr 2012 18:53:24 +0300
> Subject: PR/46282 CVS commit: [netbsd-6] src/sys/fs/msdosfs
...
> =A0To generate a diff of this commit:
> =A0cvs rdiff -u -r1.19 -r1.19.18.1 src/sys/fs/msdosfs/msdosfs_fat.c
Should not this change also be pulled up to 5, 5.0 and 5.1 branches?
From: "Juergen Hannken-Illjes" <hannken@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/46282 CVS commit: src/sys
Date: Thu, 20 Dec 2012 08:03:46 +0000
Module Name: src
Committed By: hannken
Date: Thu Dec 20 08:03:45 UTC 2012
Modified Files:
src/sys/fs/adosfs: advfsops.c advnops.c
src/sys/fs/cd9660: cd9660_lookup.c cd9660_vfsops.c cd9660_vnops.c
src/sys/fs/efs: efs_subr.c efs_vfsops.c efs_vnops.c
src/sys/fs/filecorefs: filecore_bmap.c filecore_lookup.c
filecore_utils.c filecore_vfsops.c filecore_vnops.c
src/sys/fs/msdosfs: msdosfs_denode.c msdosfs_fat.c msdosfs_lookup.c
msdosfs_vnops.c
src/sys/fs/nilfs: nilfs_subr.c nilfs_vfsops.c
src/sys/fs/ntfs: ntfs_subr.c
src/sys/kern: vfs_bio.c
src/sys/miscfs/specfs: spec_vnops.c
src/sys/sys: param.h
src/sys/ufs/ext2fs: ext2fs_alloc.c ext2fs_balloc.c ext2fs_inode.c
ext2fs_subr.c ext2fs_vfsops.c
src/sys/ufs/ffs: ffs_alloc.c ffs_balloc.c ffs_inode.c ffs_snapshot.c
ffs_vfsops.c ffs_wapbl.c
src/sys/ufs/lfs: lfs_balloc.c lfs_syscalls.c lfs_vfsops.c
src/sys/ufs/ufs: ufs_lookup.c
Log Message:
Change bread() and breadn() to never return a buffer on
error and modify all callers to not brelse() on error.
Welcome to 6.99.16
PR kern/46282 (6.0_BETA crash: msdosfs_bmap -> pcbmap -> bread -> bio_doread)
To generate a diff of this commit:
cvs rdiff -u -r1.65 -r1.66 src/sys/fs/adosfs/advfsops.c
cvs rdiff -u -r1.39 -r1.40 src/sys/fs/adosfs/advnops.c
cvs rdiff -u -r1.22 -r1.23 src/sys/fs/cd9660/cd9660_lookup.c
cvs rdiff -u -r1.75 -r1.76 src/sys/fs/cd9660/cd9660_vfsops.c
cvs rdiff -u -r1.41 -r1.42 src/sys/fs/cd9660/cd9660_vnops.c
cvs rdiff -u -r1.7 -r1.8 src/sys/fs/efs/efs_subr.c
cvs rdiff -u -r1.23 -r1.24 src/sys/fs/efs/efs_vfsops.c
cvs rdiff -u -r1.28 -r1.29 src/sys/fs/efs/efs_vnops.c
cvs rdiff -u -r1.9 -r1.10 src/sys/fs/filecorefs/filecore_bmap.c
cvs rdiff -u -r1.16 -r1.17 src/sys/fs/filecorefs/filecore_lookup.c
cvs rdiff -u -r1.10 -r1.11 src/sys/fs/filecorefs/filecore_utils.c
cvs rdiff -u -r1.69 -r1.70 src/sys/fs/filecorefs/filecore_vfsops.c
cvs rdiff -u -r1.34 -r1.35 src/sys/fs/filecorefs/filecore_vnops.c
cvs rdiff -u -r1.47 -r1.48 src/sys/fs/msdosfs/msdosfs_denode.c
cvs rdiff -u -r1.21 -r1.22 src/sys/fs/msdosfs/msdosfs_fat.c
cvs rdiff -u -r1.26 -r1.27 src/sys/fs/msdosfs/msdosfs_lookup.c
cvs rdiff -u -r1.83 -r1.84 src/sys/fs/msdosfs/msdosfs_vnops.c
cvs rdiff -u -r1.8 -r1.9 src/sys/fs/nilfs/nilfs_subr.c
cvs rdiff -u -r1.9 -r1.10 src/sys/fs/nilfs/nilfs_vfsops.c
cvs rdiff -u -r1.48 -r1.49 src/sys/fs/ntfs/ntfs_subr.c
cvs rdiff -u -r1.239 -r1.240 src/sys/kern/vfs_bio.c
cvs rdiff -u -r1.135 -r1.136 src/sys/miscfs/specfs/spec_vnops.c
cvs rdiff -u -r1.423 -r1.424 src/sys/sys/param.h
cvs rdiff -u -r1.43 -r1.44 src/sys/ufs/ext2fs/ext2fs_alloc.c
cvs rdiff -u -r1.35 -r1.36 src/sys/ufs/ext2fs/ext2fs_balloc.c
cvs rdiff -u -r1.76 -r1.77 src/sys/ufs/ext2fs/ext2fs_inode.c
cvs rdiff -u -r1.27 -r1.28 src/sys/ufs/ext2fs/ext2fs_subr.c
cvs rdiff -u -r1.167 -r1.168 src/sys/ufs/ext2fs/ext2fs_vfsops.c
cvs rdiff -u -r1.131 -r1.132 src/sys/ufs/ffs/ffs_alloc.c
cvs rdiff -u -r1.54 -r1.55 src/sys/ufs/ffs/ffs_balloc.c
cvs rdiff -u -r1.110 -r1.111 src/sys/ufs/ffs/ffs_inode.c
cvs rdiff -u -r1.119 -r1.120 src/sys/ufs/ffs/ffs_snapshot.c
cvs rdiff -u -r1.280 -r1.281 src/sys/ufs/ffs/ffs_vfsops.c
cvs rdiff -u -r1.17 -r1.18 src/sys/ufs/ffs/ffs_wapbl.c
cvs rdiff -u -r1.70 -r1.71 src/sys/ufs/lfs/lfs_balloc.c
cvs rdiff -u -r1.142 -r1.143 src/sys/ufs/lfs/lfs_syscalls.c
cvs rdiff -u -r1.296 -r1.297 src/sys/ufs/lfs/lfs_vfsops.c
cvs rdiff -u -r1.120 -r1.121 src/sys/ufs/ufs/ufs_lookup.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Juergen Hannken-Illjes" <hannken@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/46282 CVS commit: src/sys/fs/msdosfs
Date: Thu, 20 Dec 2012 11:44:39 +0000
Module Name: src
Committed By: hannken
Date: Thu Dec 20 11:44:39 UTC 2012
Modified Files:
src/sys/fs/msdosfs: msdosfs_fat.c
Log Message:
Revert rev. 1.20 now that bread() has been fixed.
PR kern/46282 (6.0_BETA crash: msdosfs_bmap -> pcbmap -> bread -> bio_doread)
To generate a diff of this commit:
cvs rdiff -u -r1.22 -r1.23 src/sys/fs/msdosfs/msdosfs_fat.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: open->closed
State-Changed-By: hannken@NetBSD.org
State-Changed-When: Thu, 20 Dec 2012 11:48:38 +0000
State-Changed-Why:
Fixed in tree.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home