[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aMnVarvAvQuJCWXy@willie-the-truck>
Date: Tue, 16 Sep 2025 22:23:54 +0100
From: Will Deacon <will@...nel.org>
To: Yeoreum Yun <yeoreum.yun@....com>
Cc: ryabinin.a.a@...il.com, glider@...gle.com, andreyknvl@...il.com,
dvyukov@...gle.com, vincenzo.frascino@....com, corbet@....net,
catalin.marinas@....com, akpm@...ux-foundation.org,
scott@...amperecomputing.com, jhubbard@...dia.com,
pankaj.gupta@....com, leitao@...ian.org, kaleshsingh@...gle.com,
maz@...nel.org, broonie@...nel.org, oliver.upton@...ux.dev,
james.morse@....com, ardb@...nel.org,
hardevsinh.palaniya@...iconsignals.io, david@...hat.com,
yang@...amperecomputing.com, kasan-dev@...glegroups.com,
workflows@...r.kernel.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mm@...ck.org
Subject: Re: [PATCH v7 1/2] kasan/hw-tags: introduce kasan.write_only option
On Tue, Sep 16, 2025 at 09:19:30PM +0100, Yeoreum Yun wrote:
> > On Wed, Sep 03, 2025 at 04:00:19PM +0100, Yeoreum Yun wrote:
> > > + switch (kasan_arg_write_only) {
> > > + case KASAN_ARG_WRITE_ONLY_DEFAULT:
> > > + /* Default is specified by kasan_flag_write_only definition. */
> > > + break;
> > > + case KASAN_ARG_WRITE_ONLY_OFF:
> > > + kasan_flag_write_only = false;
> > > + break;
> > > + case KASAN_ARG_WRITE_ONLY_ON:
> > > + kasan_flag_write_only = true;
> > > + break;
> > > + }
> > > +
> > > kasan_init_tags();
> >
> > I'm probably missing something here, but why have 'enum
> > kasan_arg_write_only' at all? What stops you from setting
> > 'kasan_flag_write_only' directly from early_kasan_flag_write_only()?
> >
> > This all looks weirdly over-engineered, as though 'kasan_flag_write_only'
> > is expected to be statically initialised to something other than 'false'.
>
> For the conherent pattern for other options.
> Since other options manage arg value and internal state separately,
> I just followed former ancestor.
I'm not sure it's the best option to blindly follow the existing code
here. To pick another kasan "mode" at random, 'kasan_flag_vmalloc' is
initialised differently depending on CONFIG_KASAN_VMALLOC and so
allowing for the default value to differ based on the kernel
configuration makes sense.
But that doesn't apply here.
I'd recommend starting simple and just having the 'flag', especially as
you already made a small mistake because of mixing up the 'flag' with
the 'arg'.
Will
Powered by blists - more mailing lists