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  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zhfz7KHJeXYNCw9/@e133380.arm.com>
Date: Thu, 11 Apr 2024 15:30:04 +0100
From: Dave Martin <Dave.Martin@....com>
To: Fenghua Yu <fenghua.yu@...el.com>
Cc: James Morse <james.morse@....com>, x86@...nel.org,
	linux-kernel@...r.kernel.org,
	Reinette Chatre <reinette.chatre@...el.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	H Peter Anvin <hpa@...or.com>, Babu Moger <Babu.Moger@....com>,
	shameerali.kolothum.thodi@...wei.com,
	D Scott Phillips OS <scott@...amperecomputing.com>,
	carl@...amperecomputing.com, lcherian@...vell.com,
	bobo.shaobowang@...wei.com, tan.shaopeng@...itsu.com,
	baolin.wang@...ux.alibaba.com, Jamie Iles <quic_jiles@...cinc.com>,
	Xin Hao <xhao@...ux.alibaba.com>, peternewman@...gle.com,
	dfustini@...libre.com, amitsinght@...vell.com,
	David Hildenbrand <david@...hat.com>,
	Rex Nie <rex.nie@...uarmicro.com>
Subject: Re: [PATCH v1 31/31] x86/resctrl: Move the resctrl filesystem code
 to /fs/resctrl

On Tue, Mar 26, 2024 at 12:44:26PM -0700, Fenghua Yu wrote:
> Hi, James,
> 
> On 3/21/24 09:51, James Morse wrote:
> > resctrl is linux's defacto interface for managing cache and bandwidth
> > policies for groups of tasks.
> > 
> > To allow other architectures to make use of this pseudo filesystem,
> > move it live in /fs/resctrl instead of /arch/x86.
> > 
> > This move leaves behind the parts of resctrl that form the architecture
> > interface for x86.
> > 
> > Signed-off-by: James Morse <james.morse@....com>
> > ---
> > Discussion needed on how/when to merge this, as it would conflict with
> > all outstanding series. It's probably worth deferring to some opportune
> > time, but is included here for illustration.
> > ---
> >   arch/x86/kernel/cpu/resctrl/core.c        |   15 -
> >   arch/x86/kernel/cpu/resctrl/ctrlmondata.c |  505 ---
> >   arch/x86/kernel/cpu/resctrl/internal.h    |  310 --
> >   arch/x86/kernel/cpu/resctrl/monitor.c     |  821 -----
> >   arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 1093 ------
> >   arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 3994 --------------------
> >   fs/resctrl/ctrlmondata.c                  |  527 +++
> >   fs/resctrl/internal.h                     |  340 ++
> >   fs/resctrl/monitor.c                      |  843 +++++
> >   fs/resctrl/psuedo_lock.c                  | 1122 ++++++
> >   fs/resctrl/rdtgroup.c                     | 4013 +++++++++++++++++++++
> >   11 files changed, 6845 insertions(+), 6738 deletions(-)
> > 
> 
> checkpatch reports warnings and checks on this patch. Please fix them. e.g.
> 
> CHECK: Blank lines aren't necessary before a close brace '}'
> #13340: FILE: fs/resctrl/rdtgroup.c:3184:
> +
> +	}

Thanks for spotting these...

However, this is a "move code around with no functional change" patch,
so I think that it should paste the original code across verbatim
without trying to address style violations.  (Otherwise, there is no
hope of checking whether this patch is correct or not...)

For the above example, see:
47820e73f5b3 ("x86/resctrl: Initialize a new resource group with default MBA values")

Other than code that is moved or cloned from previously existing code,
do you see any new style problems actually introduced by this patch?


Notwithstanding the above, this series will conflict with a lot of the
in-flight changes pending for resctrl, so it could be a good opportunity
to fix some legacy style nits.

Reinette, do you have a view on this?  If legacy style problems get
addressed in the moved code, are they essential for this series or could
that be done in a follow-up?

Cheers
---Dave

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ