From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932086AbdBNQu4 (ORCPT ); Tue, 14 Feb 2017 11:50:56 -0500 Received: from mx2.suse.de ([195.135.220.15]:55389 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754423AbdBNQut (ORCPT ); Tue, 14 Feb 2017 11:50:49 -0500 Subject: Re: [PATCH] oom_reaper: switch to struct list_head for reap queue To: Johannes Weiner References: <20170214150714.6195-1-asarai@suse.de> <20170214163005.GA2450@cmpxchg.org> Cc: Ingo Molnar , Peter Zijlstra , Andrew Morton , Michal Hocko , Oleg Nesterov , linux-kernel@vger.kernel.org, linux-mm@kvack.org, cyphar@cyphar.com From: Aleksa Sarai Message-ID: Date: Wed, 15 Feb 2017 03:52:11 +1100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <20170214163005.GA2450@cmpxchg.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >> Rather than implementing an open addressing linked list structure >> ourselves, use the standard list_head structure to improve consistency >> with the rest of the kernel and reduce confusion. >> >> Cc: Michal Hocko >> Cc: Oleg Nesterov >> Signed-off-by: Aleksa Sarai >> --- >> include/linux/sched.h | 6 +++++- >> kernel/fork.c | 4 ++++ >> mm/oom_kill.c | 24 +++++++++++++----------- >> 3 files changed, 22 insertions(+), 12 deletions(-) >> >> diff --git a/include/linux/sched.h b/include/linux/sched.h >> index e93594b88130..d8bcd0f8c5fe 100644 >> --- a/include/linux/sched.h >> +++ b/include/linux/sched.h >> @@ -1960,7 +1960,11 @@ struct task_struct { >> #endif >> int pagefault_disabled; >> #ifdef CONFIG_MMU >> - struct task_struct *oom_reaper_list; >> + /* >> + * List of threads that have to be reaped by OOM (rooted at >> + * &oom_reaper_list in mm/oom_kill.c). >> + */ >> + struct list_head oom_reaper_list; > > This is an extra pointer to task_struct and more lines of code to > accomplish the same thing. Why would we want to do that? I don't think it's more "actual" lines of code (I think the wrapping is inflating the line number count), but switching it means that it's more in line with other queues in the kernel (it took me a bit to figure out what was going on with oom_reaper_list beforehand). -- Aleksa Sarai Software Engineer (Containers) SUSE Linux GmbH https://www.cyphar.com/