lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 18 Feb 2022 15:09:26 +0900 From: Byungchul Park <byungchul.park@....com> To: Steven Rostedt <rostedt@...dmis.org> Cc: torvalds@...ux-foundation.org, damien.lemoal@...nsource.wdc.com, linux-ide@...r.kernel.org, adilger.kernel@...ger.ca, linux-ext4@...r.kernel.org, mingo@...hat.com, linux-kernel@...r.kernel.org, peterz@...radead.org, will@...nel.org, tglx@...utronix.de, joel@...lfernandes.org, sashal@...nel.org, daniel.vetter@...ll.ch, chris@...is-wilson.co.uk, duyuyang@...il.com, johannes.berg@...el.com, tj@...nel.org, tytso@....edu, willy@...radead.org, david@...morbit.com, amir73il@...il.com, bfields@...ldses.org, gregkh@...uxfoundation.org, kernel-team@....com, linux-mm@...ck.org, akpm@...ux-foundation.org, mhocko@...nel.org, minchan@...nel.org, hannes@...xchg.org, vdavydov.dev@...il.com, sj@...nel.org, jglisse@...hat.com, dennis@...nel.org, cl@...ux.com, penberg@...nel.org, rientjes@...gle.com, vbabka@...e.cz, ngupta@...are.org, linux-block@...r.kernel.org, axboe@...nel.dk, paolo.valente@...aro.org, josef@...icpanda.com, linux-fsdevel@...r.kernel.org, viro@...iv.linux.org.uk, jack@...e.cz, jack@...e.com, jlayton@...nel.org, dan.j.williams@...el.com, hch@...radead.org, djwong@...nel.org, dri-devel@...ts.freedesktop.org, airlied@...ux.ie, rodrigosiqueiramelo@...il.com, melissa.srw@...il.com, hamohammed.sa@...il.com Subject: Re: [PATCH 02/16] dept: Implement Dept(Dependency Tracker) On Thu, Feb 17, 2022 at 12:36:56PM -0500, Steven Rostedt wrote: > > +struct dept_ecxt; > > +struct dept_iecxt { > > + struct dept_ecxt *ecxt; > > + int enirq; > > + bool staled; /* for preventing to add a new ecxt */ > > +}; > > + > > +struct dept_wait; > > +struct dept_iwait { > > + struct dept_wait *wait; > > + int irq; > > + bool staled; /* for preventing to add a new wait */ > > + bool touched; > > +}; > > Nit. It makes it easier to read (and then review) if structures are spaced > where their fields are all lined up: > > struct dept_iecxt { > struct dept_ecxt *ecxt; > int enirq; > bool staled; > }; > > struct dept_iwait { > struct dept_wait *wait; > int irq; > bool staled; > bool touched; > }; > > See, the fields stand out, and is nicer on the eyes. Especially for those > of us that are getting up in age, and our eyes do not work as well as they > use to ;-) Sure! I will apply this. > > + * --- > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your ootion) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, but > > + * WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + * General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public License > > + * along with this program; if not, you can access it online at > > + * http://www.gnu.org/licenses/gpl-2.0.html. > > The SPDX at the top of the file is all that is needed. Please remove this > boiler plate. We do not use GPL boiler plates in the kernel anymore. The > SPDX code supersedes that. Thank you for informing it! > > +/* > > + * Can use llist no matter whether CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG is > > + * enabled because DEPT never race with NMI by nesting control. > > "never races with" Good eyes! > Although, I'm confused by what you mean with "by nesting control". I should've expressed it more clearly. It meant NMI and other contexts never run inside of Dept concurrently in the same CPU by preventing reentrance. > > +static void initialize_class(struct dept_class *c) > > +{ > > + int i; > > + > > + for (i = 0; i < DEPT_IRQS_NR; i++) { > > + struct dept_iecxt *ie = &c->iecxt[i]; > > + struct dept_iwait *iw = &c->iwait[i]; > > + > > + ie->ecxt = NULL; > > + ie->enirq = i; > > + ie->staled = false; > > + > > + iw->wait = NULL; > > + iw->irq = i; > > + iw->staled = false; > > + iw->touched = false; > > + } > > + c->bfs_gen = 0U; > > Is the U really necessary? I was just wondering if it's really harmful? I want to leave this if it's harmless because U let us guess the data type of ->bfs_gen correctly at a glance. Or am I missing some reason why I should fix this? Thank you very much, Steven.
Powered by blists - more mailing lists