linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Johannes Berg <johannes@sipsolutions.net>,
	Christopher Li <sparse@chrisli.org>,
	Josh Triplett <josh@freedesktop.org>
Cc: linux-sparse@vger.kernel.org,
	Anton Vorontsov <anton.vorontsov@linaro.org>
Subject: Killing off __cond_lock()
Date: Sat, 24 Mar 2012 17:23:36 +0100	[thread overview]
Message-ID: <1332606216.16159.48.camel@twins> (raw)

Hi all,

So the kernel has this __cond_lock() crap pile, which is implemented
like:

#define __acquire(x)       __context__(x,1)
#define __cond_lock(x,c)   ((c) ? ({ __acquire(x); 1; }) : 0)

Now the problem with this is that people send ugly patches like:

https://lkml.org/lkml/2012/3/24/57
http://www.spinics.net/lists/mm-commits/msg80386.html

That basically wrap an existing function in an inline function just to
use __cond_lock() on the return value.

It would be ever so much nicer if we could declare such functions like:

struct anon_vma *page_lock_anon_vma(struct page *page)
	__cond_acquires(RCU) 
	__cond_acquires(page_lock_anon_vma(page)->root->lock);

Meaning that if the return value is true (non-zero), it would acquire
that lock/context. One could of course add some shorter means of
referring to the return value, but simply using the function in the
expression should be simple enough.

In order to implement this I guess we need to extend the
__attribute__((context(expr,in,out))) thing. 

Currently in,out are explicit value constants, but I guess if we make
them expressions we could evaluate them and get dynamic behaviour.

Thus allowing something like:

int spin_trylock(spinlock_t *lock)
	__attribute__((context(lock, 0, !!spin_trylock(lock));

meaning that the context would be incremented by 1 if the return value
were true.

Having only briefly looked at the sparse source, is this feasible to
implement or do we get chicken/egg problems wrt using a function before
its declaration is complete, and referring a return value before the
function is part of an expression?

If this yields problems, are there better ways of solving this issue?

             reply	other threads:[~2012-03-24 16:23 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-24 16:23 Peter Zijlstra [this message]
2012-03-25  8:48 ` Killing off __cond_lock() Johannes Berg
2012-03-26  8:11   ` Peter Zijlstra
2012-03-26  9:00     ` Johannes Berg
2012-03-26  9:29     ` Josh Triplett

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1332606216.16159.48.camel@twins \
    --to=peterz@infradead.org \
    --cc=anton.vorontsov@linaro.org \
    --cc=johannes@sipsolutions.net \
    --cc=josh@freedesktop.org \
    --cc=linux-sparse@vger.kernel.org \
    --cc=sparse@chrisli.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).