Re: 2.6.23-rc6-mm1

[David Brownell <david-b@pacbell.net>]

On Wednesday 19 September 2007, Andrew Morton wrote:
> Head still spinning.
> 
> a) "rtc0" != "rtc" ??

One key difference between any of the numerous legacy RTC drivers
and the new framework is that the legacy drivers tended to think
(wrongly!) they would be the only RTC in the system, while the
framework recognizes that other configurations are important.

Hence names "rtc0", "rtc1", "rtc2" ... etc in framework drivers,
versus just "rtc" in legacy code.  Easily tweaked with "udev" if it
matters, but current versions of "hwclock" (in util-linux and also
in busybox) don't require a /dev/rtc file any more ... they check
for /dev/rtc0, and can be told to look at other RTCs too.


> b) The kernel is trying to register two procfs files of the same name! 
>    If that's a configuration error then we have a Kconfiguration bug.

Not so much an "error" as minor awkwardness.  The end result is
that only one driver will be active.


> > It's not a wrong thing, at any rate.  This is a fairly basic task
> > in any filesystem:  mutual exclusion.  Code is allowed to rely on
> > filesystems acting correctly...
> > 
> 
> We're relying on procfs behaviour to prevent registration of two rtc
> devices?

Not at all.  But if two chunks of code try to create /proc/x/y/z,
only one of them should succeed.  Today, there's a clear bug in
procfs whereby both of them will succeed...


> That's a strange means of arbitration.   And what happens 
> when CONFIG_PROC_FS=n?

Then nothing gets stored there; procfs is nonessential.  The same
code executes as when creating the /proc/x/y/z file fails.


> > > Yes, procfs should have been checking for this.  But it is too late now for
> > > us to just fail out of the procfs registration code.  Because this can
> > > cause previously buggy-but-works-ok code to now fail completely.
> > 
> > What do you mean by too late "now" ... just-before-2.6.23?
> 
> No.
> 
> What I mean is that we cannot change procfs as you propose until we have
> located and fixed all places in the kernel which can cause duplicated
> procfs names.

But causing duplicated names doesn't need any "fix" when that code
handles the error sanely -- by using its "PROCFS=n" code paths.

If you're arguing that you think such fault handling code may have
a lot of bugs, I might agree:  not all fault handling code gets even
basic testing.  But that would seem to be nothing more than a reason
to want some code auditing (to see how common such bugs really are,
and fix at least a few of them), and perhaps to let such bugfixes sit
in a moderately well-exposed testing tree for a while.


> (Good luck with that). 

Evidently there's no procfs maintainer ... you've said there are
patches in MM to detect and report such duplication.  What is it
turning up?


> Because we've had this happening before, and *the driver kept working*
> (apart from, presumably, some of their procfs features which, presumably,
> nobody used much). 
> 
> With the change which you propose, these drivers will probably stop
> working, because they will newly find their procfs registration failed.

The last time I had occasion to look at such stuff I observed that procfs
users were generally sane.  If they couldn't create a file, they just
continued ... after all, procfs can never be the primary interface.


> > And what would an example be of buggy-but-works code, which would
> > then be broken if the procfs stopped being buggy?
> > 
> 
> A driver which checks the return value from procfs registration and which
> then errors out (as it should) if that registration fails.

Erroring out is appropriate for fatal errors ... but procfs errors
generally can't be fatal.  (Code which uses procfs for its primary
userspace interface would be the exception.)  So most code should
never error out on such faults.

As I commented above, most code I've happend across will proceed
just fine if the extra procfs support is unavailable.  After all,
it needs to work with CONFIG_PROCFS=n so those code paths aren't
going to suddenly stop working!

- Dave

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