From owner-svn-src-all@freebsd.org Wed Aug 5 11:16:13 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id C43EA9B3285 for ; Wed, 5 Aug 2015 11:16:13 +0000 (UTC) (envelope-from steven@multiplay.co.uk) Received: from mail-wi0-f175.google.com (mail-wi0-f175.google.com [209.85.212.175]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 561C11F13 for ; Wed, 5 Aug 2015 11:16:12 +0000 (UTC) (envelope-from steven@multiplay.co.uk) Received: by wijp15 with SMTP id p15so43856208wij.0 for ; Wed, 05 Aug 2015 04:16:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-type :content-transfer-encoding; bh=zrUkd0M0EyR2nMUB57z9BEnD8YFl+FfpXBromSp83Eg=; b=GLq7Mj8EoLu3Tc4mbCGSJp1CJd+D15l+091Z8JaP1klQNER2RftrRF5rj+U50Nc0Jr 8cTg091HJCwC1IwF2luUzxJc+rmKRSwect0O7U9bJQoi7Ap/4Jo0j5+7x4aNUWjxnT1q dLeZlSHnpvuYlbTrj6KjZ/LgU4pzwV3zyAtiySs3dNZYYR82OF7o6he/LQyoXJ/4OiRC WZOOU9BWpY4UT0kTQqlMy/MRNhzyPCHGfjvs7EChTmVc1+l6A+dRkN5p6n1JcXKYRMHy TP6d4zNG4K63KJcHRl5MkyMO65tT8HsKVIneE1vvkTJ0qn83yqeJjBibAmAGAo/OsVEr iQfg== X-Gm-Message-State: ALoCoQlES23bDYG+JddzWKOKRUYb2j87khpVUEsOGbX3QkzERs9Pz/MOSx02oNDDVCRXCOjPImnk X-Received: by 10.180.208.81 with SMTP id mc17mr8727987wic.93.1438773029894; Wed, 05 Aug 2015 04:10:29 -0700 (PDT) Received: from [10.10.1.68] (82-69-141-170.dsl.in-addr.zen.co.uk. [82.69.141.170]) by smtp.gmail.com with ESMTPSA id ea2sm3300849wib.2.2015.08.05.04.10.28 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 05 Aug 2015 04:10:29 -0700 (PDT) Subject: Re: svn commit: r286223 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs To: Slawa Olhovchenkov References: <2757800.HIDNx1G49O@overcee.wemm.org> <20150803111942.GB2072@kib.kiev.ua> <55BF557B.60009@multiplay.co.uk> <20150803120359.GC2072@kib.kiev.ua> <55BFC296.5050402@freebsd.org> <20150803194412.GC8792@zxy.spb.ru> <55C07826.9070002@multiplay.co.uk> <20150804161448.GC24698@zxy.spb.ru> <55C11B5F.2080007@multiplay.co.uk> <20150804202411.GD8792@zxy.spb.ru> Cc: Warner Losh , src-committers@freebsd.org, Peter Wemm , svn-src-all@freebsd.org, svn-src-head@freebsd.org, Konstantin Belousov , Julian Elischer From: Steven Hartland Message-ID: <55C1EF1F.6040204@multiplay.co.uk> Date: Wed, 5 Aug 2015 12:10:23 +0100 User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <20150804202411.GD8792@zxy.spb.ru> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Aug 2015 11:16:13 -0000 On 04/08/2015 21:24, Slawa Olhovchenkov wrote: > On Tue, Aug 04, 2015 at 09:06:55PM +0100, Steven Hartland wrote: > >> >> On 04/08/2015 17:14, Slawa Olhovchenkov wrote: >>> On Tue, Aug 04, 2015 at 09:30:30AM +0100, Steven Hartland wrote: >>> >>>> On 03/08/2015 21:48, Warner Losh wrote: >>>>>> On Aug 3, 2015, at 1:44 PM, Slawa Olhovchenkov wrote: >>>>>> >>>>>> On Tue, Aug 04, 2015 at 03:35:50AM +0800, Julian Elischer wrote: >>>>>> >>>>>>> On 8/3/15 8:03 PM, Konstantin Belousov wrote: >>>>>>>> On Mon, Aug 03, 2015 at 12:50:19PM +0100, Steven Hartland wrote: >>>>>>>>> For this change I don't want to get into fixing the thread0 stack size, >>>>>>>>> which can be done later, just >>>>>>>>> to provide a reasonable warning to the user that smaller values could >>>>>>>>> cause a panic. >>>>>>>> Hmm, is it limited to the thread0 only ? I.e., would only increasing >>>>>>>> the initial thread stack size be enough to boot the kernel ? The zfs >>>>>>>> threads do request larger stack size, I know this. >>>>>>>> >>>>>>>> Can somebody test the following patch in the i386 configuration which >>>>>>>> does not boot ? >>>>>>> I think this is a reasonable thing to do. Thread0 (and proc0) are special. >>>>>>> I don't see why giving it a specially sized stack would be a problem. >>>>>> This is always do for ARM. >>>>>> May be need increase stack size for Thread0 on ARM too? >>>>> Seems reasonable. There should be a MI way of doing this, but all the code and defines are buried in MD files, so each architecture needs some love to make this a reality. >>>>> >>>>> Warner >>>> In the mean time are people happier with >>>> https://reviews.freebsd.org/D3279 or should I just leave it using the >>>> #define until someone has time to work on a full solution? >>> Checking by #ifdef you check only parametr at time of building zfs.ko, >>> checking variable you check actual value. >>> May be check thread stack best if only for current tread. >> Not sure I follow you as its not a #ifdef check its straight if in the >> new version i.e. >> if (kstack_pages < ZFS_MIN_KSTACK_PAGES) { > This check checked how actual kernel compile vs how compile zfs.ko. > Remeber that kenel may be compiled independed from modules? Correct that's the requirement as its thread0 that appears to be the key. > >> Just in case you didn't notice kib committed a fix for i386 thread0 in >> r286288 so this may not be needed at all any more which is good news :) > If I understund kib fix (and you about ZFS stack requirements) you > need check curthread->td_kstack_pages (for case old, unfixed kerenel, > depended from KSTACK_PAGES in Thread0). > > I.e. for all cases: > > - unfixed kernel with KSTACK_PAGES < 4 > - unfixed kernel with KSTACK_PAGES >= 4 > - fixed kernel with KSTACK_PAGES < 4 > - fixed kernel with KSTACK_PAGES >= 4 > - compiling zfs.ko separately from kernel with different KSTACK_PAGES > - using zfs.ko with old kernel > > checking curthread->td_kstack_pages is right way (or, may be > curthread->td_kstack_pages*PAGE_SIZE -- I am not cleanly understund > what need to check -- size in bytes or size in pages). > Checking KSTACK_PAGES by ifdef can produce vrong result. Its not clear to me if curthread will be thread0 however in unfixed thread0 stack size would have been kstack_pages so at the time this was still correct IMO. Just to be clear this is not checked by an ifdef, that's just OS guard which is correct. Latest revision which now explicitly checks thread0 stack size is now available: https://reviews.freebsd.org/D3279 Regards Steve