From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-f70.google.com (mail-lf0-f70.google.com [209.85.215.70]) by kanga.kvack.org (Postfix) with ESMTP id E085A6B0038 for ; Fri, 25 Nov 2016 13:34:26 -0500 (EST) Received: by mail-lf0-f70.google.com with SMTP id t196so29235125lff.3 for ; Fri, 25 Nov 2016 10:34:26 -0800 (PST) Received: from mail-lf0-x243.google.com (mail-lf0-x243.google.com. [2a00:1450:4010:c07::243]) by mx.google.com with ESMTPS id 80si21273044lfy.236.2016.11.25.10.34.25 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 25 Nov 2016 10:34:25 -0800 (PST) Received: by mail-lf0-x243.google.com with SMTP id p100so3989488lfg.2 for ; Fri, 25 Nov 2016 10:34:25 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <20161115165538.878698352bd45e212751b57a@gmail.com> <20161115170030.f0396011fa00423ff711a3b4@gmail.com> From: Dan Streetman Date: Fri, 25 Nov 2016 13:33:44 -0500 Message-ID: Subject: Re: [PATCH 2/3] z3fold: don't fail kernel build if z3fold_header is too big Content-Type: text/plain; charset=UTF-8 Sender: owner-linux-mm@kvack.org List-ID: To: Vitaly Wool Cc: Linux-MM , linux-kernel , Andrew Morton , Arnd Bergmann On Fri, Nov 25, 2016 at 11:25 AM, Vitaly Wool wrote: > On Fri, Nov 25, 2016 at 4:59 PM, Dan Streetman wrote: >> On Tue, Nov 15, 2016 at 11:00 AM, Vitaly Wool wrote: >>> Currently the whole kernel build will be stopped if the size of >>> struct z3fold_header is greater than the size of one chunk, which >>> is 64 bytes by default. This may stand in the way of automated >>> test/debug builds so let's remove that and just fail the z3fold >>> initialization in such case instead. >>> >>> Signed-off-by: Vitaly Wool >>> --- >>> mm/z3fold.c | 11 ++++++++--- >>> 1 file changed, 8 insertions(+), 3 deletions(-) >>> >>> diff --git a/mm/z3fold.c b/mm/z3fold.c >>> index 7ad70fa..ffd9353 100644 >>> --- a/mm/z3fold.c >>> +++ b/mm/z3fold.c >>> @@ -870,10 +870,15 @@ MODULE_ALIAS("zpool-z3fold"); >>> >>> static int __init init_z3fold(void) >>> { >>> - /* Make sure the z3fold header will fit in one chunk */ >>> - BUILD_BUG_ON(sizeof(struct z3fold_header) > ZHDR_SIZE_ALIGNED); >> >> Nak. this is the wrong way to handle this. The build bug is there to >> indicate to you that your patch makes the header too large, not as a >> runtime check to disable everything. > > Okay, let's agree to drop it. > >> The right way to handle it is to change the hardcoded assumption that >> the header fits into a single chunk; e.g.: >> >> #define ZHDR_SIZE_ALIGNED round_up(sizeof(struct z3fold_header), CHUNK_SIZE) >> #define ZHDR_CHUNKS (ZHDR_SIZE_ALIGNED >> CHUNK_SHIFT) >> >> then use ZHDR_CHUNKS in all places where it's currently assumed the >> header is 1 chunk, e.g. in num_free_chunks: >> >> if (zhdr->middle_chunks != 0) { >> int nfree_before = zhdr->first_chunks ? >> - 0 : zhdr->start_middle - 1; >> + 0 : zhdr->start_middle - ZHDR_CHUNKS; >> >> after changing all needed places like that, the build bug isn't needed >> anymore (unless we want to make sure the header isn't larger than some >> arbitrary number N chunks) > > That sounds overly complicated to me. I would rather use bit_spin_lock > as Arnd suggested. What would you say? using the correctly-calculated header size instead of a hardcoded value is overly complicated? i don't agree with that...i'd say it should have been done in the first place ;-) bit spin locks are hard to debug and slower and should only be used where space really is absolutely required to be minimal, which definitely isn't the case here. this should use regular spin locks and change the hardcoded assumption of zhdr size < chunk size (which always was a bad assumption) to calculate it correctly. it's really not that hard; there are only a few places where the offset position of the chunks is calculated. > > ~vitaly -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org