From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CD5B0C433E9 for ; Fri, 29 Jan 2021 16:37:44 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 84DC464DFB for ; Fri, 29 Jan 2021 16:37:44 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 84DC464DFB Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.78313.142423 (Exim 4.92) (envelope-from ) id 1l5Wm3-00012b-6n; Fri, 29 Jan 2021 16:37:31 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 78313.142423; Fri, 29 Jan 2021 16:37:31 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1l5Wm3-00012U-3r; Fri, 29 Jan 2021 16:37:31 +0000 Received: by outflank-mailman (input) for mailman id 78313; Fri, 29 Jan 2021 16:37:29 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1l5Wm1-00012P-QU for xen-devel@lists.xenproject.org; Fri, 29 Jan 2021 16:37:29 +0000 Received: from mx2.suse.de (unknown [195.135.220.15]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 1c6e23fb-a793-4d92-a99f-0cc894facf8d; Fri, 29 Jan 2021 16:37:29 +0000 (UTC) Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 29558AECE; Fri, 29 Jan 2021 16:37:28 +0000 (UTC) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 1c6e23fb-a793-4d92-a99f-0cc894facf8d X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1611938248; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Y3CP644klWC+es9IOzPIKIXWACzvqS7V5u7MMMLnEOk=; b=RDr81gyKSjBqv42AUgp21ytnzFiLPAW5HBQooAqRRMWSXsTPHHtx3AnpJ/JqXxDDqASsBL cbyWzI15z5TzomLkZovvg0jPh6kSaZQB8H/cW/5rxGbKJDUMpcN8WtmRtGbh6EDImSvnWn 0WWsnxQGJ/qi55q7jApLR+/YUlgrhZw= Subject: Re: [PATCH v7 02/10] xen/domain: Add vmtrace_frames domain creation parameter From: Jan Beulich To: Andrew Cooper Cc: =?UTF-8?Q?Micha=c5=82_Leszczy=c5=84ski?= , =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= , Wei Liu , Anthony PERARD , Tamas K Lengyel , Xen-devel References: <20210121212718.2441-1-andrew.cooper3@citrix.com> <20210121212718.2441-3-andrew.cooper3@citrix.com> <752e7de2-b95e-f7ab-0d14-877c72c66134@suse.com> Message-ID: Date: Fri, 29 Jan 2021 17:37:28 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: <752e7de2-b95e-f7ab-0d14-877c72c66134@suse.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 25.01.2021 16:08, Jan Beulich wrote: > On 21.01.2021 22:27, Andrew Cooper wrote: >> --- a/xen/common/domain.c >> +++ b/xen/common/domain.c >> @@ -132,6 +132,48 @@ static void vcpu_info_reset(struct vcpu *v) >> v->vcpu_info_mfn = INVALID_MFN; >> } >> >> +static void vmtrace_free_buffer(struct vcpu *v) >> +{ >> + const struct domain *d = v->domain; >> + struct page_info *pg = v->vmtrace.buf; >> + unsigned int i; >> + >> + if ( !pg ) >> + return; >> + >> + for ( i = 0; i < d->vmtrace_frames; i++ ) >> + { >> + put_page_alloc_ref(&pg[i]); >> + put_page_and_type(&pg[i]); >> + } >> + >> + v->vmtrace.buf = NULL; > > To set a good precedent, maybe this wants moving up ahead of > the loop and ... > >> +} >> + >> +static int vmtrace_alloc_buffer(struct vcpu *v) >> +{ >> + struct domain *d = v->domain; >> + struct page_info *pg; >> + unsigned int i; >> + >> + if ( !d->vmtrace_frames ) >> + return 0; >> + >> + pg = alloc_domheap_pages(d, get_order_from_pages(d->vmtrace_frames), >> + MEMF_no_refcount); >> + if ( !pg ) >> + return -ENOMEM; >> + >> + v->vmtrace.buf = pg; > > ... this wants moving down past the loop, to avoid > globally announcing something that isn't fully initialized > yet / anymore? So having replied on the other thread I looked back here: The suggested change actually is not just to set a good precedent. By recording the page(s) in v->vmtrace_buf, ... >> + for ( i = 0; i < d->vmtrace_frames; i++ ) >> + /* Domain can't know about this page yet - something fishy going on. */ >> + if ( !get_page_and_type(&pg[i], d, PGT_writable_page) ) >> + BUG(); ... failure here (if handled better than by BUG()) will lead vmtrace_free_buffer() to believe it needs to put references (besides freeing the page(s), which is fine). So your claimed bug really is just here, not everywhere else, and there is no reason to go backwards in terms of error handling "quality", as per the bottom of ./CODING_STYLE. Jan