lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <47FA6FDD.9060605@goop.org> Date: Mon, 07 Apr 2008 12:02:53 -0700 From: Jeremy Fitzhardinge <jeremy@...p.org> To: Andrea Arcangeli <andrea@...ranet.com> CC: Christoph Lameter <clameter@....com>, Robin Holt <holt@....com>, kvm-devel@...ts.sourceforge.net, Peter Zijlstra <a.p.zijlstra@...llo.nl>, general@...ts.openfabrics.org, steiner@....com, linux-kernel@...r.kernel.org, linux-mm@...ck.org Subject: Re: [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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists