Date: Tue, 12 Feb 2002 23:57:15 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Matthew Dillon <dillon@apollo.backplane.com> Cc: Matteo <drummino@yahoo.com>, <freebsd-stable@FreeBSD.ORG>, <freebsd-bugs@FreeBSD.ORG>, <aiuto@gufi.org> Subject: Re: Crash System with MSDOS file-system driver! Message-ID: <20020212225510.F4096-100000@gamplex.bde.org> In-Reply-To: <200202112006.g1BK6Dw56980@apollo.backplane.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 11 Feb 2002, Matthew Dillon wrote: > Well, I think you did find a bug in VFS/BIO. Even UFS will unnecessarily > break if a fully valid buffer is written and the write fails, due to > b_resid being left non-zero (B_ERROR is cleared in that case and the > write is re-queued). I think that failure is fairly safe. The read will just terminate early for the bad block. > But we have a problem here. You can't gratuitously clear b_resid for > the B_CACHE case. NFS depends on it being persistent for VLNK handling. > In fact, NFS even assumes it is persistent for directory reading but > since VMIO-backed buffers can be dropped and later reconstituted (zeroing > b_resid), there is a hack in there to magically fix it for the VDIR > case. I forgot about nfs. > I also don't think it's a good idea to just munge breadn() and leave > bread() alone. bread() just calls breadn(). > What I recommend is that the b_resid clearing code actually be put in > getblk() itself but only for the B_CACHE|B_VMIO case, and with a big > comment describing why it is there. If you want to work up a patch > for that, Bruce, I'll be happy to help test it. Or I can just do it > if you don't want to. The code is almost exactly the same. Please do it. > I do not understand the last bit of your patch that converts a non-error'd > short read into an EIO. This case cannot happen on the first bp with > either UFS or MSDOSFS because they explicitly conditionalize the breadn() > call for the case where there is at least one full buffer's worth of data > is left in the file or directory. Consequently the first buffer will > never have a short read. It is just a sanity check. Device drivers return a short read for EOF at least. The EOF case shouldn't happen because filesystems are built of whole blocks. Except I forgot the nfs case... I knew that short reads were non-errors for block devices and thought that removing block devices made them errors in all cases. > The API, in general, does not explicitly > disallow EOF occuring in the first buffer so unless you believe an API > change is in order and clearly document it in breadn()'s comments, you > should not commit this patch. (For example, NFS doesn't use breadn() > but if it did it would likely assume that EOF could occur anywhere, even > in the first buffer). It's not a good API, so some changes to it wouldn't hurt. Currently, callers of bread() are apparently required to detect short reads by comparing bp->b_bcount - bp->b_resid with their requested size. Most callers don't bother. physio() is the main exception. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-stable" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020212225510.F4096-100000>