DF-3170 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 | diff --git a/a/sys/vfs/mfs/mfsnode.h b/b/sys/vfs/mfs/mfsnode.h index 1c0b280..9e745b5 100644 --- a/sys/vfs/mfs/mfsnode.h +++ b/b/sys/vfs/mfs/mfsnode.h @@ -32,7 +32,9 @@ */ #ifndef _UFS_MFS_MFSNODE_H_ -#define _UFS_MFS_MFSNODE_H_ +#define _UFS_MFS_MFSNODE_H_ + +#include <sys/spinlock.h> /* * This structure defines the control data for the memory based file system. @@ -45,7 +47,7 @@ struct mfsnode { struct bio_queue_head bio_queue; /* list of I/O requests */ cdev_t mfs_dev; /* underlying device */ int mfs_active; - long mfs_spare[1]; + struct spinlock bio_queue_spin; /* protects bio_queue & mfs_active */ }; #ifdef MALLOC_DECLARE diff --git a/a/sys/vfs/mfs/mfs_vfsops.c b/b/sys/vfs/mfs/mfs_vfsops.c index cab8c18..b8c5074 100644 --- a/sys/vfs/mfs/mfs_vfsops.c +++ b/b/sys/vfs/mfs/mfs_vfsops.c @@ -141,8 +141,10 @@ mfsclose(struct dev_close_args *ap) if ((mfsp = dev->si_drv1) == NULL) return(0); - mfsp->mfs_active = 0; - wakeup((caddr_t)mfsp); + spin_lock(&mfsp->bio_queue_spin); + mfsp->mfs_active = 0; + spin_unlock(&mfsp->bio_queue_spin); + wakeup((caddr_t)mfsp); return(0); } @@ -184,7 +186,26 @@ mfsstrategy(struct dev_strategy_args *ap) if (mfsp->mfs_td == curthread) { mfs_doio(bio, mfsp); } else { + /* + * The queue is shared with the mfs_start server thread. + * The server drains without the BGL (MNTK_ST_MPSAFE is + * set by mfs_mount), so the producer MUST take the queue + * spinlock; the old crit/BGL pairing did not serialize + * producer against consumer across CPUs. + * + * If the server is already gone, complete the request + * here instead of queueing it forever. + */ + spin_lock(&mfsp->bio_queue_spin); + if (mfsp->mfs_active == 0) { + spin_unlock(&mfsp->bio_queue_spin); + bp->b_error = ENXIO; + bp->b_flags |= B_ERROR | B_INVAL; + biodone(bio); + return(0); + } bioq_insert_tail(&mfsp->bio_queue, bio); + spin_unlock(&mfsp->bio_queue_spin); wakeup((caddr_t)mfsp); } return(0); @@ -341,6 +362,7 @@ mfs_mount(struct mount *mp, char *path, caddr_t data, struct ucred *cred) mfsp->mfs_td = curthread; mfsp->mfs_active = 1; bioq_init(&mfsp->bio_queue); + spin_init(&mfsp->bio_queue_spin, "mfsbio"); devfs_config(); /* sync devfs work */ ksnprintf(devname, sizeof(devname), "/dev/mfs%d", minnum); @@ -450,18 +472,17 @@ mfs_start(struct mount *mp, int flags) mfsp->mfs_td = td; while (mfsp->mfs_active) { - crit_enter(); - - while ((bio = bioq_takefirst(&mfsp->bio_queue)) != NULL) { - crit_exit(); + for (;;) { + spin_lock(&mfsp->bio_queue_spin); + bio = bioq_takefirst(&mfsp->bio_queue); + spin_unlock(&mfsp->bio_queue_spin); + if (bio == NULL) + break; bp = bio->bio_buf; mfs_doio(bio, mfsp); wakeup(bp); - crit_enter(); } - crit_exit(); - /* * If a non-ignored signal is received, try to unmount. * If that fails, clear the signal (it has been "processed"), @@ -491,6 +512,24 @@ mfs_start(struct mount *mp, int flags) gotsig++; /* try to unmount in next pass */ } PRELE(curproc); + + /* + * Final drain: producers hold bio_queue_spin across the + * activity check and the insert, so once mfs_active is 0 any + * still-queued bio was inserted before the clear and must be + * serviced here rather than dropped with the mfsnode. + */ + for (;;) { + spin_lock(&mfsp->bio_queue_spin); + bio = bioq_takefirst(&mfsp->bio_queue); + spin_unlock(&mfsp->bio_queue_spin); + if (bio == NULL) + break; + bp = bio->bio_buf; + mfs_doio(bio, mfsp); + wakeup(bp); + } + if (mfsp->mfs_dev) { destroy_dev(mfsp->mfs_dev); mfsp->mfs_dev = NULL; |