From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753385AbbGKG4K (ORCPT ); Sat, 11 Jul 2015 02:56:10 -0400 Received: from mail-pd0-f175.google.com ([209.85.192.175]:34428 "EHLO mail-pd0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752630AbbGKG4I (ORCPT ); Sat, 11 Jul 2015 02:56:08 -0400 Date: Sat, 11 Jul 2015 10:48:12 +0900 From: Sergey Senozhatsky To: Andrew Morton Cc: Sergey Senozhatsky , Sergey Senozhatsky , Johannes Weiner , Minchan Kim , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [RFC] mm/shrinker: define INIT_SHRINKER macro Message-ID: <20150711014812.GD811@swordfish> References: <20150710011211.GB584@swordfish> <20150710153235.835c4992fbce526da23361d0@linux-foundation.org> <20150711012513.GB811@swordfish> <20150710183357.30605207.akpm@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150710183357.30605207.akpm@linux-foundation.org> User-Agent: Mutt/1.5.23+89 (0255b37be491) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On (07/10/15 18:33), Andrew Morton wrote: > > > > I was thinking of a trivial INIT_SHRINKER macro to init `struct shrinker' > > > > internal members (composed in email client, not tested) > > > > > > > > include/linux/shrinker.h > > > > > > > > #define INIT_SHRINKER(s) \ > > > > do { \ > > > > (s)->nr_deferred = NULL; \ > > > > INIT_LIST_HEAD(&(s)->list); \ > > > > } while (0) > > > > > > Spose so. Although it would be simpler to change unregister_shrinker() > > > to bale out if list.next==NULL and then say "all zeroes is the > > > initialized state". > > > > Yes, or '->nr_deferred == NULL' -- we can't have NULL ->nr_deferred > > in a properly registered shrinker (as of now) > > list.next seems safer because that will always be non-zero. But > whatever - we can change it later. > > > But that will not work if someone has accidentally passed not zeroed > > out pointer to unregister. > > I wouldn't worry about that really. If you pass a pointer to > uninitialized memory, the kernel will explode. That's true of just > about every pointer-accepting function in the kernel. > True. But with shrinker it's hard to say whether we have a properly initialized shrinker embedded in our `struct foo' or we don't (unless we treat register_shrinker() errors as a show stopper) by simply looking at shrinker struct (w/o touching it's private members). In zsmalloc, for instance, we don't consider failed register_shrinker() to be critical enough to forbid zs_pool creation and usage. It makes things harder later in zs_destroy_pool(), because we need to carry some sort of flag for that purpose. But `list.next' check in unregister_shrinker() would suffice in zsmalloc case, I must admit, because we kzalloc() the entire zs_pool struct. -ss