Re: [PATCH] tmpfs work update 013010 (was tmpfs initial work)

From:

Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx>

Date:

Sun, 31 Jan 2010 10:43:40 -0800 (PST)

:Hi Matt and others,
:
:Here comes 3rd iteration.
:
:1) reimplement tmpfs_read(), tmpfs_write() call by following Matt's
:...
:2) tmpfs_strategy() is a kind of temporary hack. Just call
:...
:3) implement tmpfs_advlock() with lockf structure
:
:The change makes tmpfs more stable (make nativekernel works on tmpfs
::) though, still
:some issues. 1)An umount will hit assertion. 2)There are some cases to
:observe non-res
:from tmpfs too. 3)An userland command (mount_tmpfs) is no progress,
:has a lack of features.
:
:I'll start looking a new truncation/extention API beyond this change,
:then dive into above
:issues and items.
:
:thank you, Any comments are always welcome.
:-Naoya
Your patch is really looking good now. Would it be too early for
us to start testing it or should we wait a little longer?
Here are a few things I noticed from perusing your patch:
* case 's' for the size specification uses atoi(), which is
limited to a 32 bit integer while ta_size_max is an off_t (64 bit).
I recommend using strtoimax() instead of atoi().
* I see you are using MNTK_MPSAFE. That won't apply to read,
write, getattr, and inactive. They have their own MNTK_xx_MPSAFE
flags which you also need to specify too if you want those VOPs to
be MPSAFE (and you clearly do).
* You may want to add the other MNTK_xx_MPSAFE flags but remove
MNTK_MPSAFE for initial testing until you get things rock solid,
then work MNTK_MPSAFE back in.
* For tmpfs_write() I think you can safely just use bdwrite(),
and there is no need to implement the B_CLUSTER* support (though
it won't hurt I don't think it will improve performance much
either).
* Currently you are using a separate VM object for the vnode
(via vinitvmio()) and the backing store (via tn_aobj).
This is something that only the VN device (/usr/src/sys/dev/disk/vn)
has done in the codebase. It should work and I think it is an
excellent solution to the backing store issue.
I see you even implemented the freeing of swap space in the
truncation code. Wow! Very cool!
I think shifting it to the new nvtruncbuf()/nvextendbuf() API will
be trivial. It will even simplify the tmpfs code slightly.
Again, incredible work! Please keep us posted!
-Matt