Date: Sun, 19 Dec 1999 05:50:16 +0800 From: Peter Wemm <peter@netplex.com.au> To: Terry Lambert <tlambert@primenet.com> Cc: robert+freebsd@cyrus.watson.org, freebsd-fs@FreeBSD.ORG Subject: Re: Request for objections: extended attribute and ACL interfaces Message-ID: <19991218215016.32B321CA0@overcee.netplex.com.au> In-Reply-To: Message from Terry Lambert <tlambert@primenet.com> of "Sat, 18 Dec 1999 20:05:04 GMT." <199912182005.NAA01764@usr08.primenet.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Terry Lambert wrote:
> > Terry, why don't you freaking well *look* at the code every now and then?
> > This *is* implemented and works already, for over a year now.
I note that you carefully deleted your original statements, which I'll
re-quote for context:
> The Heidemann framework supports the ability to add VOP's to an
> existing system. FreeBSD doesn't currently support this, but it
> could be fixed to do so, simply by making *vfs_op_descs[] a
> pointer reference, and reallocating it as needed in order to
> grow the descriptor list using loaded descriptors.
Fact: you said "FreeBSD doesn't support this", which is wrong. We do
add VOP's to an existing system.
Fact: your suggestion about making *vfs_op_descs[] a pointer reference
is already done, exactly as you suggested, and it was done over 12 months
ago.
My point that you should *read* the code rather than make blanket
statements still stands. Now that you appear to have read it (at last,
thank you!), your comments on the implementation are worth something.
> Peter, it should work without any handlers being present.
And what should it do? OK, suppose there is an expanded VOP_* inline
compiled into a binary to do the VCALL() etc. Now, there is no handler or
descriptor present. How is the stacking layer supposed to deal with that? It
doesn't have an offset to work with..
> At the
> FreeBSDCon, Kirk agreed that the vfs_default.c stuff damaged the
> abstraction between UFS and FFS layers, which are supposed to be
> seperated as block management policy and directory hierarchy policy
> layers. Ask Matt Dillon or Poul-Henning.
I'm not talking about UFS/FFS layers. I was responding to and talking
about one specific part, adding VOPs dynamically.
Have you looked at the vfs_default.c stuff BTW? It doesn't do anything
unless the target filesystem specifically activates it. If the target
filesystem supports some sort of passthrough (such as unionfs), then it
doesn't use vop_defaultop() as a catch-all. Unionfs (for example) provides
it's own and has absolutely no interference from vfs_default.c. The VOP
layer doesn't use vop_default.c unless the terminal filesystem
*specifically* requests it when it has no passthrough capabilities. You
will note that ffs/ffs_vnops.c (the upper layer) has it's own vop_default
handler (ufs_vnopoerate) with passes all unknown VOP's to the lower UFS layer.
If UFS can't handle them, then UFS hands it off to vop_defaultop(). FFS is
functioning as a passthrough layer.
ufs_vnops.c (bottom layer):
int
ufs_vnoperate(ap)
struct vop_generic_args /* {
struct vnodeop_desc *a_desc;
} */ *ap;
{
return (VOCALL(ufs_vnodeop_p, ap->a_desc->vdesc_offset, ap));
}
ffs_vnops.c (top layer):
vop_t **ffs_vnodeop_p;
static struct vnodeopv_entry_desc ffs_vnodeop_entries[] = {
{ &vop_default_desc, (vop_t *) ufs_vnoperate }, <<<<
{ &vop_fsync_desc, (vop_t *) ffs_fsync },
{ &vop_getpages_desc, (vop_t *) ffs_getpages },
{ &vop_putpages_desc, (vop_t *) ffs_putpages },
{ &vop_read_desc, (vop_t *) ffs_read },
{ &vop_balloc_desc, (vop_t *) ffs_balloc },
{ &vop_reallocblks_desc, (vop_t *) ffs_reallocblks },
{ &vop_write_desc, (vop_t *) ffs_write },
{ NULL, NULL }
};
ie: ffs specifically intercepts the functions it's interested and then passes
the rest through to UFS, *without interference from vfs_default.c*!
As UFS is a terminal layer, it (rightly) terminates any unhandled requests.
> Peter, there is no rendevous by name for new VOPs so that they can be
> passed from a loaded stacking layer that knows about a new VOP to
> an underlying stacking layer that knows about the new VOP, through
> one that doesn't. For example, an ACLFS layer mounted over top of
> a translucent FS mounted over top of a read only FS and backing media
> so that I can put ACLs on the contents of my CDROM that I'm making
> available to user on my system. If the backing media already knows
> about VOP_ACL, you are screwed: it's not the same VOP_ACL, unless
> you imply a dependency between kernel modules that you should not be
> implying.
The descriptors are global scope.. You can't have two different VOP's by
the same name.
vnode_if.h:
extern struct vnodeop_desc vop_islocked_desc;
static __inline int VOP_ISLOCKED(vp, p)
struct vnode *vp;
struct proc *p;
{
struct vop_islocked_args a;
int rc;
a.a_desc = VDESC(vop_islocked);
a.a_vp = vp;
a.a_p = p;
rc = VCALL(vp, VOFFSET(vop_islocked), &a);
return (rc);
}
(vnode_if.c contains the declarations of the _desc structs.)
VDESC() expands so that it's the equivalent of: a.a_desc = &vop_islocked_desc;
This cannot work with two global vop_acl_desc's.
We can add and remove descriptors on the fly. An intermediate layer operating
in passthrough mode (eg: FFS, unionfs) does not need *any* knowledge of the
VOPs being passed through it.
You state:
> there is no rendevous by name for new VOPs so that they can be
> passed from a loaded stacking layer that knows about a new VOP to
> an underlying stacking layer that knows about the new VOP, through
> one that doesn't.
This is not entirely correct. There is no "string" as such, but the
rendesvous is a global variable. (eg: vop_acl_desc). It *can* pass
through an intermediate layer without interference from vfs_default.c.
> Peter, the decriptor list isn't sorted so that a dereference
> can be done by index. Not only is the overhead for this higher, the
> addition of a new VOP is an O(3) random traversal instead of O(1)
> linear traversal for existing instanced VFSs.
So? A once-off cost at boot/load/unload time doesn't mean much as long as
there isn't a runtime penalty. I don't believe there is a runtime penalty
as it uses the exact same VCALL/VOCALL implementation as before. The cost
of indexing a 30-odd entry array at boot is nothing compared to things like
waiting for IDE disks to stabilize. If you think you can make it faster,
be my guest. If you send a single purpose patch that makes it faster without
mixing other changes in, you will have my full attention.
> This change is incomplete; it does not fully solve the problem
> noted in my previous email.
I am not denying that more work can be done, but object to you saying that
"it isn't supported". The more code I read and check the more I think the
problems you talked about aren't real either - aside from not supporting
two different VOP_ACL()'s in the same kernel at the same time (which I
think is of questionable value - that opens real cans of worms that we
could do without.. like *which* VOP_ACL() is which?).
> Don't assume the code is right and I am wrong, just because you
> are emotionally invested in the code.
I am "emotionally invested" in people spreading misinformation and blanket
statements that are clearly false. Your claim that FreeBSD does not currently
support dynamically adding VOP's is wrong.
Cheers,
-Peter
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-fs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?19991218215016.32B321CA0>