Re: [PATCH 2/2] dasd: remove dynamic ioctl registration

[Christoph Hellwig]

On Fri, Dec 16, 2005 at 03:58:19PM +0100, Martin Schwidefsky wrote:
> On Fri, 2005-12-16 at 15:33 +0100, Christoph Hellwig wrote:
> > dasd has some really messy code to allow submodule to register ioctl.
> > Right now there are two cases:  cmd and eckd.
> 
> Wrong, at least four: cmf, eckd, err, and a binary only module from EMC.
> Now don't hit me for that binary module. But it has been there for 2.4
> and we even reserved some ioctl numbers for them (240-255).

NACK, binary modules are not a reason to keep broken things, rather one
to fix it better sooner than later.

> > cmd was merged into the main module in the last patchh, so we don't
> > need the mechanism for it anymore.
> 
> Seems resonable. The same could be done for the err module. Doesn't have
> to be a module, a config option is enough.

yes, it would clean up the err code a lot.

> > Fix this second issue by adding an ioctl method to the dasd_discipline
> > structure.
> 
> That can easily be fixed by adding a check in the ioctls as well. But
> a .ioctl entry in the discipline structure makes sense and would get rid
> of all dynamically added ioctls in our code. So I'm all in favor of it.

Yepp.  I generally prefer to not just fix things but rip out surrounding
mess.  Keeps code maintainable in the long run.

> I would be cautious about ripping out the dynamic ioctls interface
> though. I have no idea if there still is an EMC module for 2.6 or other
> exploiters. It is an exported interface after all. It is not necessary
> to break these exploiters intentionally.

Yes, it is.  Unrelated modules adding ioctls is a big no-way.  Even more
for binary modules.  The EMC code deserves to be broken.
-
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/