linux-kernel - Re: [PATCH] rwsem: add rwsem_is

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] [day] [month] [year] [list]

Date:	Tue, 17 Sep 2013 08:53:24 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Josef Bacik <jbacik@...ionio.com>, linux-btrfs@...r.kernel.org,
	walken@...gle.com, mingo@...e.hu, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rwsem: add rwsem_is_contended


* Andrew Morton <akpm@...ux-foundation.org> wrote:

> On Fri, 30 Aug 2013 10:14:01 -0400 Josef Bacik <jbacik@...ionio.com> wrote:
> 
> > Btrfs uses an rwsem to control access to its extent tree.  Threads 
> > will hold a read lock on this rwsem while they scan the extent tree, 
> > and if need_resched() they will drop the lock and schedule.  The 
> > transaction commit needs to take a write lock for this rwsem for a 
> > very short period to switch out the commit roots.  If there are a lot 
> > of threads doing this caching operation we can starve out the 
> > committers which slows everybody out.  To address this we want to add 
> > this functionality to see if our rwsem has anybody waiting to take a 
> > write lock so we can drop it and schedule for a bit to allow the 
> > commit to continue. Thanks,
> 
> This sounds rather nasty and hacky.  Rather then working around a 
> locking shortcoming in a caller it would be better to fix/enhance the 
> core locking code.  What would such a change need to do?
> 
> Presently rwsem waiters are fifo-queued, are they not?  So the commit 
> thread will eventually get that lock.  Apparently that's not working 
> adequately for you but I don't fully understand what it is about these 
> dynamics which is causing observable problems.

It would be nice to see the whole solution, together with the btrfs patch.

The problem I have is that this new primitive is only superficially like 
spin_is_contended(): in the spinlock case dropping the lock will guarantee 
some sort of progress, because another CPU will almost certainly pick up 
the lock if we cpu_relax().

In the rwsem case there's no such guarantee of progress, especially if a 
read-lock is dropped. So I'd like to see how it's implemented in practice.

Thanks,

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