lkml.org 
[lkml]   [2002]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [linux-usb-devel] Re: 2.5.26 hotplug failure

On Sun, 22 Sep 2002, Linus Torvalds wrote:

> In article <20020921033137.GA26017@kroah.com>, Greg KH <greg@kroah.com> wrote:
> >On Fri, Sep 20, 2002 at 07:55:22PM -0700, David Brownell wrote:
> >>
> >> How about a facility to create the character (or block?) special file
> >> node right there in the driverfs directory? Optional of course.
> >
> >No, Linus has stated that this is not ok to do. See the lkml archives
> >for the whole discussion about this.
>
> I'm not totally against it, it's just that it has some issues:
>
> - naming policy in general. Trivially handled by just always calling
> the special node something truly boring and nautral like "node", and
> be done with it. The _path_ is the real name, the "node" would be
> just an openable entity.
>
> - the issue of persistent permissions and ownership.
>
> The latter is the real problem. And I personally think the only sane
> policy is to just let "/sbin/hotplug" handle it, which definitely
> implies _not_ having the kernel create the real device node. That way
> user-space can have any policy it damn well pleases, including having
> some default heuristics along with "a priori known nodes".
>
> But clearly that user-space hotplug entity needs to know major and minor
> numbers in order to create the real device node, and that's where the
> "node" thing may be acceptable - as a template, nothing more. Although
> I suspect that there are other, simpler and more acceptable templates
> (ie export the dang thing as just a "node" text-file, which describes
> the majors and minors and "char vs block" issues)

The concern that I had was that people would actually start using the
stupid thing, and start to rely on the path and existence of them. But, I
suppose if people really _want_ to do that, we shouldn't stop them from
hurting themselves. ;)

Userspace does need to know major/minor, as well as block vs. char. We
could encode those in one file each, but a device node gives us all three
in a standard format.

So, appended is a patch that allows you to create a device node in
driverfs. The API is:

int device_create_node(struct device *device, mode_t mode, kdev_t kdev);

The name is "node", and it's created in the device's directory. There are
plans to call /sbin/hotplug after the device is registered with its class.
It's then that the kernel will know what type of device it is, and what
the major/minor should be. The device path will be passed to
/sbin/hotplug, which can then query the node as a template for creating
other device nodes.

Note that ->fops is not set, though it appears that a request_module()
happpens when you try to write to it.

Also, the owner of the device is root, and I easily make the permissions
more stringent (i.e. make it S_IRUGO only always).

Attached is a simple little module to illustrate the use of the API. It
creates a device 'foo0' and a node inside its directory once registered:

# tree /sys/root/foo0/
/sys/root/foo0/
|-- name
|-- node
`-- power


# ls -l /sys/root/foo0/node
cr--r--r-- 1 root root 10, 16 Sep 23 10:58
/sys/root/foo0/node


Will submit as a BK patch pending feedback..


-pat


===== drivers/base/fs/device.c 1.20 vs edited =====
--- 1.20/drivers/base/fs/device.c Mon Aug 26 08:39:22 2002
+++ edited/drivers/base/fs/device.c Tue Sep 24 10:47:48 2002
@@ -80,6 +80,24 @@
};

/**
+ * device_create_node - create a device node for a device
+ * @dev: device we're creating the node for
+ * @mode: permissions of the device
+ * @kdev: major/minor of the device
+ *
+ */
+int device_create_node(struct device * dev, mode_t mode, kdev_t kdev)
+{
+ struct attribute attr = {
+ .name = "node",
+ .mode = mode,
+ };
+ printk("creating node (kdev = %d, mode = %d, dev = %s)\n",
+ kdev_t_to_nr(kdev),mode,dev->bus_id);
+ return driverfs_create_node(&attr,&dev->dir,kdev);
+}
+
+/**
* device_create_file - create a driverfs file for a device
* @dev: device requesting file
* @entry: entry describing file
@@ -243,3 +261,4 @@

EXPORT_SYMBOL(device_create_file);
EXPORT_SYMBOL(device_remove_file);
+EXPORT_SYMBOL(device_create_node);
===== fs/driverfs/inode.c 1.50 vs edited =====
--- 1.50/fs/driverfs/inode.c Wed Sep 11 13:47:46 2002
+++ edited/fs/driverfs/inode.c Tue Sep 24 10:51:26 2002
@@ -145,8 +145,6 @@
if (dentry->d_inode)
return -EEXIST;

- /* only allow create if ->d_fsdata is not NULL (so we can assume it
- * comes from the driverfs API below. */
inode = driverfs_get_inode(dir->i_sb, mode, dev);
if (inode) {
d_instantiate(dentry, inode);
@@ -598,6 +596,30 @@
up(&parent->dentry->d_inode->i_sem);
return error;
}
+
+int driverfs_create_node(struct attribute * attr,
+ struct driver_dir_entry * parent, kdev_t kdev)
+{
+ struct dentry * dentry;
+ int error = 0;
+
+ if (!attr || !parent)
+ return -EINVAL;
+
+ if (!parent->dentry)
+ return -EINVAL;
+
+ down(&parent->dentry->d_inode->i_sem);
+ dentry = get_dentry(parent->dentry,attr->name);
+ if (!IS_ERR(dentry)) {
+ error = driverfs_mknod(parent->dentry->d_inode,dentry,
+ attr->mode,kdev_t_to_nr(kdev));
+ } else
+ error = PTR_ERR(dentry);
+ up(&parent->dentry->d_inode->i_sem);
+ return error;
+}
+

/**
* driverfs_create_symlink - make a symlink
===== include/linux/device.h 1.38 vs edited =====
--- 1.38/include/linux/device.h Mon Sep 23 18:23:03 2002
+++ edited/include/linux/device.h Tue Sep 24 11:01:26 2002
@@ -346,6 +346,8 @@
extern int device_create_file(struct device *device, struct device_attribute * entry);
extern void device_remove_file(struct device * dev, struct device_attribute * attr);

+extern int device_create_node(struct device *device, mode_t mode, kdev_t kdev);
+
/*
* Platform "fixup" functions - allow the platform to have their say
* about devices and actions that the general device layer doesn't
===== include/linux/driverfs_fs.h 1.13 vs edited =====
--- 1.13/include/linux/driverfs_fs.h Thu Aug 1 12:07:33 2002
+++ edited/include/linux/driverfs_fs.h Tue Sep 24 10:54:16 2002
@@ -26,6 +26,8 @@
#ifndef _DRIVER_FS_H_
#define _DRIVER_FS_H_

+#include <linux/kdev_t.h>
+
struct driver_dir_entry;
struct attribute;

@@ -57,6 +59,10 @@
extern int
driverfs_create_file(struct attribute * attr,
struct driver_dir_entry * parent);
+
+
+extern int driverfs_create_node(struct attribute * attr,
+ struct driver_dir_entry * parent, kdev_t kdev);

extern int
driverfs_create_symlink(struct driver_dir_entry * parent, /*
* foodev.c - simple illustration of device node creation in driverfs.
*
* Copyright (c) 2002 Patrick Mochel
*
* This is simple and stupid. We declare a platform device, register it
* with the system, then create a device node for it.
* The major/minor used is the char misc device, and the first free minor
* (right after the mk712 touchscreen; stupid mk712).
*
* This is very buggy, but it's not meant to be perfect. We should check
* the return of the registration and creation functions.
* And, the module can be unloaded while there are still references to
* the device. Oh well..
*
* Also, the device has no parent or bus. Don't do that. Ever. I wrote
* the API, so I'm special. ;)
*
* Compiled with:
CFLAGS = -Wall -O2 -fomit-frame-pointer -DMODULE -D__KERNEL__
IDIR = /home/mochel/src/kernel/devel/linux-2.5/include

foodev.o::
$(CC) $(CFLAGS) -I$(IDIR) -c -o $@ $<
*/

#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/device.h>
#include <linux/init.h>
static struct device foodev = {
.bus_id = "foo0",
.name = "The Foo Device",
};
static int __init foodev_init(void)
{
device_register(&foodev);
device_create_node(&foodev,(S_IFCHR|S_IRUGO),mk_kdev(10,16));
return 0;
}
static void __exit foodev_exit(void)
{
put_device(&foodev);
}
module_init(foodev_init);
module_exit(foodev_exit);
\
 
 \ /
  Last update: 2005-03-22 13:29    [W:0.384 / U:0.152 seconds]
©2003-2011 Jasper Spaans. Advertise on this site