From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk0-f71.google.com (mail-vk0-f71.google.com [209.85.213.71]) by kanga.kvack.org (Postfix) with ESMTP id 78C9D6B025E for ; Sat, 26 Nov 2016 04:12:09 -0500 (EST) Received: by mail-vk0-f71.google.com with SMTP id p9so39267440vkd.7 for ; Sat, 26 Nov 2016 01:12:09 -0800 (PST) Received: from mail-ua0-x242.google.com (mail-ua0-x242.google.com. [2607:f8b0:400c:c08::242]) by mx.google.com with ESMTPS id h20si11060152uab.61.2016.11.26.01.12.08 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 26 Nov 2016 01:12:08 -0800 (PST) Received: by mail-ua0-x242.google.com with SMTP id 50so5418481uae.2 for ; Sat, 26 Nov 2016 01:12:08 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <20161115165538.878698352bd45e212751b57a@gmail.com> <20161115170030.f0396011fa00423ff711a3b4@gmail.com> From: Vitaly Wool Date: Sat, 26 Nov 2016 10:12:08 +0100 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: Dan Streetman Cc: Linux-MM , linux-kernel , Andrew Morton , Arnd Bergmann On Fri, Nov 25, 2016 at 7:33 PM, Dan Streetman wrote: > 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. I gave this a second thought after having run some quick benchmarking using bit_spin_lock. The perofrmance drop is substantial, so your suggestion is the way to go, thanks. ~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