d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Change the generated VOP_ macro implementations to improve type checking

Browse Source 
and KASSERT coverage.

After this check there is only one "nasty" cast in this code but there
is a KASSERT to protect against the wrong argument structure behind
that cast.

Un-inlining the meat of VOP_FOO() saves 35kB of text segment on a typical
kernel with no change in performance.

We also now run the checking and tracing on VOP's which have been layered
by nullfs, umapfs, deadfs or unionfs.

    Add new (non-inline) VOP_FOO_AP() functions which take a "struct
    foo_args" argument and does everything the VOP_FOO() macros
    used to do with checks and debugging code.

    Add KASSERT to VOP_FOO_AP() check for argument type being
    correct.

    Slim down VOP_FOO() inline functions to just stuff arguments
    into the struct foo_args and call VOP_FOO_AP().

    Put function pointer to VOP_FOO_AP() into vop_foo_desc structure
    and make VCALL() use it instead of the current offsetoff() hack.

    Retire vcall() which implemented the offsetoff()

    Make deadfs and unionfs use VOP_FOO_AP() calls instead of
    VCALL(), we know which specific call we want already.

    Remove unneeded arguments to VCALL() in nullfs and umapfs bypass
    functions.

    Remove unused vdesc_offset and VOFFSET().

    Generally improve style/readability of the generated code.
This commit is contained in:
phk 2005-01-13 07:53:01 +00:00
parent 93aaafbe16
commit 4d9781b3db
7 changed files with 103 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
sys/fs/deadfs/dead_vnops.c
View File

@ -176,7 +176,7 @@ dead_ioctl(ap)
if (!chkvnlock(ap->a_vp))
return (ENOTTY);
/* XXX: Doesn't this just recurse back here ? */
return (VCALL(ap->a_vp, VOFFSET(vop_ioctl), ap));
return (VOP_IOCTL_AP(ap));
}
@ -203,7 +203,7 @@ dead_lock(ap)
}
if (!chkvnlock(vp))
return (0);
return (VCALL(vp, VOFFSET(vop_lock), ap));
return (VOP_LOCK_AP(ap));
}
/*

2
sys/fs/nullfs/null_vnops.c
View File

@ -296,7 +296,7 @@ null_bypass(ap)
* with the modified argument structure.
*/
if (vps_p[0] && *vps_p[0])
error = VCALL(*(vps_p[0]), descp->vdesc_offset, ap);
error = VCALL(ap);
else {
printf("null_bypass: no map for %s\n", descp->vdesc_name);
error = EINVAL;

2
sys/fs/umapfs/umap_vnops.c
View File

@ -197,7 +197,7 @@ umap_bypass(ap)
* Call the operation on the lower layer
* with the modified argument structure.
*/
error = VCALL(*(vps_p[0]), descp->vdesc_offset, ap);
error = VCALL(ap);
/*
* Maintain the illusion of call-by-value

42
sys/fs/unionfs/union_vnops.c
View File

@ -829,7 +829,7 @@ union_close(ap)
vp = un->un_lowervp;
}
ap->a_vp = vp;
return (VCALL(vp, VOFFSET(vop_close), ap));
return (VOP_CLOSE_AP(ap));
}
/*
@ -872,7 +872,7 @@ union_access(ap)
if ((vp = union_lock_upper(un, td)) != NULLVP) {
ap->a_vp = vp;
error = VCALL(vp, VOFFSET(vop_access), ap);
error = VOP_ACCESS_AP(ap);
union_unlock_upper(vp, td);
return(error);
}
@ -888,7 +888,7 @@ union_access(ap)
if ((un->un_vnode->v_mount->mnt_flag & MNT_RDONLY) == 0)
ap->a_mode &= ~VWRITE;
error = VCALL(vp, VOFFSET(vop_access), ap);
error = VOP_ACCESS_AP(ap);
if (error == 0) {
struct union_mount *um;
@ -896,7 +896,7 @@ union_access(ap)
if (um->um_op == UNMNT_BELOW) {
ap->a_cred = um->um_cred;
error = VCALL(vp, VOFFSET(vop_access), ap);
error = VOP_ACCESS_AP(ap);
}
}
VOP_UNLOCK(vp, 0, td);
@ -1119,7 +1119,7 @@ union_lease(ap)
struct vnode *ovp = OTHERVP(ap->a_vp);
ap->a_vp = ovp;
return (VCALL(ovp, VOFFSET(vop_lease), ap));
return (VOP_LEASE_AP(ap));
}
static int
@ -1136,7 +1136,7 @@ union_ioctl(ap)
struct vnode *ovp = OTHERVP(ap->a_vp);
ap->a_vp = ovp;
return (VCALL(ovp, VOFFSET(vop_ioctl), ap));
return (VOP_IOCTL_AP(ap));
}
static int
@ -1151,7 +1151,7 @@ union_poll(ap)
struct vnode *ovp = OTHERVP(ap->a_vp);
ap->a_vp = ovp;
return (VCALL(ovp, VOFFSET(vop_poll), ap));
return (VOP_POLL_AP(ap));
}
static int
@ -1594,7 +1594,7 @@ union_readdir(ap)
if ((uvp = union_lock_upper(un, td)) != NULLVP) {
ap->a_vp = uvp;
error = VCALL(uvp, VOFFSET(vop_readdir), ap);
error = VOP_READDIR_AP(ap);
union_unlock_upper(uvp, td);
}
return(error);
@ -1618,7 +1618,7 @@ union_readlink(ap)
KASSERT(vp != NULL, ("union_readlink: backing vnode missing!"));
ap->a_vp = vp;
error = VCALL(vp, VOFFSET(vop_readlink), ap);
error = VOP_READLINK_AP(ap);
union_unlock_other(vp, td);
return (error);
@ -1785,7 +1785,7 @@ union_pathconf(ap)
KASSERT(vp != NULL, ("union_pathconf: backing vnode missing!"));
ap->a_vp = vp;
error = VCALL(vp, VOFFSET(vop_pathconf), ap);
error = VOP_PATHCONF_AP(ap);
union_unlock_other(vp, td);
return (error);
@ -1804,7 +1804,7 @@ union_advlock(ap)
register struct vnode *ovp = OTHERVP(ap->a_vp);
ap->a_vp = ovp;
return (VCALL(ovp, VOFFSET(vop_advlock), ap));
return (VOP_ADVLOCK_AP(ap));
}
@ -1851,7 +1851,7 @@ union_getacl(ap)
vp = union_lock_other(un, ap->a_td);
ap->a_vp = vp;
error = VCALL(vp, VOFFSET(vop_getacl), ap);
error = VOP_GETACL_AP(ap);
union_unlock_other(vp, ap->a_td);
return (error);
@ -1873,7 +1873,7 @@ union_setacl(ap)
vp = union_lock_other(un, ap->a_td);
ap->a_vp = vp;
error = VCALL(vp, VOFFSET(vop_setacl), ap);
error = VOP_SETACL_AP(ap);
union_unlock_other(vp, ap->a_td);
return (error);
@ -1892,7 +1892,7 @@ union_aclcheck(ap)
struct vnode *ovp = OTHERVP(ap->a_vp);
ap->a_vp = ovp;
return (VCALL(ovp, VOFFSET(vop_aclcheck), ap));
return (VOP_ACLCHECK_AP(ap));
}
static int
@ -1910,7 +1910,7 @@ union_closeextattr(ap)
vp = union_lock_other(un, ap->a_td);
ap->a_vp = vp;
error = VCALL(vp, VOFFSET(vop_closeextattr), ap);
error = VOP_CLOSEEXTATTR_AP(ap);
union_unlock_other(vp, ap->a_td);
return (error);
@ -1934,7 +1934,7 @@ union_getextattr(ap)
vp = union_lock_other(un, ap->a_td);
ap->a_vp = vp;
error = VCALL(vp, VOFFSET(vop_getextattr), ap);
error = VOP_GETEXTATTR_AP(ap);
union_unlock_other(vp, ap->a_td);
return (error);
@ -1957,7 +1957,7 @@ union_listextattr(ap)
vp = union_lock_other(un, ap->a_td);
ap->a_vp = vp;
error = VCALL(vp, VOFFSET(vop_listextattr), ap);
error = VOP_LISTEXTATTR_AP(ap);
union_unlock_other(vp, ap->a_td);
return (error);
@ -1977,7 +1977,7 @@ union_openextattr(ap)
vp = union_lock_other(un, ap->a_td);
ap->a_vp = vp;
error = VCALL(vp, VOFFSET(vop_openextattr), ap);
error = VOP_OPENEXTATTR_AP(ap);
union_unlock_other(vp, ap->a_td);
return (error);
@ -1999,7 +1999,7 @@ union_deleteextattr(ap)
vp = union_lock_other(un, ap->a_td);
ap->a_vp = vp;
error = VCALL(vp, VOFFSET(vop_deleteextattr), ap);
error = VOP_DELETEEXTATTR_AP(ap);
union_unlock_other(vp, ap->a_td);
return (error);
@ -2022,7 +2022,7 @@ union_setextattr(ap)
vp = union_lock_other(un, ap->a_td);
ap->a_vp = vp;
error = VCALL(vp, VOFFSET(vop_setextattr), ap);
error = VOP_SETEXTATTR_AP(ap);
union_unlock_other(vp, ap->a_td);
return (error);
@ -2043,7 +2043,7 @@ union_setlabel(ap)
vp = union_lock_other(un, ap->a_td);
ap->a_vp = vp;
error = VCALL(vp, VOFFSET(vop_setlabel), ap);
error = VOP_SETLABEL_AP(ap);
union_unlock_other(vp, ap->a_td);
return (error);

26
sys/kern/vfs_init.c
View File

@ -89,32 +89,6 @@ struct vattr va_null;
* Routines having to do with the management of the vnode table.
*/
/*
* XXX: hack alert
*/
int
vcall(struct vnode *vp, u_int off, void *ap)
{
struct vop_vector *vop = vp->v_op;
vop_bypass_t **bpt;
int rc;
for(;;) {
bpt = (void *)((u_char *)vop + off);
if (vop != NULL && *bpt == NULL && vop->vop_bypass == NULL) {
vop = vop->vop_default;
continue;
}
break;
}
KASSERT(vop != NULL, ("No VCALL(%p...)", vp));
if (*bpt != NULL)
rc = (*bpt)(ap);
else
rc = vop->vop_bypass(ap);
return (rc);
}
struct vfsconf *
vfs_byname(const char *name)
{

25
sys/sys/vnode.h
View File

@ -408,6 +408,17 @@ extern void (*lease_updatetime)(int deltat);
#define VDESC_NOMAP_VPP 0x0100
#define VDESC_VPP_WILLRELE 0x0200
/*
* A generic structure.
* This can be used by bypass routines to identify generic arguments.
*/
struct vop_generic_args {
struct vnodeop_desc *a_desc;
/* other random data follows, presumably */
};
typedef int vop_bypass_t(struct vop_generic_args *);
/*
* VDESC_NO_OFFSET is used to identify the end of the offset list
* and in places where no such field exists.
@ -418,9 +429,9 @@ extern void (*lease_updatetime)(int deltat);
* This structure describes the vnode operation taking place.
*/
struct vnodeop_desc {
int vdesc_offset; /* offset in vector,first for speed */
char *vdesc_name; /* a readable name for debugging */
int vdesc_flags; /* VDESC_* flags */
vop_bypass_t *vdesc_call; /* Function to call */
/*
* These ops are used by bypass routines to map and locate arguments.
@ -451,14 +462,6 @@ extern struct vnodeop_desc *vnodeop_descs[];
#define VOPARG_OFFSETTO(s_type, s_offset, struct_p) \
((s_type)(((char*)(struct_p)) + (s_offset)))
/*
* A generic structure.
* This can be used by bypass routines to identify generic arguments.
*/
struct vop_generic_args {
struct vnodeop_desc *a_desc;
/* other random data follows, presumably */
};
#ifdef DEBUG_VFS_LOCKS
/*
@ -521,9 +524,8 @@ void vop_unlock_pre(void *a);
/*
* This call works for vnodes in the kernel.
*/
#define VCALL(a, b, c) vcall((a), (b), (c))
#define VCALL(c) ((c)->a_desc->vdesc_call(c))
#define VDESC(OP) (& __CONCAT(OP,_desc))
#define VOFFSET(OP) (VDESC(OP)->vdesc_offset)
/*
* VMIO support inline
@ -674,7 +676,6 @@ int vop_panic(struct vop_generic_args *ap);
int vop_stdcreatevobject(struct vop_createvobject_args *ap);
int vop_stddestroyvobject(struct vop_destroyvobject_args *ap);
int vop_stdgetvobject(struct vop_getvobject_args *ap);
int vcall(struct vnode *vp, u_int off, void *ap);
void vfree(struct vnode *);
void vput(struct vnode *vp);

113
sys/tools/vnode_if.awk
View File

@ -62,25 +62,27 @@ function printh(s) {print s > hfile;}
function printp(s) {print s > pfile;}
function printq(s) {print s > qfile;}
function add_debug_code(name, arg, pos)
function add_debug_code(name, arg, pos, ind)
{
if (arg == "vpp")
arg = "*vpp";
star = "*";
else
star = "";
if (lockdata[name, arg, pos] && (lockdata[name, arg, pos] != "-")) {
if (arg ~ /^\*/) {
printh("\tif ("substr(arg, 2)" != NULL) {");
printc(ind"if ("substr(arg, 2)" != NULL) {");
}
printh("\tASSERT_VI_UNLOCKED("arg", \""uname"\");");
printc(ind"ASSERT_VI_UNLOCKED("star"a->a_"arg", \""uname"\");");
# Add assertions for locking
if (lockdata[name, arg, pos] == "L")
printh("\tASSERT_VOP_LOCKED("arg", \""uname"\");");
printc(ind"ASSERT_VOP_LOCKED(" star "a->a_"arg", \""uname"\");");
else if (lockdata[name, arg, pos] == "U")
printh("\tASSERT_VOP_UNLOCKED("arg", \""uname"\");");
printc(ind"ASSERT_VOP_UNLOCKED(" star "a->a_"arg", \""uname"\");");
else if (0) {
# XXX More checks!
}
if (arg ~ /^\*/) {
printh("\t}");
printc("ind}");
}
}
}
@ -88,18 +90,18 @@ function add_debug_code(name, arg, pos)
function add_debug_pre(name)
{
if (lockdata[name, "pre"]) {
printh("#ifdef DEBUG_VFS_LOCKS");
printh("\t"lockdata[name, "pre"]"(&a);");
printh("#endif");
printc("#ifdef DEBUG_VFS_LOCKS");
printc("\t"lockdata[name, "pre"]"(a);");
printc("#endif");
}
}
function add_debug_post(name)
{
if (lockdata[name, "post"]) {
printh("#ifdef DEBUG_VFS_LOCKS");
printh("\t"lockdata[name, "post"]"(&a, rc);");
printh("#endif");
printc("#ifdef DEBUG_VFS_LOCKS");
printc("\t"lockdata[name, "post"]"(a, rc);");
printc("#endif");
}
}
@ -159,8 +161,6 @@ if (pfile) {
if (qfile) {
printq(common_head)
printq("struct vop_generic_args;")
printq("typedef int vop_bypass_t(struct vop_generic_args *);\n")
}
if (hfile) {
@ -176,9 +176,9 @@ if (cfile) {
"#include <sys/vnode.h>\n" \
"\n" \
"struct vnodeop_desc vop_default_desc = {\n" \
" 1,\t\t\t/* special case, vop_default => 1 */\n" \
" \"default\",\n" \
" 0,\n" \
" (void *)(uintptr_t)vop_panic,\n" \
" NULL,\n" \
" VDESC_NO_OFFSET,\n" \
" VDESC_NO_OFFSET,\n" \
@ -196,8 +196,6 @@ while ((getline < srcfile) > 0) {
$2 !~ /^[a-z]+$/ || $3 !~ /^[a-z]+$/ || \
$4 !~ /^.$/ || $5 !~ /^.$/ || $6 !~ /^.$/)
continue;
if ($3 == "vpp")
$3 = "*vpp";
lockdata["vop_" $2, $3, "Entry"] = $4;
lockdata["vop_" $2, $3, "OK"] = $5;
lockdata["vop_" $2, $3, "Error"] = $6;
@ -280,9 +278,10 @@ while ((getline < srcfile) > 0) {
ctrargs = 6;
else
ctrargs = numargs;
ctrstr = "\tCTR" ctrargs "(KTR_VOP, " ctrstr ")\"";
for (i = 0; i < ctrargs; ++i)
ctrstr = ctrstr ", " args[i];
ctrstr = "\tCTR" ctrargs "(KTR_VOP,\n\t " ctrstr ")\",\n\t ";
ctrstr = ctrstr "a->a_" args[0];
for (i = 1; i < ctrargs; ++i)
ctrstr = ctrstr ", a->a_" args[i];
ctrstr = ctrstr ");";
if (pfile) {
@ -299,44 +298,30 @@ while ((getline < srcfile) > 0) {
for (i = 0; i < numargs; ++i)
printh("\t" t_spc(types[i]) "a_" args[i] ";");
printh("};");
printh("");
# Print out extern declaration.
printh("extern struct vnodeop_desc " name "_desc;");
printh("");
# Print out function.
# Print out function prototypes.
printh("int " uname "_AP(struct " name "_args *);");
printh("");
printh("static __inline int " uname "(");
for (i = 0; i < numargs; ++i) {
printh("\t" t_spc(types[i]) args[i] \
(i < numargs - 1 ? "," : ")"));
}
printh("{\n\tstruct " name "_args a;");
printh("\tint rc;");
printh("{");
printh("\tstruct " name "_args a;");
printh("");
printh("\ta.a_gen.a_desc = VDESC(" name ");");
for (i = 0; i < numargs; ++i)
printh("\ta.a_" args[i] " = " args[i] ";");
for (i = 0; i < numargs; ++i)
add_debug_code(name, args[i], "Entry");
add_debug_pre(name);
printh("\t{")
printh("\t\tstruct vop_vector *vop = "args[0]"->v_op;")
printh("\t\twhile(vop != NULL && vop->"name" == NULL && vop->vop_bypass == NULL)")
printh("\t\t\tvop = vop->vop_default;")
printh("\t\tKASSERT(vop != NULL, (\"No "name"(%p...)\", "args[0]"));")
printh("\t\tif (vop->"name" != NULL)")
printh("\t\t\trc = vop->"name"(&a);")
printh("\t\telse")
printh("\t\t\trc = vop->vop_bypass(&a.a_gen);")
printh("\t}")
printh(ctrstr);
printh("if (rc == 0) {");
for (i = 0; i < numargs; ++i)
add_debug_code(name, args[i], "OK");
printh("} else {");
for (i = 0; i < numargs; ++i)
add_debug_code(name, args[i], "Error");
printh("\treturn (" uname "_AP(&a));");
printh("}");
add_debug_post(name);
printh("\treturn (rc);\n}");
printh("");
}
if (cfile) {
@ -362,10 +347,40 @@ while ((getline < srcfile) > 0) {
printc("\tVDESC_NO_OFFSET");
printc("};");
# Print out function.
printc("\nint\n" uname "_AP(struct " name "_args *a)");
printc("{");
printc("\tint rc;");
printc("\tstruct vnode *vp = a->a_" args[0]";");
printc("\tstruct vop_vector *vop = vp->v_op;");
printc("");
printc("\tKASSERT(a->a_gen.a_desc == VDESC(" name "),");
printc("\t (\"Wrong a_desc in " name "(%p, %p)\", vp, a));");
printc("\twhile(vop != NULL && \\");
printc("\t vop->"name" == NULL && vop->vop_bypass == NULL)")
printc("\t\tvop = vop->vop_default;")
printc("\tKASSERT(vop != NULL, (\"No "name"(%p, %p)\", vp, a));")
for (i = 0; i < numargs; ++i)
add_debug_code(name, args[i], "Entry", "\t");
add_debug_pre(name);
printc("\tif (vop->"name" != NULL)")
printc("\t\trc = vop->"name"(a);")
printc("\telse")
printc("\t\trc = vop->vop_bypass(&a->a_gen);")
printc(ctrstr);
printc("\tif (rc == 0) {");
for (i = 0; i < numargs; ++i)
add_debug_code(name, args[i], "OK", "\t\t");
printc("\t} else {");
for (i = 0; i < numargs; ++i)
add_debug_code(name, args[i], "Error", "\t\t");
printc("\t}");
add_debug_post(name);
printc("\treturn (rc);");
printc("}\n");
# Print out the vnodeop_desc structure.
printc("struct vnodeop_desc " name "_desc = {");
# offset
printc("\toffsetof(struct vop_vector, "name"),");
# printable name
printc("\t\"" name "\",");
# flags
@ -381,6 +396,8 @@ while ((getline < srcfile) > 0) {
releflags = "0";
printc("\t" releflags vppwillrele ",");
# function to call
printc("\t(void*)(uintptr_t)" uname "_AP,");
# vp offsets
printc("\t" name "_vp_offsets,");
# vpp (if any)