[lkml]   [2006]   [Mar]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 2/2] Add full sysfs support to the IPMI driver
    Greg KH wrote:

    >On Wed, Mar 22, 2006 at 10:01:09AM +0100, Arjan van de Ven wrote:
    >>On Tue, 2006-03-21 at 16:13 -0600, Corey Minyard wrote:
    >>>+static void ipmi_bmc_release(struct device *dev)
    >>>+ printk(KERN_DEBUG "ipmi_bmc release\n");
    >>eehhhh NO.
    >>Please read the many comments and documentations about why a release
    >>function is NOT allowed to be empty. In fact the kernel warned you about
    >>that, didn't it?
    >Of course that's not ok.
    >Come on people, does everyone think I just put that warning message in
    >the kernel for fun to force you to create an empty release function?
    >Why do people ignore the helpful hints that the kernel provides?
    >I can take that check out and watch people get their code wrong even
    >more, as it sure doesn't seem like it is helping anyone out these
    >Corey, haven't we discussed this in the past?
    I don't think so, this is my first attempt at anything beyond a simple
    class device for the driver model. This code was a rework of
    something that came from someone else, so I never actually saw
    the kernel message.

    I did look in the include files while doing this and a little at the
    driver-model docs, and I didn't see anything that jumped out and said
    "THIS IS IMPORTANT". The release field documented in devices.txt,
    for instance, says:

    release: Callback to free the device after all references have
    gone away. This should be set by the allocator of the
    device (i.e. the bus driver that discovered the device).

    It's not altogether obvious how to use this function or its importance
    from this text, though if I had thought about it I would have figured
    it out. If you want to affect behaviour, you need to add text
    like: "NOTE: You must keep the data that holds the device structure
    around until the release function is called. If you do not, something
    could be using the device structure after you free it's data, resulting
    in bad things happening". A comment should probably appear in
    the include file, too.

    If the issue Russell brought up is really an issue, it truely needs some
    documentation written about it.

    That said, I should have looked a little deeper on this. I saw it there,
    read the docs and include file, and thought, "That's probably just there
    to aid debugging," and left it in.

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2006-03-22 22:31    [W:0.025 / U:45.484 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site