[patch] Re: 2.4.3-pre6: agpart.o causes arch/i386/mm/ioremap.c hang
"Adam J. Richter" wrote:
>
> In case anyone is interested, the conflicting lock of
> init_mm.page_table_lock was acquired in line 1318 of mm/memory.c,
> in pte_alloc.
You can sorta blame me for that. I reviewed the locking in
the mmap_sem patch and said it was correct :(
I only checked that the new locking was correct, rather than
checking that the new locking *rules* were being complied
with kernel-wide.
pmd_alloc() has the same problem. It requires ->page_table_lock,
and we have bugs there. Fixed in this patch.
Linus, this patch includes the mm->rss locking stuff which
I sent yesterday.
Also, is the comment over remap_page_range true? Should the caller
be doing a down_write(mmap_sem)? If so, then there are
about thirty callers who don't. I've looked at each one,
and it is safe to do the down_write() *within* remap_page_range().
--- linux-2.4.3-pre6/fs/exec.c Thu Mar 22 18:52:52 2001
+++ linux-akpm/fs/exec.cFri Mar 23 22:08:48 2001
@@ -252,6 +252,8 @@
/*
* This routine is used to map in a page into an address space: needed by
* execve() for the initial stack and environment pages.
+ *
+ * tsk->mmap_sem is held for writing.
*/
void put_dirty_page(struct task_struct * tsk, struct page *page, unsigned long
address)
{
@@ -291,6 +293,7 @@
unsigned long stack_base;
struct vm_area_struct *mpnt;
int i;
+ unsigned long rss_increment = 0;
stack_base = STACK_TOP - MAX_ARG_PAGES*PAGE_SIZE;
@@ -322,11 +325,14 @@
struct page *page = bprm->page[i];
if (page) {
bprm->page[i] = NULL;
- current->mm->rss++;
+ rss_increment++;
put_dirty_page(current,page,stack_base);
}
stack_base += PAGE_SIZE;
}
+ spin_lock(>mm->page_table_lock);
+ current->mm->rss += rss_increment;
+ spin_unlock(>mm->page_table_lock);
up_write(>mm->mmap_sem);
return 0;
--- linux-2.4.3-pre6/include/linux/mm.h Thu Mar 22 18:52:52 2001
+++ linux-akpm/include/linux/mm.h Fri Mar 23 23:32:00 2001
@@ -407,6 +407,8 @@
* On a two-level page table, this ends up being trivial. Thus the
* inlining and the symmetry break with pte_alloc() that does all
* of this out-of-line.
+ *
+ * __pmd_alloc() requires that mm->page_table_lock be held, so we do too.
*/
static inline pmd_t *pmd_alloc(struct mm_struct *mm, pgd_t *pgd, unsigned long
address)
{
--- linux-2.4.3-pre6/include/linux/sched.h Thu Mar 22 18:52:52 2001
+++ linux-akpm/include/linux/sched.hFri Mar 23 22:08:48 2001
@@ -209,9 +209,12 @@
atomic_t mm_count; /* How many references to "struct
mm_struct" (users count as 1) */
int map_count; /* number of VMAs */
struct rw_semaphore mmap_sem;
- spinlock_t page_table_lock;
+ spinlock_t page_table_lock; /* Protects task page tables and
+mm->rss */
- struct list_head mmlist;/* List of all active mm's */
+ struct list_head mmlist;/* List of all active mm's. These are
+globally strung
+* together off init_mm.mmlist, and
+are protected
+* by mmlist_lock
+*/
unsigned long start_code, end_code, start_data, end_data;
unsigned long start_brk, brk, start_stack;
--- linux-2.4.3-pre6/mm/memory.cFri Mar 23 22:16:55 2001
+++ linux-akpm/mm/memory.c Fri Mar 23 23:32:05 2001
@@ -374,7 +374,6 @@
address = (address + PGDIR_SIZE) & PGDIR_MASK;
dir++;
} while (address && (address < end));
- spin_unlock(>page_table_lock);
/*
* Update rss for the mm_struct (not necessarily current->mm)
* Notice that rss is an unsigned long.
@@ -383,6 +382,7 @@
mm->rss -= freed;
else
mm->rss = 0;
+ spin_unlock(>page_table_lock);
}
@@ -655,6 +655,7 @@
} while (address && (address < end));
}
+/* mm->page_table_lock must be held */
static inline int zeromap_pmd_range(struct mm_struct *mm, pmd_t * pmd, unsigned long
address,
unsigned long size, pgprot_t prot)
{
@@ -734,6 +735,7 @@
} while (address && (address < end));
}
+/* mm->page_table_lock must be held */
static inline int remap_pmd_range(struct mm_struct *mm, pmd_t * pmd, unsigned long
address, unsigned long size,
unsigned long phys_addr, pgprot_t prot)
{
@@ -792,6 +794,8 @@
* - flush the old one
* - update the page tables
* - inform the TLB about the new one
+ *
+ * We hold the mm semaphore for reading and vma->vm_mm->page_table_lock
*/
static inline void establish_pte(struct vm_area_struct * vma, unsigned long address,
pte_t
Re: 2.4.3-pre6: agpart.o causes arch/i386/mm/ioremap.c hang
I wrote: > Under linux-2.4.3-pre6 compiled for SMP, loading agpgart.o >hangs the system in remap_area_pages (arch/i386/mm/ioremap.c) at >the call to spin_lock(_mm.page_table_lock), which is not in 2.4.2. [...] > agp_backend_initialize > agp_generic_create_gatt_table > io_remap_nocache > __ioremap > remap_area_pages [...] > I'm rebuilding the kernel now with a modified spin_lock() >routine that should tell me who acquired the lock previously [...] In case anyone is interested, the conflicting lock of init_mm.page_table_lock was acquired in line 1318 of mm/memory.c, in pte_alloc. One way that this might be happening is that it looks like no page_table_lock is every acquired by vmalloc, which results in the following call graph: vmalloc __vmalloc vmalloc_area_pages alloc_area_pmd pte_alloc ...which assumes (here incorrectly) that mm->page_table_lock is held, and sometimes releases and reacquires mm->page_table_lock. I will attempt to analyze this further tomorrow if nobody beats me to it. Adam J. Richter __ __ 4880 Stevens Creek Blvd, Suite 104 [EMAIL PROTECTED] \ / San Jose, California 95129-1034 +1 408 261-6630 | g g d r a s i l United States of America fax +1 408 261-6631 "Free Software For The Rest Of Us." - 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/
2.4.3-pre6: agpart.o causes arch/i386/mm/ioremap.c hang
Under linux-2.4.3-pre6 compiled for SMP, loading agpgart.o hangs the system in remap_area_pages (arch/i386/mm/ioremap.c) at the call to spin_lock(_mm.page_table_lock), which is not in 2.4.2. When I load agpgart.o, I get the following messages: Linux agpgart interface v0.99 (c) Jeff Hartmann agpgart: Maximum main memory to use for agp memory: 690M agpgart: Detected Via Apollo Pro chipset After that, the console keys (RightAlt ScrollLock, Alt-F2, etc.) but there is not other response to my keystrokes and the system is no longer pingable. The call graphic is basically: agp_backend_initialize agp_generic_create_gatt_table io_remap_nocache __ioremap remap_area_pages I've made a cursory search through the kernel sources for what else might be holding this lock, but I have not yet found anything. I'm rebuilding the kernel now with a modified spin_lock() routine that should tell me who acquired the lock previously; however, I really do not understand this part of the kernel enough to know what the changes were intended to do in the first place. So, knowing where else the lock was acquired will not necessarily be enough for me to be able to generate a patch. Anyhow, I imagine that this lock is being held by some code that can block. We'll see. Adam J. Richter __ __ 4880 Stevens Creek Blvd, Suite 104 [EMAIL PROTECTED] \ / San Jose, California 95129-1034 +1 408 261-6630 | g g d r a s i l United States of America fax +1 408 261-6631 "Free Software For The Rest Of Us." - 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/
2.4.3-pre6: agpart.o causes arch/i386/mm/ioremap.c hang
Under linux-2.4.3-pre6 compiled for SMP, loading agpgart.o hangs the system in remap_area_pages (arch/i386/mm/ioremap.c) at the call to spin_lock(init_mm.page_table_lock), which is not in 2.4.2. When I load agpgart.o, I get the following messages: Linux agpgart interface v0.99 (c) Jeff Hartmann agpgart: Maximum main memory to use for agp memory: 690M agpgart: Detected Via Apollo Pro chipset After that, the console keys (RightAlt ScrollLock, Alt-F2, etc.) but there is not other response to my keystrokes and the system is no longer pingable. The call graphic is basically: agp_backend_initialize agp_generic_create_gatt_table io_remap_nocache __ioremap remap_area_pages I've made a cursory search through the kernel sources for what else might be holding this lock, but I have not yet found anything. I'm rebuilding the kernel now with a modified spin_lock() routine that should tell me who acquired the lock previously; however, I really do not understand this part of the kernel enough to know what the changes were intended to do in the first place. So, knowing where else the lock was acquired will not necessarily be enough for me to be able to generate a patch. Anyhow, I imagine that this lock is being held by some code that can block. We'll see. Adam J. Richter __ __ 4880 Stevens Creek Blvd, Suite 104 [EMAIL PROTECTED] \ / San Jose, California 95129-1034 +1 408 261-6630 | g g d r a s i l United States of America fax +1 408 261-6631 "Free Software For The Rest Of Us." - 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: 2.4.3-pre6: agpart.o causes arch/i386/mm/ioremap.c hang
I wrote: Under linux-2.4.3-pre6 compiled for SMP, loading agpgart.o hangs the system in remap_area_pages (arch/i386/mm/ioremap.c) at the call to spin_lock(init_mm.page_table_lock), which is not in 2.4.2. [...] agp_backend_initialize agp_generic_create_gatt_table io_remap_nocache __ioremap remap_area_pages [...] I'm rebuilding the kernel now with a modified spin_lock() routine that should tell me who acquired the lock previously [...] In case anyone is interested, the conflicting lock of init_mm.page_table_lock was acquired in line 1318 of mm/memory.c, in pte_alloc. One way that this might be happening is that it looks like no page_table_lock is every acquired by vmalloc, which results in the following call graph: vmalloc __vmalloc vmalloc_area_pages alloc_area_pmd pte_alloc ...which assumes (here incorrectly) that mm-page_table_lock is held, and sometimes releases and reacquires mm-page_table_lock. I will attempt to analyze this further tomorrow if nobody beats me to it. Adam J. Richter __ __ 4880 Stevens Creek Blvd, Suite 104 [EMAIL PROTECTED] \ / San Jose, California 95129-1034 +1 408 261-6630 | g g d r a s i l United States of America fax +1 408 261-6631 "Free Software For The Rest Of Us." - 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/
