Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 26 Feb 2009 19:00:05 GMT
From:      dfilter@FreeBSD.ORG (dfilter service)
To:        freebsd-bugs@FreeBSD.org
Subject:   Re: kern/120967: commit references a PR
Message-ID:  <200902261900.n1QJ05dd002807@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help 
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, &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);
 _______________________________________________
 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"

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