Christoph Hellwig <[email protected]> wrote on 04.02.2006 14:14:19:
>
> NACK again. (although andrew unfortunately pushed this to Linus
> already, it would be good if this went out again before 2.6.16).
>
> If you really want this functionality in a separate module you need
> to call try_module_get on the module structure of the eer module
> everytime before calling into it. You need to register a structure
> ala
>
> struct dasd_eer_ops {
> struct module *owner;
> void (*disable_on_device)(struct dasd_device *);
> void (*write_trigger)(struct dasd_eer_trigger *);
> };
>
> and then have wrappers in the core dasd code ala
>
> eer_disable_on_device(struct dasd_device *dev)
> {
> if (try_module_get(eer_ops->owner)) {
> eer_ops->write_trigger(dev);
> module_put(eer_ops->owner);
> }
> }
>
> the module_get/put on THIS_MODULE in the patch are completely broken.
For the dynamically registered ioctls there is exactly such a
concept in place. So a try_module_get is done _before_ the
dasd_ioctl_set_eer function is called. That function again calls
dasd_eer_enable_on_device which does the try_module_get you complain
about. So, for each device on which the error reporing is enabled, a
try_module_get is done, and it should be safe for those devices to
call the respective notifier functions later on.
>
> the ioctl registration is also totally not acceptable. firtly this
> interface is absolutely suitable for sysfs, although that might only
work
> if it's in the main module. But even if you go for ioctls it should be
> on your separate char device and not use the broken dynamic ioctl
> registration which _must_ not be used in any new code.
I know that you don't like that dynamic ioctl interface, but I would
like to leave that discussion to Horst.
But besides that, the ioctl in question is needed on the DASD device,
since we want to enable or disable error reporting per DASD device.
Best Regards / Mit freundlichen Grüßen
Stefan Weinhuber
-------------------------------------------------------------------
IBM Deutschland Entwicklung GmbH
Linux for zSeries Development & Services
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
