Subject: Re: Redoing file system suspension API (update)
To: None <firstname.lastname@example.org>
From: Bill Studenmund <email@example.com>
Date: 06/21/2006 13:41:52
Content-Type: text/plain; charset=us-ascii
On Sat, Jun 17, 2006 at 03:58:32PM +0200, Juergen Hannken-Illjes wrote:
> On Tue, Jun 13, 2006 at 04:18:09PM +0200, Juergen Hannken-Illjes wrote:
> > We have an API for file system suspension which consists of the functio=
> This one differs from the initial proposal by two items:
> - The per-thread state is initialized on thread creation instead of
> doing it on the fly (see vngate_initialize()).
> - Instead of tracking possible long sleeps I put the suspend/resume into
> We have an API for file system suspension which consists of the functions
> vfs_write_suspend, vfs_write_resume, vn_start_write and vn_finished_write.
> ** This implementation of file system suspension has some serious problem=
> 1) Its definition (Prepare to start a file system write operation) is vag=
> and file system dependent. It is impossible to determine which VOP's
> will change file system data or metadata without knowing details of the
> underlying file system. It does not allow recursion.
Ok, so we work on the definition. I agree the name needs changing.
I'm not sure I agree about not knowing what will change metadata w/o=20
knowing the file system.
How is recursion an issue? We can either adjust the upper level call=20
layers to cope (pass in a state parameter indicating if we've locked or=20
not, be clear about entry points that are called w/ the lock), or have=20
code cope with a "You already have the lock" error code.
> 2) Its implementation adds a layer above file systems AND a layer inside
> file systems.
Given our current vnode locking protocol, I'm not sure that this is really=
an issue. While it'd be nice to not need upper-level calls (and for=20
some things like VOP_WRITE() calls, we probably should just have the fs do=
it), there are places where upper-level code wants to perform an atomic=20
sequence of actions. Having that upper level need to explicitly lock seems=
fine to me.
Snapshotting really needs the whole OS's help to get right. Applications=20
and everything on down have to help, so that you get a good snapshot. So=20
having things above and below the VFS/VOP layer help out seems quite=20
As an aside, if you have a journaling file system and want to make=20
sure there is space in the journal before you start an operation, you need=
checkpoint calls in exactly the same places where you have the=20
vn_start_write() and vn_stop_write() calls. So the idea of a call or calls=
that say, "I'm about to start a sequence of operations, please make sure=20
everything's ok" is a general one, and something we need.
> 3) It may take forever to suspend a file system if there is a high load
> on other file systems. Even a high "read load" on the file system we
> are suspending makes the suspension take a long time. If softdep file
> systems are involved the suspension may not succeed at all.
Ok, then we definitely need either a redesign or re-work.
> ** The approach described here resolves these issues. It replaces the
> "write gates" by "access file system gates". Goal is to make every system
> call atomic with regards to file system suspension. It will do all opera=
> on a file system either before or after a suspension but will never do on=
> before and another part after the suspension. Allowing recursion makes it
> easier to place the gates. So the advantages are:
I dislike the idea of "gates". They are locks, let's call them such.
I feel strongly about this as you're inventing a whole new locking=20
protocolon the fly. You may be right, but it's not clear to me that you=20
are. And one thing I've learned (painfully) is that even if something I=20
make would work fine, if others don't understand it, it really won't work=
> - It is semantically well defined.
> - It is possible to add a DEBUG option to check it. Every VOP called on
> a suspending or suspended file system is an error (there are minor
> exceptions: syncer and part of pagedaemon).
> - No modifications of file system internals are needed to implement it.
> - A suspended file system is really quiet. No vnodes are locked during
You still have locks, though. I think we should keep calling them locks.=20
Also, we already have spin locks and lockmgr locks. Do we really want to=20
add another type of lock?? I'm not saying you must use lockmgr locks, but=
we really should look at the overall complexity of our locking mechanisms.
> - It solves 1), 2) and the "read part" of 3).
> To solve the rest of 3) it adds a throttling on the first gate not involv=
> in a suspending file system.
> ** The new API is:
> Using explicit enter()/leave() pairs adds much complexity so I took anoth=
> approach. I use two types of gates. Normal gates need a "leave" operatio=
> Permanent gates are valid until the thread returns to user mode.
I really really dislike the idea of adding locks that are automatically
released upon return to user mode.
> void vngate_initialize(struct lwp *l)
> Initialize the per-thread state. Will be freed on vngate_leave_all(...=
> int vngate_enter(struct mount *mp, int flags)
> Enter a vnode gate for the file system "mp". "flags" is a combination =
> V_WAIT Thread must sleep until a suspension is over.
> V_NOWAIT Returns an error if a suspension is active.
> V_NOERROR Panic on error. No need for the caller to check the result.
> V_PERMANENT Enter a permanent gate.
> void vngate_leave(struct mount *mp)
> Leave a (non-permanent) gate for the file system "mp".
> void vngate_leave_all(struct mount *mp, int destroy)
> Leave all permanent gates of this thread. Assumes all normal gates are
> already closed. If "mp" is set, only leave the gates for this file sys=
> "destroy" is set if all state has to be freed because this thread will
> void vngate_sleep(struct mount *mp)
> Sleep until a suspension on this file system is over.
> void vngate_suspend(void)
> Suspend all gates of this thread. Called during suspension and just be=
> we go to long (interruptible) sleep. Further vngate_(enter|leave) call=
> void vngate_resume(void)
> Resume all gates of this thread to the state before the last vngate_sus=
> May wait if a suspension is in progress.
> int vfs_suspend(struct mount *mp, int wait)
> Suspend the file system "mp". If "wait" is set, wait for a current sus=
> otherwise return an error.
> int vfs_resume(struct mount *mp)
> Resume the file system "mp".
> option VNODE_GATEDEBUG
> Add code to check
> - No VOP's are called without a taken gate.
> - Internal integrity checks.
> ** Attached is an implementation on a recent -current.
> Adding permanent gates to lookup(), (new) FILE_USE_GATED() and nfsrv_fhto=
> covers most system calls. The various compat parts are not ready. A scan=
> all "FILE_USE()" should be sufficient here. The implementation renames
> "vfs_write_XXX" to "vfs_XXX" and adds the state pointer to "struct lwp".
> All other changes depend on "option NEWVNGATE" to make it easy to test the
> new API.
I dislike the idea of adding automatic gate calls. Well, something where=20
returning with the "gate" locked is a side-effect.
I agree with Yamamoto-san that a file system should do as much of this=20
inside itself that it can. I disagree that it has to do it all, but I=20
agree with the spirit of things that it should do as much as it can.
I'm not 100% on the details, but I think that you've already found the=20
places above the file system that need to know about snapshot locking.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (NetBSD)
-----END PGP SIGNATURE-----