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=-8.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,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 3B902C433E2 for ; Mon, 14 Sep 2020 12:48:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0F8A1208DB for ; Mon, 14 Sep 2020 12:48:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726512AbgINMsi (ORCPT ); Mon, 14 Sep 2020 08:48:38 -0400 Received: from mx2.suse.de ([195.135.220.15]:55928 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726608AbgINMoG (ORCPT ); Mon, 14 Sep 2020 08:44:06 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 1AAE7AD25; Mon, 14 Sep 2020 12:44:21 +0000 (UTC) Date: Mon, 14 Sep 2020 14:44:05 +0200 From: Petr Mladek To: John Ogness Cc: Sergey Senozhatsky , Sergey Senozhatsky , Steven Rostedt , linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] printk: ringbuffer: avoid memcpy() on state_var Message-ID: <20200914124404.GD11154@alley> References: <20200914094803.27365-1-john.ogness@linutronix.de> <20200914094803.27365-2-john.ogness@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200914094803.27365-2-john.ogness@linutronix.de> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon 2020-09-14 11:54:03, John Ogness wrote: > @state_var is copied as part of the descriptor copying via > memcpy(). This is not allowed because @state_var is an atomic type, > which in some implementations may contain a spinlock. Great catch! > Avoid using memcpy() with @state_var by explicitly copying the other > fields of the descriptor. @state_var is set using atomic set > operator before returning. Just thinking loudly. Strictly speaking, memcpy() might be used when the atomic variable is later corrected by an atomic operation. And it is done by the first patch. I think that it is a matter of taste what solution is more error prone. Either this function must be updated when a new field is added into struct prb_desc or the atomic_long_set() must not get removed. Hmm, missing memcpy() should be rather easy to debug. While missing atomic_long_set() is really subtle problem problem. So, the new code is better from my POV. > Fixes: b6cf8b3f3312 ("printk: add lockless ringbuffer") > Signed-off-by: John Ogness Reviewed-by: Petr Mladek Best Regards, Petr