linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Gerd Knorr <kraxel@bytesex.org>
Cc: Kernel List <linux-kernel@vger.kernel.org>,
	video4linux list <video4linux-list@redhat.com>
Subject: Re: [RFC/PATCH] sysfs'ify video4linux
Date: Wed, 16 Jul 2003 09:19:24 -0700	[thread overview]
Message-ID: <20030716161924.GA7406@kroah.com> (raw)
In-Reply-To: <20030716084448.GC27600@bytesex.org>

On Wed, Jul 16, 2003 at 10:44:48AM +0200, Gerd Knorr wrote:
> > > Comments?
> > 
> > You _have_ to set up a release function for your class device.  You
> > can't just kfree it like I think you are doing, otherwise any users of
> > the sysfs files will oops the kernel after the video class device is
> > gone.
> 
> class_device_unregister() is called from video_unregister_device() which
> looks fine to me.  Or do you talk about something else?

Heh, yes, that is correct.  But then what do you do with the object that
had the class_device in it after class_device_unregister() returns?  I
don't see you freeing the device directly, which is good, but who does?

You _have_ to let the structure that the kobject is in free itself in a
release() function, you can't guess at what the kobject reference count
is at any time and just assume you can throw it away at that moment.

LWN did an article about how to do this properly at:
	http://lwn.net/Articles/36850/

But for some code that does this in a simple manner, look at
drivers/char/tty_io.c, the release_tty_dev() function.  It's set up in
the tty_class to handle destroying the structure when the last reference
to the tty_dev structure goes away.  Look at the
tty_remove_class_device() to see that the structure isn't kfreed on it's
own.

It looks like the video drivers include a "struct video_device"
structure within their own structures, right?  That will have to be
changed to a pointer to that structure in order for the lifetime rules
to work properly.  I know Hanna is fighting this same battle with the
input layer right now...

Does this help out?

As an example of why you _have_ to do this, consider a user who opens a
sysfs file of a video device, and then removes the video device from the
system.  Then they do a read on the sysfs file.  Oops...

> > Other than that, how about exporting the dev_t value for the video
> > device?  Then you automatically get udev support, and I don't have to go
> > add it to this code later :)
> 
> Do you have a pointer to sample code for that?

Look at the dev file in /sys/class/tty/*, or in /sys/block/hd* or in
/sys/class/usb/*, and so on...

I need to make a general function to support this to make it easier for
everyone to export a dev_t to userspace, but until then, if you want to
look at the show_dev() function in drivers/usb/core/file.c it shows what
you need to do.

thanks,

greg k-h

  reply	other threads:[~2003-07-16 16:04 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-07-15 14:31 [RFC/PATCH] sysfs'ify video4linux Gerd Knorr
2003-07-15 15:21 ` Ronald Bultje
2003-07-15 16:19   ` Matt Porter
2003-07-15 21:27 ` Greg KH
2003-07-16  8:44   ` Gerd Knorr
2003-07-16 16:19     ` Greg KH [this message]
2003-07-16 20:20       ` Gerd Knorr
2003-07-16 21:08         ` Greg KH
2003-07-17 12:01           ` Gerd Knorr
2003-07-17 14:57             ` Greg KH
2003-07-17 16:37               ` Gerd Knorr
2003-07-17 21:49                 ` Greg KH
2003-07-18  9:59                   ` Gerd Knorr
     [not found]                     ` <20030718234359.GK1583@kroah.com>
2003-07-21  7:28                       ` Gerd Knorr
2003-07-21  7:55                         ` Ronald Bultje
2003-07-21 15:43                         ` [RFC/PATCH] 1/2 v4l: sysfs'ify video4linux core Gerd Knorr
2003-07-21 15:47                           ` [RFC/PATCH] 2/2 v4l: sysfs'ify bttv driver Gerd Knorr
2003-07-21 16:27                           ` [RFC/PATCH] 1/2 v4l: sysfs'ify video4linux core Greg KH
2003-07-16 13:33   ` [RFC/PATCH] sysfs'ify video4linux Mark McClelland
2003-07-16 14:10     ` root
2003-07-16 16:23     ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20030716161924.GA7406@kroah.com \
    --to=greg@kroah.com \
    --cc=kraxel@bytesex.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=video4linux-list@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).