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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id AD285C7EE3A for ; Wed, 3 May 2023 17:03:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1DE646B0072; Wed, 3 May 2023 13:03:50 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 16A4A6B0075; Wed, 3 May 2023 13:03:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 057CA6B0078; Wed, 3 May 2023 13:03:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from mail-yb1-f202.google.com (mail-yb1-f202.google.com [209.85.219.202]) by kanga.kvack.org (Postfix) with ESMTP id D6EE76B0072 for ; Wed, 3 May 2023 13:03:49 -0400 (EDT) Received: by mail-yb1-f202.google.com with SMTP id 3f1490d57ef6-b9a792ff423so10840254276.1 for ; Wed, 03 May 2023 10:03:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1683133429; x=1685725429; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=zhaj39JNTZzzP0OJ4q1Fsmn7E11iG53wyK5Xw6bPaLM=; b=40UVaor955aGvd53qm0GFgMypuIN/TDh5/Cj/jhcvxQS4Yv/e/MDzudJeFjocQMl4z toBL0aXdf0tGpY68t5/f3Xd8q1uty1AXWrjuKlkXdxUSy5JJf0g6JX5bHceDViBje8Ae S4DQJAuiQ4DPqxbEfLltixvnWbZE10OBCPPh6IRMMDa3+zwbFH1Vju9/ofezaBO1VD+G 43AtH8T8f2cQsgdkHTRD+KZjDCWhi5KxitNJ9TQ3NsS1v+tf/j9rzn0S66YkQjlPBTr0 tspOJhBqJiPsCSAFK5lm2iw8hqxg6Mm+8sRlkCwipalTTk+0WxoYaygjt0Pwj3bvWEHz l0aQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683133429; x=1685725429; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=zhaj39JNTZzzP0OJ4q1Fsmn7E11iG53wyK5Xw6bPaLM=; b=gk8t6N0gLkJ3+tl7vB+Oxp0ssMacWekrSLuJnrhAvjs7McRBrX41PAfzOdAtQYXFT/ MtX1FHNJGX78t5x/qYiu0u72yXTkPFf4RcbPhalv3dsq0Nb0DB7Yx1OO0wFyRH+fZdiU SRJmSd8o+y3Ax2HOGCjZd+QiN7f+wF//4R0VImVh60ahw6o+6X2y4w5vd1rKpNJG/FZx a5D6A9pNRA0I3/sMcAftfvjsyEMT0AcbFcUom0VuRdQz0RxPEz3MWM24+QYefffxab61 C4E9+i3IwfODVRYSRR67XXDCyjEKdXFYeUmMupRHdtFDcIW20z9DM/s95PzSJLCECcgH eovA== X-Gm-Message-State: AC+VfDyrz+/780jKF91ODTATDDmo7/2tZswdKMPzuqrzh0HOtS5Ov0g9 WsRm2gRTQrV90096HewwUAxdhnKiqI5sDQ== X-Google-Smtp-Source: ACHHUZ6YXpLpwm/R0qKToJvq9sHa5Q0iIMTC/TPyNin1GGc7jjHykTcutQN60S4l4xLAspZDXhD3eA3oBvFeWA== X-Received: from shakeelb.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:262e]) (user=shakeelb job=sendgmr) by 2002:a05:6902:154a:b0:b9e:1436:990e with SMTP id r10-20020a056902154a00b00b9e1436990emr6476589ybu.3.1683133429450; Wed, 03 May 2023 10:03:49 -0700 (PDT) Date: Wed, 3 May 2023 17:03:47 +0000 In-Reply-To: Mime-Version: 1.0 References: <20230502160839.361544-1-roman.gushchin@linux.dev> Message-ID: <20230503170347.ldrrtenh57trfpdy@google.com> Subject: Re: [PATCH v2 1/2] mm: kmem: fix a NULL pointer dereference in obj_stock_flush_required() From: Shakeel Butt To: Roman Gushchin Cc: Yosry Ahmed , linux-mm@kvack.org, Andrew Morton , Johannes Weiner , Michal Hocko , Muchun Song , linux-kernel@vger.kernel.org, syzbot+774c29891415ab0fd29d@syzkaller.appspotmail.com, Dmitry Vyukov Content-Type: text/plain; charset="us-ascii" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, May 02, 2023 at 02:38:19PM -0700, Roman Gushchin wrote: [...] > > > > I believe all read accesses other than obj_stock_flush_required() are > > done under the lock, so READ_ONCE() wouldn't be needed AFAICT. Having > > READ_ONCE() only around the racy read can be useful to document the > > racy read and differentiate it from others. > > > > With that said, it's also inconvenient to keep track moving forward of > > which reading sites are racy, and it may be simpler to just annotate > > all readers with READ_ONCE(). > > > > I am not sure which approach is better, just thinking out loud. > > Yeah, I wasn't sure either. I believe that all changes except the original > READ_ONCE() are not leading to any meaningful asm changes, so it's a matter > of taste. > > The reason why I went with the "change them all" approach: > reads without READ_ONCE() and subsequent writes with WRITE_ONCE() > inside a single function looked really weird. > Change them all is the right approach. This code will evolve in future and having partial tagging will cause confusion or might be missed altogether. Also the automated tools prefer change them all.