lkml.org 
[lkml]   [1997]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: 2.1.57 /proc problem
    Richard Guenther wrote:
    > This is a bug in procfs not deleting the dentry for the file.
    > It's been there for a long time now... (You can easy cause this
    > with binfmt_misc, too)
    > I suppose, this will not be fixed until the VFS has finally moved
    > to dentries and procfs has got a 'little' overhaul.

    Hi Richard,
    I sent Heinz a patch to try out, and if you have a similar problem
    perhaps you could test it as well? I've appended the text of my prior
    message.

    > BTW I reported this bug twice some time ago...

    I remember your posting on the binfmt_misc problem, but didn't make the
    connection with the dentry caching at the time.

    Regards,
    Bill

    [appended message]
    Heinz Mauelshagen wrote:
    In 2.1.57 it seemed to work using proc_register and proc_unregister.
    >
    > But strange results occured after proc_unregiste (module unload):
    >
    > "ls -l /proc/e*" doesn't show /proc/egg.
    > "ls -l /proc/egg" shows it!
    > "cat /proc/egg" segfaults :-(

    Does your /proc code install a custom inode_operations in the inodes?
    I'm pretty sure I know what's going wrong: the dcache is keeping your
    inodes around after you've unregistered the module, and the subsequent
    call through the i_op vector goes to never-never land.

    As luck would have it, I've been working on a patch for /proc in the
    past couple of days that should fix this problem (and others). Could
    you try the attached patch and see if it helps?

    Regards,
    Bill--- linux-2.1.57/kernel/fork.c.old Fri Sep 26 08:10:52 1997
    +++ linux-2.1.57/kernel/fork.c Sun Sep 28 13:27:14 1997
    @@ -208,7 +208,6 @@
    struct vm_area_struct * mpnt, *tmp, **pprev;
    int retval;

    - mm->mmap = mm->mmap_cache = NULL;
    flush_cache_mm(current->mm);
    pprev = &mm->mmap;
    for (mpnt = current->mm->mmap ; mpnt ; mpnt = mpnt->vm_next) {
    @@ -254,8 +253,7 @@
    if (retval)
    goto fail_nomem;
    }
    - flush_tlb_mm(current->mm);
    - return 0;
    + retval = 0;

    fail_nomem:
    flush_tlb_mm(current->mm);
    @@ -276,7 +274,11 @@
    mm->count = 1;
    mm->def_flags = 0;
    mm->mmap_sem = MUTEX;
    - mm->pgd = NULL;
    + /*
    + * Install the kernel page directory so
    + * pgd_offset() is always valid.
    + */
    + mm->pgd = swapper_pg_dir;
    mm->mmap = mm->mmap_cache = NULL;

    /* It has not run yet, so cannot be present in anyone's
    @@ -324,10 +326,12 @@
    goto free_mm;
    retval = dup_mmap(mm);
    if (retval)
    - goto free_mm;
    + goto free_pt;
    return 0;

    free_mm:
    + mm->pgd = NULL;
    +free_pt:
    tsk->mm = NULL;
    mmput(mm);
    fail_nomem:
    --- linux-2.1.57/fs/proc/array.c.old Fri Sep 26 08:10:51 1997
    +++ linux-2.1.57/fs/proc/array.c Sun Sep 28 12:01:33 1997
    @@ -348,6 +348,14 @@

    if (!p || !p->mm || ptr >= TASK_SIZE)
    return 0;
    + /*
    + * Note: a new task may (briefly) have mm->pgd == NULL
    + */
    + if (!p->mm->pgd) {
    + printk("get_phys_addr: pid %d has NULL pgd!\n", p->pid);
    + return 0;
    + }
    +
    page_dir = pgd_offset(p->mm,ptr);
    if (pgd_none(*page_dir))
    return 0;
    @@ -861,6 +869,7 @@
    struct vm_area_struct * vma = tsk->mm->mmap;

    while (vma) {
    + /* Note: if the task has mmaps, mm->pgd is valid */
    pgd_t *pgd = pgd_offset(tsk->mm, vma->vm_start);
    int pages = 0, shared = 0, dirty = 0, total = 0;

    @@ -917,24 +926,31 @@
    #define MAPS_LINE_MAX MAPS_LINE_MAX8


    -static long read_maps (int pid, struct file * file,
    - char * buf, unsigned long count)
    +static long read_maps (int pid, struct file * file, char * buf,
    + unsigned long count)
    {
    - struct task_struct *p = find_task_by_pid(pid);
    - char * destptr;
    + struct task_struct *p;
    + struct vm_area_struct * map, * next;
    + char * destptr = buf, * buffer;
    loff_t lineno;
    - int column;
    - struct vm_area_struct * map;
    - int i;
    - char * buffer;
    + int column, i;
    + long retval;
    +
    + /*
    + * We might sleep getting the page, so get it first.
    + */
    + retval = -ENOMEM;
    + buffer = (char*)__get_free_page(GFP_KERNEL);
    + if (!buffer)
    + goto out;

    + retval = -EINVAL;
    + p = find_task_by_pid(pid);
    if (!p)
    - return -EINVAL;
    + goto freepage_out;

    if (!p->mm || p->mm == &init_mm || count == 0)
    - return 0;
    -
    - buffer = (char*)__get_free_page(GFP_KERNEL);
    + goto getlen_out;

    /* decode f_pos */
    lineno = file->f_pos >> MAPS_LINE_SHIFT;
    @@ -944,9 +960,7 @@
    for (map = p->mm->mmap, i = 0; map && (i < lineno); map = map->vm_next, i++)
    continue;

    - destptr = buf;
    -
    - for ( ; map ; ) {
    + for ( ; map ; map = next ) {
    /* produce the next line */
    char *line;
    char str[5], *cp = str;
    @@ -957,6 +971,10 @@
    MAPS_LINE_MAX4 : MAPS_LINE_MAX8;
    int len;

    + /*
    + * Get the next vma now (but it won't be used if we sleep).
    + */
    + next = map->vm_next;
    flags = map->vm_flags;

    *cp++ = flags & VM_READ ? 'r' : '-';
    @@ -993,20 +1011,19 @@
    if (column >= len) {
    column = 0; /* continue with next line at column 0 */
    lineno++;
    - map = map->vm_next;
    - continue;
    + continue; /* we haven't slept */
    }

    i = len-column;
    if (i > count)
    i = count;
    - copy_to_user(destptr, line+column, i);
    - destptr += i; count -= i;
    - column += i;
    + copy_to_user(destptr, line+column, i); /* may have slept */
    + destptr += i;
    + count -= i;
    + column += i;
    if (column >= len) {
    column = 0; /* next time: next line at column 0 */
    lineno++;
    - map = map->vm_next;
    }

    /* done? */
    @@ -1015,6 +1032,7 @@

    /* By writing to user space, we might have slept.
    * Stop the loop, to avoid a race condition.
    + * N.B. cloned task might have had mmaps changed ...
    */
    if (p != current)
    break;
    @@ -1023,8 +1041,13 @@
    /* encode f_pos */
    file->f_pos = (lineno << MAPS_LINE_SHIFT) + column;

    +getlen_out:
    + retval = destptr - buf;
    +
    +freepage_out:
    free_page((unsigned long)buffer);
    - return destptr-buf;
    +out:
    + return retval;
    }

    #ifdef CONFIG_MODULES
    --- linux-2.1.57/fs/proc/root.c.old Wed Sep 10 09:21:27 1997
    +++ linux-2.1.57/fs/proc/root.c Fri Sep 26 10:39:01 1997
    @@ -173,7 +173,8 @@

    int proc_openprom_regdev(struct openpromfs_dev *d)
    {
    - if (proc_openpromdev_ino == PROC_OPENPROMD_FIRST + PROC_NOPENPROMD) return -1;
    + if (proc_openpromdev_ino == PROC_OPENPROMD_FIRST + PROC_NOPENPROMD)
    + return -1;
    d->next = proc_openprom_devices;
    d->inode = proc_openpromdev_ino++;
    proc_openprom_devices = d;
    @@ -218,6 +219,7 @@
    (inode, filp, dirent, filldir);
    return -EINVAL;
    }
    +#define OPENPROM_DEFREADDIR proc_openprom_defreaddir

    static int
    proc_openprom_deflookup(struct inode * dir, struct dentry *dentry)
    @@ -229,17 +231,17 @@
    (dir, dentry);
    return -ENOENT;
    }
    +#define OPENPROM_DEFLOOKUP proc_openprom_deflookup
    +#else
    +#define OPENPROM_DEFREADDIR NULL
    +#define OPENPROM_DEFLOOKUP NULL
    #endif

    static struct file_operations proc_openprom_operations = {
    NULL, /* lseek - default */
    NULL, /* read - bad */
    NULL, /* write - bad */
    -#if defined(CONFIG_SUN_OPENPROMFS_MODULE) && defined(CONFIG_KERNELD)
    - proc_openprom_defreaddir,/* readdir */
    -#else
    - NULL, /* readdir */
    -#endif
    + OPENPROM_DEFREADDIR, /* readdir */
    NULL, /* poll - default */
    NULL, /* ioctl - default */
    NULL, /* mmap */
    @@ -251,11 +253,7 @@
    struct inode_operations proc_openprom_inode_operations = {
    &proc_openprom_operations,/* default net directory file-ops */
    NULL, /* create */
    -#if defined(CONFIG_SUN_OPENPROMFS_MODULE) && defined(CONFIG_KERNELD)
    - proc_openprom_deflookup,/* lookup */
    -#else
    - NULL, /* lookup */
    -#endif
    + OPENPROM_DEFLOOKUP, /* lookup */
    NULL, /* link */
    NULL, /* unlink */
    NULL, /* symlink */
    @@ -639,6 +637,26 @@
    }

    /*
    + * As some entries in /proc are volatile, we want to
    + * get rid of unused dentries. This could be made
    + * smarter: we could keep a "volatile" flag in the
    + * inode to indicate which ones to keep.
    + */
    +static void
    +proc_delete_dentry(struct dentry * dentry)
    +{
    + d_drop(dentry);
    +}
    +
    +static struct dentry_operations proc_dentry_operations =
    +{
    + NULL, /* revalidate */
    + NULL, /* d_hash */
    + NULL, /* d_compare */
    + proc_delete_dentry /* d_delete(struct dentry *) */
    +};
    +
    +/*
    * Don't create negative dentries here, return -ENOENT by hand
    * instead.
    */
    @@ -646,12 +664,15 @@
    {
    struct inode *inode;
    struct proc_dir_entry * de;
    + int error;

    + error = -ENOTDIR;
    if (!dir || !S_ISDIR(dir->i_mode))
    - return -ENOTDIR;
    + goto out;

    - de = (struct proc_dir_entry *) dir->u.generic_ip;
    + error = -ENOENT;
    inode = NULL;
    + de = (struct proc_dir_entry *) dir->u.generic_ip;
    if (de) {
    for (de = de->subdir; de ; de = de->next) {
    if (!de || !de->low_ino)
    @@ -660,18 +681,20 @@
    continue;
    if (!memcmp(dentry->d_name.name, de->name, de->namelen)) {
    int ino = de->low_ino | (dir->i_ino & ~(0xffff));
    + error = -EINVAL;
    inode = proc_get_inode(dir->i_sb, ino, de);
    - if (!inode)
    - return -EINVAL;
    break;
    }
    }
    }
    - if (!inode)
    - return -ENOENT;

    - d_add(dentry, inode);
    - return 0;
    + if (inode) {
    + dentry->d_op = &proc_dentry_operations;
    + d_add(dentry, inode);
    + error = 0;
    + }
    +out:
    + return error;
    }

    static int proc_root_lookup(struct inode * dir, struct dentry * dentry)
    @@ -721,6 +744,8 @@
    if (!inode)
    return -EINVAL;
    }
    +
    + dentry->d_op = &proc_dentry_operations;
    d_add(dentry, inode);
    return 0;
    }
    @@ -827,3 +852,4 @@
    read_unlock(&tasklist_lock);
    return 0;
    }
    +
    --- linux-2.1.57/fs/proc/inode.c.old Sat Jul 19 08:17:14 1997
    +++ linux-2.1.57/fs/proc/inode.c Fri Sep 26 10:39:01 1997
    @@ -20,21 +20,28 @@
    static void proc_put_inode(struct inode *inode)
    {
    #ifdef CONFIG_SUN_OPENPROMFS_MODULE
    - if ((inode->i_ino >= PROC_OPENPROM_FIRST)
    - && (inode->i_ino < PROC_OPENPROM_FIRST + PROC_NOPENPROM)
    - && proc_openprom_use)
    + if ((inode->i_ino >= PROC_OPENPROM_FIRST) &&
    + (inode->i_ino < PROC_OPENPROM_FIRST + PROC_NOPENPROM) &&
    + proc_openprom_use)
    (*proc_openprom_use)(inode, 0);
    #endif
    + /*
    + * Kill off unused inodes ... VFS will unhash
    + * and delete if we set i_nlink to zero.
    + */
    + if (inode->i_count == 1)
    + inode->i_nlink = 0;
    }

    /*
    - * Does this ever happen?
    + * Note: as of 2.1.56 we don't need a delete_inode routine
    + * if no special cleanup is required.
    */
    +#if 0
    static void proc_delete_inode(struct inode *inode)
    {
    - printk("proc_delete_inode()?\n");
    - inode->i_size = 0;
    }
    +#endif

    static void proc_put_super(struct super_block *sb)
    {
    @@ -47,7 +54,7 @@
    proc_read_inode,
    proc_write_inode,
    proc_put_inode,
    - proc_delete_inode,
    + NULL, /* delete_inode(struct inode *) */
    NULL,
    proc_put_super,
    NULL,
    @@ -85,9 +92,14 @@
    return 1;
    }

    -struct inode * proc_get_inode(struct super_block * s, int ino, struct proc_dir_entry * de)
    +struct inode * proc_get_inode(struct super_block * s, int ino,
    + struct proc_dir_entry * de)
    {
    - struct inode * inode = iget(s, ino);
    + struct inode * inode;
    +
    + inode = iget(s, ino);
    + if (!inode)
    + goto out;

    #ifdef CONFIG_SUN_OPENPROMFS_MODULE
    if ((inode->i_ino >= PROC_OPENPROM_FIRST)
    @@ -95,7 +107,9 @@
    && proc_openprom_use)
    (*proc_openprom_use)(inode, 1);
    #endif
    - if (inode && inode->i_sb == s) {
    + /* N.B. How can this test ever fail?? */
    + if (inode->i_sb == s) {
    + /* N.B. could de disappear while we allocated an inode?? */
    inode->u.generic_ip = (void *) de;
    if (de) {
    if (de->mode) {
    @@ -126,26 +140,36 @@
    }
    read_unlock(&tasklist_lock);
    }
    +out:
    return inode;
    }

    struct super_block *proc_read_super(struct super_block *s,void *data,
    int silent)
    {
    + struct inode * root_inode;
    +
    lock_super(s);
    s->s_blocksize = 1024;
    s->s_blocksize_bits = 10;
    s->s_magic = PROC_SUPER_MAGIC;
    s->s_op = &proc_sops;
    + root_inode = proc_get_inode(s, PROC_ROOT_INO, &proc_root);
    + if (!root_inode)
    + goto out_no_root;
    + s->s_root = d_alloc_root(root_inode, NULL);
    + if (!s->s_root)
    + goto out_no_root;
    + parse_options(data, &root_inode->i_uid, &root_inode->i_gid);
    unlock_super(s);
    - s->s_root = d_alloc_root(proc_get_inode(s, PROC_ROOT_INO, &proc_root), NULL);
    - if (!s->s_root) {
    - s->s_dev = 0;
    - printk("get root inode failed\n");
    - return NULL;
    - }
    - parse_options(data, &s->s_root->d_inode->i_uid, &s->s_root->d_inode->i_gid);
    return s;
    +
    +out_no_root:
    + printk("proc_read_super: get root inode failed\n");
    + iput(root_inode);
    + s->s_dev = 0;
    + unlock_super(s);
    + return NULL;
    }

    int proc_statfs(struct super_block *sb, struct statfs *buf, int bufsiz)
    \
     
     \ /
      Last update: 2005-03-22 13:40    [W:0.080 / U:30.360 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site