Hi Sudeep, On 2021/7/20 21:37, Sudeep Holla wrote: > On Tue, Jul 20, 2021 at 07:26:35PM +0800, Xiongfeng Wang wrote: >> When I added might_sleep() in down_timeout(), I got the following > > Sorry it is not clear if you are able to reproduce this issue without > any other modifications in the mainline kernel ? Without any modifications, the mainline kernel does not print the Calltrace. But the risk of sleeping in atomic context still exists. > >> Calltrace: >> >> [ 8.775671] BUG: sleeping function called from invalid context at kernel/locking/semaphore.c:160 >> [ 8.777070] in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 14, name: cpuhp/0 > >>From this I guess you are adding sleep after raw_spin_lock_irqsave > in down_timeout(kernel/locking/semaphore.c). I add 'might_sleep()' which is used to check whether there are problems if I sleep here. It's not a real sleep. The Document for might_sleep is as below. /** * might_sleep - annotation for functions that can sleep * * this macro will print a stack trace if it is executed in an atomic * context (spinlock, irq-handler, ...). Additional sections where blocking is * not allowed can be annotated with non_block_start() and non_block_end() * pairs. * * This is a useful debugging help to be able to catch problems early and not * be bitten later when the calling function happens to sleep when it is not * supposed to. */ > >> >> It is because generic_exec_single() will disable local irq before >> calling _init_cache_level(). _init_cache_level() use acpi_get_table() to >> get the PPTT table, but this function could schedule out. >> >> To fix this issue, we use a static pointer to record the mapped PPTT >> table in the first beginning. Later, we use that pointer to reference >> the PPTT table in acpi_find_last_cache_level(). We also modify other >> functions in pptt.c to use the pointer to reference PPTT table. The main problem is that we called local_irq_save() in generic_exec_single(), and then we called down_timeout() in acpi_os_wait_semaphore(). down_timeout() could enter into sleep if failed to acquire the semaphore. There are risks of sleeping in irq disabled context. Thanks, Xiongfeng >> > > I don't follow this change at all. >
