lkml.org 
[lkml]   [2007]   [Jul]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: Concerning a post that you made about expandable anonymous shared mappings

    Hugh Dickins wrote:
    > On Mon, 2 Jul 2007, Stas Sergeev wrote:
    >> Hugh Dickins wrote:
    >>> You've answered your own question: we did not make the change Stas
    >>> suggested, IIRC because I remained a little uneasy with that change
    >>> in behaviour, and nobody else spoke up for it.
    >> IIRC your argument, that made sense to me,
    >> was that with such an approach, you can only
    >> expand the backing-store, but never shrink.
    >> I agree this is a problem from some point of
    >> view. I have no idea whether it is important
    >> or not, but it definitely _looks_ not very good.
    >
    > You were gracious enough to accept my arguments back then, but after
    > mulling this over overnight, I've come to think I was just too timid
    > back then, and gave too much weight to the issue of there being no
    > shrink, and to the issue that child might expand the object without
    > parent knowing (that surplus remaining allocated until both exited).
    >
    > I've come right around to your original view, Stas, and William's:
    > if that mmap creates such an object, then the expanding mremap really
    > ought to be useful, and allow the underlying object to be expanded.
    > The shared anonymous object is already anomalous: expanding it on
    > fault makes it more consistent with its own nature, not less.
    >
    >>>> //why does this failed. I am well in the interval [4096, 8192]
    >>>> *(unsigned int *)(ptr + 4096 + 8)= 10;
    >>>> }
    >> Well, this generates a bus error, while, for
    >> example, doing the same trick with having a
    >> /dev/mem as a backing-store, simply maps the
    >> "enlarged" space with the anonymous memory,
    >> and so does not generate a SIGBUS (not producing
    >> a desired effect either, of course).
    >> Why do we have it both ways? Shouldn't they
    >> (i.e. /dev/zero and /dev/mem mapping) behave
    >> the same after expanding with mremap?
    >
    > They've very different: mapping /dev/mem maps an existing object;
    > whereas mapping /dev/zero is a convention by which a new object
    > is created - and we can't afford the memory to make it of infinite
    > extent, so have limited it to the mapped size.
    >
    >>> I haven't given it any thought since then:
    >> I was thinking about it a bit, and I imagined
    >> we need something like
    >> int mopen(void *addr, size_t len, unsigned int flags);
    >> which will give you an fd for the memory area,
    >> which you can then ftruncate() and mmap() (and
    >> mremap).
    >
    > Ah, if we added an mopen(), there's no end to the discussions
    > we could have about what it can do. It may be a great idea:
    > but it's really not needed to solve this particular little
    > problem. As last time around, you were suggesting .mremap
    > callouts; but I much prefer your original shmem_zero_nopage,
    > which is a solution of the scale appropriate to the problem.
    >
    >> Such a thing could solve that mremap() problem
    >> that me and William Tambe have.
    >
    > If you're thinking of going that way, for this problem it
    > would be better just to stick with POSIX shm_open, as Christoph
    > suggested before, and you suggest to William in other mail.
    >
    > But I now accept your view, that the shared anonymous object
    > is not behaving consistently in response to mremap, and would
    > like to put in a patch for that. This isn't a particularly good
    > time for such a patch - it's unclear right now whether 2.6.23 will
    > have shmem_nopage like 2.6.22 or shmem_fault like 2.6.22-rc-mm;
    > but we can easily adjust to whichever.
    >
    > Here's a patch against 2.6.22-rc7: would you, Stas, put your
    > Signed-off-by into this, and accept authorship - although I'm
    > sending this back to you, it's very much your idea, and only
    > trivially modified from your three-year-old patch by me. If
    > you're agreeable, I can then forward it or its shmem_zero_fault
    > equivalent to Andrew when we see which way 2.6.23 is going.
    >
    > [I'll fill in the patch comment later]
    >
    > Signed-off-by: Hugh Dickins <hugh@veritas.com>
    > ----
    >
    > mm/shmem.c | 41 ++++++++++++++++++++++++++++++++++++++++-
    > 1 file changed, 40 insertions(+), 1 deletion(-)
    >
    > --- 2.6.22-rc7/mm/shmem.c 2007-06-17 10:51:02.000000000 +0100
    > +++ linux/mm/shmem.c 2007-07-03 15:35:32.000000000 +0100
    > @@ -1320,6 +1320,36 @@ static struct page *shmem_nopage(struct
    > return page;
    > }
    >
    > +struct page *shmem_zero_nopage(struct vm_area_struct *vma,
    > + unsigned long address, int *type)
    > +{
    > + struct page *page;
    > +
    > + page = shmem_nopage(vma, address, type);
    > + if (unlikely(page == NOPAGE_SIGBUS)) {
    > + struct inode *inode = vma->vm_file->f_dentry->d_inode;
    > + struct shmem_inode_info *info = SHMEM_I(inode);
    > + loff_t vm_size, i_size;
    > +
    > + /*
    > + * If mremap has been used to extend the vma,
    > + * now extend the underlying object to include this page.
    > + */
    > + vm_size = (PAGE_ALIGN(address) - vma->vm_start) +
    > + ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
    > + spin_lock(&info->lock);
    > + i_size = i_size_read(inode);
    > + if (i_size < vm_size && vm_size <= SHMEM_MAX_BYTES &&
    > + !shmem_acct_size(info->flags, vm_size - i_size)) {
    > + i_size_write(inode, vm_size);
    > + inode->i_ctime = inode->i_mtime = CURRENT_TIME;
    > + }
    > + spin_unlock(&info->lock);
    > + page = shmem_nopage(vma, address, type);
    > + }
    > + return page;
    > +}
    > +
    > static int shmem_populate(struct vm_area_struct *vma,
    > unsigned long addr, unsigned long len,
    > pgprot_t prot, unsigned long pgoff, int nonblock)
    > @@ -2471,6 +2501,14 @@ static struct vm_operations_struct shmem
    > #endif
    > };
    >
    > +static struct vm_operations_struct shmem_zero_vm_ops = {
    > + .nopage = shmem_zero_nopage,
    > + .populate = shmem_populate,
    > +#ifdef CONFIG_NUMA
    > + .set_policy = shmem_set_policy,
    > + .get_policy = shmem_get_policy,
    > +#endif
    > +};
    >
    > static int shmem_get_sb(struct file_system_type *fs_type,
    > int flags, const char *dev_name, void *data, struct vfsmount *mnt)
    > @@ -2599,6 +2637,7 @@ int shmem_zero_setup(struct vm_area_stru
    > if (vma->vm_file)
    > fput(vma->vm_file);
    > vma->vm_file = file;
    > - vma->vm_ops = &shmem_vm_ops;
    > + vma->vm_ops = &shmem_zero_vm_ops;
    > + vma->vm_pgoff = 0;
    > return 0;
    > }
    >

    Will this patch be added to stable versions of the linux kernel?
    Please let me know.

    Sincerely,
    William Tambe
    -
    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: 2007-07-10 03:55    [W:0.035 / U:32.800 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site