Re: Recent wave of additional false negative warnings?

[Oleg Drokin <green@xxxxxxxxxxxxxx>]

Hello!

   I think I just see another common false positive pattern. It looks like this:
lustre/obdclass/lprocfs_status_server.c:361 lprocfs_exp_setup() warn: we tested 'exp->exp_nid_stats' before and it was 'false'

The code:
        spin_lock(&exp->exp_lock);
        if (exp->exp_nid_stats != NULL) {
                spin_unlock(&exp->exp_lock);
                RETURN(-EALREADY);
        }
        spin_unlock(&exp->exp_lock);
… do stuff….
        if (old_stat != new_stat) {
                spin_lock(&exp->exp_lock);
                if (exp->exp_nid_stats) {
                        LASSERT(exp->exp_nid_stats == old_stat);
                        nidstat_putref(exp->exp_nid_stats);
                }
…

Another one:
lustre/ldlm/ldlm_lib.c:1633 target_start_recovery_timer() warn: we tested 'obd->obd_recovery_start' before and it was 'false'

static void target_start_recovery_timer(struct obd_device *obd)
{
        if (obd->obd_recovery_start != 0)
                return;

        spin_lock(&obd->obd_dev_lock);
… do stuff…
        if (obd->obd_recovery_start != 0) {
                spin_unlock(&obd->obd_dev_lock);
                return;
        }
…

You get the idea.

Now it's obvious that we do check the same value in a row twice, but what's obvious to me and is not taken into account yet is that there might be a parallel thread that modifies the value.
Kernel does not really use 'volatile" modifiers for this sort of stuff for a variety of reason anyway.
But there is still a hint - the locks taken around this sort of accesses.

I wonder if it's possible to reset such non-local variables state when we encounter a memory barrier (I think locks have explicit memory barrier right in the defines, so we should be able
to see them trivially)? After all memory barrier purpose is exactly like this - to stop caching whatever previous state compiler might think the variable has.

Thoughts?

Bye,
    Oleg--
To unsubscribe from this list: send the line "unsubscribe smatch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html