lkml.org 
[lkml]   [2006]   [Feb]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 0/5] new dasd ioctl patchkit
    From
    Date
    Christoph Hellwig <hch@lst.de> wrote on 14.02.2006 20:09:09:

    > (1) devices have pretty clear ownership. Implementing ioctls in
    > different modules means we get a very undefined API. Given that
    > ioctls are generally deprecated adding extensions to that is
    > a bad idea.

    So you are saying: 'If it's a dasd function, do it in the dasd
    driver!'. Right?
    Well, I can understand that point. As the invocation of the new
    ioctls is actually changing the behavior of a dasd device, I'm
    looking into moving that part from dasd_eer into the main dasd module.
    Since this will remove the dynamic ioctl registration as well, it
    will also remove the conflict with your cleanup code.

    > (4) adding new ioctls is not okay in general, you should be using
    > better APIs.

    Hm, you suggested sysfs in a previous mail. Once I have moved
    the eerset/eerget logic into the main dasd driver, I could provide
    a sysfs interface instead of the ioctls (right now that would be
    difficult).

    But I'd like to know why an ioctl is so bad. I know that the ioctl
    interface as such can be error prone, but in our case we just
    transfer an integer back and forth. Sysfs always struck me as an
    end user interface, something that a human can use with echo and
    cat instead of specialized tools. The new ioctls however should not
    be used directly, but by a tool or daemon that knows what it does
    and can handle the changed behavior of the dasd device.
    Until now we have always implemented functionality like that as
    ioctls (e.g. quiesce/resume, reserve/release) so the new ioctls are
    more consistent with the existing interface, and I'd like to keep it
    that way.

    > (5) you're not just adding new ioctls but also add them using the
    > dynamic registration facility that I NACKed before and remove in
    > earlier sent patches, but you copletly ignored all objections
    > and just resent the patches anyway

    As I said, the dynamic ioctls can be removed from my code.
    (I don't want to dwell on it, but your first NACK against dasd_eer
    was because it added code to compat_ioctl.c. I wasn't aware of your
    discussion with Martin about the dynamic ioctls.)

    > (6) in addition to the dynamic ioctls it's also adding a misc device,
    > leading to a totally incoherent interface.

    Well, the dasd ioctl will be moved so the misc device will be the
    only interface implemented by dasd_eer.

    > that's totally fine with me. As long as we backout the bogus eer
    > patch before 2.6.16 all the cleanups and even the eckd ioctl fix
    > can wait. But don't put this crappy interface into 1.6.16 and thus
    > SLES10 so that applications start to rely on it.

    As said above, I still prefer the ioctls over a sysfs interface.
    Since the other issues do not cause interface changes I would
    prefer to keep the code in.

    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 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/

    \
     
     \ /
      Last update: 2006-02-15 22:16    [W:0.025 / U:119.700 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site