On Dec 8, Jaromir Dolecek wrote
> Anyway - interesting point about ext2fs_sync():
> The code is very similar to ffs_sync(). In sync loop, vget() is called
> for each "dirty" vnode to lock it.  If the vnode is busy (some other process
> writing to it), vget() returns with EBUSY. In both ffs_sync() and "old"
> ext2fs_sync(), the error is ignored - the only handled error is
> ENOENT, which re-starts sync() loop. So when vget() returns EBUSY,
> it's ignored and we get nice panic later, when vput() aka VOP_UNLOCK()
> is called - the vnode is locked by other process. It works that way
> with ext2fs at least, but there is no panic for ffs.  I spent
> quite a big time just by looking for difference between ffs & ext2fs
> code, which make it work under ffs (and didn't find any, unfortunately).

You didn't read carefully ffs_sync (and I can point it out: I did the same
mistacke when I read it :) All errors are handled, but differently:
if the error is ENOENT, the sync loop is restarted.
For any other error, the error is just ignored, and we go sync the next
vnode (note the 'continue' keyword). So if a vnode is busy, it's not flushed,
that' all.
The only problem I can see here is if vget() returns an error with is something
else than EBUSY or ENOENT. Is it possible ? Is the inode locked in this
case ? If so maybe it should be unlocked here (I didn't investigate further).
> 
> Another point: maybe it would be worth to actually (physically)
> share more code between ffs/ufs/ext2fs. Something similar to way
> ffs and lfs are sharing code in ufs_readwrite.c, maybe (not that
> I like the way it's done there very much). I noticed even come parts from
> ext2fs_readwrite.c are more or less a copy of ufs_readwrite.c code,
> just with slightly other structure names.

And that's the problem: ext2fs and ffs have the same logic, but different
data layout. It's hard to share code when the data structures to be used are
different.

About ufs_readwrite.c/ext2fs_readwrite.c specifically: they're not shared
because eventually the clustering code should go here. But I din't look
further either ... fixing vfs_cluster for blocks < pagesize would be good,
but I'm not sure it's enouth for ext2fs ...

--
Manuel Bouyer, LIP6, Universite Paris VI.           Manuel.Bouyer@lip6.fr
--