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>