lkml.org 
[lkml]   [2001]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: VM in 2.4.7-pre hurts...
    Date
    Hi Linus,

    On Mon, 9 Jul 2001, Linus Torvalds wrote:
    >
    > On 9 Jul 2001, Christoph Rohland wrote:
    >>
    >> No, it does matter. It prevents races against getpage.
    >
    > No it doesn't.
    >
    > We have the page locked.
    >
    > And if somebody does "getpage()" and doesn't check for the page
    > lock, then that test _still_ doesn't prevent races, because the
    > getpage might happen just _after_ the "atomic_read()".
    >
    > As it stands now, that atomic_read() does _nothing_. If you think
    > something depends on it, then that something is already buggy.

    Yep, you are right. This check hides another error: We cannot use
    find_get_page for shmem since this is getting the page without the
    lock like you described. I removed this optimization. Also
    __find_lock_page has to check that mapping and index are still the
    ones we looked for.

    I append a patch to fix these errors (and the other obvious buglets in
    shmem.c I did send to you several times).

    Stephen, could you crosscheck? You had the test case which triggered
    the count > 2 bug.

    Greetings
    Christoph


    diff -uNr 7-pre6/mm/filemap.c 7-pre6-fix/mm/filemap.c
    --- 7-pre6/mm/filemap.c Wed Jul 11 09:59:01 2001
    +++ 7-pre6-fix/mm/filemap.c Wed Jul 11 20:49:14 2001
    @@ -760,7 +760,7 @@
    lock_page(page);

    /* Is the page still hashed? Ok, good.. */
    - if (page->mapping)
    + if (page->mapping == mapping && page->index == offset)
    return page;

    /* Nope: we raced. Release and try again.. */
    diff -uNr 7-pre6/mm/shmem.c 7-pre6-fix/mm/shmem.c
    --- 7-pre6/mm/shmem.c Wed Jul 11 09:59:01 2001
    +++ 7-pre6-fix/mm/shmem.c Wed Jul 11 20:44:35 2001
    @@ -3,7 +3,8 @@
    *
    * Copyright (C) 2000 Linus Torvalds.
    * 2000 Transmeta Corp.
    - * 2000 Christoph Rohland
    + * 2000-2001 Christoph Rohland
    + * 2000-2001 SAP AG
    *
    * This file is released under the GPL.
    */
    @@ -33,7 +34,7 @@
    #define TMPFS_MAGIC 0x01021994

    #define ENTRIES_PER_PAGE (PAGE_SIZE/sizeof(unsigned long))
    -#define NR_SINGLE (ENTRIES_PER_PAGE + SHMEM_NR_DIRECT)
    +#define SHMEM_MAX_BLOCKS (SHMEM_NR_DIRECT + ENTRIES_PER_PAGE*ENTRIES_PER_PAGE)

    static struct super_operations shmem_ops;
    static struct address_space_operations shmem_aops;
    @@ -193,7 +194,14 @@
    }

    out:
    - info->max_index = index;
    + /*
    + * We have no chance to give an error, so we limit it to max
    + * size here and the application will fail later
    + */
    + if (index > SHMEM_MAX_BLOCKS)
    + info->max_index = SHMEM_MAX_BLOCKS;
    + else
    + info->max_index = index;
    info->swapped -= freed;
    shmem_recalc_inode(inode);
    spin_unlock (&info->lock);
    @@ -311,6 +319,7 @@
    return page;
    }

    + shmem_recalc_inode(inode);
    if (entry->val) {
    unsigned long flags;

    @@ -390,22 +399,9 @@

    static int shmem_getpage(struct inode * inode, unsigned long idx, struct page **ptr)
    {
    - struct address_space * mapping = inode->i_mapping;
    int error;

    - *ptr = NOPAGE_SIGBUS;
    - if (inode->i_size <= (loff_t) idx * PAGE_CACHE_SIZE)
    - return -EFAULT;
    -
    - *ptr = __find_get_page(mapping, idx, page_hash(mapping, idx));
    - if (*ptr) {
    - if (Page_Uptodate(*ptr))
    - return 0;
    - page_cache_release(*ptr);
    - }
    -
    down (&inode->i_sem);
    - /* retest we may have slept */
    if (inode->i_size < (loff_t) idx * PAGE_CACHE_SIZE)
    goto sigbus;
    *ptr = shmem_getpage_locked(inode, idx);
    @@ -1024,6 +1020,8 @@
    unsigned long max_inodes, inodes;
    struct shmem_sb_info *info = &sb->u.shmem_sb;

    + max_blocks = info->max_blocks;
    + max_inodes = info->max_inodes;
    if (shmem_parse_options (data, NULL, &max_blocks, &max_inodes))
    return -EINVAL;

    @@ -1071,7 +1069,7 @@
    sb->u.shmem_sb.free_blocks = blocks;
    sb->u.shmem_sb.max_inodes = inodes;
    sb->u.shmem_sb.free_inodes = inodes;
    - sb->s_maxbytes = (unsigned long long)(SHMEM_NR_DIRECT + (ENTRIES_PER_PAGE*ENTRIES_PER_PAGE)) << PAGE_CACHE_SHIFT;
    + sb->s_maxbytes = (unsigned long long)SHMEM_MAX_BLOCKS << PAGE_CACHE_SHIFT;
    sb->s_blocksize = PAGE_CACHE_SIZE;
    sb->s_blocksize_bits = PAGE_CACHE_SHIFT;
    sb->s_magic = TMPFS_MAGIC;
    @@ -1279,9 +1277,11 @@
    struct qstr this;
    int vm_enough_memory(long pages);

    - error = -ENOMEM;
    + if (size > (unsigned long long) SHMEM_MAX_BLOCKS << PAGE_CACHE_SHIFT)
    + return ERR_PTR(-EINVAL);
    +
    if (!vm_enough_memory((size) >> PAGE_SHIFT))
    - goto out;
    + return ERR_PTR(-ENOMEM);

    this.name = name;
    this.len = strlen(name);
    @@ -1289,7 +1289,7 @@
    root = tmpfs_fs_type.kern_mnt->mnt_root;
    dentry = d_alloc(root, &this);
    if (!dentry)
    - goto out;
    + return ERR_PTR(-ENOMEM);

    error = -ENFILE;
    file = get_empty_filp();
    @@ -1315,7 +1315,6 @@
    put_filp(file);
    put_dentry:
    dput (dentry);
    -out:
    return ERR_PTR(error);
    }
    /*
    -
    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:17    [W:0.033 / U:1.344 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site