Subject: Re: Redoing file system suspension API (update)
To: None <hannken@eis.cs.tu-bs.de>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-kern
Date: 06/18/2006 22:59:33
thanks for taking a look of this.
although i have many negative comments, i think it's a step forward. :-)
first of all, i tend to think filesystem snapshot thing should be done
entirely in filesystem-dependent code.
> - Instead of tracking possible long sleeps I put the suspend/resume into
> ltsleep().
why?
i don't think it's desirable for each subsystems to put their own
random hooks in these places.
what happens if a filesystem itself sleeps with PCATCH?
(maybe you can call it a bug, but we currently have such a code.)
besides, vngate_resume itself seems to sleep.
is it safe to call it from ltsleep?
> To solve the rest of 3) it adds a throttling on the first gate not involved
> in a suspending file system.
- isn't it normal that an operation become slow when the system has
other activities?
- why you check P_SYSTEM?
> ** The new API is:
>
> Using explicit enter()/leave() pairs adds much complexity so I took another
> approach. I use two types of gates. Normal gates need a "leave" operation.
> Permanent gates are valid until the thread returns to user mode.
while it can make your patch smaller, i think it's actually more complex
and harder to understand and maintain.
> void vngate_initialize(struct lwp *l)
>
> Initialize the per-thread state. Will be freed on vngate_leave_all(..., 1).
please try to avoid putting subsystem-specific data to struct lwp.
> V_NOERROR Panic on error. No need for the caller to check the result.
what's the point of this?
> Adding permanent gates to lookup(), (new) FILE_USE_GATED() and nfsrv_fhtovp()
> covers most system calls. The various compat parts are not ready. A scan over
> 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.
why you put vngate_enter into FILE_USE, rather than VOPs?
YAMAMOTO Takashi