On Fri, Nov 6, 2020 at 9:30 AM Song Liu <songliubraving@xxxxxx> wrote:
>
> I know very little about sparse internal. But I feel this is more
> like a minor bug.
It's not really a bug, and the pattern you show isn't actually all
that easy to see for a compiler.
> Here is the simplified inlined code of htab_map_delete_elem()
>
> if (some_condition) {
> ret = -EBUSY;
> } else {
> spin_lock();
> ret = 0;
> }
So here the issue for the sparse optimizer is that while both "ret"
and "spin_lock" is tied to "some_condition", it's not really obvious.
A compiler would have to follow all the value analysis - and good
compilers do that, but sparse is not a "great" compiler, it's more of
a "solid and not horribly stupid" one.
> if (ret)
> return ret;
So I'd really suggest you make the code more obvious - it will be more
obvious to humans too.
Write it as
if (some_condition)
return -EBUSY;
spin_lock();
instead, and now sparse will see that obviously spin_lock() and
"some_condition" are mutually incompatible.
Linus
