From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f53.google.com (mail-wm1-f53.google.com [209.85.128.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CA0813BBA09 for ; Thu, 19 Mar 2026 11:17:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.53 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773919029; cv=none; b=VgP6ujAHszrD+Vp2LqcBMu7oThycL7ZqPnZ2SNzETfIeoqymys9GJHQzidHR7OwkGkV8hEULIJFWhv3c62mIk5hGEmR2KikORbuLW8fmtD1KZZ5jLqbXtnRvU5cTPqT/mdXvcqZ9d1YeyQEU5gCTl3V6JgLhN9F+5jwKTMSRd1c= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773919029; c=relaxed/simple; bh=yHw/35XNMej1lUifL+ZFpTub6WjTZHUsGA3jJOnWl0s=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=JgnCix+ZQRF9FYWne8f0DKvCcoy8a5kqcopKUfp9Q5KvIwFLOpjwcL2V0xx3rHP9OFjOmpdkc4JIYRfl5viQ3kGebwyX0AGyJsXTwuqNdtiZ/HQIxAgNsJdKVHYZF8h8wg5HlpvQYUJIfm2Oxoj39q8UJTvivTdQal8twTn3xDU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=YQo9tqAb; arc=none smtp.client-ip=209.85.128.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="YQo9tqAb" Received: by mail-wm1-f53.google.com with SMTP id 5b1f17b1804b1-4852e9ca034so7148175e9.2 for ; Thu, 19 Mar 2026 04:17:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1773919026; x=1774523826; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=e2wM6sGnXsNFSp1BD9GkQIxbDImOFriLb2HV5PVZ7fI=; b=YQo9tqAbmfIOIBqZrxYfCjuO4dtudl7znIj7C5pZzvEJYF5StLVYbvxV4yvPAO2PX8 MgKvCvWkHL0MQJ/Sw4OUlryii8vE/aIyU/sTtN/nzdF6rgRfjxyGCnX+olrE5/ksWbUB T2CKpNVjL8/y8fqT3eWWcL/qDZQcxDPkskCFO6Cm2xDI8OG0PFHWy67E8i/DaOr1q2ah Yhxq6GlbhP3uszXc/G0JjWkO5OAkWdBfsGYf0WqRYOZnbm+fALedGnE+U4IlQfoEtVR7 AfxpTgVST202b/iFd7LnSL/MydeemcNnS3aSIzG21yanqIDm5Ew5YJWMRHQK5v4lUwmq YsOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1773919026; x=1774523826; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=e2wM6sGnXsNFSp1BD9GkQIxbDImOFriLb2HV5PVZ7fI=; b=Vf+4gyFFjhNPi27sImQnyl9VoaOo7FVOeVQNmlxswcWkF2XtSwaGHqdlCWivFDF/FA WLtXMlsBFwqdWqWFP5JnV2mAjKW69jJ8zbe/YuhgVBhMCHrfmj8t/FqVpptF8RnrNNAb YTMRGqDYfrizT4jOxXu0XjTYVmcg7M9LXfpECEoCCfMAD3i0P/NJgWbyJfoj/Mvtd4rw XstBDQPanX6EG6e2zHzZZsn91tcTzIrjeMjQc4tMvzkLPLHytzjmDqZepe2iyzbQCa/+ ieMtu0Ne+4hJdi0XwvxEEP20z4zGhpXmrpxSdEVdE8Qrk0qeHYHE6yiEOv9Yo+WJsnK7 IfKg== X-Forwarded-Encrypted: i=1; AJvYcCUAUiglDK44xp8eOB4i5gAvy+23jGzTAMRpz055BoF6xiW7zQMmDP3jh/KRdYr+gyOSLPGe/+MsKKrVPbw=@vger.kernel.org X-Gm-Message-State: AOJu0Yy4cRPyEvmgeYSw1Q+cC25Xe06InFcQM2OmC8OM7jTyiRGApEng zuqNE/M26vcblE0W8NXx1giLAvtRAqDubvaNJ2L1L/2VIdpw7MY1tmIF54fhQKCb8Fc= X-Gm-Gg: ATEYQzyMUu+XZJPONWEwV+PjteO7yRwrBxoC/tOIIob+synPZtnv1XhQH1t9XPhcVcv U2uV8r/N0y3V27OosneOMyWF2txnRP9phN/d9b4FgB++RYXjQEGqH9cq7B7mMszJv+RFqgL4oMr weWT0hbtolW4upZ1jnLRfyooLXYkoz0Xl4iH+ZLM2fMaZwf8LHnYb2c3X0TbsSW/F8NaowvAPzD WKXQFGjaKsHht012Jvo/QjTP6iphaLCy0XuiYtdSbHyu3sqg8bUQ7LB1WMW1bpp3WyEMkNGobjI tl6QuGrWlRsU4SVex5frSmEUiCPMoKPe23R5kMhgzbcmJRVUK/0JWB7Srl3ODEJ7cX8ERhlaL2b 7t9iSkUYvO9AqzG8DcZekw34pS7ufSKgbzRtXUBnVlzKlKkOtyZ28D7FvIKJ7J+nQ05bSSO4Cfj eWQ6fITAl38lsOoQJQBRIATrMG8SPnuSMNkCbK X-Received: by 2002:a05:600c:c163:b0:485:3abe:ab86 with SMTP id 5b1f17b1804b1-486f441b801mr128405905e9.4.1773919026005; Thu, 19 Mar 2026 04:17:06 -0700 (PDT) Received: from pathway.suse.cz ([176.114.240.130]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-486f4bc96c6sm68794225e9.5.2026.03.19.04.17.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Mar 2026 04:17:05 -0700 (PDT) Date: Thu, 19 Mar 2026 12:17:04 +0100 From: Petr Mladek To: John Ogness Cc: Jianzhou Zhao , linux-kernel@vger.kernel.org, senozhatsky@chromium.org, rostedt@goodmis.org Subject: Re: KCSAN: data-race in desc_read / prb_reserve_in_last Message-ID: References: <1a32d127.3865.19cdaf2c166.Coremail.luckd0g@163.com> <87ikazitc4.fsf@jogness.linutronix.de> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87ikazitc4.fsf@jogness.linutronix.de> On Fri 2026-03-13 17:37:55, John Ogness wrote: > Hi Petr, > > On 2026-03-12, Petr Mladek wrote: > > [rfc v2]: > > > > diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringbuffer.c > > index 56c8e3d031f4..c080edf5636a 100644 > > --- a/kernel/printk/printk_ringbuffer.c > > +++ b/kernel/printk/printk_ringbuffer.c > > @@ -454,11 +454,12 @@ static enum desc_state desc_read(struct prb_desc_ring *desc_ring, > > struct printk_info *info = to_info(desc_ring, id); > > struct prb_desc *desc = to_desc(desc_ring, id); > > atomic_long_t *state_var = &desc->state_var; > > + unsigned long state_val, state_val_check; > > enum desc_state d_state; > > - unsigned long state_val; > > > > /* Check the descriptor state. */ > > state_val = atomic_long_read(state_var); /* LMM(desc_read:A) */ > > +try_again: > > d_state = get_desc_state(id, state_val); > > if (d_state == desc_miss || d_state == desc_reserved) { > > /* > > @@ -466,7 +467,9 @@ static enum desc_state desc_read(struct prb_desc_ring *desc_ring, > > * @state_var so that the caller can see the details of > > * the inconsistent state. > > */ > > - goto out; > > + if (desc_out) > > + atomic_long_set(&desc_out->state_var, state_val); > > + return d_state; > > I would keep the "goto out" so that the success scenario can also set > the state_var of @desc_out. You are right. > > } > > > > /* > > @@ -542,14 +545,19 @@ static enum desc_state desc_read(struct prb_desc_ring *desc_ring, > > smp_rmb(); /* LMM(desc_read:D) */ > > > > /* > > - * The data has been copied. Return the current descriptor state, > > - * which may have changed since the load above. > > + * The data has been copied. Make sure that they match the given @id > > + * and @d_state read at the beginning. Re-read them otherwise. > > + * > > + * Note that this simplifies the logic. Especially, the reader API > > + * expects that all values are stable when the record is finalized. > > + * But they are stable only when it stays finalized all the time. > > */ > > - state_val = atomic_long_read(state_var); /* LMM(desc_read:E) */ > > - d_state = get_desc_state(id, state_val); > > -out: > > - if (desc_out) > > - atomic_long_set(&desc_out->state_var, state_val); > > + state_val_check = atomic_long_read(state_var); /* LMM(desc_read:E) */ > > + if (state_val_check != state_val) { > > + state_val = state_val_check; > > + goto try_again; > > + } > > + > > The out label and code could stay as it was. Or is there some reason you > do not want to set the state_var in the success case? It was a mistake. I was confused by the following code: /* * Copy the descriptor data. The data is not valid until the * state has been re-checked. A memcpy() for all of @desc * cannot be used because of the atomic_t @state_var field. */ if (desc_out) { memcpy(&desc_out->text_blk_lpos, &desc->text_blk_lpos, sizeof(desc_out->text_blk_lpos)); /* LMM(desc_read:C) */ } I somehow assumed that the memcpy copied everything. And we will not longer need to update state_var when state_val did not change. But the code copies only text_blk_lpos. /o\ I am going to send a proper patch where this will be fixed. > out: > if (desc_out) > atomic_long_set(&desc_out->state_var, state_val); > > > return d_state; > > } > > We should probably annotate the memcpy() with a wrapping > data_race(). Sadly we were aware of and expected the data race, but did > not realize that commit 4cfc7258f876 ("printk: ringbuffer: add > finalization/extension support") made the data race a problem. Yup. I'll add the data_race() as well. Thanks a lot for review. Best Regards, Petr