From owner-freebsd-bugs@FreeBSD.ORG Thu Feb 26 19:00:05 2009 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D9E2B1065904 for ; Thu, 26 Feb 2009 19:00:05 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id B905C8FC1A for ; Thu, 26 Feb 2009 19:00:05 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id n1QJ05n0002808 for ; Thu, 26 Feb 2009 19:00:05 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id n1QJ05dd002807; Thu, 26 Feb 2009 19:00:05 GMT (envelope-from gnats) Date: Thu, 26 Feb 2009 19:00:05 GMT Message-Id: <200902261900.n1QJ05dd002807@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: dfilter@FreeBSD.ORG (dfilter service) Cc: Subject: Re: kern/120967: commit references a PR X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: dfilter service List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 Feb 2009 19:00:11 -0000 The following reply was made to PR kern/120967; it has been noted by GNATS. From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/120967: commit references a PR Date: Thu, 26 Feb 2009 18:58:58 +0000 (UTC) Author: avg Date: Thu Feb 26 18:58:41 2009 New Revision: 189082 URL: http://svn.freebsd.org/changeset/base/189082 Log: udf_readatoffset: read through directory vnode, do not read > MAXBSIZE Currently bread()-ing through device vnode with (1) VMIO enabled, (2) bo_bsize != DEV_BSIZE (3) more than 1 block results in data being incorrectly cached. So instead a more common approach of using a vnode belonging to fs is now employed. Also, prevent attempt to bread more than MAXBSIZE bytes because of adjustments made to account for offset that doesn't start on block boundary. Add expanded comments to explain the calculations. Also drop unused inline function while here. PR: kern/120967 PR: kern/129084 Reviewed by: scottl, kib Approved by: jhb (mentor) Modified: head/sys/fs/udf/udf.h head/sys/fs/udf/udf_vfsops.c head/sys/fs/udf/udf_vnops.c Modified: head/sys/fs/udf/udf.h ============================================================================== --- head/sys/fs/udf/udf.h Thu Feb 26 18:55:55 2009 (r189081) +++ head/sys/fs/udf/udf.h Thu Feb 26 18:58:41 2009 (r189082) @@ -95,27 +95,12 @@ struct ifid { MALLOC_DECLARE(M_UDFFENTRY); static __inline int -udf_readlblks(struct udf_mnt *udfmp, int sector, int size, struct buf **bp) +udf_readdevblks(struct udf_mnt *udfmp, int sector, int size, struct buf **bp) { return (RDSECTOR(udfmp->im_devvp, sector, (size + udfmp->bmask) & ~udfmp->bmask, bp)); } -static __inline int -udf_readalblks(struct udf_mnt *udfmp, int lsector, int size, struct buf **bp) -{ - daddr_t rablock, lblk; - int rasize; - - lblk = (lsector + udfmp->part_start) << (udfmp->bshift - DEV_BSHIFT); - rablock = (lblk + 1) << udfmp->bshift; - rasize = size; - - return (breadn(udfmp->im_devvp, lblk, - (size + udfmp->bmask) & ~udfmp->bmask, - &rablock, &rasize, 1, NOCRED, bp)); -} - /* * Produce a suitable file number from an ICB. The passed in ICB is expected * to be in little endian (meaning that it hasn't been swapped for big Modified: head/sys/fs/udf/udf_vfsops.c ============================================================================== --- head/sys/fs/udf/udf_vfsops.c Thu Feb 26 18:55:55 2009 (r189081) +++ head/sys/fs/udf/udf_vfsops.c Thu Feb 26 18:58:41 2009 (r189082) @@ -476,7 +476,7 @@ udf_mountfs(struct vnode *devvp, struct */ sector = le32toh(udfmp->root_icb.loc.lb_num) + udfmp->part_start; size = le32toh(udfmp->root_icb.len); - if ((error = udf_readlblks(udfmp, sector, size, &bp)) != 0) { + if ((error = udf_readdevblks(udfmp, sector, size, &bp)) != 0) { printf("Cannot read sector %d\n", sector); goto bail; } @@ -794,7 +794,7 @@ udf_find_partmaps(struct udf_mnt *udfmp, * XXX If reading the first Sparing Table fails, should look * for another table. */ - if ((error = udf_readlblks(udfmp, le32toh(pms->st_loc[0]), + if ((error = udf_readdevblks(udfmp, le32toh(pms->st_loc[0]), le32toh(pms->st_size), &bp)) != 0) { if (bp != NULL) brelse(bp); Modified: head/sys/fs/udf/udf_vnops.c ============================================================================== --- head/sys/fs/udf/udf_vnops.c Thu Feb 26 18:55:55 2009 (r189081) +++ head/sys/fs/udf/udf_vnops.c Thu Feb 26 18:58:41 2009 (r189082) @@ -1296,16 +1296,20 @@ static int udf_readatoffset(struct udf_node *node, int *size, off_t offset, struct buf **bp, uint8_t **data) { - struct udf_mnt *udfmp; - struct file_entry *fentry = NULL; + struct udf_mnt *udfmp = node->udfmp; + struct vnode *vp = node->i_vnode; + struct file_entry *fentry; struct buf *bp1; uint32_t max_size; daddr_t sector; + off_t off; + int adj_size; int error; - udfmp = node->udfmp; - - *bp = NULL; + /* + * This call is made *not* only to detect UDF_INVALID_BMAP case, + * max_size is used as an ad-hoc read-ahead hint for "normal" case. + */ error = udf_bmap_internal(node, offset, §or, &max_size); if (error == UDF_INVALID_BMAP) { /* @@ -1323,9 +1327,18 @@ udf_readatoffset(struct udf_node *node, /* Adjust the size so that it is within range */ if (*size == 0 || *size > max_size) *size = max_size; - *size = min(*size, MAXBSIZE); - if ((error = udf_readlblks(udfmp, sector, *size + (offset & udfmp->bmask), bp))) { + /* + * Because we will read starting at block boundary, we need to adjust + * how much we need to read so that all promised data is in. + * Also, we can't promise to read more than MAXBSIZE bytes starting + * from block boundary, so adjust what we promise too. + */ + off = blkoff(udfmp, offset); + *size = min(*size, MAXBSIZE - off); + adj_size = (*size + off + udfmp->bmask) & ~udfmp->bmask; + *bp = NULL; + if ((error = bread(vp, lblkno(udfmp, offset), adj_size, NOCRED, bp))) { printf("warning: udf_readlblks returned error %d\n", error); /* note: *bp may be non-NULL */ return (error); _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"