Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755336AbbLDQQm (ORCPT ); Fri, 4 Dec 2015 11:16:42 -0500 Received: from resqmta-ch2-07v.sys.comcast.net ([69.252.207.39]:44683 "EHLO resqmta-ch2-07v.sys.comcast.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753823AbbLDQQj (ORCPT ); Fri, 4 Dec 2015 11:16:39 -0500 Date: Fri, 4 Dec 2015 10:16:38 -0600 (CST) From: Christoph Lameter X-X-Sender: cl@east.gentwo.org To: Geliang Tang cc: Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] mm/slab.c: use list_{empty_careful,last_entry} in drain_freelist In-Reply-To: <20151204134302.GA6388@bogon> Message-ID: References: <3ea815dc52bf1a2bb5e324d7398315597900be84.1449151365.git.geliangtang@163.com> <20151204134302.GA6388@bogon> Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1424 Lines: 46 On Fri, 4 Dec 2015, Geliang Tang wrote: > On Thu, Dec 03, 2015 at 08:53:21AM -0600, Christoph Lameter wrote: > > On Thu, 3 Dec 2015, Geliang Tang wrote: > > > > > while (nr_freed < tofree && !list_empty(&n->slabs_free)) { > > > > > > spin_lock_irq(&n->list_lock); > > > - p = n->slabs_free.prev; > > > - if (p == &n->slabs_free) { > > > + if (list_empty_careful(&n->slabs_free)) { > > > > We have taken the lock. Why do we need to be "careful"? list_empty() > > shoudl work right? > > Yes. list_empty() is OK. > > > > > > spin_unlock_irq(&n->list_lock); > > > goto out; > > > } > > > > > > - page = list_entry(p, struct page, lru); > > > + page = list_last_entry(&n->slabs_free, struct page, lru); > > > > last??? > > The original code delete the page from the tail of slabs_free list. Maybe make the code clearer by using another method to get the page pointer? > > > > Would the the other new function that returns NULL on the empty list or > > the pointer not be useful here too and save some code? > > Sorry, I don't really understand what do you mean. Can you please specify > it a little bit? I take that back. list_empty is the best choice here. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/