All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Cyrill Gorcunov <gorcunov@openvz.org>,
	David Howells <dhowells@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Sasha Levin <levinsasha928@gmail.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/7] fs/proc/task_mmu.c: don't use task->mm in m_start() and show_*map()
Date: Wed, 6 Aug 2014 21:41:29 +0200	[thread overview]
Message-ID: <20140806194129.GA943@redhat.com> (raw)
In-Reply-To: <20140806141005.GA1487@redhat.com>

On 08/06, Oleg Nesterov wrote:
>
> On 08/06, Kirill A. Shutemov wrote:
> >
> > On Tue, Aug 05, 2014 at 09:46:44PM +0200, Oleg Nesterov wrote:
> > >
> > >  	if (m->count < m->size)  /* vma is copied successfully */
> > > -		m->version = (vma != get_gate_vma(task->mm))
> > > +		m->version = (vma != priv->tail_vma)
> > >  			? vma->vm_start : 0;
> >
> > Drop excessive parenthesis while you're there? And in the next hunk.
>
> I agree. But lets do this in another patch? I forgot to mention this in
> the yesterday's TODO, but we should factor out this code. Perhaps move
> this into vma_stop(). Or at least add a trivial helper, this pattern
> repeats 3 times, plus another one in m_start(). This problem is that I
> do not really understand this logic right now, but I'll do something
> today in any case.

Damn, this needs even more cleanups than I thought. Even "version == -1"
logic is absolutely wrong/dead. And "last_addr rather than next_addr" is
wrong too afaics, this just means we need the extra ->vm_next step plus
vma-or-tail_vma recheck.

Hopefully I'll finish this tomorrow.

Oleg.

  reply	other threads:[~2014-08-06 19:41 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-05 19:46 [PATCH v2 0/7] /proc/PID/*maps* fixes/cleanups Oleg Nesterov
2014-08-05 19:46 ` [PATCH v2 1/7] fs/proc/task_mmu.c: don't use task->mm in m_start() and show_*map() Oleg Nesterov
2014-08-06  9:52   ` Kirill A. Shutemov
2014-08-06 14:10     ` Oleg Nesterov
2014-08-06 19:41       ` Oleg Nesterov [this message]
2014-08-05 19:46 ` [PATCH v2 2/7] fs/proc/task_mmu.c: unify/simplify do_maps_open() and numa_maps_open() Oleg Nesterov
2014-08-06  9:55   ` Kirill A. Shutemov
2014-08-05 19:46 ` [PATCH v2 3/7] proc: introduce proc_mem_open() Oleg Nesterov
2014-08-06  9:57   ` Kirill A. Shutemov
2014-08-05 19:46 ` [PATCH v2 4/7] fs/proc/task_mmu.c: shift mm_access() from m_start() to proc_maps_open() Oleg Nesterov
2014-08-06 10:05   ` Kirill A. Shutemov
2014-12-03 14:14   ` Kirill A. Shutemov
2014-12-03 16:59     ` Eric W. Biederman
2014-12-04 16:17       ` Kirill A. Shutemov
2014-12-03 17:34     ` Oleg Nesterov
2014-08-05 19:46 ` [PATCH v2 5/7] fs/proc/task_mmu.c: simplify the vma_stop() logic Oleg Nesterov
2014-08-06 10:12   ` Kirill A. Shutemov
2014-08-05 19:47 ` [PATCH v2 6/7] fs/proc/task_mmu.c: cleanup the "tail_vma" horror in m_next() Oleg Nesterov
2014-08-06 10:17   ` Kirill A. Shutemov
2014-08-05 19:47 ` [PATCH v2 7/7] fs/proc/task_mmu.c: shift "priv->task = NULL" from m_start() to m_stop() Oleg Nesterov
2014-08-06 10:18   ` Kirill A. Shutemov
2014-08-06 10:19 ` [PATCH v2 0/7] /proc/PID/*maps* fixes/cleanups Kirill A. Shutemov
2014-08-06 18:48 ` Cyrill Gorcunov
2014-08-07 19:17 ` [PATCH 0/5] fs/proc/task_mmu.c: cleanup tail_vma/last_addr mess Oleg Nesterov
2014-08-07 19:17   ` [PATCH 1/5] fs/proc/task_mmu.c: kill the suboptimal and confusing m->version logic Oleg Nesterov
2014-08-07 19:17   ` [PATCH 2/5] fs/proc/task_mmu.c: simplify m_start() to make it readable Oleg Nesterov
2014-08-07 19:17   ` [PATCH 3/5] fs/proc/task_mmu.c: introduce m_next_vma() helper Oleg Nesterov
2014-08-07 19:17   ` [PATCH 4/5] fs/proc/task_mmu.c: reintroduce m->version logic Oleg Nesterov
2014-08-07 19:18   ` [PATCH 5/5] fs/proc/task_mmu.c: update m->version in the main loop in m_start() Oleg Nesterov
2014-08-11 17:00 ` [PATCH 0/3] fs/proc/task_nommu.c: copy-and-paste the changes from task_mmu.c Oleg Nesterov
2014-08-11 17:00   ` [PATCH 1/3] fs/proc/task_nommu.c: change maps_open() to use __seq_open_private() Oleg Nesterov
2014-08-11 17:00   ` [PATCH 2/3] fs/proc/task_nommu.c: shift mm_access() from m_start() to proc_maps_open() Oleg Nesterov
2014-08-11 17:00   ` [PATCH 3/3] fs/proc/task_nommu.c: don't use priv->task->mm Oleg Nesterov
2014-08-12 13:57   ` [PATCH 0/3] fs/proc/task_nommu.c: copy-and-paste the changes from task_mmu.c Greg Ungerer
2014-08-12 17:35     ` Oleg Nesterov
2014-08-20 16:49 ` [PATCH -mm 0/2] make /proc/PID/*maps* namespace friendly Oleg Nesterov
2014-08-20 16:49   ` [PATCH -mm 1/2] proc/maps: replace proc_maps_private->pid with "struct inode *inode" Oleg Nesterov
2014-08-20 16:50   ` [PATCH -mm 2/2] proc/maps: make vm_is_stack() logic namespace-friendly Oleg Nesterov
2014-08-20 19:22 ` [PATCH 0/4] mempolicy: get_task_policy() cleanups/preparations Oleg Nesterov
2014-08-20 19:22   ` [PATCH 1/4] mempolicy: change alloc_pages_vma() to use mpol_cond_put() Oleg Nesterov
2014-08-20 19:23   ` [PATCH 2/4] mempolicy: change get_task_policy() to return default_policy rather than NULL Oleg Nesterov
2014-08-20 19:23   ` [PATCH 3/4] mempolicy: sanitize the usage of get_task_policy() Oleg Nesterov
2014-08-20 19:23   ` [PATCH 4/4] mempolicy: remove the "task" arg of vma_policy_mof() and simplify it Oleg Nesterov
2014-08-21 19:00   ` [PATCH 0/4] mempolicy: fix show_numa_map() vs exec() + do_set_mempolicy() race Oleg Nesterov
2014-08-21 19:00     ` [PATCH 1/4] mempolicy: introduce __get_vma_policy(), export get_task_policy() Oleg Nesterov
2014-08-21 19:01     ` [PATCH 2/4] mempolicy: fix show_numa_map() vs exec() + do_set_mempolicy() race Oleg Nesterov
2014-08-21 19:01     ` [PATCH 3/4] mempolicy: kill do_set_mempolicy()->down_write(&mm->mmap_sem) Oleg Nesterov
2014-08-21 19:01     ` [PATCH 4/4] mempolicy: unexport get_vma_policy() and remove its "task" arg Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140806194129.GA943@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=dhowells@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=gorcunov@openvz.org \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kirill@shutemov.name \
    --cc=levinsasha928@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.