lkml.org 
[lkml]   [2008]   [Apr]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch 01/10] emm: mm_lock: Lock a process against reclaim
Andrea Arcangeli wrote:
> On Fri, Apr 04, 2008 at 04:12:42PM -0700, Jeremy Fitzhardinge wrote:
>
>> I think you can break this if() down a bit:
>>
>> if (!(vma->vm_file && vma->vm_file->f_mapping))
>> continue;
>>
>
> It makes no difference at runtime, coding style preferences are quite
> subjective.
>

Well, overall the formatting of that if statement is very hard to read.
Separating out the logically distinct pieces in to different ifs at
least shows the reader that they are distinct.
Aside from that, doing some manual CSE to remove all the casts and
expose the actual thing you're testing for would help a lot (are the
casts even necessary?).

>> So this is an O(n^2) algorithm to take the i_mmap_locks from low to high
>> order? A comment would be nice. And O(n^2)? Ouch. How often is it
>> called?
>>
>
> It's called a single time when the mmu notifier is registered. It's a
> very slow path of course. Any other approach to reduce the complexity
> would require memory allocations and it would require
> mmu_notifier_register to return -ENOMEM failure. It didn't seem worth
> it.
>

It's per-mm though. How many processes would need to have notifiers?


>> And is it necessary to mush lock and unlock together? Unlock ordering
>> doesn't matter, so you should just be able to have a much simpler loop, no?
>>
>
> That avoids duplicating .text. Originally they were separated. unlock
> can't be a simpler loop because I didn't reserve vm_flags bitflags to
> do a single O(N) loop for unlock. If you do malloc+fork+munmap two
> vmas will point to the same anon-vma lock, that's why the unlock isn't
> simpler unless I mark what I locked with a vm_flags bitflag.

Well, its definitely going to need more comments then. I assumed it
would end up locking everything, so unlocking everything would be
sufficient.

J


\
 
 \ /
  Last update: 2008-04-07 21:07    [W:0.122 / U:0.284 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site