[PATCH 7/11] 2.6.18-mm3 pktcdvd: make procfs interface optional

[PATCH 7/11] 2.6.18-mm3 pktcdvd: make procfs interface optional

[Thomas Maier]

[-- Attachment #1: Type: text/plain, Size: 293 bytes --]

Hello,

this patch makes the procfs interface optional and groups
the procfs functions together.
New kernel config parameter: CDROM_PKTCDVD_PROCINTF

http://people.freenet.de/BalaGi/download/pktcdvd-7-procfs-optional_2.6.18.patch

Signed-off-by: Thomas Maier<balagi@justmail.de>

-Thomas Maier

[-- Attachment #2: pktcdvd-7-procfs-optional_2.6.18.patch --]
[-- Type: application/octet-stream, Size: 10474 bytes --]

diff -urpN 6-sysfs/drivers/block/Kconfig 7-procfs-optional/drivers/block/Kconfig
--- 6-sysfs/drivers/block/Kconfig	2006-10-03 11:49:17.000000000 +0200
+++ 7-procfs-optional/drivers/block/Kconfig	2006-10-03 13:20:02.000000000 +0200
@@ -428,17 +428,22 @@ config CDROM_PKTCDVD
 	tristate "Packet writing on CD/DVD media"
 	depends on !UML
 	help
-	  If you have a CDROM drive that supports packet writing, say Y to
-	  include preliminary support. It should work with any MMC/Mt Fuji
-	  compliant ATAPI or SCSI drive, which is just about any newer CD
-	  writer.
+	  If you have a CDROM/DVD drive that supports packet writing, say
+	  Y to include preliminary support. It should work with any
+	  MMC/Mt Fuji compliant ATAPI or SCSI drive, which is just about
+	  any newer DVD/CD writer.
 
-	  Currently only writing to CD-RW, DVD-RW and DVD+RW discs is possible.
+	  Currently only writing to CD-RW, DVD-RW, DVD+RW and DVDRAM discs
+	  is possible.
 	  DVD-RW disks must be in restricted overwrite mode.
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called pktcdvd.
 
+	  For further information see the file
+
+	    Documentation/cdrom/packet-writing.txt
+	  
 config CDROM_PKTCDVD_BUFFERS
 	int "Free buffers for data gathering"
 	depends on CDROM_PKTCDVD
@@ -458,6 +463,21 @@ config CDROM_PKTCDVD_WCACHE
 	  this option is dangerous unless the CD-RW media is known good, as we
 	  don't do deferred write error handling yet.
 
+config CDROM_PKTCDVD_PROCINTF
+	bool "Enable procfs interface"
+	depends on CDROM_PKTCDVD
+	default y
+	help
+	  Enable the proc filesystem interface for pktcdvd.
+	  The files can be found as:
+	  
+	  /proc/driver/pktcdvd/pktcdvd<idx>
+	  
+	  Also a misc device is added with a dynamic minor device id.
+	  See the entry in  /proc/misc  for the minor number.
+	  The misc pktcdvd driver supports ioctl calls, needed
+	  by the pktsetup tool found in udftools package.
+
 source "drivers/s390/block/Kconfig"
 
 config ATA_OVER_ETH
diff -urpN 6-sysfs/drivers/block/pktcdvd.c 7-procfs-optional/drivers/block/pktcdvd.c
--- 6-sysfs/drivers/block/pktcdvd.c	2006-10-03 13:54:19.000000000 +0200
+++ 7-procfs-optional/drivers/block/pktcdvd.c	2006-10-03 13:54:31.000000000 +0200
@@ -52,9 +52,7 @@
 #include <linux/errno.h>
 #include <linux/spinlock.h>
 #include <linux/file.h>
-#include <linux/proc_fs.h>
 #include <linux/seq_file.h>
-#include <linux/miscdevice.h>
 #include <linux/suspend.h>
 #include <linux/mutex.h>
 #include <scsi/scsi_cmnd.h>
@@ -63,6 +61,11 @@
 #include <linux/debugfs.h>
 #include <linux/device.h>
 
+#if PKT_USE_PROCFS
+#include <linux/proc_fs.h>
+#include <linux/miscdevice.h>
+#endif
+
 #include <asm/uaccess.h>
 
 #define DRIVER_NAME	"pktcdvd"
@@ -84,7 +87,6 @@
 #define ZONE(sector, pd) (((sector) + (pd)->offset) & ~((pd)->settings.size - 1))
 
 static struct pktcdvd_device *pkt_devs[MAX_WRITERS];
-static struct proc_dir_entry *pkt_proc;
 static int pktdev_major = 0; /* default: dynamic major number */
 static struct mutex ctl_mutex;	/* Serialize open/close/setup/teardown */
 static mempool_t *psd_pool;
@@ -547,6 +549,108 @@ static void pkt_debugfs_cleanup(void)
 	pkt_debugfs_root = NULL;
 }
 
+/********************************************************
+ *
+ * (old) procfs interface
+ *
+ *******************************************************/
+#if PKT_USE_PROCFS
+static struct proc_dir_entry *pkt_proc;
+
+/* file operations for /proc/driver/pktcdvd/.. files */
+
+static int pkt_seq_show(struct seq_file *m, void *p)
+{
+	struct pktcdvd_device *pd = m->private;
+	char buf[1024];
+	
+	pkt_print_info(pd, buf, sizeof(buf));
+	seq_printf(m, "%s", buf);
+	return 0;
+}
+
+static int pkt_seq_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, pkt_seq_show, PDE(inode)->data);
+}
+
+static struct file_operations pkt_proc_fops = {
+	.open	= pkt_seq_open,
+	.read	= seq_read,
+	.llseek	= seq_lseek,
+	.release = single_release
+};
+
+static void pkt_get_status(struct pkt_ctrl_command *ctrl_cmd)
+{
+	struct pktcdvd_device *pd;
+	
+	mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
+	
+	pd = pkt_find_dev_from_minor(ctrl_cmd->dev_index);
+	if (pd) {
+		ctrl_cmd->dev = new_encode_dev(pd->bdev->bd_dev);
+		ctrl_cmd->pkt_dev = new_encode_dev(pd->pkt_dev);
+	} else {
+		ctrl_cmd->dev = 0;
+		ctrl_cmd->pkt_dev = 0;
+	}
+	ctrl_cmd->num_devices = MAX_WRITERS;
+	
+	mutex_unlock(&ctl_mutex);
+}
+
+static int pkt_ctl_ioctl(struct inode *inode, struct file *file,
+			unsigned int cmd, unsigned long arg)
+{
+	void __user *argp = (void __user *)arg;
+	struct pkt_ctrl_command ctrl_cmd;
+	int ret = 0;
+	dev_t pkt_dev = 0;
+
+	if (cmd != PACKET_CTRL_CMD)
+		return -ENOTTY;
+
+	if (copy_from_user(&ctrl_cmd, argp, sizeof(struct pkt_ctrl_command)))
+		return -EFAULT;
+
+	switch (ctrl_cmd.command) {
+	case PKT_CTRL_CMD_SETUP:
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+		ret = pkt_setup_dev(new_decode_dev(ctrl_cmd.dev), &pkt_dev);
+		ctrl_cmd.pkt_dev = new_encode_dev(pkt_dev);
+		break;
+	case PKT_CTRL_CMD_TEARDOWN:
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+		ret = pkt_remove_dev(new_decode_dev(ctrl_cmd.pkt_dev));
+		break;
+	case PKT_CTRL_CMD_STATUS:
+		pkt_get_status(&ctrl_cmd);
+		break;
+	default:
+		return -ENOTTY;
+	}
+
+	if (copy_to_user(argp, &ctrl_cmd, sizeof(struct pkt_ctrl_command)))
+		return -EFAULT;
+	return ret;
+}
+
+
+static struct file_operations pkt_ctl_fops = {
+	.ioctl	 = pkt_ctl_ioctl,
+	.owner	 = THIS_MODULE,
+};
+
+static struct miscdevice pkt_misc = {
+	.minor 		= MISC_DYNAMIC_MINOR,
+	.name  		= DRIVER_NAME,
+	.fops  		= &pkt_ctl_fops
+};
+
+#endif /* PKT_USE_PROCFS */
 
 /*****************************************************************/
 
@@ -2682,34 +2786,11 @@ static void pkt_init_queue(struct pktcdv
 	q->queuedata = pd;
 }
 
-static int pkt_seq_show(struct seq_file *m, void *p)
-{
-	struct pktcdvd_device *pd = m->private;
-	char buf[1024];
-	
-	pkt_print_info(pd, buf, sizeof(buf));
-	seq_printf(m, "%s", buf);
-	return 0;
-}
-
-static int pkt_seq_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, pkt_seq_show, PDE(inode)->data);
-}
-
-static struct file_operations pkt_proc_fops = {
-	.open	= pkt_seq_open,
-	.read	= seq_read,
-	.llseek	= seq_lseek,
-	.release = single_release
-};
-
 static int pkt_new_dev(struct pktcdvd_device *pd, dev_t dev)
 {
 	int i;
 	int ret = 0;
 	char b[BDEVNAME_SIZE];
-	struct proc_dir_entry *proc;
 	struct block_device *bdev;
 
 	if (pd->pkt_dev == dev) {
@@ -2753,11 +2834,16 @@ static int pkt_new_dev(struct pktcdvd_de
 		goto out_mem;
 	}
 
-	proc = create_proc_entry(pd->name, 0, pkt_proc);
-	if (proc) {
-		proc->data = pd;
-		proc->proc_fops = &pkt_proc_fops;
+#if PKT_USE_PROCFS
+	{
+		struct proc_dir_entry *proc = create_proc_entry(pd->name,
+								0, pkt_proc);
+		if (proc) {
+			proc->data = pd;
+			proc->proc_fops = &pkt_proc_fops;
+		}
 	}
+#endif
 	DPRINTK(DRIVER_NAME": writer %s mapped to %s\n", pd->name, bdevname(bdev, b));
 	return 0;
 
@@ -2946,7 +3032,9 @@ static int pkt_remove_dev(dev_t pkt_dev)
 
 	blkdev_put(pd->bdev);
 
+#if PKT_USE_PROCFS
 	remove_proc_entry(pd->name, pkt_proc);
+#endif
 	DPRINTK(DRIVER_NAME": writer %s unmapped\n", pd->name);
 
 	del_gendisk(pd->disk);
@@ -2964,73 +3052,6 @@ out:
 	return ret;
 }
 
-static void pkt_get_status(struct pkt_ctrl_command *ctrl_cmd)
-{
-	struct pktcdvd_device *pd;
-	
-	mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
-	
-	pd = pkt_find_dev_from_minor(ctrl_cmd->dev_index);
-	if (pd) {
-		ctrl_cmd->dev = new_encode_dev(pd->bdev->bd_dev);
-		ctrl_cmd->pkt_dev = new_encode_dev(pd->pkt_dev);
-	} else {
-		ctrl_cmd->dev = 0;
-		ctrl_cmd->pkt_dev = 0;
-	}
-	ctrl_cmd->num_devices = MAX_WRITERS;
-	
-	mutex_unlock(&ctl_mutex);
-}
-
-static int pkt_ctl_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
-{
-	void __user *argp = (void __user *)arg;
-	struct pkt_ctrl_command ctrl_cmd;
-	int ret = 0;
-	dev_t pkt_dev = 0;
-
-	if (cmd != PACKET_CTRL_CMD)
-		return -ENOTTY;
-
-	if (copy_from_user(&ctrl_cmd, argp, sizeof(struct pkt_ctrl_command)))
-		return -EFAULT;
-
-	switch (ctrl_cmd.command) {
-	case PKT_CTRL_CMD_SETUP:
-		if (!capable(CAP_SYS_ADMIN))
-			return -EPERM;
-		ret = pkt_setup_dev(new_decode_dev(ctrl_cmd.dev), &pkt_dev);
-		ctrl_cmd.pkt_dev = new_encode_dev(pkt_dev);
-		break;
-	case PKT_CTRL_CMD_TEARDOWN:
-		if (!capable(CAP_SYS_ADMIN))
-			return -EPERM;
-		ret = pkt_remove_dev(new_decode_dev(ctrl_cmd.pkt_dev));
-		break;
-	case PKT_CTRL_CMD_STATUS:
-		pkt_get_status(&ctrl_cmd);
-		break;
-	default:
-		return -ENOTTY;
-	}
-
-	if (copy_to_user(argp, &ctrl_cmd, sizeof(struct pkt_ctrl_command)))
-		return -EFAULT;
-	return ret;
-}
-
-
-static struct file_operations pkt_ctl_fops = {
-	.ioctl	 = pkt_ctl_ioctl,
-	.owner	 = THIS_MODULE,
-};
-
-static struct miscdevice pkt_misc = {
-	.minor 		= MISC_DYNAMIC_MINOR,
-	.name  		= DRIVER_NAME,
-	.fops  		= &pkt_ctl_fops
-};
 
 static int __init pkt_init(void)
 {
@@ -3057,19 +3078,21 @@ static int __init pkt_init(void)
 
 	pkt_debugfs_init();
 
+#if PKT_USE_PROCFS
 	ret = misc_register(&pkt_misc);
 	if (ret) {
 		printk(DRIVER_NAME": Unable to register misc device\n");
 		goto out_misc;
 	}
-
 	pkt_proc = proc_mkdir(DRIVER_NAME, proc_root_driver);
-
+#endif
 	return 0;
 
+#if PKT_USE_PROCFS
 out_misc:
 	pkt_debugfs_cleanup();
 	pkt_sysfs_cleanup();
+#endif
 out:
 	unregister_blkdev(pktdev_major, DRIVER_NAME);
 out2:
@@ -3079,9 +3102,10 @@ out2:
 
 static void __exit pkt_exit(void)
 {
+#if PKT_USE_PROCFS
 	remove_proc_entry(DRIVER_NAME, proc_root_driver);
 	misc_deregister(&pkt_misc);
-	
+#endif
 	pkt_debugfs_cleanup();
 	pkt_sysfs_cleanup();
 	
diff -urpN 6-sysfs/include/linux/pktcdvd.h 7-procfs-optional/include/linux/pktcdvd.h
--- 6-sysfs/include/linux/pktcdvd.h	2006-10-03 12:54:41.000000000 +0200
+++ 7-procfs-optional/include/linux/pktcdvd.h	2006-10-03 13:09:18.000000000 +0200
@@ -108,12 +108,21 @@ struct pkt_ctrl_command {
 #define PACKET_CTRL_CMD		_IOWR(PACKET_IOCTL_MAGIC, 1, struct pkt_ctrl_command)
 
 #ifdef __KERNEL__
+#include <linux/config.h>
 #include <linux/blkdev.h>
 #include <linux/completion.h>
 #include <linux/cdrom.h>
 #include <linux/kobject.h>
 #include <linux/sysfs.h>
 
+/* use (old) procfs interface? */
+#ifdef CONFIG_CDROM_PKTCDVD_PROCINTF
+#define PKT_USE_PROCFS  1
+#else
+#define PKT_USE_PROCFS  0
+#endif
+
+
 struct packet_settings
 {
 	__u32			size;		/* packet size in (512 byte) sectors */

Re: [PATCH 7/11] 2.6.18-mm3 pktcdvd: make procfs interface optional

[Peter Osterlund]

"Thomas Maier" <balagi@justmail.de> writes:

> this patch makes the procfs interface optional and groups
> the procfs functions together.
> New kernel config parameter: CDROM_PKTCDVD_PROCINTF

The Kconfig help text update looks good (slightly modified). Andrew,
please apply:


From: Thomas Maier <balagi@justmail.de>

pktcdvd: Update Kconfig help text.

Signed-off-by: Thomas Maier <balagi@justmail.de>
Signed-off-by: Peter Osterlund <petero2@telia.com>

---

 drivers/block/Kconfig |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index 17dc222..f7eb08b 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -429,14 +429,18 @@ config CDROM_PKTCDVD
 	tristate "Packet writing on CD/DVD media"
 	depends on !UML
 	help
-	  If you have a CDROM drive that supports packet writing, say Y to
-	  include preliminary support. It should work with any MMC/Mt Fuji
-	  compliant ATAPI or SCSI drive, which is just about any newer CD
-	  writer.
+	  If you have a CDROM/DVD drive that supports packet writing, say
+	  Y to include support. It should work with any MMC/Mt Fuji
+	  compliant ATAPI or SCSI drive, which is just about any newer
+	  DVD/CD writer.
 
-	  Currently only writing to CD-RW, DVD-RW and DVD+RW discs is possible.
+	  Currently only writing to CD-RW, DVD-RW, DVD+RW and DVDRAM discs
+	  is possible.
 	  DVD-RW disks must be in restricted overwrite mode.
 
+	  See the file <file:Documentation/cdrom/packet-writing.txt>
+	  for further information on the use of this driver.
+
 	  To compile this driver as a module, choose M here: the
 	  module will be called pktcdvd.
 

-- 
Peter Osterlund - petero2@telia.com
http://web.telia.com/~u89404340

Re: [PATCH 7/11] 2.6.18-mm3 pktcdvd: make procfs interface optional

[Peter Osterlund]

"Thomas Maier" <balagi@justmail.de> writes:

> this patch makes the procfs interface optional and groups
> the procfs functions together.
> New kernel config parameter: CDROM_PKTCDVD_PROCINTF

Given the fact that Linus doesn't allow breaking user space tools
unless absolutely necessary, I don't think it makes sense to be able
to disable the character device control code.

The /proc/driver/pktcdvd/pktcdvd? file only contains debugging stuff
though, and the main reason it's not already in debugfs is that
debugfs didn't exist when Jens wrote this driver.

Therefore a patch that unconditionally moves
/proc/driver/pktcdvd/pktcdvd? to debugfs would make a lot of sense.

Also, the current change has another problem:

static int pkt_seq_show(struct seq_file *m, void *p)
+{
+       struct pktcdvd_device *pd = m->private;
+       char buf[1024];
+
+       pkt_print_info(pd, buf, sizeof(buf));
+       seq_printf(m, "%s", buf);
+       return 0;
+}

This wastes 1K stack space, and it can corrupt the stack if the
pkt_print_info() function wants to write more than 1K data.

Unconditionally moving to debugfs would remove the need for the
pkt_print_info() function so the buf array wouldn't be needed any
more.

-- 
Peter Osterlund - petero2@telia.com
http://web.telia.com/~u89404340

Re: [PATCH 7/11] 2.6.18-mm3 pktcdvd: make procfs interface optional

[Thomas Maier]

Hello,

Am 05.10.2006, 21:59 Uhr, schrieb Peter Osterlund <petero2@telia.com>:

> Given the fact that Linus doesn't allow breaking user space tools
> unless absolutely necessary, I don't think it makes sense to be able
> to disable the character device control code.

Hmm, since it will be optional to leave the procfs interface in the
kernel, it is on the admin / distribution builder to decide this.
I know only one tool that uses this interface (pktsetup), maybe there
are others, but if the sysfs interface will be available in future (?),
there is another (simple) option to control the driver, using simple
shell commands e.g.
And i read anywhere, that procfs should only contain process information
in a far away (?) future ;)

> Therefore a patch that unconditionally moves
> /proc/driver/pktcdvd/pktcdvd? to debugfs would make a lot of sense.

Then move it to debugfs and drop it from procfs.

> Also, the current change has another problem:
>
> static int pkt_seq_show(struct seq_file *m, void *p)
> +{
> +       struct pktcdvd_device *pd = m->private;
> +       char buf[1024];
> +
> +       pkt_print_info(pd, buf, sizeof(buf));
> +       seq_printf(m, "%s", buf);
> +       return 0;
> +}
>
> This wastes 1K stack space, and it can corrupt the stack if the
> pkt_print_info() function wants to write more than 1K data.

Ok, 1k is a little bit high, since only about 300 chars are written
into the buffer currently.
But also, pkt_print_info() would only write sizeof(buf) chars into
the buffer.

-Thomas

Re: [PATCH 7/11] 2.6.18-mm3 pktcdvd: make procfs interface optional

[Ingo Oeser]

Hi,

On Monday, 9. October 2006 12:05, Thomas Maier wrote:
> Ok, 1k is a little bit high, since only about 300 chars are written
> into the buffer currently.
> But also, pkt_print_info() would only write sizeof(buf) chars into
> the buffer.

What about making pkt_print_info() accept a "struct seq_file *" 
instead and print directly into the seq_file buffer? 


Regards

Ingo Oeser

[PATCH 1/2] 2.6.19-rc1-mm1 pktcdvd: init pktdev_major

[Thomas Maier]

[-- Attachment #1: Type: text/plain, Size: 131 bytes --]

Hello,

this is a bugfix for pktcdvd.c:

pktdev_major must be initialized to 0.

Signed-off-by: Thomas Maier <balagi@justmail.de>


[-- Attachment #2: pktcdvd-1-init-pktdev_major.patch --]
[-- Type: application/octet-stream, Size: 545 bytes --]

diff -urpN linux-2.6.19-rc1-mm1/drivers/block/pktcdvd.c 1-init-pktdev_major/drivers/block/pktcdvd.c
--- linux-2.6.19-rc1-mm1/drivers/block/pktcdvd.c	2006-10-14 13:46:04.000000000 +0200
+++ 1-init-pktdev_major/drivers/block/pktcdvd.c	2006-10-14 13:50:54.000000000 +0200
@@ -83,7 +83,7 @@
 
 static struct pktcdvd_device *pkt_devs[MAX_WRITERS];
 static struct proc_dir_entry *pkt_proc;
-static int pktdev_major;
+static int pktdev_major = 0;
 static struct mutex ctl_mutex;	/* Serialize open/close/setup/teardown */
 static mempool_t *psd_pool;
 

Re: [PATCH 1/2] 2.6.19-rc1-mm1 pktcdvd: init pktdev_major

[Peter Osterlund]

On Sat, 14 Oct 2006, Thomas Maier wrote:

> pktdev_major must be initialized to 0.

No it doesn't. The BSS section is automatically initialized to 0. In fact, 
in the past there have been several patches for various parts of the 
kernel to remove explicit 0 initialization of global variables.

-- 
Peter Osterlund - petero2@telia.com
http://web.telia.com/~u89404340