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>