Re: [PATCH 6/13] maps: Move the page walker code to lib/
Nick Piggin wrote: Matt Mackall wrote: On Wed, Apr 04, 2007 at 03:50:56PM +1000, Nick Piggin wrote: Just put it in its own file in mm/ rather than its own file in lib. lib should be for almost-standalone stuff, IMO (ie. only using basic kernel functionality). Arguably that's what lib/ should be for, but it's currently largely I disagree. There is code everywhere that exists to provide some functionality via an API to other parts of the kernel. You don't think mm/page_alloc.c should go in lib/? Oh, I think I misunderstood you. Anyway, we have lots of conditionally compiled code in mm as well. It doesn't seem to be much hardship. -- SUSE Labs, Novell Inc. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/13] maps: Move the page walker code to lib/
Matt Mackall wrote: On Wed, Apr 04, 2007 at 03:50:56PM +1000, Nick Piggin wrote: Matt Mackall wrote: On Wed, Apr 04, 2007 at 01:51:37PM +1000, Nick Piggin wrote: Matt Mackall wrote: Move the page walker code to lib/ This lets it get shared outside of proc/ and linked in only when needed. I think it would be better in mm/. I originally was looking at putting it in mm/memory.c and possibly Just put it in its own file in mm/ rather than its own file in lib. lib should be for almost-standalone stuff, IMO (ie. only using basic kernel functionality). Arguably that's what lib/ should be for, but it's currently largely I disagree. There is code everywhere that exists to provide some functionality via an API to other parts of the kernel. You don't think mm/page_alloc.c should go in lib/? used to avoid linking in unused code without adding more hair to Kconfig. Which is what I'm trying to do here. Well if you're doing a nice big reorganisation and jumble, then why would you worry about a few lines in Kconfig? Apart from these users outside mm/, I don't see much point in converting things over. The page table walking API we have now is neat and simple. It takes a few lines of code, but is it a big problem? I don't think it really qualifies as either neat or simple. It may be about as neat as walking a heterogenous tree with an inconsistent naming scheme can be, but it's still a headache. A maze of twisty little passages, all slightly different. All page table range walking code should be the same. It is a simple template to use to walk a range of ptes. It is a headache outside mm/, sure, but not because of its incredible complexity. -- SUSE Labs, Novell Inc. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/13] maps: Move the page walker code to lib/
On Wed, Apr 04, 2007 at 03:50:56PM +1000, Nick Piggin wrote: > Matt Mackall wrote: > >On Wed, Apr 04, 2007 at 01:51:37PM +1000, Nick Piggin wrote: > > > >>Matt Mackall wrote: > >> > >>>Move the page walker code to lib/ > >>> > >>>This lets it get shared outside of proc/ and linked in only when > >>>needed. > >> > >>I think it would be better in mm/. > > > > > >I originally was looking at putting it in mm/memory.c and possibly > > Just put it in its own file in mm/ rather than its own file in lib. > lib should be for almost-standalone stuff, IMO (ie. only using basic > kernel functionality). Arguably that's what lib/ should be for, but it's currently largely used to avoid linking in unused code without adding more hair to Kconfig. Which is what I'm trying to do here. > Apart from these users outside mm/, I don't see much point in converting > things over. The page table walking API we have now is neat and simple. > It takes a few lines of code, but is it a big problem? I don't think it really qualifies as either neat or simple. It may be about as neat as walking a heterogenous tree with an inconsistent naming scheme can be, but it's still a headache. A maze of twisty little passages, all slightly different. -- Mathematics is the supreme nostalgia of our time. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/13] maps: Move the page walker code to lib/
Matt Mackall wrote: On Wed, Apr 04, 2007 at 01:51:37PM +1000, Nick Piggin wrote: Matt Mackall wrote: Move the page walker code to lib/ This lets it get shared outside of proc/ and linked in only when needed. I think it would be better in mm/. I originally was looking at putting it in mm/memory.c and possibly Just put it in its own file in mm/ rather than its own file in lib. lib should be for almost-standalone stuff, IMO (ie. only using basic kernel functionality). Yes, I think ioremap.c should be in mm/ as well. converting some users there. But I decided it ought to be conditionally linked until it had some core users and I wasn't quite ready to tackle the tricky mm/ bits. Apart from these users outside mm/, I don't see much point in converting things over. The page table walking API we have now is neat and simple. It takes a few lines of code, but is it a big problem? -- SUSE Labs, Novell Inc. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/13] maps: Move the page walker code to lib/
On Wed, Apr 04, 2007 at 01:51:37PM +1000, Nick Piggin wrote: > Matt Mackall wrote: > >Move the page walker code to lib/ > > > >This lets it get shared outside of proc/ and linked in only when > >needed. > > I think it would be better in mm/. I originally was looking at putting it in mm/memory.c and possibly converting some users there. But I decided it ought to be conditionally linked until it had some core users and I wasn't quite ready to tackle the tricky mm/ bits. > So would clear_refs_pte_range, and clear_refs_write (in a more > generic form), IMO. > > Sweet patchset, though. Thanks. -- Mathematics is the supreme nostalgia of our time. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/13] maps: Move the page walker code to lib/
Matt Mackall wrote: Move the page walker code to lib/ This lets it get shared outside of proc/ and linked in only when needed. I think it would be better in mm/. So would clear_refs_pte_range, and clear_refs_write (in a more generic form), IMO. Sweet patchset, though. -- SUSE Labs, Novell Inc. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 6/13] maps: Move the page walker code to lib/
Move the page walker code to lib/
This lets it get shared outside of proc/ and linked in only when
needed.
Signed-off-by: Matt Mackall <[EMAIL PROTECTED]>
Index: mm/fs/proc/task_mmu.c
===
--- mm.orig/fs/proc/task_mmu.c 2007-03-27 22:13:43.0 -0500
+++ mm/fs/proc/task_mmu.c 2007-03-27 22:13:51.0 -0500
@@ -280,123 +280,6 @@ static int clear_refs_pte_range(pmd_t *p
return 0;
}
-struct mm_walk {
- int (*pgd_entry)(pgd_t *, unsigned long, unsigned long, void *);
- int (*pud_entry)(pud_t *, unsigned long, unsigned long, void *);
- int (*pmd_entry)(pmd_t *, unsigned long, unsigned long, void *);
- int (*pte_entry)(pte_t *, unsigned long, unsigned long, void *);
-};
-
-static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
- struct mm_walk *walk, void *private)
-{
- pte_t *pte;
- int err;
-
- for (pte = pte_offset_map(pmd, addr); addr != end;
-addr += PAGE_SIZE, pte++) {
- if (pte_none(*pte))
- continue;
- err = walk->pte_entry(pte, addr, addr, private);
- if (err) {
- pte_unmap(pte);
- return err;
- }
- }
- pte_unmap(pte);
- return 0;
-}
-
-static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end,
- struct mm_walk *walk, void *private)
-{
- pmd_t *pmd;
- unsigned long next;
- int err;
-
- for (pmd = pmd_offset(pud, addr); addr != end;
-pmd++, addr = next) {
- next = pmd_addr_end(addr, end);
- if (pmd_none_or_clear_bad(pmd))
- continue;
- if (walk->pmd_entry) {
- err = walk->pmd_entry(pmd, addr, next, private);
- if (err)
- return err;
- }
- if (walk->pte_entry) {
- err = walk_pte_range(pmd, addr, next, walk, private);
- if (err)
- return err;
- }
- }
- return 0;
-}
-
-static int walk_pud_range(pgd_t *pgd, unsigned long addr, unsigned long end,
- struct mm_walk *walk, void *private)
-{
- pud_t *pud;
- unsigned long next;
- int err;
-
- for (pud = pud_offset(pgd, addr); addr != end;
-pud++, addr = next) {
- next = pud_addr_end(addr, end);
- if (pud_none_or_clear_bad(pud))
- continue;
- if (walk->pud_entry) {
- err = walk->pud_entry(pud, addr, next, private);
- if (err)
- return err;
- }
- if (walk->pmd_entry || walk->pte_entry) {
- err = walk_pmd_range(pud, addr, next, walk, private);
- if (err)
- return err;
- }
- }
- return 0;
-}
-
-/*
- * walk_page_range - walk a memory map's page tables with a callback
- * @mm - memory map to walk
- * @addr - starting address
- * @end - ending address
- * @action - callback invoked for every bottom-level (PTE) page table
- * @private - private data passed to the callback function
- *
- * Recursively walk the page table for the memory area in a VMA, calling
- * a callback for every bottom-level (PTE) page table.
- */
-static int walk_page_range(struct mm_struct *mm,
- unsigned long addr, unsigned long end,
- struct mm_walk *walk, void *private)
-{
- pgd_t *pgd;
- unsigned long next;
- int err;
-
- for (pgd = pgd_offset(mm, addr); addr != end;
-pgd++, addr = next) {
- next = pgd_addr_end(addr, end);
- if (pgd_none_or_clear_bad(pgd))
- continue;
- if (walk->pgd_entry) {
- err = walk->pgd_entry(pgd, addr, next, private);
- if (err)
- return err;
- }
- if (walk->pud_entry || walk->pmd_entry || walk->pte_entry) {
- err = walk_pud_range(pgd, addr, next, walk, private);
- if (err)
- return err;
- }
- }
- return 0;
-}
-
static struct mm_walk smaps_walk = { .pmd_entry = smaps_pte_range };
static int show_smap(struct seq_file *m, void *v)
Index: mm/include/linux/mm.h
===
--- mm.orig/include/linux/mm.h 2007-03-27 22:13:42.0 -0500
+++ mm/include/linux/mm.h 2007-03-27 22:13:51.0 -0500
@@ -747,6 +747,16 @@ unsigned long unmap_vmas(struct mmu_gath
stru
