Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752993AbbGACvZ (ORCPT ); Tue, 30 Jun 2015 22:51:25 -0400 Received: from hqemgate15.nvidia.com ([216.228.121.64]:3169 "EHLO hqemgate15.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750794AbbGACvX (ORCPT ); Tue, 30 Jun 2015 22:51:23 -0400 X-PGP-Universal: processed; by hqnvupgp07.nvidia.com on Tue, 30 Jun 2015 19:51:07 -0700 Date: Tue, 30 Jun 2015 19:51:12 -0700 From: Mark Hairgrove To: Jerome Glisse CC: "akpm@linux-foundation.org" , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , Linus Torvalds , "joro@8bytes.org" , Mel Gorman , "H. Peter Anvin" , Peter Zijlstra , Andrea Arcangeli , Johannes Weiner , Larry Woodman , Rik van Riel , Dave Airlie , Brendan Conoboy , Joe Donohue , Duncan Poole , Sherry Cheung , Subhash Gutti , John Hubbard , Lucien Dunning , Cameron Buschardt , Arvind Gopalakrishnan , Haggai Eran , Shachar Raindel , Liran Liss , Roland Dreier , Ben Sander , Greg Stoner , John Bridgman , Michael Mantor , Paul Blinzer , Laurent Morichetti , Alexander Deucher , Oded Gabbay , =?ISO-8859-15?Q?J=E9r=F4me_Glisse?= , Jatin Kumar Subject: Re: [PATCH 06/36] HMM: add HMM page table v2. In-Reply-To: <20150629144305.GA2173@gmail.com> Message-ID: References: <1432236705-4209-1-git-send-email-j.glisse@gmail.com> <1432236705-4209-7-git-send-email-j.glisse@gmail.com> <20150626163030.GA3748@gmail.com> <20150629144305.GA2173@gmail.com> User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) X-NVConfidentiality: public MIME-Version: 1.0 X-Originating-IP: [172.17.163.10] X-ClientProxiedBy: HQMAIL103.nvidia.com (172.20.187.11) To HQMAIL108.nvidia.com (172.18.146.13) Content-Type: multipart/mixed; boundary="8323329-43040936-1435719081=:31456" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3271 Lines: 86 --8323329-43040936-1435719081=:31456 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 8BIT On Mon, 29 Jun 2015, Jerome Glisse wrote: > [...] > > Iterator is what protect against concurrent freeing of the directory so it > has to return to caller on directory boundary (for 64bits arch with 64bits > pte it has return every 512 entries). Otherwise pt_iter_fini() would have > to walk over the whole directory range again just to drop reference and this > doesn't sound like a good idea. I don't understand why it would have to return to the caller to unprotect the directory. The iterator would simply drop the reference to the previous directory, take a reference on the next one, and keep searching for a valid entry. Why would pt_iter_fini have to walk over the entire range? The iterator would keep at most one directory per level referenced. _fini would walk the per-level ptd array and unprotect each level, the same way it does now. > > So really with what you are asking it whould be: > > hmm_pt_iter_init(&iter, start, end); > for(next=pt_iter_next(&iter,&ptep); next { > // Here ptep is valid until next address. Above you have to call > // pt_iter_next() to switch to next directory. > addr = max(start, next - (~HMM_PMD_MASK + 1)); > for (; addr < next; addr += PAGE_SIZE, ptep++) { > // access ptep > } > } > > My point is that internally pt_iter_next() will do the exact same test it is > doing now btw cur and addr. Just that the addr is no longer explicit but iter > infer it. But this way, the iteration across directories is more efficient because the iterator can simply walk the directory array. Take a directory that has one valid entry at the very end. The existing iteration will do this: hmm_pt_iter_next(dir_addr[0], end) Walk up the ptd array Compute level start and end and compare them to dir_addr[0] Compute dir_addr[1] using addr and pt->mask Return dir_addr[1] hmm_pt_iter_update(dir_addr[1]) Walk up the ptd array, compute level start and end Compute level index of dir_addr[1] Read entry for dir_addr[1] Return NULL hmm_pt_iter_next(dir_addr[1], end) ... And so on 511 times until the last entry is read. This is really more suited to a for loop iteration, which it could be if this were fully contained within the _next call. > > > If _next only returned to the caller when it hit a valid hmm_pte (or end), > > then only one function would be needed (_next) instead of two > > (_update/_walk and _next). > > On the valid entry side, this is because when you are walking the page table > you have no garanty that the entry will not be clear below you (in case of > concurrent invalidation). The only garanty you have is that if you are able > to read a valid entry from the update() callback then this entry is valid > until you get a new update() callback telling you otherwise. > > Cheers, > J?r?me > --8323329-43040936-1435719081=:31456-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/