Re: + acpi_os_acquire_object-gfp_kernel-called-with-irqs.patch added to -mm tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 13 Feb 2006 18:52:44 -0500
Dave Jones <[email protected]> wrote:

> On Mon, Feb 13, 2006 at 03:13:55PM -0800, Andrew Morton wrote:
> 
>  > The patch titled
>  > 
>  >      acpi_os_acquire_object (GFP_KERNEL) called with IRQs disabled through suspend-resume
>  > 
>  > has been added to the -mm tree.  Its filename is
>  > 
>  >      acpi_os_acquire_object-gfp_kernel-called-with-irqs.patch
>  > 
>  > From: Davi Arnaut <[email protected]>
>  > 
>  > acpi_os_acquire_object() gets called, with IRQs disabled, from:
>  > 
>  > Debug: sleeping function called from invalid context at mm/slab.c:2499
>  > in_atomic():0, irqs_disabled():1
>  >  [<c01462f3>] kmem_cache_alloc+0x40/0x4f     [<c0202c85>] acpi_os_acquire_object+0xb/0x3c
>  >  [<c02171b1>] acpi_ut_allocate_object_desc_dbg+0x13/0x49     [<c021704b>] acpi_ut_create_internal_object_dbg+0xf/0x5e
>  >  [<c02136d4>] acpi_rs_set_srs_method_data+0x3d/0xb9     [<c021aa3d>] acpi_pci_link_set+0x102/0x17b
>  >  [<c021aecb>] irqrouter_resume+0x1e/0x3c     [<c024d921>] __sysdev_resume+0x11/0x6b
>  >  [<c024dbde>] sysdev_resume+0x34/0x52     [<c0251cb7>] device_power_up+0x5/0xa
>  >  [<c0138787>] suspend_enter+0x44/0x46     [<c01386e5>] suspend_prepare+0x63/0xc1
>  >  [<c0138813>] enter_state+0x5e/0x7c     [<c013894c>] state_store+0x81/0x8f
>  >  [<c01388cb>] state_store+0x0/0x8f     [<c0196a0a>] subsys_attr_store+0x1e/0x22
>  >  [<c0196c12>] flush_write_buffer+0x22/0x28     [<c0196c64>] sysfs_write_file+0x4c/0x71
>  >  [<c0196c18>] sysfs_write_file+0x0/0x71     [<c015b2c9>] vfs_write+0xa2/0x15a
>  >  [<c015b42c>] sys_write+0x41/0x6a     [<c0102e75>] syscall_call+0x7/0xb
>  > 
>  > The patch also fixes a missing check for NULL return from
>  > acpi_os_acquire_object().
>  > 
>  > Signed-off-by: Davi Arnaut <[email protected]>
>  > Cc: "Brown, Len" <[email protected]>
>  > Cc: Pavel Machek <[email protected]>
>  > Signed-off-by: Andrew Morton <[email protected]>
>  > ---
>  > 
>  >  drivers/acpi/osl.c            |    2 +-
>  >  drivers/acpi/parser/psutils.c |    8 ++++++--
>  >  2 files changed, 7 insertions(+), 3 deletions(-)
>  > 
>  > diff -puN drivers/acpi/osl.c~acpi_os_acquire_object-gfp_kernel-called-with-irqs drivers/acpi/osl.c
>  > --- devel/drivers/acpi/osl.c~acpi_os_acquire_object-gfp_kernel-called-with-irqs	2006-02-13 15:13:26.000000000 -0800
>  > +++ devel-akpm/drivers/acpi/osl.c	2006-02-13 15:13:26.000000000 -0800
>  > @@ -1175,7 +1175,7 @@ acpi_status acpi_os_release_object(acpi_
>  >  
>  >  void *acpi_os_acquire_object(acpi_cache_t * cache)
>  >  {
>  > -	void *object = kmem_cache_alloc(cache, GFP_KERNEL);
>  > +	void *object = kmem_cache_alloc(cache, GFP_ATOMIC);
>  >  	WARN_ON(!object);
>  >  	return object;
>  >  }
> 
> This is potentially a lot of atomic allocations.
> 
> acpi_os_acquire_object is called by..
> 
> drivers/acpi/parser/psutils.c  acpi_ps_alloc_op()
> drivers/acpi/utilities/utobject.c:      acpi_ut_allocate_object_desc_dbg()
> drivers/acpi/utilities/utstate.c:       acpi_ut_create_generic_state()
> 
> Those three functions are called all over the place in acpi.
> 
> drivers/acpi/dispatcher/dsmethod.c:     op = acpi_ps_alloc_op(AML_METHOD_OP);
> drivers/acpi/dispatcher/dsopcode.c:     op = acpi_ps_alloc_op(AML_INT_EVAL_SUBTREE_OP);
> drivers/acpi/dispatcher/dsopcode.c:     op = acpi_ps_alloc_op(AML_INT_EVAL_SUBTREE_OP);
> drivers/acpi/dispatcher/dswload.c:              op = acpi_ps_alloc_op(walk_state->opcode);
> drivers/acpi/dispatcher/dswload.c:              op = acpi_ps_alloc_op(walk_state->opcode);
> drivers/acpi/parser/psargs.c:           name_op = acpi_ps_alloc_op(AML_INT_NAMEPATH_OP);
> drivers/acpi/parser/psargs.c:   field = acpi_ps_alloc_op(opcode);
> drivers/acpi/parser/psargs.c:           arg = acpi_ps_alloc_op(AML_BYTE_OP);
> drivers/acpi/parser/psargs.c:                   arg = acpi_ps_alloc_op(AML_INT_BYTELIST_OP);
> drivers/acpi/parser/psargs.c:                   arg = acpi_ps_alloc_op(AML_INT_NAMEPATH_OP);
> drivers/acpi/parser/psloop.c:                                       acpi_ps_alloc_op(walk_state->
> drivers/acpi/parser/psloop.c:                           op = acpi_ps_alloc_op(walk_state->opcode);
> drivers/acpi/parser/psparse.c:                      acpi_ps_alloc_op(AML_INT_RETURN_VALUE_OP);
> drivers/acpi/parser/psparse.c:                              acpi_ps_alloc_op(AML_INT_RETURN_VALUE_OP);
> drivers/acpi/parser/psparse.c:                                      acpi_ps_alloc_op(op->common.
> drivers/acpi/parser/psparse.c:                      acpi_ps_alloc_op(AML_INT_RETURN_VALUE_OP);
> drivers/acpi/parser/psutils.c:  scope_op = acpi_ps_alloc_op(AML_SCOPE_OP);
> drivers/acpi/utilities/utobject.c:          acpi_ut_allocate_object_desc_dbg(module_name, line_number,
> drivers/acpi/utilities/utobject.c:              second_object = acpi_ut_allocate_object_desc_dbg(module_name,
> drivers/acpi/dispatcher/dswscope.c:     scope_info = acpi_ut_create_generic_state();
> drivers/acpi/dispatcher/dswstate.c:     state = acpi_ut_create_generic_state();
> drivers/acpi/events/evmisc.c:           notify_info = acpi_ut_create_generic_state();
> drivers/acpi/namespace/nseval.c:        scope_info = acpi_ut_create_generic_state();
> drivers/acpi/parser/psscope.c:  scope = acpi_ut_create_generic_state();
> drivers/acpi/parser/psscope.c:  scope = acpi_ut_create_generic_state();
> drivers/acpi/utilities/utstate.c:       state = acpi_ut_create_generic_state();
> drivers/acpi/utilities/utstate.c:       state = acpi_ut_create_generic_state();
> drivers/acpi/utilities/utstate.c:       state = acpi_ut_create_generic_state();
> drivers/acpi/utilities/utstate.c:       state = acpi_ut_create_generic_state();
> 
> I'll bet these callsites are all also called from multiple locations.
> 
> 		Dave
> 

You are right, this one instead should work better.

Signed-off-by: Davi Arnaut <[email protected]>
--

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index ac5bbae..8d44b0d 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -1175,7 +1175,12 @@ acpi_status acpi_os_release_object(acpi_
 
 void *acpi_os_acquire_object(acpi_cache_t * cache)
 {
-	void *object = kmem_cache_alloc(cache, GFP_KERNEL);
+	void *object;
+	
+	if (acpi_in_resume)
+		object = kmem_cache_alloc(cache, GFP_ATOMIC);
+	else
+		object = kmem_cache_alloc(cache, GFP_KERNEL);
 	WARN_ON(!object);
 	return object;
 }
diff --git a/drivers/acpi/parser/psutils.c b/drivers/acpi/parser/psutils.c
index 3e07cb9..0d3c56a 100644
--- a/drivers/acpi/parser/psutils.c
+++ b/drivers/acpi/parser/psutils.c
@@ -138,12 +138,16 @@ union acpi_parse_object *acpi_ps_alloc_o
 		/* The generic op (default) is by far the most common (16 to 1) */
 
 		op = acpi_os_acquire_object(acpi_gbl_ps_node_cache);
-		memset(op, 0, sizeof(struct acpi_parse_obj_common));
+
+		if (op)
+			memset(op, 0, sizeof(struct acpi_parse_obj_common));
 	} else {
 		/* Extended parseop */
 
 		op = acpi_os_acquire_object(acpi_gbl_ps_node_ext_cache);
-		memset(op, 0, sizeof(struct acpi_parse_obj_named));
+
+		if (op)
+			memset(op, 0, sizeof(struct acpi_parse_obj_named));
 	}
 
 	/* Initialize the Op */

-
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/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux