lkml.org 
[lkml]   [2003]   [May]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] PAG support, try #2
    On Wed, May 14, 2003 at 11:43:31AM +0100, David Howells wrote:
    > +extern long sys_setpag(pag_t);
    > +extern long sys_getpag(void);

    These have to be a marked asmlinkage. What's the reason you have
    the syscall in this header anyway?

    > +static kmem_cache_t *vfs_token_cache;
    > +static kmem_cache_t *vfs_pag_cache;

    So do you have an estimate for the number of users here yet?
    Adding two more slab caches that are unused for 99% of the users
    might not be the best choice if there's no strong need.

    > +inline pag_t vfs_leave_pag(void)

    Inline but not static seems strange..

    > +/*
    > + * join an existing PAG (+ve), run without PAG (0), or create and join new PAG (-1)
    > + * - PAG IDs must be +ve, >0 and unique
    > + * - returns ID of PAG joined or 0 if now running without a PAG
    > + */
    > +long sys_setpag(pag_t pag)
    > +{
    > + if (pag > 0) return vfs_join_pag(pag);
    > + else if (pag == 0) return vfs_leave_pag();
    > + else if (pag == -1) return vfs_new_pag();
    > + else return -EINVAL;
    > +}

    We already discussed the coding style issue, but anyway, why aren't
    these three separate syscalls?

    > +void vfs_pag_put(struct vfs_pag *vfspag)
    > +{
    > + struct vfs_token *vtoken;
    > +
    > + if (vfspag && atomic_dec_and_lock(&vfspag->usage, &vfs_pag_lock)) {
    > + rb_erase(&vfspag->node, &vfs_pag_tree);
    > + spin_unlock(&vfs_pag_lock);
    > +
    > + while (!list_empty(&vfspag->tokens)) {

    What protects vfspag->tokens?

    > +/*
    > + * search for a token covering a particular filesystem key in the specified pag list
    > + */

    Please linwrap after 80 chars.

    > +
    > + if (p->vfspag)
    > + vfs_pag_get(p->vfspag);
    > +

    Shouldn't vfs_pag_get hanlde a NULL argument instead?

    > diff -uNr linux-2.5.69/kernel/Makefile linux-2.5.69-pag/kernel/Makefile
    > --- linux-2.5.69/kernel/Makefile 2003-05-06 15:04:56.000000000 +0100
    > +++ linux-2.5.69-pag/kernel/Makefile 2003-05-13 10:45:27.000000000 +0100
    > @@ -3,7 +3,7 @@
    > #
    >
    > obj-y = sched.o fork.o exec_domain.o panic.o printk.o profile.o \
    > - exit.o itimer.o time.o softirq.o resource.o \
    > + cred.o exit.o itimer.o time.o softirq.o resource.o \

    What happened to the suggestion to make the pag code optional? It's
    really easy to stub it out properly and most people don't need it.

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 13:35    [W:0.025 / U:0.096 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site