2006-10-03 15:25:51

by Thomas Maier

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

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<[email protected]>

-Thomas Maier


Attachments:
pktcdvd-7-procfs-optional_2.6.18.patch (10.23 kB)

2006-10-05 19:48:41

by Peter Osterlund

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

"Thomas Maier" <[email protected]> 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 <[email protected]>

pktcdvd: Update Kconfig help text.

Signed-off-by: Thomas Maier <[email protected]>
Signed-off-by: Peter Osterlund <[email protected]>

---

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 - [email protected]
http://web.telia.com/~u89404340

2006-10-05 20:00:01

by Peter Osterlund

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

"Thomas Maier" <[email protected]> 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 - [email protected]
http://web.telia.com/~u89404340

2006-10-09 10:03:44

by Thomas Maier

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

Hello,

Am 05.10.2006, 21:59 Uhr, schrieb Peter Osterlund <[email protected]>:

> 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

2006-10-09 17:07:33

by Ingo Oeser

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

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

2006-10-14 18:01:38

by Thomas Maier

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

Hello,

this is a bugfix for pktcdvd.c:

pktdev_major must be initialized to 0.

Signed-off-by: Thomas Maier <[email protected]>


Attachments:
pktcdvd-1-init-pktdev_major.patch (545.00 B)

2006-10-14 18:05:21

by Peter Osterlund

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

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 - [email protected]
http://web.telia.com/~u89404340