From win-pv-devel-bounces@lists.xenproject.org Fri May 03 10:33:50 2024 Return-path: Envelope-to: archives@lists.xenproject.org Delivery-date: Fri, 03 May 2024 10:33:50 +0000 Received: from list by lists.xenproject.org with outflank-mailman.716354.1118485 (Exim 4.92) (envelope-from ) id 1s2qEe-00032p-89; Fri, 03 May 2024 10:33:48 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 716354.1118485; Fri, 03 May 2024 10:33:48 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1s2qEe-00032i-49; Fri, 03 May 2024 10:33:48 +0000 Received: by outflank-mailman (input) for mailman id 716354; Fri, 03 May 2024 10:33:47 +0000 Received: from se1-gles-sth1-in.inumbo.com ([159.253.27.254] helo=se1-gles-sth1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1s2qEd-00031e-Hc for win-pv-devel@lists.xenproject.org; Fri, 03 May 2024 10:33:47 +0000 Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [2a00:1450:4864:20::42e]) by se1-gles-sth1.inumbo.com (Halon) with ESMTPS id 9e4c197a-0938-11ef-909c-e314d9c70b13; Fri, 03 May 2024 12:33:46 +0200 (CEST) Received: by mail-wr1-x42e.google.com with SMTP id ffacd0b85a97d-34ddc9fe4a1so1421771f8f.0 for ; Fri, 03 May 2024 03:33:45 -0700 (PDT) Received: from localhost.localdomain (host86-140-227-241.range86-140.btcentralplus.com. [86.140.227.241]) by smtp.gmail.com with ESMTPSA id v28-20020a5d591c000000b0034db47c7e6dsm3414719wrd.115.2024.05.03.03.33.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 03 May 2024 03:33:44 -0700 (PDT) X-BeenThere: win-pv-devel@lists.xenproject.org List-Id: Developer list for the Windows PV Drivers subproject List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: win-pv-devel-bounces@lists.xenproject.org Precedence: list Sender: "win-pv-devel" X-Inumbo-ID: 9e4c197a-0938-11ef-909c-e314d9c70b13 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cloud.com; s=cloud; t=1714732424; x=1715337224; darn=lists.xenproject.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=2f0bRAh6H9kHIA13d+xge+7TBirtft5uR6LXQYULhro=; b=EiRYYrnlHZokx/7CfMBsLasPT8pLXrayxtnwkmXTKLvQMhLlHpv9NQK9sBkKSNSk9a M3yadD5OfGvbD8jHvoTMiQep23PBc16SwxlpGcj1h+cF/HV+h/Y3RbgiaGbqrPsi2XcK yCBtGX3Vij13hvbRkWYrsKnTGrS12xOrjYpd8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714732424; x=1715337224; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=2f0bRAh6H9kHIA13d+xge+7TBirtft5uR6LXQYULhro=; b=cuVVZ3iIN20M9DPujTxtL3uCYBEPzQBJqDEs+pV/yvJdefifX+pDL/XCa2L3mpp0UW psX8bFEYPg0YjFw9xHVcsIDCa8zH/MTo4jCvdHsXaTFc3VJCreC1tMrFVraEgNtd5AMC Q+kB6IpKZYTl8to5XaJm3wlvtpuYGN5xYGua/PAfGgismr1rbUB5hFGCe/edzfJWD2xT oJBfm2l0lpm70j4HsDOjZtX0xzPPi6W4kwM6paM4FcAIwltiP18/72fEj0gRFh/gYaGQ uWLwYmsUW1B+yZll1ab+P0YEv8mwmT/gmSqwoTjxvNlPTxMhbqoYNJETtK+fMm8XhLiB eUsg== X-Gm-Message-State: AOJu0Ywe5kMq/yp9EgY8llTe6rEbOB35Xmu4qG9khJXPlWzBq+suZWUW 7wC3N1LV6NB+1e3dKI+ot5dqa/V0ghEvUao1sAaBcQi5ohDglXhU1/A057HSFZj+D4hKnmCqwIA = X-Google-Smtp-Source: AGHT+IEZbkr4xI0axe10tSMCpu0raNxqmv7Q/lsFQbH/SvLPD/A1DDyOW4euQ7CvUKV4qkPvff6DJA== X-Received: by 2002:adf:959a:0:b0:34c:e2c4:74d0 with SMTP id p26-20020adf959a000000b0034ce2c474d0mr1572144wrp.68.1714732424327; Fri, 03 May 2024 03:33:44 -0700 (PDT) From: Owen Smith To: win-pv-devel@lists.xenproject.org Cc: Owen Smith Subject: [PATCH] Cache->Cursor may point to Slab Date: Fri, 3 May 2024 11:33:28 +0100 Message-ID: <20240503103328.320-1-owen.smith@cloud.com> X-Mailer: git-send-email 2.44.0.windows.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Its possible that the Cursor refers to the only Slab that has no allocated objects, but there are other slabs which are fully allocated. The current ASSERTions are not valid, and needs rework. Also adds more comments and ASSERTions to validate whats going on. Signed-off-by: Owen Smith --- src/xenbus/cache.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/xenbus/cache.c b/src/xenbus/cache.c index 6229aed..821f66a 100644 --- a/src/xenbus/cache.c +++ b/src/xenbus/cache.c @@ -479,20 +479,25 @@ CacheDestroySlab( // // The only reason the cursor should be pointing at this slab is - // if it is the only one in the list. + // if it is the only one in the list with no allocated objects. // if (Cache->Cursor == &Slab->ListEntry) { ASSERT3P(Slab->ListEntry.Flink, ==, &Cache->SlabList); - ASSERT3P(Slab->ListEntry.Blink, ==, &Cache->SlabList); + // Slab->ListEntry.Blink could refer to a fully allocated Slab + // Set Cursor to SlabList, so next CacheGet allocates a new Slab Cache->Cursor = &Cache->SlabList; } RemoveEntryList(&Slab->ListEntry); + // If Cache->Cursor == &Cache->SlabList, + // either; all entries in SlabList are fully allocated; or + // SlabList is empty + // If Cache->Cursor != &Cache->SlabList, + // Cache->Cursor is the first Slab that is not fully allocated - ASSERT(IMPLY(Cache->Cursor == &Cache->SlabList, - IsListEmpty(&Cache->SlabList))); + CacheAudit(Cache); - Index = CacheMaskSize(Slab->Allocated); + Index = CacheMaskSize(Slab->Constructed); while (--Index >= 0) { PVOID Object = (PVOID)&Slab->Buffer[Index * Cache->Size]; @@ -501,6 +506,7 @@ CacheDestroySlab( __CacheMaskClear(Slab->Constructed, Index); } } + ASSERT3U(CacheMaskCount(Slab->Constructed), ==, 0); ASSERT(Cache->CurrentSlabs != 0); InterlockedDecrement(&Cache->CurrentSlabs); -- 2.44.0.windows.1 From win-pv-devel-bounces@lists.xenproject.org Fri May 03 10:34:22 2024 Return-path: Envelope-to: archives@lists.xenproject.org Delivery-date: Fri, 03 May 2024 10:34:22 +0000 Received: from list by lists.xenproject.org with outflank-mailman.716361.1118488 (Exim 4.92) (envelope-from ) id 1s2qFC-0003PZ-9o; Fri, 03 May 2024 10:34:22 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 716361.1118488; Fri, 03 May 2024 10:34:22 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1s2qFC-0003PS-6r; Fri, 03 May 2024 10:34:22 +0000 Received: by outflank-mailman (input) for mailman id 716361; Fri, 03 May 2024 10:34:21 +0000 Received: from se1-gles-sth1-in.inumbo.com ([159.253.27.254] helo=se1-gles-sth1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1s2qFB-00031e-Fj for win-pv-devel@lists.xenproject.org; Fri, 03 May 2024 10:34:21 +0000 Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [2a00:1450:4864:20::332]) by se1-gles-sth1.inumbo.com (Halon) with ESMTPS id b3a5e8ff-0938-11ef-909c-e314d9c70b13; Fri, 03 May 2024 12:34:21 +0200 (CEST) Received: by mail-wm1-x332.google.com with SMTP id 5b1f17b1804b1-41bab13ca4eso48326315e9.1 for ; Fri, 03 May 2024 03:34:21 -0700 (PDT) Received: from localhost.localdomain (host86-140-227-241.range86-140.btcentralplus.com. [86.140.227.241]) by smtp.gmail.com with ESMTPSA id fm9-20020a05600c0c0900b0041c23148330sm8882474wmb.10.2024.05.03.03.34.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 03 May 2024 03:34:19 -0700 (PDT) X-BeenThere: win-pv-devel@lists.xenproject.org List-Id: Developer list for the Windows PV Drivers subproject List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: win-pv-devel-bounces@lists.xenproject.org Precedence: list Sender: "win-pv-devel" X-Inumbo-ID: b3a5e8ff-0938-11ef-909c-e314d9c70b13 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cloud.com; s=cloud; t=1714732460; x=1715337260; darn=lists.xenproject.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=+s89ucL5DOr3F0mogaO8ruuW7s5hwjE5NCGxaqGI1zE=; b=RWcdaAxDMMTvbYSdLiMSM8azMmaLL2JCg3l/lEMG93mIjgJVhK/NRZUzWVOFutXr/h 8pK9y+HU4IVFYRb8fWqPC41PP3fMbfkrMQIKdxePgnZEe3LucrvzBdVJooGnDBqyh1xh +KO2sGoseeFFmhVH9DNQk33jOKI3+SE4/Pm3M= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714732460; x=1715337260; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=+s89ucL5DOr3F0mogaO8ruuW7s5hwjE5NCGxaqGI1zE=; b=uf5J6f8/utGTzlQBwOHWDdaitxd5kl9oMgJwuTjwFrg0TBRy2XAt/QjTAeZ09W9LUH KTQ3tz4PIFFfwwluHqYwNeVVWkrEoqG6dRZ7nS9oBqgzYCkwPuzlgGIPvjUQEiEH7Xhe or9wFzvGZ9VkOQrMAoQIwwZ6vt8PiEJI6gsnKxBUmxgqckmgpOtL2DqiM13WNfiT/OTq vTjf6CmITzdO+9AiURSNU8Z82iXQKcV4zFKPj+2SG/tllqn+ewTVJCOGd6CUr6cYWXMG 4rucpdJvom226dqfQ0SlutOzSf6Y+wOFzXvaGGKLT2/nZC0mj80RRKR07DEn1gJ8wF44 fDOg== X-Gm-Message-State: AOJu0Yx4QvJIYpXq8aLzJ2KaVGcZYMiIvZb97l2q+Cfk2fkNMQ6UYyzi OJ0tZaHDIffsa4Uz3fZ8qo0GnI8SJXSB2pkR+LE/CE87uKBdGBS/Cil8FL4EiLmYmmgOhJrEDnQ = X-Google-Smtp-Source: AGHT+IHUWQBEillxgCJHOGCzkys0pSCT2Q/sz1n6ykoJOvh4606KKEGgkxez1suwr0smUb/Sc/p97g== X-Received: by 2002:a05:600c:a48:b0:41b:da7b:9b94 with SMTP id c8-20020a05600c0a4800b0041bda7b9b94mr1900254wmq.2.1714732460230; Fri, 03 May 2024 03:34:20 -0700 (PDT) From: Owen Smith To: win-pv-devel@lists.xenproject.org Cc: Owen Smith Subject: [PATCH] Fix ASSERT in PdoUnplugRequest Date: Fri, 3 May 2024 11:34:09 +0100 Message-ID: <20240503103409.1108-1-owen.smith@cloud.com> X-Mailer: git-send-email 2.44.0.windows.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit PdoUnplugRequest can be called multiple times on the same Pdo if the child device is restarted during child device installation. Avoid ASSERT in checked build and prevent incorrect increment of the unplug values. Signed-off-by: Owen Smith --- src/xenvif/pdo.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/xenvif/pdo.c b/src/xenvif/pdo.c index 46b43a9..a3ae061 100644 --- a/src/xenvif/pdo.c +++ b/src/xenvif/pdo.c @@ -1234,7 +1234,11 @@ PdoUnplugRequest( { NTSTATUS status; - ASSERT3U(Pdo->UnplugRequested, !=, Make); + // When a driver is restarted, PdoUnplugRequest is called again, + // dont increment the unplug count again. + if (Pdo->UnplugRequested == Make) + return; + Pdo->UnplugRequested = Make; status = XENBUS_UNPLUG(Acquire, &Pdo->UnplugInterface); -- 2.44.0.windows.1 From win-pv-devel-bounces@lists.xenproject.org Mon May 20 08:21:11 2024 Return-path: Envelope-to: archives@lists.xenproject.org Delivery-date: Mon, 20 May 2024 08:21:11 +0000 Received: from list by lists.xenproject.org with outflank-mailman.725856.1130160 (Exim 4.92) (envelope-from ) id 1s8yGa-0006EH-J1; Mon, 20 May 2024 08:21:08 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 725856.1130160; Mon, 20 May 2024 08:21:08 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1s8yGa-0006EA-GW; Mon, 20 May 2024 08:21:08 +0000 Received: by outflank-mailman (input) for mailman id 725856; Mon, 20 May 2024 08:21:06 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1s8yGY-0006E4-KL for win-pv-devel@lists.xenproject.org; Mon, 20 May 2024 08:21:06 +0000 Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [2a00:1450:4864:20::431]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id e5f7bb4a-1681-11ef-b4bb-af5377834399; Mon, 20 May 2024 10:21:04 +0200 (CEST) Received: by mail-wr1-x431.google.com with SMTP id ffacd0b85a97d-351cda41b53so1780284f8f.2 for ; Mon, 20 May 2024 01:21:03 -0700 (PDT) Received: from ?IPV6:2a00:23c7:df82:3001:20f1:15b5:79c7:cb20? ([2a00:23c7:df82:3001:20f1:15b5:79c7:cb20]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-354c0d87b0bsm4102010f8f.9.2024.05.20.01.21.02 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 20 May 2024 01:21:02 -0700 (PDT) X-BeenThere: win-pv-devel@lists.xenproject.org List-Id: Developer list for the Windows PV Drivers subproject List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: win-pv-devel-bounces@lists.xenproject.org Precedence: list Sender: "win-pv-devel" X-Inumbo-ID: e5f7bb4a-1681-11ef-b4bb-af5377834399 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1716193263; x=1716798063; darn=lists.xenproject.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:reply-to:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=uOWk67Zai5ANXqeNwKtIlpICRpm8y3bYST+NR+2U+Ls=; b=lKfzVaDcivonV5FVfko2C1SEkxA4O+4sozNZCiMJ6LQ4fAIxZYZLZ3Gdkb72z5W+W4 uOHhIZljPR4gNSVl09y6Okq7Ttua3LllZ2P7zTPVFNF8vuGb+ZE0hBvvkGqcIIEB9u2E kjOf1BfXP2SnjbMti8HniunuNM0K47zZsklcH3E8jfzFSx2rMAT1cNKH6X0lnmz1mfbr 7AilEW6fEWzN/9LSALm2J6Q+GOpULc5UTELtWtUhRQoz02Q2GAqBpiTpU9gnGqs/7I7N 6BnvGU3EzKKTjyA7s3tBhxZL5Ww6G/abzb9L9Z9AgyXADMWhHa1CYux2B6rn1NgqBcZS e7Pw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716193263; x=1716798063; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:reply-to:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=uOWk67Zai5ANXqeNwKtIlpICRpm8y3bYST+NR+2U+Ls=; b=hpXdTmcr4vncWDokYYqWWz/9tdKSWTQCVnCXvWAj9bgZY4wZ7ra3ZxLXYbroWoyiNN qSrOgYo0LXW6nlHFEozdTN/DJYiDrTauqDgpaOJHnqdkBLjVI6UjexI++nPHYdGLyyYv 2Jqv9/zylOK0h4mCeHYsrWzxHrVkFa7g9gqubPWPavGkfKvRhFlj9rAyUingvaPjbgYf ObpYvRMXdpuwHLvCL0mHMmGM9Kd3HECEHTBHxPY7JOiksZCzD6yGrkBtZT/W8SMUPKjM /OkLlZaeQQISIEeCy7Mt0Ju3wrb/mm+ovUWdw4UhEpqSvKmuiQ3Gw20bJ7plT6COcXno BdBQ== X-Gm-Message-State: AOJu0YzwH5/2I0zhIzZvKCEy+lmVppHmHzAgFRtEpPLl1tStynndqkEZ bykc+pu+EIbmC8VmcWrbE6t21VmoaFYiv6eMTCY3wba/f+SjEwYSkyyFpA== X-Google-Smtp-Source: AGHT+IHGMRdv3JS5D2AjWl0srXmc+kNuulHsKMyEMafxsg/r38iPZpvQfMqMzw/n0TlKcS+6+FfbMw== X-Received: by 2002:a5d:61cd:0:b0:34d:b47c:7e6f with SMTP id ffacd0b85a97d-3504a6334c4mr18445545f8f.27.1716193263084; Mon, 20 May 2024 01:21:03 -0700 (PDT) Message-ID: Date: Mon, 20 May 2024 09:21:02 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Reply-To: paul@xen.org Subject: Re: [PATCH] Cache->Cursor may point to Slab To: win-pv-devel@lists.xenproject.org References: <20240503103328.320-1-owen.smith@cloud.com> Content-Language: en-US From: "Durrant, Paul" In-Reply-To: <20240503103328.320-1-owen.smith@cloud.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 03/05/2024 11:33, Owen Smith wrote: > Its possible that the Cursor refers to the only Slab that has no > allocated objects, but there are other slabs which are fully > allocated. The current ASSERTions are not valid, and needs rework. > > Also adds more comments and ASSERTions to validate whats going on. > > Signed-off-by: Owen Smith > --- > src/xenbus/cache.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/src/xenbus/cache.c b/src/xenbus/cache.c > index 6229aed..821f66a 100644 > --- a/src/xenbus/cache.c > +++ b/src/xenbus/cache.c > @@ -479,20 +479,25 @@ CacheDestroySlab( > > // > // The only reason the cursor should be pointing at this slab is > - // if it is the only one in the list. > + // if it is the only one in the list with no allocated objects. Now that we don't spill empty slabs immediately, I think the cursor could actually point at any one of several empty slabs. > // > if (Cache->Cursor == &Slab->ListEntry) { > ASSERT3P(Slab->ListEntry.Flink, ==, &Cache->SlabList); > - ASSERT3P(Slab->ListEntry.Blink, ==, &Cache->SlabList); > + // Slab->ListEntry.Blink could refer to a fully allocated Slab > + // Set Cursor to SlabList, so next CacheGet allocates a new Slab > Cache->Cursor = &Cache->SlabList; > } > > RemoveEntryList(&Slab->ListEntry); > + // If Cache->Cursor == &Cache->SlabList, > + // either; all entries in SlabList are fully allocated; or > + // SlabList is empty > + // If Cache->Cursor != &Cache->SlabList, > + // Cache->Cursor is the first Slab that is not fully allocated > > - ASSERT(IMPLY(Cache->Cursor == &Cache->SlabList, > - IsListEmpty(&Cache->SlabList))); > + CacheAudit(Cache); > > - Index = CacheMaskSize(Slab->Allocated); > + Index = CacheMaskSize(Slab->Constructed); > while (--Index >= 0) { > PVOID Object = (PVOID)&Slab->Buffer[Index * Cache->Size]; > > @@ -501,6 +506,7 @@ CacheDestroySlab( > __CacheMaskClear(Slab->Constructed, Index); > } > } > + ASSERT3U(CacheMaskCount(Slab->Constructed), ==, 0); > > ASSERT(Cache->CurrentSlabs != 0); > InterlockedDecrement(&Cache->CurrentSlabs); Acked-by: Paul Durrant From win-pv-devel-bounces@lists.xenproject.org Mon May 20 08:22:37 2024 Return-path: Envelope-to: archives@lists.xenproject.org Delivery-date: Mon, 20 May 2024 08:22:37 +0000 Received: from list by lists.xenproject.org with outflank-mailman.725867.1130175 (Exim 4.92) (envelope-from ) id 1s8yI0-0006n6-VK; Mon, 20 May 2024 08:22:36 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 725867.1130175; Mon, 20 May 2024 08:22:36 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1s8yI0-0006mz-SC; Mon, 20 May 2024 08:22:36 +0000 Received: by outflank-mailman (input) for mailman id 725867; Mon, 20 May 2024 08:22:35 +0000 Received: from se1-gles-sth1-in.inumbo.com ([159.253.27.254] helo=se1-gles-sth1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1s8yHz-0006HY-Lg for win-pv-devel@lists.xenproject.org; Mon, 20 May 2024 08:22:35 +0000 Received: from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com [2a00:1450:4864:20::12e]) by se1-gles-sth1.inumbo.com (Halon) with ESMTPS id 1c144415-1682-11ef-909f-e314d9c70b13; Mon, 20 May 2024 10:22:35 +0200 (CEST) Received: by mail-lf1-x12e.google.com with SMTP id 2adb3069b0e04-523b20e2615so4701279e87.1 for ; Mon, 20 May 2024 01:22:34 -0700 (PDT) Received: from ?IPV6:2a00:23c7:df82:3001:20f1:15b5:79c7:cb20? ([2a00:23c7:df82:3001:20f1:15b5:79c7:cb20]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-41f87c25459sm450017625e9.18.2024.05.20.01.22.33 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 20 May 2024 01:22:33 -0700 (PDT) X-BeenThere: win-pv-devel@lists.xenproject.org List-Id: Developer list for the Windows PV Drivers subproject List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: win-pv-devel-bounces@lists.xenproject.org Precedence: list Sender: "win-pv-devel" X-Inumbo-ID: 1c144415-1682-11ef-909f-e314d9c70b13 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1716193354; x=1716798154; darn=lists.xenproject.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:reply-to:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=jobkO9motA8y1Jm4cwF61u7oLcSaANKUUX3P3xSxoDI=; b=Bf6IJVLzmivBr7chZCYHliQzGjcIc/DHSng6KLtMG5fyhiTAAPVZz1V5hlZdksU7CB PrRztLeGcXKkgAzYUX6mdpNM1xybLylqvwnDDJ0Wy44E5FZQ38zfRPhariCzHIkX2SGf 6dpOoUpV1QITYKeA71lXVzQlH+fjkndjAuyWDHpDjjti3Q/lz3GjVN00bo2IInD69GHR miFxiyMiHT0JsR/UkkAbAeo3SRcsbIeWlWfReva5Elm5W7wGeyJaivOL/pBK4X/9v4Eq Ychg4yYxmAzqXCrahDlKoKfFKNrJ/wEkrhjEspoviQTEPv9aejakQfEsbd8xwKCyO/qN 1mTA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716193354; x=1716798154; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:reply-to:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=jobkO9motA8y1Jm4cwF61u7oLcSaANKUUX3P3xSxoDI=; b=cZUpK+AvpoJTUUav4wBKrIR35373va82e3wjxW1h8mkKIie+T7Y0WkM6hWM5WRZ31I 2EnY2OlBHQ6PbGIGp+Y7hRBKKHcBYkxD/Oiuo3PN8hPZyz6NKGq1bSBML8che8fUIF7v BUVthwrY+Gu2kIaLgQFZamd2PygeRLVrXUJmOa1XBrzZf3Nso630v9pBocvD7yu5vFYt YZo5jVYoBQfrK5ss3y5qv5QNbYjNX2Y3Ka8n7q8LgRAbIpz1ZzJ8fCD3LEADoXH8uMF6 E7GrZioHfOVwkE09T665PSb8bAxWGXO9uY83oQUrdR9Rz+phhuvrooUw8qqdIBQrI7LF CvPg== X-Gm-Message-State: AOJu0Yz7/ozq/OUiGB+j/IpftLIlUcMCElgtLU+jiiqrLvE6K090yn2M qWxpZaXa5TIEoSBuujXwfhN82O4H6EOBHM9r+Wjqw3wU8S7iBZiTe2Ul9g== X-Google-Smtp-Source: AGHT+IFi1twW0ONK6UwXFeho3j3DyDX+Py66eOTUkQc7REkahjQTgT/e2ze+5H6vvVaxqcYWra1thg== X-Received: by 2002:a2e:2c0c:0:b0:2e7:102f:7aeb with SMTP id 38308e7fff4ca-2e7102f7d5cmr52075351fa.40.1716193353831; Mon, 20 May 2024 01:22:33 -0700 (PDT) Message-ID: Date: Mon, 20 May 2024 09:22:33 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Reply-To: paul@xen.org Subject: Re: [PATCH] Fix ASSERT in PdoUnplugRequest To: win-pv-devel@lists.xenproject.org References: <20240503103409.1108-1-owen.smith@cloud.com> Content-Language: en-US From: "Durrant, Paul" In-Reply-To: <20240503103409.1108-1-owen.smith@cloud.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 03/05/2024 11:34, Owen Smith wrote: > PdoUnplugRequest can be called multiple times on the same Pdo if > the child device is restarted during child device installation. > Avoid ASSERT in checked build and prevent incorrect increment of the > unplug values. > > Signed-off-by: Owen Smith Acked-by: Paul Durrant > --- > src/xenvif/pdo.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/xenvif/pdo.c b/src/xenvif/pdo.c > index 46b43a9..a3ae061 100644 > --- a/src/xenvif/pdo.c > +++ b/src/xenvif/pdo.c > @@ -1234,7 +1234,11 @@ PdoUnplugRequest( > { > NTSTATUS status; > > - ASSERT3U(Pdo->UnplugRequested, !=, Make); > + // When a driver is restarted, PdoUnplugRequest is called again, > + // dont increment the unplug count again. > + if (Pdo->UnplugRequested == Make) > + return; > + > Pdo->UnplugRequested = Make; > > status = XENBUS_UNPLUG(Acquire, &Pdo->UnplugInterface); From win-pv-devel-bounces@lists.xenproject.org Mon May 20 08:32:31 2024 Return-path: Envelope-to: archives@lists.xenproject.org Delivery-date: Mon, 20 May 2024 08:32:31 +0000 Received: from list by lists.xenproject.org with outflank-mailman.725890.1130198 (Exim 4.92) (envelope-from ) id 1s8yRb-0000wx-3q; Mon, 20 May 2024 08:32:31 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 725890.1130198; Mon, 20 May 2024 08:32: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 1s8yRb-0000wq-1P; Mon, 20 May 2024 08:32:31 +0000 Received: by outflank-mailman (input) for mailman id 725890; Mon, 20 May 2024 08:32:30 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1s8yRa-0000wi-2H for win-pv-devel@lists.xenproject.org; Mon, 20 May 2024 08:32:30 +0000 Received: from mail-lj1-x231.google.com (mail-lj1-x231.google.com [2a00:1450:4864:20::231]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id 7deb4599-1683-11ef-b4bb-af5377834399; Mon, 20 May 2024 10:32:28 +0200 (CEST) Received: by mail-lj1-x231.google.com with SMTP id 38308e7fff4ca-2e43c481b53so47216991fa.2 for ; Mon, 20 May 2024 01:32:28 -0700 (PDT) Received: from ?IPV6:2a00:23c7:df82:3001:20f1:15b5:79c7:cb20? ([2a00:23c7:df82:3001:20f1:15b5:79c7:cb20]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-41fccbe8fc6sm411646335e9.4.2024.05.20.01.32.27 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 20 May 2024 01:32:27 -0700 (PDT) X-BeenThere: win-pv-devel@lists.xenproject.org List-Id: Developer list for the Windows PV Drivers subproject List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: win-pv-devel-bounces@lists.xenproject.org Precedence: list Sender: "win-pv-devel" X-Inumbo-ID: 7deb4599-1683-11ef-b4bb-af5377834399 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1716193948; x=1716798748; darn=lists.xenproject.org; h=content-transfer-encoding:in-reply-to:content-language:references :to:from:subject:reply-to:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=csGREk5CcvcZf5UKIYiep0rx2R5L8oJwNhLCjURweZI=; b=Qjd6RoOH8d+eUPVV5JcqhTQWWFCUKYzEsI0tpE/uW1Ug3DQzlS/ZBJ7TGY2npAO/sl cVVU5fV3+h2YbMdzWNOCXWhOzKF4nTVRkVk9PX9MzaNQEpRooWKAPRVW5FZmWkATCdkT m6B0sIiB65xAz4iP/lweuoAjUO5axptPU+pPCh+aay8IxwfUm/ODFOQSnR863uFTmqqV zOZ5is2kpB+0dvdJIIf9f1lZcJ0UxhunocHzt9K1Z5ZG/NVk9ETYVrkIj6/SPKIsrLT4 WEx2E4K9kCTcpQoi8IytLbK28bAe1I6UDolbQLPFbijpElEC56vVm5wEDIvPZkLC3Xwo NDMg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716193948; x=1716798748; h=content-transfer-encoding:in-reply-to:content-language:references :to:from:subject:reply-to:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=csGREk5CcvcZf5UKIYiep0rx2R5L8oJwNhLCjURweZI=; b=K0FO9IwgXEIxgB+al9ZvyG/ZXQ9d5V2PShCTBr1gRnolD3xkzOPVjaWlRulimof7UP QbFdFQM3yOTSdv2vC+aI9zOKkKUUakX2ojmdM+AcsKLyL4/ptKK6SPXK0djMJN81SvRJ /qixCze48WjdkUuwu/+xbyZ3DFI5lp3UyCAhvIDyCjVstHmdxQHrud4oNHQxnKV0nGMP d84TQ4rgl65bp3gjkonjx2YLp/4NgqdtkMYiWhDT3HMyRcHG78qnGDfY93qcFKzmL7pP GE1oB5yF6ijIsOKVBqlaTX6zDH7+iMe8pZnODos3VMmVGWrR1oeIpiKvQJGeLdx0H1bg 9bHw== X-Gm-Message-State: AOJu0YwUqTd8yD3KPIOkwgBm4M2Lf1XwILbq3P11se0WGmgA5sKHX0HL UwldbydaTZ3zvTyFHv62ZTpzslzDMGQy5t+HIo7u1519vJeSi3v7RwHilQ== X-Google-Smtp-Source: AGHT+IFVkdUkX9anjwouHsfxcT50Ypd0Q++UUTMQnFIau9CSN0lgSkF9PB0T6CUmsBCxlLQrlyIiMQ== X-Received: by 2002:a2e:7a0b:0:b0:2e3:603e:4697 with SMTP id 38308e7fff4ca-2e52028ec1cmr211534031fa.36.1716193947478; Mon, 20 May 2024 01:32:27 -0700 (PDT) Message-ID: <54a61870-f1f1-4712-9510-82ad15d8030b@gmail.com> Date: Mon, 20 May 2024 09:32:26 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Reply-To: paul@xen.org Subject: Re: [PATCH] Cache->Cursor may point to Slab From: "Durrant, Paul" To: win-pv-devel@lists.xenproject.org References: <20240503103328.320-1-owen.smith@cloud.com> Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 20/05/2024 09:21, Durrant, Paul wrote: > On 03/05/2024 11:33, Owen Smith wrote: >> Its possible that the Cursor refers to the only Slab that has no >> allocated objects, but there are other slabs which are fully >> allocated. The current ASSERTions are not valid, and needs rework. >> >> Also adds more comments and ASSERTions to validate whats going on. >> >> Signed-off-by: Owen Smith >> --- >>   src/xenbus/cache.c | 16 +++++++++++----- >>   1 file changed, 11 insertions(+), 5 deletions(-) >> >> diff --git a/src/xenbus/cache.c b/src/xenbus/cache.c >> index 6229aed..821f66a 100644 >> --- a/src/xenbus/cache.c >> +++ b/src/xenbus/cache.c >> @@ -479,20 +479,25 @@ CacheDestroySlab( >>       // >>       // The only reason the cursor should be pointing at this slab is >> -    // if it is the only one in the list. >> +    // if it is the only one in the list with no allocated objects. > > Now that we don't spill empty slabs immediately, I think the cursor > could actually point at any one of several empty slabs. > >>       // >>       if (Cache->Cursor == &Slab->ListEntry) { >>           ASSERT3P(Slab->ListEntry.Flink, ==, &Cache->SlabList); >> -        ASSERT3P(Slab->ListEntry.Blink, ==, &Cache->SlabList); >> +        // Slab->ListEntry.Blink could refer to a fully allocated Slab >> +        // Set Cursor to SlabList, so next CacheGet allocates a new Slab >>           Cache->Cursor = &Cache->SlabList; >>       } >>       RemoveEntryList(&Slab->ListEntry); >> +    // If Cache->Cursor == &Cache->SlabList, >> +    //     either; all entries in SlabList are fully allocated; or >> +    //             SlabList is empty >> +    // If Cache->Cursor != &Cache->SlabList, >> +    //     Cache->Cursor is the first Slab that is not fully allocated >> -    ASSERT(IMPLY(Cache->Cursor == &Cache->SlabList, >> -                 IsListEmpty(&Cache->SlabList))); >> +    CacheAudit(Cache); Actually, I think this might cause a problem. The cursor is reset to the anchor above but the audit could find a slab in the list that is not fully occupied, I think. >> -    Index = CacheMaskSize(Slab->Allocated); >> +    Index = CacheMaskSize(Slab->Constructed); >>       while (--Index >= 0) { >>           PVOID Object = (PVOID)&Slab->Buffer[Index * Cache->Size]; >> @@ -501,6 +506,7 @@ CacheDestroySlab( >>               __CacheMaskClear(Slab->Constructed, Index); >>           } >>       } >> +    ASSERT3U(CacheMaskCount(Slab->Constructed), ==, 0); >>       ASSERT(Cache->CurrentSlabs != 0); >>       InterlockedDecrement(&Cache->CurrentSlabs); > > Acked-by: Paul Durrant I'll withdraw that and take a closer look at this. Paul > From win-pv-devel-bounces@lists.xenproject.org Mon May 20 09:15:36 2024 Return-path: Envelope-to: archives@lists.xenproject.org Delivery-date: Mon, 20 May 2024 09:15:36 +0000 Received: from list by lists.xenproject.org with outflank-mailman.725912.1130213 (Exim 4.92) (envelope-from ) id 1s8z7H-0007G5-Bp; Mon, 20 May 2024 09:15:35 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 725912.1130213; Mon, 20 May 2024 09:15:35 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1s8z7H-0007Fx-91; Mon, 20 May 2024 09:15:35 +0000 Received: by outflank-mailman (input) for mailman id 725912; Mon, 20 May 2024 09:15:33 +0000 Received: from mail.xenproject.org ([104.130.215.37]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1s8z7F-0007Fr-P4 for win-pv-devel@lists.xenproject.org; Mon, 20 May 2024 09:15:33 +0000 Received: from xenbits.xenproject.org ([104.239.192.120]) by mail.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1s8z7F-0006UH-LV; Mon, 20 May 2024 09:15:33 +0000 Received: from host86-178-29-159.range86-178.btcentralplus.com ([86.178.29.159] helo=CBG-R90WXYV0.amazon.com) by xenbits.xenproject.org with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1s8z7F-0007Z3-B2; Mon, 20 May 2024 09:15:33 +0000 X-BeenThere: win-pv-devel@lists.xenproject.org List-Id: Developer list for the Windows PV Drivers subproject List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: win-pv-devel-bounces@lists.xenproject.org Precedence: list Sender: "win-pv-devel" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=xen.org; s=20200302mail; h=Message-Id:Date:Subject:Cc:To:From; bh=XNV4Zt5GCLumkQPs4VuSpyrUBv08qaLo7r2UQ/fQQro=; b=upV/4ARAMOzms43ZWHkjchqZZy 464cbcl5FX79WP9qcY9Kwkrz/xit0HUh1Cn+858tEaaws0NZrkZyoXMASQzmXuRxMmtGNG8oYpeDV Ic/Ef7RRcf4rwGchdQR2YvXfuusoqq0FjHcdN0R2h+pO+XWJeEnRkl92dBWBAaJhi9Fs=; From: Paul Durrant To: win-pv-devel@lists.xenproject.org Cc: Owen Smith , Paul Durrant Subject: [PATCH xenbus] Re-work Cache->Cursor handling in CacheDestroySlab() Date: Mon, 20 May 2024 10:15:20 +0100 Message-Id: <20240520091520.769-1-paul@xen.org> X-Mailer: git-send-email 2.17.1 From: Owen Smith Since the advent of lazy slab spilling in commit 7f8b622668fb ("Improve the performance of the slab allocator"), if the cursor slab is being destroyed, it no longer means it is the only slab in the last. Remove the ASSERTions and simply set the new cursor to the current cursor's Flink. This will either be the next slab which (because slabs are kept in decreasing order of occupancy) will also be empty, or it will be the list anchor (which indicates that all slabs in the list are full). Call CacheAudit() after slab removal to re-verify these invariants. Also clean up a typo in a commit comment in CacheAudit(). Signed-off-by: Owen Smith [Re-worked original patch] Signed-off-by: Paul Durrant --- src/xenbus/cache.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/xenbus/cache.c b/src/xenbus/cache.c index 6229aeddefc2..b67c5dd7526d 100644 --- a/src/xenbus/cache.c +++ b/src/xenbus/cache.c @@ -360,7 +360,7 @@ CacheAudit( PLIST_ENTRY ListEntry; // - // The cursror should point at the first slab that is not fully + // The cursor should point at the first slab that is not fully // occupied. // for (ListEntry = Cache->SlabList.Flink; @@ -478,21 +478,19 @@ CacheDestroySlab( Cache->Count -= CacheMaskSize(Slab->Allocated); // - // The only reason the cursor should be pointing at this slab is - // if it is the only one in the list. + // The cursor slab should always be the first slab in the list that is not + // fully occupied. If we are destroying it then clearly it is empty, but + // it may be one of several empty slabs. Set the cursor to the current + // cursor's Flink so that it will either point at the next empty slab, or + // the list anchor if there are no more empty slabs. // - if (Cache->Cursor == &Slab->ListEntry) { - ASSERT3P(Slab->ListEntry.Flink, ==, &Cache->SlabList); - ASSERT3P(Slab->ListEntry.Blink, ==, &Cache->SlabList); - Cache->Cursor = &Cache->SlabList; - } + if (Cache->Cursor == &Slab->ListEntry) + Cache->Cursor = Slab->ListEntry.Flink; RemoveEntryList(&Slab->ListEntry); + CacheAudit(Cache); - ASSERT(IMPLY(Cache->Cursor == &Cache->SlabList, - IsListEmpty(&Cache->SlabList))); - - Index = CacheMaskSize(Slab->Allocated); + Index = CacheMaskSize(Slab->Constructed); while (--Index >= 0) { PVOID Object = (PVOID)&Slab->Buffer[Index * Cache->Size]; -- 2.17.1