linux-kernel - Re: [PATCH] generic dynamic per cpu refcounting

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: <20130128182737.GC22465@mtj.dyndns.org>
Date:	Mon, 28 Jan 2013 10:27:37 -0800
From:	Tejun Heo <tj@...nel.org>
To:	Kent Overstreet <koverstreet@...gle.com>
Cc:	Oleg Nesterov <oleg@...hat.com>, srivatsa.bhat@...ux.vnet.ibm.com,
	rusty@...tcorp.com.au, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] generic dynamic per cpu refcounting

Hello, guys.

On Mon, Jan 28, 2013 at 10:15:28AM -0800, Kent Overstreet wrote:
> > 	percpu_ref_kill();
> > 	put_and_dsetroy();
> > 
> > And this can race with another holder which drops the last reference,
> > its put_and_dsetroy() can see PCPU_REF_DYING and return false.
> > 
> > Or I misunderstood the code/interface?
> 
> Nope, nailed it :) That should _definitely_ be in the documentation.

Can we just combine kill initiation and base ref put and make that the
responsibility of the owner?  Extra features on basic constructs may
seem good for certain use cases but tend to bring more confusion than
good in the long run.  If a user needs to synchronize among multiple
killers, let the user deal with the issue.

> Actually - I think it'd be better to have the default percpu_ref_kill()
> do the second synchronize_rcu(), and have an unsafe version that skips
> it.

Note that synchronize_rcu/sched() can be very slow and cause problems
in paths which are frequently traveled and visible to userland.  It's
fine for things like module destruction but can be a problem even
during device destruction - blkcg had synchronize_rcu() in
request_queue destruction which led to huge latencies during boot
because SCSI wants to create and then destroy request_queues for all
possible LUNs on certain configurations.  So, if you put
synchronize_rcu/sched() in percpu_ref_kill(), that better not be used
from e.g. close(2).

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/