smatch warnings on the IB core

smatch warnings on the IB core

[Or Gerlitz]

Hi Dan,

With the latest smatch I still see these hits on the IB core, basically, 
I would be happy to see the IB stack free from such warnings and then we 
can easily require each new patch not to introduce them... can you shed 
some light on the problem and possible solutions?

> drivers/infiniband/core/user_mad.c:912 ib_umad_sm_open() warn: 
> inconsistent returns sem:&port->sm_sem: locked (908) unlocked (886 
> [(-6)], 912 [s32min-(-1),1-s32max], 912 [(-512)], 912 [(-11)])
>
> drivers/infiniband/core/cma.c:1336 cma_req_handler() warn: 
> inconsistent returns mutex:&listen_id->handler_mutex: locked (1271 
> [(-22)], 1274 [(-103)]) unlocked (1323 [0], 1336 
> [s32min-(-1),1-s32max], 1336 [(-12)])
>
> drivers/infiniband/core/cma.c:1535 iw_conn_req_handler() warn: 
> inconsistent returns mutex:&listen_id->handler_mutex: locked (1463 
> [(-103)]) unlocked (1535 [0], 1535 [s32min-(-1),1-s32max], 1535 
> [s32min-(-1),1-s32max], 1535 [s32min-(-1),1-s32max], 1535 
> [s32min-(-1),1-s32max], 1535 [(-12)])

Sean, if you want to reproduce clone git://repo.or.cz/smatch.git and use 
this command

$ make C=2 CHECK="/patch/to/smatch/smatch_scripts/../smatch 
--project=kernel"  drivers/infiniband/core/ -j 12
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: smatch warnings on the IB core

[Hefty, Sean]

> > drivers/infiniband/core/cma.c:1336 cma_req_handler() warn:
> > inconsistent returns mutex:&listen_id->handler_mutex: locked (1271
> > [(-22)], 1274 [(-103)]) unlocked (1323 [0], 1336
> > [s32min-(-1),1-s32max], 1336 [(-12)])
> >
> > drivers/infiniband/core/cma.c:1535 iw_conn_req_handler() warn:
> > inconsistent returns mutex:&listen_id->handler_mutex: locked (1463
> > [(-103)]) unlocked (1535 [0], 1535 [s32min-(-1),1-s32max], 1535
> > [s32min-(-1),1-s32max], 1535 [s32min-(-1),1-s32max], 1535
> > [s32min-(-1),1-s32max], 1535 [(-12)])

Visually inspecting the code for both of these, I don't see anything wrong.  In both cases the listen mutex is acquired near the top of the function and released at the end.  I don't see how you exit either function with the mutex locked.

In the top case, line 1271 is a return statement that occurs before we've acquired the mutex.  Line 1274 is an abort case where we acquire the mutex, check a state, release the mutex, and return.

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

Re: smatch warnings on the IB core

[Or Gerlitz]

On 28/10/2013 18:32, Hefty, Sean wrote:
> Visually inspecting the code for both of these, I don't see anything wrong.  In both cases the listen mutex is acquired near the top of the function and released at the end.  I don't see how you exit either function with the mutex locked.
>
> In the top case, line 1271 is a return statement that occurs before we've acquired the mutex.  Line 1274 is an abort case where we acquire the mutex, check a state, release the mutex, and return.

Dan, do you agree this is false-positive? anything we can do to teach 
smatch not to shout on that?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: smatch warnings on the IB core

[Dan Carpenter]

On Sun, Oct 27, 2013 at 08:45:25AM +0200, Or Gerlitz wrote:
> Hi Dan,
> 
> With the latest smatch I still see these hits on the IB core,
> basically, I would be happy to see the IB stack free from such
> warnings and then we can easily require each new patch not to
> introduce them... can you shed some light on the problem and
> possible solutions?
> 

The first one is because it thinks nonseekable_open() can fail.
If you build the cross function database first:

~/path/to/smatch/smatch_scripts/build_kernel_data.sh

Then the warning should go away.  But building the database takes three
hours on my system.  The database is around 4GB.

The other two are because smatch isn't doing cross function analysis of
locks.  It doesn't see that we take the lock in cma_disable_callback().
I want to add this at some point but I haven't got around to it.

In the end, what I do is I only look at new warnings.  With Smatch, I've
deliberately set it to have more false positives than GCC.  It's always
a tradeoff.

I wish there were a way to carry a list of false positives in the smatch
git tree and filter them out automatically...

regards,
dan carpenter

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