[uml-devel] [UML] fix crash in block layer

[uml-devel] [UML] fix crash in block layer

[Jason Lunz]

[resent, axboe@suse.de bounced]

When the device-mapper DM_DEV_CREATE_CMD ioctl is called to create a new
device, dev_create()->dm_create()->alloc_dev()->
blk_queue_bounce_limit(md->queue, BLK_BOUNCE_ANY) happens.

blk_queue_bounce_limit(BLK_BOUNCE_ANY) calls init_emergency_isa_pool()
if blk_max_pfn < blk_max_low_pfn. This is the case on UML, but
init_emergency_isa_pool() hits BUG_ON(!isa_page_pool) because there
doesn't seem to be a dma zone on UML for mempool_create() to allocate from.

Most architectures seem to have max_low_pfn == max_pfn, but UML doesn't
because of the uml_reserved chunk it keeps for itself. From what I can
see, max_pfn and max_low_pfn don't get much use after the
bootmem-allocator stops being used anyway, except that they initialize
the block layer's blk_max_low_pfn/blk_max_pfn.

With this applied I can use lvm to create logical volumes without
crashing UML.

Signed-off-by: Jason Lunz <lunz@falooley.org>

---
 arch/um/kernel/mem.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-2.6.20-rc5-uml/arch/um/kernel/mem.c
===================================================================
--- linux-2.6.20-rc5-uml.orig/arch/um/kernel/mem.c
+++ linux-2.6.20-rc5-uml/arch/um/kernel/mem.c
@@ -63,7 +63,7 @@
 
 void mem_init(void)
 {
-	max_low_pfn = (high_physmem - uml_physmem) >> PAGE_SHIFT;
+	max_pfn = max_low_pfn = (high_physmem - uml_physmem) >> PAGE_SHIFT;
 
         /* clear the zero-page */
         memset((void *) empty_zero_page, 0, PAGE_SIZE);
@@ -84,7 +84,6 @@
 	totalram_pages += totalhigh_pages;
 #endif
 	num_physpages = totalram_pages;
-	max_pfn = totalram_pages;
 	printk(KERN_INFO "Memory: %luk available\n", 
 	       (unsigned long) nr_free_pages() << (PAGE_SHIFT-10));
 	kmalloc_ok = 1;

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [UML] fix crash in block layer

[Jeff Dike]

On Thu, Feb 15, 2007 at 11:00:01AM -0500, Jason Lunz wrote:
> Permit lvm to create logical volumes without crashing UML.

Thanks, this is in my tree.

			Jeff

-- 
Work email - jdike at linux dot intel dot com

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [UML] fix crash in block layer

[Blaisorblade]

On Thursday 15 February 2007 18:09, Jeff Dike wrote:
> On Thu, Feb 15, 2007 at 11:00:01AM -0500, Jason Lunz wrote:
> > Permit lvm to create logical volumes without crashing UML.
>
> Thanks, this is in my tree.

Hmm, this seems not a _correct_ fix - at least it will be buggy with high 
memory (which we could drop anyway - actually we _should_ drop it probably). 
And at a closer review other potential bugs (or at least misconceptions) 
surface.

The difference between low_pfn and pfn should be just high memory accounting - 
however also uml_reserved enters the picture, and that is bad.

From a closer look, it seems that uml_reserved is ignored for max_low_pfn and 
considered for max_pfn - with your code, you are going to ignore it 
everywhere, while probably you should consider it everywhere (i.e. use 
totalram_pages everywhere) - if it is correct to consider it (and I believe 
the code was written to do all that for an important reason).

The following could be a suggestion, if max_low_pfn is not used between the 
old and the new moment of assignment (and it seems it is not). This is just 
an idea however:

mem_init:

-        max_low_pfn = ...
        /* this will put all low memory onto the freelists */
        totalram_pages = free_all_bootmem();
+        max_low_pfn = totalram_pages;
#ifdef CONFIG_HIGHMEM
        totalhigh_pages = highmem >> PAGE_SHIFT;
        totalram_pages += totalhigh_pages;
#endif
        num_physpages = totalram_pages;
        max_pfn = totalram_pages;

Please note that I did not spend a lot of time on this, so everything could be 
wrong. However, testing cannot help with uml_reserved handling, and this is a 
dark corner.  So things should be better understood before merging the patch.

The code is too convoluted for a brief look - drawing a picture which explains 
all those variables would help. Both for UML and for every arch...

BTW: init_bootmem is not called by us and we probably should - min_low_pfn is 
not initialized.
-- 
Inform me of my mistakes, so I can add them to my list!
Paolo Giarrusso, aka Blaisorblade
http://www.user-mode-linux.org/~blaisorblade

Chiacchiera con i tuoi amici in tempo reale! 
 http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com 


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [UML] fix crash in block layer

[Jason Lunz]

On Thu, Feb 15, 2007 at 08:29:28PM +0100, Blaisorblade wrote:
> The following could be a suggestion, if max_low_pfn is not used between the 
> old and the new moment of assignment (and it seems it is not). This is just 
> an idea however:
> 
> mem_init:
> 
> -        max_low_pfn = ...
>         /* this will put all low memory onto the freelists */
>         totalram_pages = free_all_bootmem();
> +        max_low_pfn = totalram_pages;
> #ifdef CONFIG_HIGHMEM
>         totalhigh_pages = highmem >> PAGE_SHIFT;
>         totalram_pages += totalhigh_pages;
> #endif
>         num_physpages = totalram_pages;
>         max_pfn = totalram_pages;
>
> Please note that I did not spend a lot of time on this, so everything
> could be wrong. However, testing cannot help with uml_reserved
> handling, and this is a dark corner.  So things should be better
> understood before merging the patch.

I agree - I have only a vague idea about what uml_reserved means.

> The code is too convoluted for a brief look - drawing a picture which
> explains all those variables would help. Both for UML and for every
> arch...

blk_queue_bounce_limit() calls init_emergency_isa_pool() to get dma-zone
pages to use as bounce buffers when its caller passes a dma_addr limit
that is less than max_low_pfn. The BLK_BOUNCE_ANY macro is supposed to
mean "never bounce", and it's defined as:

#define BLK_BOUNCE_ANY          ((u64)blk_max_pfn << PAGE_SHIFT)

So it presumes that max_pfn >= max_low_pfn. uml's mem_init() is
violating this assumption - when uml_reserved is subtracted from
max_pfn, we end up with max_pfn < max_low_pfn, so BLK_BOUNCE_ANY has the
opposite of the intended effect. blk_queue_bounce_limit therefore tries
to create a mempool with zone-dma pages on a no-dma-zone arch and the
kernel goes BUG().

So I think your idea is correct. It passes my testing - I can still use
lvm within uml. I have not tested CONFIG_HIGHMEM, but here's an
implementation against 2.6.20.

Jeff, please drop my other patch and use this one.

Signed-off-by: Jason Lunz <lunz@falooley.org>

---
 arch/um/kernel/mem.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-2.6.20-uml/arch/um/kernel/mem.c
===================================================================
--- linux-2.6.20-uml.orig/arch/um/kernel/mem.c
+++ linux-2.6.20-uml/arch/um/kernel/mem.c
@@ -63,8 +63,6 @@
 
 void mem_init(void)
 {
-	max_low_pfn = (high_physmem - uml_physmem) >> PAGE_SHIFT;
-
         /* clear the zero-page */
         memset((void *) empty_zero_page, 0, PAGE_SIZE);
 
@@ -79,6 +77,7 @@
 
 	/* this will put all low memory onto the freelists */
 	totalram_pages = free_all_bootmem();
+	max_low_pfn = totalram_pages;
 #ifdef CONFIG_HIGHMEM
 	totalhigh_pages = highmem >> PAGE_SHIFT;
 	totalram_pages += totalhigh_pages;

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [UML] fix crash in block layer

[Jeff Dike]

On Fri, Feb 16, 2007 at 12:02:08PM -0500, Jason Lunz wrote:
> I agree - I have only a vague idea about what uml_reserved means.

This is ancient code - after a quick look through it, I think what is
happening is this:
	Early in boot, there are both libc and kernel (bootmem) memory
allocations happening.  We can't redirect malloc to kmalloc yet, so
mallocs are allowed to happen until kmalloc is running.  This requires
that the memory setup code leave some empty room in the address space
for malloc to grow into.  The end of this area is uml_reserved.
	When we are ready to turn on kmalloc, the rest of UML physical
memory (beyond uml_reserved) was already available to the bootmem
allocator, and it is just released to the page allocator.  The area
that wasn't malloced by libc is released separately to the page
allocator.
	At that point, uml_reserved loses its meaning, since memory on
either side of it is treated identically by the page allocator.

> Jeff, please drop my other patch and use this one.

OK.

				Jeff

-- 
Work email - jdike at linux dot intel dot com

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [UML] fix crash in block layer

[Blaisorblade]

On Friday 16 February 2007 19:40, Jeff Dike wrote:
> On Fri, Feb 16, 2007 at 12:02:08PM -0500, Jason Lunz wrote:
> > I agree - I have only a vague idea about what uml_reserved means.
>
> This is ancient code - after a quick look through it, I think what is
> happening is this:
> 	Early in boot, there are both libc and kernel (bootmem) memory
> allocations happening.  We can't redirect malloc to kmalloc yet, so
> mallocs are allowed to happen until kmalloc is running.  This requires
> that the memory setup code leave some empty room in the address space
> for malloc to grow into.  The end of this area is uml_reserved.
> 	When we are ready to turn on kmalloc, the rest of UML physical
> memory (beyond uml_reserved) was already available to the bootmem
> allocator, and it is just released to the page allocator.  The area
> that wasn't malloced by libc is released separately to the page
> allocator.

I was also curious about the "high_physmem - uml_physmem" difference, which I 
now guess I understand - and about seeing this documented so that redundant 
vars could be deleted.

> 	At that point, uml_reserved loses its meaning, since memory on
> either side of it is treated identically by the page allocator.

But the purpose of:
        uml_reserved = brk_end;
is probably to restore this meaning: after this, uml_reserved marks again the 
end of the reserved memory, right?

And now that you explain this me, I'm wondering.
How do things work when kmalloc_ok is disabled (i.e. initial_thread_cb)? I 
thought all reserved memory would stay reserved for the whole UML kernel 
lifetime.
-- 
Inform me of my mistakes, so I can add them to my list!
Paolo Giarrusso, aka Blaisorblade
http://www.user-mode-linux.org/~blaisorblade

Chiacchiera con i tuoi amici in tempo reale! 
 http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com 


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [UML] fix crash in block layer

[Blaisorblade]

On Friday 16 February 2007 18:02, Jason Lunz wrote:
> On Thu, Feb 15, 2007 at 08:29:28PM +0100, Blaisorblade wrote:
> > The following could be a suggestion, if max_low_pfn is not used between
> > the old and the new moment of assignment (and it seems it is not). This
> > is just an idea however:

> blk_queue_bounce_limit() calls init_emergency_isa_pool() to get dma-zone
> pages to use as bounce buffers when its caller passes a dma_addr limit
> that is less than max_low_pfn. The BLK_BOUNCE_ANY macro is supposed to
> mean "never bounce", and it's defined as:

> #define BLK_BOUNCE_ANY          ((u64)blk_max_pfn << PAGE_SHIFT)

> So it presumes that max_pfn >= max_low_pfn. uml's mem_init() is
> violating this assumption - when uml_reserved is subtracted from
> max_pfn, we end up with max_pfn < max_low_pfn, so BLK_BOUNCE_ANY has the
> opposite of the intended effect. blk_queue_bounce_limit therefore tries
> to create a mempool with zone-dma pages on a no-dma-zone arch and the
> kernel goes BUG().

Thanks for the explaination; but I meant that the meaning of those variables 
and the code to set them was convoluted.

> So I think your idea is correct. It passes my testing - I can still use
> lvm within uml. I have not tested CONFIG_HIGHMEM, but here's an 
> implementation against 2.6.20.

Do not worry for HIGHMEM - I doubt it builds currently; uml_reserved is also 
used for other stuff.

I've also verified that moving the assignment to max_low_pfn to later cannot 
possibly introduce bugs, as max_{low_,}pfn are not used by core kernel code.

> Jeff, please drop my other patch and use this one.

> Signed-off-by: Jason Lunz <lunz@falooley.org>
>
> ---
>  arch/um/kernel/mem.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> Index: linux-2.6.20-uml/arch/um/kernel/mem.c
> ===================================================================
> --- linux-2.6.20-uml.orig/arch/um/kernel/mem.c
> +++ linux-2.6.20-uml/arch/um/kernel/mem.c
> @@ -63,8 +63,6 @@
>
>  void mem_init(void)
>  {
> -	max_low_pfn = (high_physmem - uml_physmem) >> PAGE_SHIFT;
> -
>          /* clear the zero-page */
>          memset((void *) empty_zero_page, 0, PAGE_SIZE);
>
> @@ -79,6 +77,7 @@
>
>  	/* this will put all low memory onto the freelists */
>  	totalram_pages = free_all_bootmem();
> +	max_low_pfn = totalram_pages;
>  #ifdef CONFIG_HIGHMEM
>  	totalhigh_pages = highmem >> PAGE_SHIFT;
>  	totalram_pages += totalhigh_pages;

-- 
Inform me of my mistakes, so I can add them to my list!
Paolo Giarrusso, aka Blaisorblade
http://www.user-mode-linux.org/~blaisorblade
Chiacchiera con i tuoi amici in tempo reale! 
 http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com 


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

[uml-devel] [UML] fix crash in block layer

[Jason Lunz]

When the device-mapper DM_DEV_CREATE_CMD ioctl is called to create a new
device, dev_create()->dm_create()->alloc_dev()->
blk_queue_bounce_limit(md->queue, BLK_BOUNCE_ANY) happens.

blk_queue_bounce_limit(BLK_BOUNCE_ANY) calls init_emergency_isa_pool()
if blk_max_pfn < blk_max_low_pfn. This is the case on UML, but
init_emergency_isa_pool() hits BUG_ON(!isa_page_pool) because there
doesn't seem to be a dma zone on UML for mempool_create() to allocate from.

Most architectures seem to have max_low_pfn == max_pfn, but UML doesn't
because of the uml_reserved chunk it keeps for itself. From what I can
see, max_pfn and max_low_pfn don't get much use after the
bootmem-allocator stops being used anyway, except that they initialize
the block layer's blk_max_low_pfn/blk_max_pfn.

With this applied I can use lvm to create logical volumes without
crashing UML.

Signed-off-by: Jason Lunz <lunz@falooley.org>

---
 arch/um/kernel/mem.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-2.6.20-rc5-uml/arch/um/kernel/mem.c
===================================================================
--- linux-2.6.20-rc5-uml.orig/arch/um/kernel/mem.c
+++ linux-2.6.20-rc5-uml/arch/um/kernel/mem.c
@@ -63,7 +63,7 @@
 
 void mem_init(void)
 {
-	max_low_pfn = (high_physmem - uml_physmem) >> PAGE_SHIFT;
+	max_pfn = max_low_pfn = (high_physmem - uml_physmem) >> PAGE_SHIFT;
 
         /* clear the zero-page */
         memset((void *) empty_zero_page, 0, PAGE_SIZE);
@@ -84,7 +84,6 @@
 	totalram_pages += totalhigh_pages;
 #endif
 	num_physpages = totalram_pages;
-	max_pfn = totalram_pages;
 	printk(KERN_INFO "Memory: %luk available\n", 
 	       (unsigned long) nr_free_pages() << (PAGE_SHIFT-10));
 	kmalloc_ok = 1;

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel