Re: [PATCH] binder: fix sparse warnings on locking context
On Wed, Dec 05, 2018 at 06:02:22AM -0800, Todd Kjos wrote: > On Wed, Dec 5, 2018 at 2:57 AM Greg KH wrote: > > > On Mon, Dec 03, 2018 at 12:24:54PM -0800, Todd Kjos wrote: > > > Add __acquire()/__release() annnotations to fix warnings > > > in sparse context checking > > > > > > There is one case where the warning was due to a lack of > > > a "default:" case in a switch statement where a lock was > > > being released in each of the cases, so the default > > > case was added. > > > > > > Signed-off-by: Todd Kjos > > > > You sent out 4 patches here, as a series, but with no numbering so I > > don't know what order to put them in :( > > > > Can you resend them properly numbered so I have a chance to get it > > right? > > > > I didn't number them because they are independent and can go in any order. Ah, that wasn't obvious :) > They are not really a series. I can resend with numbers though if you want > to reflect the order I sent them in. Ok, no need for numbering them, but they do need to be resent based on the feedback I gave. I've dropped them all from my queue because of that. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] binder: fix sparse warnings on locking context
On Mon, Dec 03, 2018 at 12:24:54PM -0800, Todd Kjos wrote: > Add __acquire()/__release() annnotations to fix warnings > in sparse context checking > > There is one case where the warning was due to a lack of > a "default:" case in a switch statement where a lock was > being released in each of the cases, so the default > case was added. > > Signed-off-by: Todd Kjos You sent out 4 patches here, as a series, but with no numbering so I don't know what order to put them in :( Can you resend them properly numbered so I have a chance to get it right? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] binder: fix sparse warnings on locking context
On Mon, Dec 03, 2018 at 12:24:54PM -0800, Todd Kjos wrote:
> Add __acquire()/__release() annnotations to fix warnings
> in sparse context checking
>
> There is one case where the warning was due to a lack of
> a "default:" case in a switch statement where a lock was
> being released in each of the cases, so the default
> case was added.
>
> Signed-off-by: Todd Kjos
> ---
> drivers/android/binder.c | 43 +-
> drivers/android/binder_alloc.c | 1 +
> 2 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 9f1000d2a40c7..9f2059d24ae2d 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -751,12 +757,16 @@ _binder_node_unlock(struct binder_node *node, int line)
> #define binder_node_inner_lock(node) _binder_node_inner_lock(node, __LINE__)
> static void
> _binder_node_inner_lock(struct binder_node *node, int line)
> + __acquires(&node->lock) __acquires(&node->proc->inner_lock)
> {
> binder_debug(BINDER_DEBUG_SPINLOCKS,
>"%s: line=%d\n", __func__, line);
> spin_lock(&node->lock);
> if (node->proc)
> binder_inner_proc_lock(node->proc);
> + else
> + /* annotation for sparse */
> + __acquire(&node->proc->inner_lock);
This one is questionnable because:
1) if !node->proc, then '&node->proc->inner_lock' is not acquired
since it doesn't even exist.
2) OTOH, the function can't have the annotation 100% right because
it semantics allows unbalanced locking depending on node->proc
being null or not.
But I see very well the intent and maybe it's a right solution.
I dunno.
Same for most of the following ones.
Best regards,
-- Luc Van Oostenryck
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] binder: fix sparse warnings on locking context
Add __acquire()/__release() annnotations to fix warnings
in sparse context checking
There is one case where the warning was due to a lack of
a "default:" case in a switch statement where a lock was
being released in each of the cases, so the default
case was added.
Signed-off-by: Todd Kjos
---
drivers/android/binder.c | 43 +-
drivers/android/binder_alloc.c | 1 +
2 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 9f1000d2a40c7..9f2059d24ae2d 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -660,6 +660,7 @@ struct binder_transaction {
#define binder_proc_lock(proc) _binder_proc_lock(proc, __LINE__)
static void
_binder_proc_lock(struct binder_proc *proc, int line)
+ __acquires(&proc->outer_lock)
{
binder_debug(BINDER_DEBUG_SPINLOCKS,
"%s: line=%d\n", __func__, line);
@@ -675,6 +676,7 @@ _binder_proc_lock(struct binder_proc *proc, int line)
#define binder_proc_unlock(_proc) _binder_proc_unlock(_proc, __LINE__)
static void
_binder_proc_unlock(struct binder_proc *proc, int line)
+ __releases(&proc->outer_lock)
{
binder_debug(BINDER_DEBUG_SPINLOCKS,
"%s: line=%d\n", __func__, line);
@@ -690,6 +692,7 @@ _binder_proc_unlock(struct binder_proc *proc, int line)
#define binder_inner_proc_lock(proc) _binder_inner_proc_lock(proc, __LINE__)
static void
_binder_inner_proc_lock(struct binder_proc *proc, int line)
+ __acquires(&proc->inner_lock)
{
binder_debug(BINDER_DEBUG_SPINLOCKS,
"%s: line=%d\n", __func__, line);
@@ -705,6 +708,7 @@ _binder_inner_proc_lock(struct binder_proc *proc, int line)
#define binder_inner_proc_unlock(proc) _binder_inner_proc_unlock(proc,
__LINE__)
static void
_binder_inner_proc_unlock(struct binder_proc *proc, int line)
+ __releases(&proc->inner_lock)
{
binder_debug(BINDER_DEBUG_SPINLOCKS,
"%s: line=%d\n", __func__, line);
@@ -720,6 +724,7 @@ _binder_inner_proc_unlock(struct binder_proc *proc, int
line)
#define binder_node_lock(node) _binder_node_lock(node, __LINE__)
static void
_binder_node_lock(struct binder_node *node, int line)
+ __acquires(&node->lock)
{
binder_debug(BINDER_DEBUG_SPINLOCKS,
"%s: line=%d\n", __func__, line);
@@ -735,6 +740,7 @@ _binder_node_lock(struct binder_node *node, int line)
#define binder_node_unlock(node) _binder_node_unlock(node, __LINE__)
static void
_binder_node_unlock(struct binder_node *node, int line)
+ __releases(&node->lock)
{
binder_debug(BINDER_DEBUG_SPINLOCKS,
"%s: line=%d\n", __func__, line);
@@ -751,12 +757,16 @@ _binder_node_unlock(struct binder_node *node, int line)
#define binder_node_inner_lock(node) _binder_node_inner_lock(node, __LINE__)
static void
_binder_node_inner_lock(struct binder_node *node, int line)
+ __acquires(&node->lock) __acquires(&node->proc->inner_lock)
{
binder_debug(BINDER_DEBUG_SPINLOCKS,
"%s: line=%d\n", __func__, line);
spin_lock(&node->lock);
if (node->proc)
binder_inner_proc_lock(node->proc);
+ else
+ /* annotation for sparse */
+ __acquire(&node->proc->inner_lock);
}
/**
@@ -768,6 +778,7 @@ _binder_node_inner_lock(struct binder_node *node, int line)
#define binder_node_inner_unlock(node) _binder_node_inner_unlock(node,
__LINE__)
static void
_binder_node_inner_unlock(struct binder_node *node, int line)
+ __releases(&node->lock) __releases(&node->proc->inner_lock)
{
struct binder_proc *proc = node->proc;
@@ -775,6 +786,9 @@ _binder_node_inner_unlock(struct binder_node *node, int
line)
"%s: line=%d\n", __func__, line);
if (proc)
binder_inner_proc_unlock(proc);
+ else
+ /* annotation for sparse */
+ __release(&node->proc->inner_lock);
spin_unlock(&node->lock);
}
@@ -1384,10 +1398,14 @@ static void binder_dec_node_tmpref(struct binder_node
*node)
binder_node_inner_lock(node);
if (!node->proc)
spin_lock(&binder_dead_nodes_lock);
+ else
+ __acquire(&binder_dead_nodes_lock);
node->tmp_refs--;
BUG_ON(node->tmp_refs < 0);
if (!node->proc)
spin_unlock(&binder_dead_nodes_lock);
+ else
+ __release(&binder_dead_nodes_lock);
/*
* Call binder_dec_node() to check if all refcounts are 0
* and cleanup is needed. Calling with strong=0 and internal=1
@@ -1890,18 +1908,22 @@ static struct binder_thread *binder_get_txn_from(
*/
static struct binder_thread *binder_get_txn_from_and_acq_inner(
struct binder_transaction *t)
+ __acquires(&t->from->proc->inner_lock)
{
struct binder_thread *from;
[PATCH] binder: fix sparse warnings on locking context
Add __acquire()/__release() annnotations to fix warnings
in sparse context checking
There is one case where the warning was due to a lack of
a "default:" case in a switch statement where a lock was
being released in each of the cases, so the default
case was added.
Signed-off-by: Todd Kjos
---
drivers/android/binder.c | 43 +-
drivers/android/binder_alloc.c | 1 +
2 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 9f1000d2a40c7..9f2059d24ae2d 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -660,6 +660,7 @@ struct binder_transaction {
#define binder_proc_lock(proc) _binder_proc_lock(proc, __LINE__)
static void
_binder_proc_lock(struct binder_proc *proc, int line)
+ __acquires(&proc->outer_lock)
{
binder_debug(BINDER_DEBUG_SPINLOCKS,
"%s: line=%d\n", __func__, line);
@@ -675,6 +676,7 @@ _binder_proc_lock(struct binder_proc *proc, int line)
#define binder_proc_unlock(_proc) _binder_proc_unlock(_proc, __LINE__)
static void
_binder_proc_unlock(struct binder_proc *proc, int line)
+ __releases(&proc->outer_lock)
{
binder_debug(BINDER_DEBUG_SPINLOCKS,
"%s: line=%d\n", __func__, line);
@@ -690,6 +692,7 @@ _binder_proc_unlock(struct binder_proc *proc, int line)
#define binder_inner_proc_lock(proc) _binder_inner_proc_lock(proc, __LINE__)
static void
_binder_inner_proc_lock(struct binder_proc *proc, int line)
+ __acquires(&proc->inner_lock)
{
binder_debug(BINDER_DEBUG_SPINLOCKS,
"%s: line=%d\n", __func__, line);
@@ -705,6 +708,7 @@ _binder_inner_proc_lock(struct binder_proc *proc, int line)
#define binder_inner_proc_unlock(proc) _binder_inner_proc_unlock(proc,
__LINE__)
static void
_binder_inner_proc_unlock(struct binder_proc *proc, int line)
+ __releases(&proc->inner_lock)
{
binder_debug(BINDER_DEBUG_SPINLOCKS,
"%s: line=%d\n", __func__, line);
@@ -720,6 +724,7 @@ _binder_inner_proc_unlock(struct binder_proc *proc, int
line)
#define binder_node_lock(node) _binder_node_lock(node, __LINE__)
static void
_binder_node_lock(struct binder_node *node, int line)
+ __acquires(&node->lock)
{
binder_debug(BINDER_DEBUG_SPINLOCKS,
"%s: line=%d\n", __func__, line);
@@ -735,6 +740,7 @@ _binder_node_lock(struct binder_node *node, int line)
#define binder_node_unlock(node) _binder_node_unlock(node, __LINE__)
static void
_binder_node_unlock(struct binder_node *node, int line)
+ __releases(&node->lock)
{
binder_debug(BINDER_DEBUG_SPINLOCKS,
"%s: line=%d\n", __func__, line);
@@ -751,12 +757,16 @@ _binder_node_unlock(struct binder_node *node, int line)
#define binder_node_inner_lock(node) _binder_node_inner_lock(node, __LINE__)
static void
_binder_node_inner_lock(struct binder_node *node, int line)
+ __acquires(&node->lock) __acquires(&node->proc->inner_lock)
{
binder_debug(BINDER_DEBUG_SPINLOCKS,
"%s: line=%d\n", __func__, line);
spin_lock(&node->lock);
if (node->proc)
binder_inner_proc_lock(node->proc);
+ else
+ /* annotation for sparse */
+ __acquire(&node->proc->inner_lock);
}
/**
@@ -768,6 +778,7 @@ _binder_node_inner_lock(struct binder_node *node, int line)
#define binder_node_inner_unlock(node) _binder_node_inner_unlock(node,
__LINE__)
static void
_binder_node_inner_unlock(struct binder_node *node, int line)
+ __releases(&node->lock) __releases(&node->proc->inner_lock)
{
struct binder_proc *proc = node->proc;
@@ -775,6 +786,9 @@ _binder_node_inner_unlock(struct binder_node *node, int
line)
"%s: line=%d\n", __func__, line);
if (proc)
binder_inner_proc_unlock(proc);
+ else
+ /* annotation for sparse */
+ __release(&node->proc->inner_lock);
spin_unlock(&node->lock);
}
@@ -1384,10 +1398,14 @@ static void binder_dec_node_tmpref(struct binder_node
*node)
binder_node_inner_lock(node);
if (!node->proc)
spin_lock(&binder_dead_nodes_lock);
+ else
+ __acquire(&binder_dead_nodes_lock);
node->tmp_refs--;
BUG_ON(node->tmp_refs < 0);
if (!node->proc)
spin_unlock(&binder_dead_nodes_lock);
+ else
+ __release(&binder_dead_nodes_lock);
/*
* Call binder_dec_node() to check if all refcounts are 0
* and cleanup is needed. Calling with strong=0 and internal=1
@@ -1890,18 +1908,22 @@ static struct binder_thread *binder_get_txn_from(
*/
static struct binder_thread *binder_get_txn_from_and_acq_inner(
struct binder_transaction *t)
+ __acquires(&t->from->proc->inner_lock)
{
struct binder_thread *from;
