Received: by 2002:a25:8b12:0:0:0:0:0 with SMTP id i18csp1240015ybl; Fri, 23 Aug 2019 16:00:54 -0700 (PDT) X-Google-Smtp-Source: APXvYqwwZjnTpoXz8HgY6DEm81yPfUPjH2CzI1zzw30kv33TeFF9TnEZPBhvrI37NyaejszuTqAu X-Received: by 2002:a65:640a:: with SMTP id a10mr5903460pgv.338.1566601254105; Fri, 23 Aug 2019 16:00:54 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1566601254; cv=none; d=google.com; s=arc-20160816; b=pi299XYiA1AaNgsybcnL3oGps2JBkXZAp6XYosw4tQRRwRayi04ZYO9uKhAZStdKRf AEgRE8707xtIX4feMo6ls9jilwaCEgcE7RTs972Z7Co0U/8UBOc70JSw5Ep9bjcRLB3j xoYlrGD/muo8IEP7fxUvb3zS7lvqT9MFTEpUtiaLjD4t764VcOhgX95Myf7MH4rb/ycz xZzc+RGeCK1RIOR4c7sRQKSqon7eHFlrCWbpeATBDJ/IfQWjta2IRgBcpTalR0zascq6 839ZGrKgIn9D8vXzXEs/cRqYpikbx7Uu1VkC+vXwLVgN1DWnDCAzYN5MioF+4TI+6ZuX KFXg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date; bh=fxuhS80oQDA3AvdS7zagXQ2qBZsVfi3XqeUXp8EG2q8=; b=Dgkckl6KDg8X8hFuj/UpHltk5tnXYFjmjelmnxAwDpxDpsZOkEZiPa69dN1imViaT/ SwQZulBDYgRz4kKhBgycz2u3FvWo6xyiNpzVppCJ7ID3qHvQMcEMBZX2OJabLxL/1yHc XzPik4actEucAKPRx9sEcxUjTzVSGK8KEHWjK0zje+ueCsHIhukcQXwQ/S+njSFAwB7w 0ADhS4CwjHNlOCaTGyUC/Zdt4x1LC3mJofUFQY4vSGPogNF+xtAUAXdBMncQ25KayYpU 8XgQN5sVWb+bJxkwiBYPGmheXrYT5m9ZMXgqKH4HEGLp0OUkqVXvhzAKYHMngCPJcruk EYUQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id i4si3015789pgj.558.2019.08.23.16.00.38; Fri, 23 Aug 2019 16:00:54 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387720AbfHWK3F (ORCPT + 99 others); Fri, 23 Aug 2019 06:29:05 -0400 Received: from mx2.suse.de ([195.135.220.15]:53940 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726142AbfHWK3F (ORCPT ); Fri, 23 Aug 2019 06:29:05 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 49845ACB4; Fri, 23 Aug 2019 10:29:03 +0000 (UTC) Date: Fri, 23 Aug 2019 12:29:02 +0200 From: Petr Mladek To: Sergey Senozhatsky Cc: John Ogness , Andrea Parri , Sergey Senozhatsky , Steven Rostedt , Brendan Higgins , Peter Zijlstra , Thomas Gleixner , Linus Torvalds , Greg Kroah-Hartman , linux-kernel@vger.kernel.org Subject: Re: comments style: Re: [RFC PATCH v4 1/9] printk-rb: add a new printk ringbuffer implementation Message-ID: <20190823102902.c2l2h3qinykrcmep@pathway.suse.cz> References: <20190807222634.1723-1-john.ogness@linutronix.de> <20190807222634.1723-2-john.ogness@linutronix.de> <20190820085554.deuejmxn4kbqnq7n@pathway.suse.cz> <20190820092731.GA14137@jagdpanzerIV> <87a7c3f4uj.fsf@linutronix.de> <20190823055445.GA7195@jagdpanzerIV> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190823055445.GA7195@jagdpanzerIV> User-Agent: NeoMutt/20170912 (1.9.0) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri 2019-08-23 14:54:45, Sergey Senozhatsky wrote: > On (08/21/19 07:46), John Ogness wrote: > [..] > > The labels are necessary for the technical documentation of the > > barriers. And, after spending much time in this, I find them very > > useful. But I agree that there needs to be a better way to assign label > > names. > [..] > > > Where dp stands for descriptor push. For dataring we can add a 'dr' > > > prefix, to avoid confusion with desc barriers, which have 'd' prefix. > > > And so on. Dunno. > > > > Yeah, I spent a lot of time going in circles on this one. > [..] > > I hope that we can agree that the labels are important. And that a > > formal documentation of the barriers is also important. Yes, they are a > > lot of work, but I find it makes it a lot easier to go back to the code > > after I've been away for a while. Even now, as I go through your > > feedback on code that I wrote over a month ago, I find the formal > > comments critical to quickly understand _exactly_ why the memory > > barriers exist. > > Yeah. I like those tagsi/labels, and appreciate your efforts. > > Speaking about it in general, not necessarily related to printk patch set. > With or without labels/tags we still have to grep. But grep-ing is much > easier when we have labels/tags. Otherwise it's sometimes hard to understand > what to grep for - _acquire, _relaxed, smp barrier, write_once, or > anything else. Grepping is not needed when function names are used in the comment and cscope might be used. Each function should be short and easy enough so that any nested label can be found by eyes. A custom script is an alternative but it would be better to use existing tools. In each case, two letter labels would get redundant sooner or later when the semantic gets used widely. And I hope that we use a semantic that is going to be used widely. > > Perhaps we should choose labels that are more clear, like: > > > > dataring_push:A > > dataring_push:B > > > > Then we would see comments like: > > > > Memory barrier involvement: > > > > If _dataring_pop:B reads from dataring_datablock_setid:A, then > > _dataring_pop:C reads from dataring_push:G. > [..] > > RELEASE from dataring_push:E to dataring_datablock_setid:A > > matching > > ACQUIRE from _dataring_pop:B to _dataring_pop:E > > I thought about it. That's very informative, albeit pretty hard to maintain. > The same applies to drA or prA and any other context dependent prefix. The maintenance is my concern as well. The labels should be primary for an automatized consistency checker. They make sense only when they are in sync with the code. Best Regards, Petr