[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJrWOzCjXQ8D=OprqHCH7NXn81-rVqZyjbz6+8mAP=GHgV3_vg@mail.gmail.com>
Date: Thu, 11 Aug 2016 11:07:14 +0200
From: Roman Penyaev <roman.penyaev@...fitbricks.com>
To: Tejun Heo <tj@...nel.org>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] percpu-refcount: do not forget to rcu_barrier()
just before freeing
On Thu, Aug 11, 2016 at 12:09 AM, Tejun Heo <tj@...nel.org> wrote:
> On Wed, Aug 10, 2016 at 09:55:39PM +0200, Roman Pen wrote:
>> percpu issues some RCU callbacks to synchronize its state, so before
>> freeing we have to wait all those callbacks to finish.
>>
>> E.g. the following simple sequence on stack causes nasty crash:
>>
>> struct percpu_ref ref;
>>
>> percpu_ref_init(&ref, release, 0, GFP_KERNEL);
>> percpu_ref_kill(&ref);
>> percpu_ref_exit(&ref);
>
> Hmmm... that's just an illegal sequence of operations. You can't exit
> a ref which hasn't completed killing yet (the kill callback hasn't
> been called).
Yes, exactly, this is an illegal operation. But it is not more illegal
than calling kill() twice or reinit() when counter is not yet zero.
And those illegals are covered with warnings, which can be observed
for example with this freeze/unfreeze blk-mq bug.
So what I want to say is that bugs exist above percpu-ref and can
easily trigger illegal sequence and nasty crash is not a good way
to say that someone did a mistake.
But of course, that is very minor and was discovered by my stupidity
and tests which use percpu-ref in not a kosher way :)
--
Roman
Powered by blists - more mailing lists