Re: [Patch 0/2] sysfs: fix s_active lockdep warning

[Eric W. Biederman]

Greg KH <gregkh@xxxxxxx> writes:

> Heh, this whole mess is the very reason we didn't add lockdep support to
> the driver core.  Nested devices that all look alike from the driver
> core, are really different objects and the locking lifetimes are
> separate, but lockdep can't see that.
>
> I suggest we just remove the original patch, as it seems to be causing
> way too many problems.
>
> Any objections to that?

I think the hit rate for real problems has been about 25-50%.  Of the
false positives a lot of those have been, code that is at least
questionable.

Furthermore there are problems we can find this way that we won't know
about any other way.  Unfortunately I haven't had much time to do
anything kernel related lately, or I would have done more with this.
My comment was about simply about finding a good way to increase the
signal to noise ration so investigations can reasonably start with the
presumption that code lockdep is complaining about real problems.

The deadlocks that we can hit in sysfs are very nasty to find, they
have persisted for years, and they pop back up after they are fixed.
So far the pain from lockdep annotations seems a lot lower.

Right now annotating with subclasses as Amerigo is attempting will work,
and remove the false positives.  I was simply hoping to find a faster
way to get there.

So yes, I do object to removing the original patch.  Let's put in the
work to find a good path to remove the handful of cases that cause
false positives.

It's a shame we aren't getting stack overflow errors on the same paths
that are removing sysfs attributes from the callback handlers of sysfs
attributes, or we could just rule out that questionable practice and
call all of the lockdep warnings valid.  Unfortunately that would just
be the tail wagging the horse.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/