lkml.org 
[lkml]   [2010]   [Nov]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [resend][PATCH 4/4] oom: don't ignore rss in nascent mm
    On 11/29, KOSAKI Motohiro wrote:
    >
    > > The patch is not complete, compat_copy_strings() needs changes.
    > > But, shouldn't it use get_arg_page() too? Otherwise, where do
    > > we check RLIMIT_STACK?
    >
    > Because NOMMU doesn't have variable length argv. Instead it is still
    > using MAX_ARG_STRLEN as old MMU code.
    >
    > 32 pages hard coded argv limitation naturally prevent this nascent mm
    > issue.

    Ah, I didn't mean NOMMU. I meant compat_execve()->compat_copy_strings().
    If a 32bit process execs we seem to miss the RLIMIT_STACK check, no?

    > > The patch asks for the cleanups. In particular, I think exec_mmap()
    > > should accept bprm, not mm. But I'd prefer to do this later.
    > >
    > > Oleg.
    >
    > General request. Please consider to keep Brad's reported-by tag.

    Yes, yes, sure.

    > > +static void acct_arg_size(struct linux_binprm *bprm, unsigned long pages)

    OK.

    > Please move this function into #ifdef CONFIG_MMU. nommu code doesn't use it.

    Well it does, to revert the MM_ANONPAGES counter. I'll add the empty
    function for NOMMU.

    > > +{
    > > + struct mm_struct *mm = current->mm;
    > > + long diff = pages - bprm->vma_pages;
    >
    > I prefer to cast signed before assignment. It's safer more.

    OK.

    > > @@ -1003,6 +1024,7 @@ int flush_old_exec(struct linux_binprm *
    > > /*
    > > * Release all of the old mmap stuff
    > > */
    > > + acct_arg_size(bprm, 0);
    >
    > Why do we need this unacct here? I mean 1) if exec_mmap() is success,
    > we don't need unaccount at all

    Yes, we already killed all sub-threads. But this doesn't mean nobody
    else can use current->mm, think about CLONE_VM. The simplest example
    is vfork().

    > 2) if exec_mmap() is failure, an epilogue of
    > do_execve() does unaccount thing.

    Yes.

    Thanks Kosaki!

    I'll resend v2 today. I am still not sure about compat_copy_strings()...

    Oleg.



    \
     
     \ /
      Last update: 2010-11-29 12:43    [W:0.028 / U:0.348 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site