Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 26 Feb 2009 18:58:41 +0000 (UTC)
From:      Andriy Gapon <avg@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r189082 - head/sys/fs/udf
Message-ID:  <200902261858.n1QIwfei035609@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
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, &sector, &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);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200902261858.n1QIwfei035609>