spa_active_count seemed useless

[thegreatgazoo]

I just came across static variable spa_active_count in module/zfs/spa_misc.c. It gets incremented and decremented but never read/used anywhere. Maybe I missed something, but it seemed useless.

[chrisrd]

Yup, he's dead Jim.

It was previously (only) used in spa_busy(), but that function went away in @60101509 (Aug 2010).

Would you like to submit a patch?

[behlendorf]

We might want to consider reintroducing spa_busy() just for consistency with illumos. It's not an interface we need because the kernel modules interface already tracks a busy count. But it would be one less difference.

[chrisrd]

I'm in two minds about reintroducing spa_busy() for consistency with Illumos...

On one hand, having been merging a bunch of Illumos patches, I'm all for consistency.

On the other hand, having been delving into lots of unfamiliar code and struggling to wrap my head around it all, coming across pieces of code that don't do anything can be a time sink because you're not sure if there's something you're missing, or if it really doesn't do anything (...and then, when you're convinced it _really_ doesn't do anything, you triumphantly shoot off a dead code removal patch, only to be told "oh yeah, we know about that, it's only for consistency...").

Then there's the matter of having little bits of useless code continuously running on everyone's machines. It's not going to cost anything measurable, but still...

I guess you could annotate it to indicate it's only for consistency, and the annotation could even be in the form of #ifdef so the code isn't actually run at all, but that could get very messy very quickly.

I guess in an ideal world this stuff would be abstracted away into platform dependent pieces, but we're quite some ways away from that ideal world.

In writing this, I'm coming down on the side of killing it. For me, it's not a big deal to have an occasional patch rejection and looking into the git log to see a "we don't need this" message and the occasional patch rejection keeps you on your toes!

[behlendorf]

@chrisrd I completely understand, I go back and forth about this kind of thing myself. Usually I err on the side removing dead code like this because it results in more understandable code. It would be easier to make an argument for keeping it if we could think of at least one potential consumer, but I can't think of any.

OK, let's kill it. If someone could propose a patch which removes spa_busy() from include/sys/spa.h and spa_active_count that would be great.

[behlendorf]

@thegreatgazoo thanks for pushing a patch for this.