linux-kernel - Re: [PATCH-v3 1/4] idr: Percpu ida

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130828204454.GC1357@kmo-pixel>
Date:	Wed, 28 Aug 2013 13:44:54 -0700
From:	Kent Overstreet <kmo@...erainc.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	"Nicholas A. Bellinger" <nab@...ux-iscsi.org>,
	target-devel <target-devel@...r.kernel.org>,
	lf-virt <virtualization@...ts.linux-foundation.org>,
	lkml <linux-kernel@...r.kernel.org>,
	kvm-devel <kvm@...r.kernel.org>,
	"Michael S. Tsirkin" <mst@...hat.com>, Asias He <asias@...hat.com>,
	Jens Axboe <axboe@...nel.dk>, Tejun Heo <tj@...nel.org>,
	Ingo Molnar <mingo@...hat.com>,
	Andi Kleen <andi@...stfloor.org>,
	Christoph Lameter <cl@...two.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Christoph Lameter <cl@...ux-foundation.org>
Subject: Re: [PATCH-v3 1/4] idr: Percpu ida

On Wed, Aug 28, 2013 at 01:23:32PM -0700, Andrew Morton wrote:
> On Wed, 28 Aug 2013 12:53:17 -0700 Kent Overstreet <kmo@...erainc.com> wrote:
> 
> > > > +	while (1) {
> > > > +		spin_lock(&pool->lock);
> > > > +
> > > > +		/*
> > > > +		 * prepare_to_wait() must come before steal_tags(), in case
> > > > +		 * percpu_ida_free() on another cpu flips a bit in
> > > > +		 * cpus_have_tags
> > > > +		 *
> > > > +		 * global lock held and irqs disabled, don't need percpu lock
> > > > +		 */
> > > > +		prepare_to_wait(&pool->wait, &wait, TASK_UNINTERRUPTIBLE);
> > > > +
> > > > +		if (!tags->nr_free)
> > > > +			alloc_global_tags(pool, tags);
> > > > +		if (!tags->nr_free)
> > > > +			steal_tags(pool, tags);
> > > > +
> > > > +		if (tags->nr_free) {
> > > > +			tag = tags->freelist[--tags->nr_free];
> > > > +			if (tags->nr_free)
> > > > +				set_bit(smp_processor_id(),
> > > > +					pool->cpus_have_tags);
> > > > +		}
> > > > +
> > > > +		spin_unlock(&pool->lock);
> > > > +		local_irq_restore(flags);
> > > > +
> > > > +		if (tag >= 0 || !(gfp & __GFP_WAIT))
> > > > +			break;
> > > > +
> > > > +		schedule();
> > > > +
> > > > +		local_irq_save(flags);
> > > > +		tags = this_cpu_ptr(pool->tag_cpu);
> > > > +	}
> > > 
> > > What guarantees that this wait will terminate?
> > 
> > It seems fairly clear to me from the break statement a couple lines up;
> > if we were passed __GFP_WAIT we terminate iff we succesfully allocated a
> > tag. If we weren't passed __GFP_WAIT we never actually sleep.
> 
> OK ;)  Let me rephrase.  What guarantees that a tag will become available?
> 
> If what we have here is an open-coded __GFP_NOFAIL then that is
> potentially problematic.

It's the same semantics as a mempool, really - it'll succeed when a tag
gets freed.  If we are sleeping then there isn't really anything else we
can do, there isn't anything we're trying in the __GFP_WAIT case that
we're not trying in the GFP_NOWAIT case.
--
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/