User question for __context__ or similiar

User question for __context__ or similiar

[Holger Freyther]

Hi,

I would like to use sparse on the OpenBSC[1] userspace application
and I'm searching for a hint on how to realize it or how to adopt the
sparse code to do the following.

In the code we have a struct called "struct gsm_subscriber" and it
is reference counted. We have various methods that query the 
database (VLR) to find a subscriber and then we have subscr_put
and subscr_get to operate on the reference count.

One rule we have established in the code is that the "reference" is
never borrowed. This means code that is doing a subscr_get or a query
will need to do subscr_put on all exits paths. The only exception is
e.g. if work needs to be scheduled and the pointer is put into a talloc
allocated structure.

My first thought was that this almost works like locking in the kernel
but I was wrong. I have used the CHECKER macros from linux/compiler.h

and changed code to

struct gsm_subscriber *subscr_get(struct gsm_subscriber *subscr)
                                          __acquires(subscr);
struct gsm_subscriber *subscr_put(struct gsm_subscriber *subscr)
                                         __releases(subscr);
struct gsm_subscriber *subscr_find_by_tmsi(int tmsi)
                                         __acquires(subscr);

and the code like

do_something()
{
    subscr = subscr_find_by_tmsi(tmsi);
    if (!subscr)
           return;

   if (some_condition) {
...
          subscr_put(subscr);
   }

...
   return;
}

My assumption would be that sparse is looking at the flows and figures
out that one path exits without subscr_put being called. Is my assumption
wrong, would it be worth adding a check that goes through the flow and
counts ref's/unref's? Am I doing it completely wrong?

help and pointers to the code are very much appreciated.

      z.

PS: I will update the wiki with some explanation when I get it to work.


[1] http://openbsc.gnumonks.org

Re: User question for __context__ or similiar

[Christopher Li]

On Wed, Oct 28, 2009 at 3:56 AM, Holger Freyther <zecke@selfish.org> wrote:
>
> My first thought was that this almost works like locking in the kernel
> but I was wrong. I have used the CHECKER macros from linux/compiler.h
>
> and changed code to
>
> struct gsm_subscriber *subscr_get(struct gsm_subscriber *subscr)
>                                          __acquires(subscr);
> struct gsm_subscriber *subscr_put(struct gsm_subscriber *subscr)
>                                         __releases(subscr);
> struct gsm_subscriber *subscr_find_by_tmsi(int tmsi)
>                                         __acquires(subscr);
>
> and the code like
>
> do_something()
> {
>    subscr = subscr_find_by_tmsi(tmsi);
>    if (!subscr)
>           return;
>
>   if (some_condition) {
> ...
>          subscr_put(subscr);
>   }

The context checker will complain here. In the sparse context checking,
it assert that for each basic block, all entry path of the basic block
should have same context level.

In this case, there are two code path merge into one. The if true path
change the context value to release once(-1). The if not true path has
not changed.
There for, sparse will complain here.

> My assumption would be that sparse is looking at the flows and figures
> out that one path exits without subscr_put being called. Is my assumption

Sparse don't know about subscr_put. It only knows about the context value.
It assert all entry point has same context value, which is a pretty strong
assertion.

e.g. sparse can not handle the following code:

if (flags & NEED_LOCK)
    lock()

do_some_thing()

if (flags & NEED_LOCK)
   unlock()

Even though the test condition is exactly the same for
lock() vs unlock() sparse will still complain at do_some_thing().
Because the one of the incoming code path has lock taken
and the other does not.

In real life execution, there will not be context imbalance,
because same condition controls lock vs unlock. But sparse
has no knowledge about two test condition are exactly the same.
Even the expression are exactly the same, it might yield different
results. Some sparse complain when it is *possible* to get context
imbalanced.


> wrong, would it be worth adding a check that goes through the flow and
> counts ref's/unref's? Am I doing it completely wrong?

It really depends on what you expect sparse to catch.
Sparse will complain if you put lock in a conditional branch,
have one function lock it but use in another function etc.

Hope that helps.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html