Date: Wed, 8 Dec 2010 23:49:09 +0200 From: Kostik Belousov <kostikbel@gmail.com> To: Julian Elischer <julian@freebsd.org> Cc: freebsd-fs@freebsd.org, pjd@freebsd.org, Oliver Fromme <olli@lurza.secnetix.de>, Kirk McKusick <mckusick@McKusick.COM> Subject: Re: TRIM support for UFS? Message-ID: <20101208214909.GH33073@deviant.kiev.zoral.com.ua> In-Reply-To: <4CFFBB6C.1050400@freebsd.org> References: <201012081658.oB8Gw9w3010495@lurza.secnetix.de> <4CFFBB6C.1050400@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--v2Uk6McLiE8OV1El Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Dec 08, 2010 at 09:07:56AM -0800, Julian Elischer wrote: >=20 > From: Kirk McKusick <mckusick@chez.mckusick.com> > Date: Wed, 21 May 2008 13:19:18 -0700 > To: Poul-Henning Kamp <phk@critter.freebsd.dk> > Subject: UFS and BIO_DELETE > X-URL: http://WWW.McKusick.COM/ > Reply-To: Kirk McKusick <mckusick@McKusick.COM> >=20 > I enclose below my proposed patch to add BIO_DELETE to UFS > (goes at the end of ffs_blkfree). As I have no way to test > it, I am wondering if you could let me know if it works. >=20 > Also, I am thinking of only enabling it for filesystems mounted > with a new flag requesting the behavior since the geteblk is a > rather expensive call for the usual no-op case. >=20 > I did look at just allocating a `struct bio' as a local variable > and using that, but it looked like I needed to also come up with a > `producer' and/or `consumer' if I wanted to pass it to GEOM directly, > so in the end I went with this more expensive solution. If there > is an easy way to just pass a bio structure to GEOM, I would much > prefer that approach. >=20 > ~Kirk >=20 >=20 > *** ffs_alloc.c Wed May 21 20:11:04 2008 > --- ffs_alloc.c.new Wed May 21 20:10:50 2008 > *************** > *** 1945,1950 **** > --- 1945,1962 ---- > ACTIVECLEAR(fs, cg); > UFS_UNLOCK(ump); > bdwrite(bp); > + /* > + * Request that the block be cleared. > + */ > + bp =3D geteblk(size); > + bp->b_iocmd =3D BIO_DELETE; > + bp->b_vp =3D devvp; > + bp->b_blkno =3D fsbtodb(fs, bno); > + bp->b_offset =3D dbtob(bp->b_blkno); > + bp->b_iooffset =3D bp->b_offset; > + bp->b_bcount =3D size; > + BUF_KERNPROC(bp); > + BO_STRATEGY(&devvp->v_bufobj, bp); > } > =20 > #ifdef INVARIANTS >=20 The UFS devvp contains the pointer to struct g_consumer in devpp->v_bufobj->bo_private, so g_alloc_bio() and g_io_request() can be used to start BIO_DELETE command without fake bufer allocation. g_io_strategy() may be used as the sample. But, I have a question about the patch. The bitmap block in ffs_blkfree() is handled with delayed write, by bdwrite() call. I think that it is quite possible for BIO_DELETE to be executed before the bitmap block is written and on-disk bit is set in bitmap. Would not this allow for the blocks to be deleted before the bitmap is updated, and potentially before on-disk pointers are cleared that points to the blocks ? SU just rolls back the unfinished dependencies on write, but BIO_DELETE cannot. Also, shouldn't the BIO_DELETE only be issued for real devices, and not the snapshots ? diff --git a/sys/ufs/ffs/ffs_alloc.c b/sys/ufs/ffs/ffs_alloc.c index b740bbb..4ff57ae 100644 --- a/sys/ufs/ffs/ffs_alloc.c +++ b/sys/ufs/ffs/ffs_alloc.c @@ -83,6 +83,8 @@ __FBSDID("$FreeBSD$"); =20 #include <security/audit/audit.h> =20 +#include <geom/geom.h> + #include <ufs/ufs/dir.h> #include <ufs/ufs/extattr.h> #include <ufs/ufs/quota.h> @@ -1850,6 +1852,7 @@ ffs_blkfree(ump, fs, devvp, bno, size, inum, dephd) u_int cg; u_int8_t *blksfree; struct cdev *dev; + struct bio *bip; =20 cg =3D dtog(fs, bno); if (devvp->v_type =3D=3D VREG) { @@ -1962,6 +1965,16 @@ ffs_blkfree(ump, fs, devvp, bno, size, inum, dephd) softdep_setup_blkfree(UFSTOVFS(ump), bp, bno, numfrags(fs, size), dephd); bdwrite(bp); + + if (devvp->v_type !=3D VREG) { + bip =3D g_alloc_bio(); + bip->bio_cmd =3D BIO_DELETE; + bip->bio_offset =3D dbtob(fsbtodb(fs, bno)); + bip->bio_done =3D g_destroy_bio; + bip->bio_length =3D size; + g_io_request(bip, + (struct g_consumer *)devvp->v_bufobj.bo_private); + } } =20 #ifdef INVARIANTS --v2Uk6McLiE8OV1El Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (FreeBSD) iEYEARECAAYFAkz//VUACgkQC3+MBN1Mb4hnDwCgqAJkWJ12JeMLqu4+Zq8t3sE9 AnYAnjFi+KlwMkY/sFhicTHejYSOelXv =ngSY -----END PGP SIGNATURE----- --v2Uk6McLiE8OV1El--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20101208214909.GH33073>