From: Michel Lespinasse on
On Wed, May 26, 2010 at 10:23:59AM +0900, KAMEZAWA Hiroyuki wrote:
> On Tue, 25 May 2010 11:27:55 +0200
> Peter Zijlstra <peterz(a)infradead.org> wrote:
> > Also, do you really think doing something like:
> >
> > /*
> > * Check the vma index is within the range and do
> > * sequential scan until m_index.
> > */
> > vma = NULL;
> > if ((unsigned long)l < mm->map_count) {
> > vma = mm->mmap;
> > while (l-- && vma)
> > vma = vma->vm_next;
> > goto out;
> > }
> >
> > with preemption disabled is a _good_ thing?
> >
> > People were talking about raising our vma limit of 64k...

All right, this is clearly a problem then.

> Hmm..can't we use something speculative lookup for reading maps ?
> (as we played in several months ago...)

Would you happen to have a link to that conversation / remember the
subject line of the thread ?

Thanks,

--
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Peter Zijlstra on
On Thu, 2010-05-27 at 03:59 -0700, Michel Lespinasse wrote:
> On Tue, May 25, 2010 at 11:27:55AM +0200, Peter Zijlstra wrote:
> > On Tue, 2010-05-25 at 02:12 -0700, Michel Lespinasse wrote:
> > > Yes, we do have patches trying to release the mmap_sem when a page
> > > fault for a file backed VMA blocks on accessing the corresponding
> > > file. We have not given up on these, and we intend to try submitting
> > > them again. However, these patches do *not* address the case of a page
> > > fault blocking while trying to get a free page (i.e. when you get
> > > under high memory pressure).
> >
> > But I guess they could, right? Simply make the allocation under mmap_sem
> > be __GFP_HARDWALL|__GFP_HIGHMEM|__GFP_MOVABLE__GFP_NOWARN or
> > (GFP_HUGHUSER_MOVABLE & ~(__GFP_WAIT|__GFP_IO|__GFP_FS))|__GFP_NOWARN
> >
> > and drop the mmap_sem when that fails.
>
> It's not clear to me if this can lead to a clean uncontroversial solution.
> Doing this for file backed VMAs does not sound any harder in principle,
> but we could not get it past linus's NACK last time. I think it's worth
> exploring again, but I don't expect it to be so easy :)

Right, I was planning to have a look again, but my todo list is getting
out of hand again.

> > > > I really don't like people tinkering with the lock implementations like
> > > > this. Nor do I like the naming, stats are in no way _critical_.
> > >
> > > Critical here refers to the fact that you're not allowed to block
> > > while holding the unfairly acquired rwsem.
> >
> > We usually call that atomic, your 0/n patch didn't explain any of that.
>
> Would replacing the 'critical' name with 'atomic' address your concern
> though, or would you remain fundamentally opposed to anything that involves
> an unfair acquire path ?

Yeah, I don't think its a good idea to add unfairness to the lock, such
things tend to backfire and generate starvation cases. For instance, a
tight loop around your /proc/$pid/maps mod will starve $pid from doing
anything requiring mmap_sem for writing.

> What about patches 1-7 which don't deal with the critical/atomic API;
> can we agree to get these in before we we figure out what to do with
> the the last 4 ?

patch 6 looks like it will break fairness, either that or the changelog
got me totally confused (already had to read it twice).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Michel Lespinasse on
On Thu, May 27, 2010 at 4:18 AM, Peter Zijlstra <peterz(a)infradead.org> wrote:
> On Thu, 2010-05-27 at 03:59 -0700, Michel Lespinasse wrote:
>> What about patches 1-7 which don't deal with the critical/atomic API;
>> can we agree to get these in before we we figure out what to do with
>> the the last 4 ?
>
> patch 6 looks like it will break fairness, either that or the changelog
> got me totally confused (already had to read it twice).

Hmmm. I must be bad at explaining this :)

It avoids a situation where threads got blocked waiting for all active
readers to release, but (due to a race condition when blocking) the
first queued thread is also a reader. Letting that reader proceed in
parallel with the currently active ones will not delay queued writers;
they would have had to wait for the first queued reader to proceed
anyway since they were queued behind it.

--
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/