[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20161220093051.GB12200@kroah.com>
Date: Tue, 20 Dec 2016 10:30:51 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: "Reshetova, Elena" <elena.reshetova@...el.com>
Cc: Peter Zijlstra <peterz@...radead.org>,
Liljestrand Hans <ishkamiel@...il.com>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>,
Kees Cook <keescook@...omium.org>,
"will.deacon@....com" <will.deacon@....com>,
Boqun Feng <boqun.feng@...il.com>,
David Windsor <dwindsor@...il.com>, "aik@...abs.ru" <aik@...abs.ru>,
"david@...son.dropbear.id.au" <david@...son.dropbear.id.au>
Subject: Re: Conversion from atomic_t to refcount_t: summary of issues
On Tue, Dec 20, 2016 at 09:13:58AM +0000, Reshetova, Elena wrote:
> > On Mon, Dec 19, 2016 at 07:55:15AM +0000, Reshetova, Elena wrote:
> > > Well, again, you are right in theory, but in practice for example for struct
> > sched_group { atomic_t ref; ... }:
> > >
> > > http://lxr.free-electrons.com/source/kernel/sched/core.c#L6178
> > >
> > > To me this is a refcounter that needs the protection.
> >
> > Only if you have more than UINT_MAX CPUs or something like that.
> >
> > And if you really really want to use refcount_t there, you could +1 the
> > scheme and it'd work again.
>
> Well, yes, probably, but there are many cases like this in practice,
> so we would need to have a good plan how to get it all submitted and
> tested properly. The current patch set is already bigger than what we
> had before and it is only growing.
kernel programming is hard :)
Don't get frustrated, it's going to be a lot of work, just break it up
into chunks and go at it...
> Hans will provide more info later today based on his testing, which
> shows many places in kernel core where we DO actually have increment
> on zero happening in practice and whole kernel doesn't even boot with
> the strictest approach (refusing to inc on zero). And we are only able
> to test for x86....
>
> Given the massive amount of changes, it would be good to merge this at
> least in couple of stages:
>
> 1) first soft version of refcount_t API which at least allows
> increment on zero and all atomic_t used as refcounter occurrences that
> don't require reference counter scheme change (+1 or other)
Why not merge the "correct" implementation? Don't submit something that
doesn't work well. Then fix up the instances that are broken when you
convert them to this new api.
> 2) patch set that fixes all problematic places (potentially with code rewrite)
> 3) patch that removes possibility of inc on zero from refcount_t
That implies that 3) would not happen for another year or so, not good.
Do it right the first time.
thanks,
greg k-h
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
