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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 9A0BEC54E58 for ; Mon, 18 Mar 2024 07:51:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=iMhB4l5Z3O31QHb3ZBg4yPQo5XMVpz2ndBUXs6CnyUk=; b=rCgu4c7+/tqYHB HldFj5O1IJZpbHTGwrthQ+XyZZ7SbcBCOXdXArZ9XptWU3GncP7DaqISPxvndcwijoSrQMSjrd994 57yrFpCrpSSpGIrUaOBl8VdznH6v4GdbAE4hp8THBGwT0JI8E3Fq8somDGFZ1ocifjw6B4BlfbWhZ lZ5twwxpB9WAtaY8hSN0hSuEKGfKWfpw83OH1aQfY+O6k4Uv2qLs9LM0joiZYDE/azeGdeOW1zZUo UUZ6l/qEpEqxCugTJuR1+ds3CTCM3yyZ129yVmXWP+BocB/mgDW03yhbEgjRWdezn7A+I2JDO4ULl kF7RGm3QuPPIUn1XavgQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rm7lr-00000007fDP-0nP9; Mon, 18 Mar 2024 07:50:59 +0000 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rm7ln-00000007fCc-2OPy for linux-arm-kernel@lists.infradead.org; Mon, 18 Mar 2024 07:50:57 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1710748254; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=M61ri+LtQ+sPozYEZ1McUz+TEhyS+suTMSA9oAYcUks=; b=TS+xE6sfDwATF+zc4bCqH6xk7gLpQOxoWkA/HiYDB0W/D525IFg6TDEO3BWs+IIqbaCyXm Qn8A+qaPrPubSH9R9C7gXl7by1oSk4Or/WUWZc3ZfoQgld4XJ5tlhDZ24mi8wONvWYTQsZ +FKZHuylE8VqIpr7M7ejGl9mvIZCVN4= Received: from mail-ot1-f72.google.com (mail-ot1-f72.google.com [209.85.210.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-215-gOALD944MMOnzWTz_4rONg-1; Mon, 18 Mar 2024 03:50:50 -0400 X-MC-Unique: gOALD944MMOnzWTz_4rONg-1 Received: by mail-ot1-f72.google.com with SMTP id 46e09a7af769-6e67ea99696so2505100a34.3 for ; Mon, 18 Mar 2024 00:50:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710748249; x=1711353049; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=M61ri+LtQ+sPozYEZ1McUz+TEhyS+suTMSA9oAYcUks=; b=VPeR680ln1/71PEnjpWcoo2lCzIfnDKTUVoKKhdu5TcS6dQs48K+rpTuqUQH1JFtRn 9tOwK48I34x5sGZYEDuXRrxGrp5AN8kdGTHjo1LqyDHojGI6D25f3fcymMuLglDEyjmP bctQlffX22JAc+cohXEXqI6DrchfiJXM8YJpwE2gDGRQkG1udCG63CnCEKHfYnAjuOnI DXQZ1xB5hGbN6LW7E1b70q8K/3Kl6f4KyL8Zgu+wnIjbTzdiWba7L6aksOQUkSUUsBiL QigYwNH8SFSy1UR/6nhwUa6l5P9G5UBo1tiTUPnwohnKeyXywtpI4bnvoqRE892e/oPB 9HTw== X-Forwarded-Encrypted: i=1; AJvYcCWnhV+JqsARizUK2HOPkAamyR/vhPoeRswZkoVmA4w9zCLNUKwD9/FRHxUF2WHR1ZDujDIusi9AxMZKZFjbGfzvpLVtYFQ7fhEeb8UA2PYlupHJWUg= X-Gm-Message-State: AOJu0YyF6DBmzwI/MbFUKx8JaOu/E4c2VOVtQfRb4BTlhXAgcjXhfk03 iV30dnfFPmMGVvCWmundhiCGtgVejC/Ve4VuNrN66Yxw5WUoD2pocrd3wYdkOittKmscubyanly 3lozTXWiEgrdhWccNfxf4z9AfFGZ6rztx2QNnAV2mdgramAHYiSb0PHQsA6rIUYsVi4Oobc1q X-Received: by 2002:a9d:6e8e:0:b0:6e4:9456:6b04 with SMTP id a14-20020a9d6e8e000000b006e494566b04mr12857357otr.5.1710748249607; Mon, 18 Mar 2024 00:50:49 -0700 (PDT) X-Google-Smtp-Source: AGHT+IE083dlEzBLt+Sky3z69bRCGJ12X1u/z5aEMtKwBNnHV8LauRWAFwR09RP3WUCW/rBcAqaHMg== X-Received: by 2002:a9d:6e8e:0:b0:6e4:9456:6b04 with SMTP id a14-20020a9d6e8e000000b006e494566b04mr12857341otr.5.1710748249106; Mon, 18 Mar 2024 00:50:49 -0700 (PDT) Received: from redhat.com ([2.52.5.113]) by smtp.gmail.com with ESMTPSA id g9-20020a056830084900b006e682c00e59sm843274ott.57.2024.03.18.00.50.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 18 Mar 2024 00:50:48 -0700 (PDT) Date: Mon, 18 Mar 2024 03:50:41 -0400 From: "Michael S. Tsirkin" To: Gavin Shan Cc: virtualization@lists.linux.dev, linux-kernel@vger.kernel.org, jasowang@redhat.com, xuanzhuo@linux.alibaba.com, yihyu@redhat.com, shan.gavin@gmail.com, Will Deacon , Catalin Marinas , linux-arm-kernel@lists.infradead.org, mochs@nvidia.com Subject: Re: [PATCH] virtio_ring: Fix the stale index in available ring Message-ID: <20240318034710-mutt-send-email-mst@kernel.org> References: <20240314040443-mutt-send-email-mst@kernel.org> <9b148de7-b687-4d10-b177-5608b8dc7046@redhat.com> <20240314074216-mutt-send-email-mst@kernel.org> <23dc6d00-6a57-4ddf-8611-f3c6f6a8e43c@redhat.com> <20240314085630-mutt-send-email-mst@kernel.org> <63002c24-8117-458f-84c7-fa4f7acd8cc6@redhat.com> <20240315065318-mutt-send-email-mst@kernel.org> <66e12633-b2d6-4b9a-9103-bb79770fcafa@redhat.com> <20240317124214-mutt-send-email-mst@kernel.org> <589d980f-2e4d-47b4-9dc7-8c64dbe271ce@redhat.com> MIME-Version: 1.0 In-Reply-To: <589d980f-2e4d-47b4-9dc7-8c64dbe271ce@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240318_005055_816510_B97453DC X-CRM114-Status: GOOD ( 34.92 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Mar 18, 2024 at 09:41:45AM +1000, Gavin Shan wrote: > On 3/18/24 02:50, Michael S. Tsirkin wrote: > > On Fri, Mar 15, 2024 at 09:24:36PM +1000, Gavin Shan wrote: > > > > > > On 3/15/24 21:05, Michael S. Tsirkin wrote: > > > > On Fri, Mar 15, 2024 at 08:45:10PM +1000, Gavin Shan wrote: > > > > > > > Yes, I guess smp_wmb() ('dmb') is buggy on NVidia's grace-hopper platform. I tried > > > > > to reproduce it with my own driver where one thread writes to the shared buffer > > > > > and another thread reads from the buffer. I don't hit the out-of-order issue so > > > > > far. > > > > > > > > Make sure the 2 areas you are accessing are in different cache lines. > > > > > > > > > > Yes, I already put those 2 areas to separate cache lines. > > > > > > > > > > > > My driver may be not correct somewhere and I will update if I can reproduce > > > > > the issue with my driver in the future. > > > > > > > > Then maybe your change is just making virtio slower and masks the bug > > > > that is actually elsewhere? > > > > > > > > You don't really need a driver. Here's a simple test: without barriers > > > > assertion will fail. With barriers it will not. > > > > (Warning: didn't bother testing too much, could be buggy. > > > > > > > > --- > > > > > > > > #include > > > > #include > > > > #include > > > > #include > > > > > > > > #define FIRST values[0] > > > > #define SECOND values[64] > > > > > > > > volatile int values[100] = {}; > > > > > > > > void* writer_thread(void* arg) { > > > > while (1) { > > > > FIRST++; > > > > // NEED smp_wmb here > > > __asm__ volatile("dmb ishst" : : : "memory"); > > > > SECOND++; > > > > } > > > > } > > > > > > > > void* reader_thread(void* arg) { > > > > while (1) { > > > > int first = FIRST; > > > > // NEED smp_rmb here > > > __asm__ volatile("dmb ishld" : : : "memory"); > > > > int second = SECOND; > > > > assert(first - second == 1 || first - second == 0); > > > > } > > > > } > > > > > > > > int main() { > > > > pthread_t writer, reader; > > > > > > > > pthread_create(&writer, NULL, writer_thread, NULL); > > > > pthread_create(&reader, NULL, reader_thread, NULL); > > > > > > > > pthread_join(writer, NULL); > > > > pthread_join(reader, NULL); > > > > > > > > return 0; > > > > } > > > > > > > > > > Had a quick test on NVidia's grace-hopper and Ampere's CPUs. I hit > > > the assert on both of them. After replacing 'dmb' with 'dsb', I can > > > hit assert on both of them too. I need to look at the code closely. > > > > > > [root@virt-mtcollins-02 test]# ./a > > > a: a.c:26: reader_thread: Assertion `first - second == 1 || first - second == 0' failed. > > > Aborted (core dumped) > > > > > > [root@nvidia-grace-hopper-05 test]# ./a > > > a: a.c:26: reader_thread: Assertion `first - second == 1 || first - second == 0' failed. > > > Aborted (core dumped) > > > > > > Thanks, > > > Gavin > > > > > > Actually this test is broken. No need for ordering it's a simple race. > > The following works on x86 though (x86 does not need barriers > > though). > > > > > > #include > > #include > > #include > > #include > > > > #if 0 > > #define x86_rmb() asm volatile("lfence":::"memory") > > #define x86_mb() asm volatile("mfence":::"memory") > > #define x86_smb() asm volatile("sfence":::"memory") > > #else > > #define x86_rmb() asm volatile("":::"memory") > > #define x86_mb() asm volatile("":::"memory") > > #define x86_smb() asm volatile("":::"memory") > > #endif > > > > #define FIRST values[0] > > #define SECOND values[640] > > #define FLAG values[1280] > > > > volatile unsigned values[2000] = {}; > > > > void* writer_thread(void* arg) { > > while (1) { > > /* Now synchronize with reader */ > > while(FLAG); > > FIRST++; > > x86_smb(); > > SECOND++; > > x86_smb(); > > FLAG = 1; > > } > > } > > > > void* reader_thread(void* arg) { > > while (1) { > > /* Now synchronize with writer */ > > while(!FLAG); > > x86_rmb(); > > unsigned first = FIRST; > > x86_rmb(); > > unsigned second = SECOND; > > assert(first - second == 1 || first - second == 0); > > FLAG = 0; > > > > if (!(first %1000000)) > > printf("%d\n", first); > > } > > } > > > > int main() { > > pthread_t writer, reader; > > > > pthread_create(&writer, NULL, writer_thread, NULL); > > pthread_create(&reader, NULL, reader_thread, NULL); > > > > pthread_join(writer, NULL); > > pthread_join(reader, NULL); > > > > return 0; > > } > > > > I tried it on host and VM of NVidia's grace-hopper. Without the barriers, I > can hit assert. With the barriers, it's working fine without hitting the > assert. > > I also had some code to mimic virtio vring last weekend, and it's just > working well. Back to our original issue, __smb_wmb() is issued by guest > while __smb_rmb() is executed on host. The VM and host are running at > different exception level: EL2 vs EL1. I'm not sure it's the cause. I > need to modify my code so that __smb_wmb() and __smb_rmb() can be executed > from guest and host. It is thinkably possible that on grace-hopper barriers work differently somehow. We need to find out more though. Anyone from Nvidia can chime in? -- MST _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel