Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 14 Dec 2002 16:56:07 +0000
From:      Ian Dowse <iedowse@maths.tcd.ie>
To:        Jake Burkholder <jake@locore.ca>
Cc:        Kirk McKusick <mckusick@mckusick.com>, cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/sys/ufs/ufs inode.h src/sys/sys conf.h src/sys/ufs/ffs ffs_snapshot.c 
Message-ID:   <200212141656.aa51164@salmon.maths.tcd.ie>
In-Reply-To: Your message of "Sat, 14 Dec 2002 03:16:31 EST." <20021214031631.D93389@locore.ca> 

next in thread | previous in thread | raw e-mail | index | archive | help
In message <20021214031631.D93389@locore.ca>, Jake Burkholder writes:
>Apparently, On Fri, Dec 13, 2002 at 05:37:00PM -0800,
>	Kirk McKusick said words to the effect of;
>>   Only the most recent snapshot contains the complete list of blocks
>>   that were copied in all of the earlier snapshots, thus its precomputed
>>   list must be used in the copyonwrite test. Using incomplete lists may
>>   lead to deadlock. Also do not include the blocks used for the indirect
>>   pointers in the indirect pointers as this may lead to inconsistent
>>   snapshots.
>
>This tries to stuff a pointer into a 32 bit integer.
>
>+	((daddr_t *)(ip->i_offset)) = &snapblklist[1];
>
>Please find a field that is wide enough to hold a pointer on all platforms.

How about just putting back the i_snapblklist field into struct
inode for now? I suppose we could eventually use a union with the
directory modification state fields, though the size reduction is
fairly marginal. Patch below - sorry, this is completely untested.

Ian

Index: ffs/ffs_snapshot.c
===================================================================
RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_snapshot.c,v
retrieving revision 1.54
diff -u -r1.54 ffs_snapshot.c
--- ffs/ffs_snapshot.c	14 Dec 2002 01:36:59 -0000	1.54
+++ ffs/ffs_snapshot.c	14 Dec 2002 16:46:36 -0000
@@ -532,18 +532,16 @@
 	}
 	/*
 	 * Allocate the space for the list of preallocated snapshot blocks.
-	 * The i_offset field is borrowed to pass the value of snapblklist
-	 * down into the expunge functions.
 	 */
 	snaplistsize = fs->fs_ncg + howmany(fs->fs_cssize, fs->fs_bsize) +
 	    FSMAXSNAP + 1 /* superblock */ + 1 /* last block */ + 1 /* size */;
 	MALLOC(snapblklist, daddr_t *, snaplistsize * sizeof(daddr_t),
 	    M_UFSMNT, M_WAITOK);
-	((daddr_t *)(ip->i_offset)) = &snapblklist[1];
+	ip->i_snapblklist = &snapblklist[1];
 	/*
 	 * Expunge the blocks used by the snapshots from the set of
 	 * blocks marked as used in the snapshot bitmaps. Also, collect
-	 * the list of allocated blocks in i_offset.
+	 * the list of allocated blocks in i_snapblklist.
 	 */
 	if (ip->i_ump->um_fstype == UFS1)
 		error = expunge_ufs1(vp, ip, copy_fs, mapacct_ufs1, BLK_SNAP);
@@ -554,9 +552,9 @@
 		FREE(snapblklist, M_UFSMNT);
 		goto done;
 	}
-	snaplistsize = ((daddr_t *)(ip->i_offset)) - snapblklist;
+	snaplistsize = ip->i_snapblklist - snapblklist;
 	snapblklist[0] = snaplistsize;
-	ip->i_offset = 0;
+	ip->i_snapblklist = NULL;
 	/*
 	 * Write out the list of allocated blocks to the end of the snapshot.
 	 */
@@ -999,7 +997,7 @@
 		if (blkno == 0 || blkno == BLK_NOCOPY)
 			continue;
 		if (expungetype == BLK_SNAP && blkno != BLK_SNAP)
-			*((daddr_t *)(ip->i_offset))++ = lblkno;
+			*ip->i_snapblklist++ = lblkno;
 		if (blkno == BLK_SNAP)
 			blkno = blkstofrags(fs, lblkno);
 		ffs_blkfree(fs, vp, blkno, fs->fs_bsize, inum);
@@ -1275,7 +1273,7 @@
 		if (blkno == 0 || blkno == BLK_NOCOPY)
 			continue;
 		if (expungetype == BLK_SNAP && blkno != BLK_SNAP)
-			*((daddr_t *)(ip->i_offset))++ = lblkno;
+			*ip->i_snapblklist++ = lblkno;
 		if (blkno == BLK_SNAP)
 			blkno = blkstofrags(fs, lblkno);
 		ffs_blkfree(fs, vp, blkno, fs->fs_bsize, inum);
Index: ufs/inode.h
===================================================================
RCS file: /home/ncvs/src/sys/ufs/ufs/inode.h,v
retrieving revision 1.42
diff -u -r1.42 inode.h
--- ufs/inode.h	14 Dec 2002 01:36:59 -0000	1.42
+++ ufs/inode.h	14 Dec 2002 16:46:40 -0000
@@ -63,6 +63,7 @@
 struct inode {
 	LIST_ENTRY(inode) i_hash;/* Hash chain. */
 	TAILQ_ENTRY(inode) i_nextsnap; /* snapshot file list. */
+	daddr_t	*i_snapblklist;	/* snapshot blocks for expunge functions. */
 	struct	vnode  *i_vnode;/* Vnode associated with this inode. */
 	struct	ufsmount *i_ump;/* Ufsmount point associated with this inode. */
 	struct	vnode  *i_devvp;/* Vnode for block I/O. */

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message




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