linux-kernel - Re: [patch] cpufreq: mark cpufreq_tsc() as core_initcall

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]

Date:	Fri, 24 Nov 2006 22:24:33 -0500 (EST)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Oleg Nesterov <oleg@...sign.ru>
cc:	"Paul E. McKenney" <paulmck@...ibm.com>,
	Jens Axboe <jens.axboe@...cle.com>,
	Kernel development list <linux-kernel@...r.kernel.org>
Subject: Re: [patch] cpufreq: mark cpufreq_tsc() as core_initcall_sync

On Sat, 25 Nov 2006, Oleg Nesterov wrote:

> > Given that you aren't using per-cpu data, why not just rely on a spinlock?
> 
> I thought about this too, and we can re-use sp->wq.lock,

Yes, although it would be a layering violation.

> > Then everything will be simple and easy to verify,
> 
> xxx_read_lock() will be simpler, but not too much. synchronize_xxx() needs
> some complication.

Look at the (untested) example below.  The code may be a little bit 
longer, but it's a lot easier to understand and verify.

> spin_lock() + spin_unlock() doesn't imply mb(), it allows subsequent loads
> to move into the the critical region.

No, that's wrong.  Subsequent loads are allowed to move into the region 
protected by the spinlock, but not past it (into the xxx critical 
section).

> I personally prefer this way, but may be you are right.

See what you think...

Alan

//-----------------------------------------------------------------------------
struct xxx_struct {
	int completed;
	int ctr[2];
	struct mutex mutex;
	spinlock_t lock;
	wait_queue_head_t wq;
};

void init_xxx_struct(struct xxx_struct *sp)
{
	sp->completed = 0;
	sp->ctr[0] = 1;
	sp->ctr[1] = 0;
	spin_lock_init(&sp->lock);
	mutex_init(&sp->mutex);
	init_waitqueue_head(&sp->wq);
}

int xxx_read_lock(struct xxx_struct *sp)
{
	int idx;

	spin_lock(&sp->lock);
	idx = sp->completed & 0x1;
	++sp->ctr[idx];
	spin_unlock(&sp->lock);
	return idx;
}

void xxx_read_unlock(struct xxx_struct *sp, int idx)
{
	spin_lock(&sp->lock);
	if (--sp->ctr[idx] == 0)
		wake_up(&sp->wq);
	spin_unlock(&sp->lock);
}

void synchronize_xxx(struct xxx_struct *sp)
{
	int idx;

	mutex_lock(&sp->mutex);

	spin_lock(&sp->lock);
	idx = sp->completed & 0x1;
	++sp->completed;
	--sp->ctr[idx];
	sp->ctr[idx ^ 1] = 1;
	spin_unlock(&sp->lock);

	wait_event(sp->wq, sp->ctr[idx] == 0);
	mutex_unlock(&sp->mutex);
}

-
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/