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

[Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx>]

: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