From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754886AbeARJCl (ORCPT ); Thu, 18 Jan 2018 04:02:41 -0500 Received: from Galois.linutronix.de ([146.0.238.70]:48217 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750974AbeARJCi (ORCPT ); Thu, 18 Jan 2018 04:02:38 -0500 Date: Thu, 18 Jan 2018 10:02:25 +0100 (CET) From: Thomas Gleixner To: Yang Shi cc: longman@redhat.com, LKML , Peter Zijlstra Subject: Re: [PATCH 2/2 v3] lib: debugobjects: touch watchdog to avoid softlockup when !CONFIG_PREEMPT In-Reply-To: Message-ID: References: <1515023802-54196-1-git-send-email-yang.s@alibaba-inc.com> <1515023802-54196-2-git-send-email-yang.s@alibaba-inc.com> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 18 Jan 2018, Yang Shi wrote: > On 1/17/18 4:21 AM, Thomas Gleixner wrote: > > There are two things which can be done here: > > > > 1) The collected objects can be put on a global free list and work > > scheduled to free them piecewise. > > I don't get your point here. objects free has already been done in a work. > free_object() -> schedule_work() But it's doing it in a loop for each freed object. > Do you mean free all of them out of the for loop in a batch? Then don't call > free_object() in the for loop? Somehing like this: diff --git a/lib/debugobjects.c b/lib/debugobjects.c index 2f5349c6e81a..d36940cdc658 100644 --- a/lib/debugobjects.c +++ b/lib/debugobjects.c @@ -42,6 +42,7 @@ static struct debug_obj obj_static_pool[ODEBUG_POOL_SIZE] __initdata; static DEFINE_RAW_SPINLOCK(pool_lock); static HLIST_HEAD(obj_pool); +static HLIST_HEAD(obj_to_free); static int obj_pool_min_free = ODEBUG_POOL_SIZE; static int obj_pool_free = ODEBUG_POOL_SIZE; @@ -714,13 +715,12 @@ EXPORT_SYMBOL_GPL(debug_object_active_state); static void __debug_check_no_obj_freed(const void *address, unsigned long size) { unsigned long flags, oaddr, saddr, eaddr, paddr, chunks; - struct hlist_node *tmp; - HLIST_HEAD(freelist); struct debug_obj_descr *descr; enum debug_obj_state state; struct debug_bucket *db; + struct hlist_node *tmp; struct debug_obj *obj; - int cnt; + int cnt, freed = 0; saddr = (unsigned long) address; eaddr = saddr + size; @@ -751,21 +751,22 @@ static void __debug_check_no_obj_freed(const void *address, unsigned long size) goto repeat; default: hlist_del(&obj->node); - hlist_add_head(&obj->node, &freelist); + /* Put them on the freelist */ + raw_spin_lock_irqsave(&pool_lock, flags); + hlist_add_head(&obj->node, &obj_to_free); + raw_spin_lock_irqrestore(&pool_lock, flags); + freed++; break; } } raw_spin_unlock_irqrestore(&db->lock, flags); - /* Now free them */ - hlist_for_each_entry_safe(obj, tmp, &freelist, node) { - hlist_del(&obj->node); - free_object(obj); - } - if (cnt > debug_objects_maxchain) debug_objects_maxchain = cnt; } + + if (freed) + schedule_work(.....); The allocation side can look at the free list as well and grab objects from there if the pool level is low if that happens before the work can do that. > > > > 2) We can do a cond_resched() if not in atomic context and interrupts are > > enabled. > > I did try this before I went with touching softlockup watchdog approach. The > problem is in_atomic() can't tell if it is in atomic context on non-preempt > kernel. For preempt kernel, it is easy. Peter, can we do anything about that? Thanks, tglx