> On 23. Jan 2020, at 17:37, Andrew Doran <ad%netbsd.org@localhost> wrote:
>
> Hi,
>
> I would like to unify these back into a single struct vnode.
>
> I think I understand the motivation for the split in the first place and I
> very much agree code outside the VFS core should not be playing around with
> a lot of the stuff in struct vnode, but I think the split causes more
> trouble than it's worth.
Please do not, I strongly object.
It was hard work to clean up file systems and kernel components using
and abusing vnode fields that should be opaque outside the VFS implementation.
I'm quite sure people will start again using these fields because
it is so simple. For me the unique vnode was a maintenance nightmare.
> Here are my reasons for wanting to go back to a single struct:
>
> 1) Performance. I want to be able to cluster the fields around the kind of
> usage they get. For example changing v_usecount on libc.so's vnode or the
> root vnode shouldn't force a cache miss on a different CPU that's just
> trying to look up pages, or get at v_op or v_type or whatever; they're
> different kinds of activity (see below for example).
>
> 2) Memory usage; to do the above in a way that doesn't use more memory than
> needed. With the structs split in two it's difficult to arrange all of
> this cleanly.
>
> 3) Clarity. I'm having a hard time working in the code with the split and
> the different macros to convert back and forth. It's visually untidy. I
> also think that if NetBSD's is to be a reference implementation or teaching
> aid then well known stuff like this should be straightforward.
>
> Thoughts?
>
> Cheers,
> Andrew
>
> struct vnode {
> /*
> * more stable stuff goes at the front.
> */
> struct uvm_object v_uobj;
> ...
> enum vtype v_type;
> ...
>
> /*
> * counters usecounts etc. these guys get hammered on.
> * think of v_usecount on "rootvnode" for example.
> * (v_usecount will come out of struct uvm_object with uvm
> * rwlock changes.)
> */
> int v_usecount
> __cacheline_aligned(COHERENCY_UNIT);
> int v_holdcount;
> int v_numoutput;
> int v_writecount;
> int v_someothercounter;
> ...
All contained in struct vnode, there are no counters in struct vnode_impl.
> /*
> * then locks at the back in their own line so they don't
> * impinge on any of the above either; different kind of activity
> * again.
> */
> krwlock_t v_lock
> __cacheline_aligned(COHERENCY_UNIT);
> krwlock_t v_nc_lock; /* namecache locks */
> krwlock_t v_nc_listlock;
All (v_lock at least) contained in struct vnode_impl, all locks should
go here as they are opaque to any consumer.
> };
--
J. Hannken-Illjes - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig
Attachment:
signature.asc
Description: Message signed with OpenPGP