* [PATCH 1/5] pktcdvd: Fix overflow for discs with large packets @ 2006-02-03 20:16 Peter Osterlund 2006-02-03 20:18 ` [PATCH 2/5] pktcdvd: Remove version string Peter Osterlund 0 siblings, 1 reply; 7+ messages in thread From: Peter Osterlund @ 2006-02-03 20:16 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, Phillip Susi From: Phillip Susi <psusi@cfl.rr.com> The pktcdvd driver was using an 8 bit field to store the packet length obtained from the disc track info. This causes it to overflow packet length values of 128KB or more. I changed the field to 32 bits to fix this. The pktcdvd driver defaulted to its maximum allowed packet length when it detected a 0 in the track info field. I changed this to fail the operation and refuse to access the media. This seems more sane than attempting to access it with a value that almost certainly will not work. Signed-off-by: Peter Osterlund <petero2@telia.com> --- drivers/block/pktcdvd.c | 2 +- include/linux/pktcdvd.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c index 81ad466..f0a0ad4 100644 --- a/drivers/block/pktcdvd.c +++ b/drivers/block/pktcdvd.c @@ -1640,7 +1640,7 @@ static int pkt_probe_settings(struct pkt pd->settings.size = be32_to_cpu(ti.fixed_packet_size) << 2; if (pd->settings.size == 0) { printk("pktcdvd: detected zero packet size!\n"); - pd->settings.size = 128; + return -ENXIO; } if (pd->settings.size > PACKET_MAX_SECTORS) { printk("pktcdvd: packet size is too big\n"); diff --git a/include/linux/pktcdvd.h b/include/linux/pktcdvd.h index 2c177e4..d1c9c4a 100644 --- a/include/linux/pktcdvd.h +++ b/include/linux/pktcdvd.h @@ -114,7 +114,7 @@ struct pkt_ctrl_command { struct packet_settings { - __u8 size; /* packet size in (512 byte) sectors */ + __u32 size; /* packet size in (512 byte) sectors */ __u8 fp; /* fixed packets */ __u8 link_loss; /* the rest is specified * as per Mt Fuji */ -- Peter Osterlund - petero2@telia.com http://web.telia.com/~u89404340 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/5] pktcdvd: Remove version string 2006-02-03 20:16 [PATCH 1/5] pktcdvd: Fix overflow for discs with large packets Peter Osterlund @ 2006-02-03 20:18 ` Peter Osterlund 2006-02-03 20:19 ` [PATCH 3/5] Let CDROM_PKTCDVD_WCACHE depend on EXPERIMENTAL Peter Osterlund 2006-02-03 20:29 ` [PATCH 2/5] pktcdvd: Remove version string Ismail Donmez 0 siblings, 2 replies; 7+ messages in thread From: Peter Osterlund @ 2006-02-03 20:18 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel The version information is not useful for a driver that is maintained in Linus' kernel tree. Signed-off-by: Peter Osterlund <petero2@telia.com> --- drivers/block/pktcdvd.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c index f0a0ad4..01f070a 100644 --- a/drivers/block/pktcdvd.c +++ b/drivers/block/pktcdvd.c @@ -43,8 +43,6 @@ * *************************************************************************/ -#define VERSION_CODE "v0.2.0a 2004-07-14 Jens Axboe (axboe@suse.de) and petero2@telia.com" - #include <linux/pktcdvd.h> #include <linux/config.h> #include <linux/module.h> @@ -2679,7 +2677,6 @@ static int __init pkt_init(void) pkt_proc = proc_mkdir("pktcdvd", proc_root_driver); - DPRINTK("pktcdvd: %s\n", VERSION_CODE); return 0; out: -- Peter Osterlund - petero2@telia.com http://web.telia.com/~u89404340 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/5] Let CDROM_PKTCDVD_WCACHE depend on EXPERIMENTAL 2006-02-03 20:18 ` [PATCH 2/5] pktcdvd: Remove version string Peter Osterlund @ 2006-02-03 20:19 ` Peter Osterlund 2006-02-03 20:20 ` [PATCH 4/5] pktcdvd: Don't waste kernel memory Peter Osterlund 2006-02-03 20:29 ` [PATCH 2/5] pktcdvd: Remove version string Ismail Donmez 1 sibling, 1 reply; 7+ messages in thread From: Peter Osterlund @ 2006-02-03 20:19 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, Adrian Bunk From: Adrian Bunk <bunk@stusta.de> Unless the help text is outdated, this seems to be logical. Signed-off-by: Adrian Bunk <bunk@stusta.de> Signed-off-by: Peter Osterlund <petero2@telia.com> --- drivers/block/Kconfig | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index 139cbba..db6818f 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -437,8 +437,8 @@ config CDROM_PKTCDVD_BUFFERS pktsetup time. config CDROM_PKTCDVD_WCACHE - bool "Enable write caching" - depends on CDROM_PKTCDVD + bool "Enable write caching (EXPERIMENTAL)" + depends on CDROM_PKTCDVD && EXPERIMENTAL help If enabled, write caching will be set for the CD-R/W device. For now this option is dangerous unless the CD-RW media is known good, as we -- Peter Osterlund - petero2@telia.com http://web.telia.com/~u89404340 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/5] pktcdvd: Don't waste kernel memory 2006-02-03 20:19 ` [PATCH 3/5] Let CDROM_PKTCDVD_WCACHE depend on EXPERIMENTAL Peter Osterlund @ 2006-02-03 20:20 ` Peter Osterlund 2006-02-03 20:21 ` [PATCH 5/5] pktcdvd: Allow larger packets Peter Osterlund 0 siblings, 1 reply; 7+ messages in thread From: Peter Osterlund @ 2006-02-03 20:20 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel Allocate memory for read-gathering at open time, when it is known just how much memory is needed. This avoids wasting kernel memory when the real packet size is smaller than the maximum packet size supported by the driver. This is always the case when using DVD discs. Signed-off-by: Peter Osterlund <petero2@telia.com> --- drivers/block/Kconfig | 4 ++-- drivers/block/pktcdvd.c | 53 +++++++++++++++++++++++++---------------------- include/linux/pktcdvd.h | 4 ++-- 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index db6818f..8b13316 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -433,8 +433,8 @@ config CDROM_PKTCDVD_BUFFERS This controls the maximum number of active concurrent packets. More concurrent packets can increase write performance, but also require more memory. Each concurrent packet will require approximately 64Kb - of non-swappable kernel memory, memory which will be allocated at - pktsetup time. + of non-swappable kernel memory, memory which will be allocated when + a disc is opened for writing. config CDROM_PKTCDVD_WCACHE bool "Enable write caching (EXPERIMENTAL)" diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c index 01f070a..18d5979 100644 --- a/drivers/block/pktcdvd.c +++ b/drivers/block/pktcdvd.c @@ -130,7 +130,7 @@ static struct bio *pkt_bio_alloc(int nr_ /* * Allocate a packet_data struct */ -static struct packet_data *pkt_alloc_packet_data(void) +static struct packet_data *pkt_alloc_packet_data(int frames) { int i; struct packet_data *pkt; @@ -139,11 +139,12 @@ static struct packet_data *pkt_alloc_pac if (!pkt) goto no_pkt; - pkt->w_bio = pkt_bio_alloc(PACKET_MAX_SIZE); + pkt->frames = frames; + pkt->w_bio = pkt_bio_alloc(frames); if (!pkt->w_bio) goto no_bio; - for (i = 0; i < PAGES_PER_PACKET; i++) { + for (i = 0; i < frames / FRAMES_PER_PAGE; i++) { pkt->pages[i] = alloc_page(GFP_KERNEL|__GFP_ZERO); if (!pkt->pages[i]) goto no_page; @@ -151,7 +152,7 @@ static struct packet_data *pkt_alloc_pac spin_lock_init(&pkt->lock); - for (i = 0; i < PACKET_MAX_SIZE; i++) { + for (i = 0; i < frames; i++) { struct bio *bio = pkt_bio_alloc(1); if (!bio) goto no_rd_bio; @@ -161,14 +162,14 @@ static struct packet_data *pkt_alloc_pac return pkt; no_rd_bio: - for (i = 0; i < PACKET_MAX_SIZE; i++) { + for (i = 0; i < frames; i++) { struct bio *bio = pkt->r_bios[i]; if (bio) bio_put(bio); } no_page: - for (i = 0; i < PAGES_PER_PACKET; i++) + for (i = 0; i < frames / FRAMES_PER_PAGE; i++) if (pkt->pages[i]) __free_page(pkt->pages[i]); bio_put(pkt->w_bio); @@ -185,12 +186,12 @@ static void pkt_free_packet_data(struct { int i; - for (i = 0; i < PACKET_MAX_SIZE; i++) { + for (i = 0; i < pkt->frames; i++) { struct bio *bio = pkt->r_bios[i]; if (bio) bio_put(bio); } - for (i = 0; i < PAGES_PER_PACKET; i++) + for (i = 0; i < pkt->frames / FRAMES_PER_PAGE; i++) __free_page(pkt->pages[i]); bio_put(pkt->w_bio); kfree(pkt); @@ -205,17 +206,17 @@ static void pkt_shrink_pktlist(struct pk list_for_each_entry_safe(pkt, next, &pd->cdrw.pkt_free_list, list) { pkt_free_packet_data(pkt); } + INIT_LIST_HEAD(&pd->cdrw.pkt_free_list); } static int pkt_grow_pktlist(struct pktcdvd_device *pd, int nr_packets) { struct packet_data *pkt; - INIT_LIST_HEAD(&pd->cdrw.pkt_free_list); - INIT_LIST_HEAD(&pd->cdrw.pkt_active_list); - spin_lock_init(&pd->cdrw.active_list_lock); + BUG_ON(!list_empty(&pd->cdrw.pkt_free_list)); + while (nr_packets > 0) { - pkt = pkt_alloc_packet_data(); + pkt = pkt_alloc_packet_data(pd->settings.size >> 2); if (!pkt) { pkt_shrink_pktlist(pd); return 0; @@ -950,7 +951,7 @@ try_next_bio: pd->current_sector = zone + pd->settings.size; pkt->sector = zone; - pkt->frames = pd->settings.size >> 2; + BUG_ON(pkt->frames != pd->settings.size >> 2); pkt->write_size = 0; /* @@ -1986,8 +1987,14 @@ static int pkt_open_dev(struct pktcdvd_d if ((ret = pkt_set_segment_merging(pd, q))) goto out_unclaim; - if (write) + if (write) { + if (!pkt_grow_pktlist(pd, CONFIG_CDROM_PKTCDVD_BUFFERS)) { + printk("pktcdvd: not enough memory for buffers\n"); + ret = -ENOMEM; + goto out_unclaim; + } printk("pktcdvd: %lukB available on disc\n", lba << 1); + } return 0; @@ -2013,6 +2020,8 @@ static void pkt_release_dev(struct pktcd pkt_set_speed(pd, MAX_SPEED, MAX_SPEED); bd_release(pd->bdev); blkdev_put(pd->bdev); + + pkt_shrink_pktlist(pd); } static struct pktcdvd_device *pkt_find_dev_from_minor(int dev_minor) @@ -2378,12 +2387,6 @@ static int pkt_new_dev(struct pktcdvd_de /* This is safe, since we have a reference from open(). */ __module_get(THIS_MODULE); - if (!pkt_grow_pktlist(pd, CONFIG_CDROM_PKTCDVD_BUFFERS)) { - printk("pktcdvd: not enough memory for buffers\n"); - ret = -ENOMEM; - goto out_mem; - } - pd->bdev = bdev; set_blocksize(bdev, CD_FRAMESIZE); @@ -2394,7 +2397,7 @@ static int pkt_new_dev(struct pktcdvd_de if (IS_ERR(pd->cdrw.thread)) { printk("pktcdvd: can't start kernel thread\n"); ret = -ENOMEM; - goto out_thread; + goto out_mem; } proc = create_proc_entry(pd->name, 0, pkt_proc); @@ -2405,8 +2408,6 @@ static int pkt_new_dev(struct pktcdvd_de DPRINTK("pktcdvd: writer %s mapped to %s\n", pd->name, bdevname(bdev, b)); return 0; -out_thread: - pkt_shrink_pktlist(pd); out_mem: blkdev_put(bdev); /* This is safe: open() is still holding a reference. */ @@ -2502,6 +2503,10 @@ static int pkt_setup_dev(struct pkt_ctrl goto out_mem; pd->disk = disk; + INIT_LIST_HEAD(&pd->cdrw.pkt_free_list); + INIT_LIST_HEAD(&pd->cdrw.pkt_active_list); + spin_lock_init(&pd->cdrw.active_list_lock); + spin_lock_init(&pd->lock); spin_lock_init(&pd->iosched.lock); sprintf(pd->name, "pktcdvd%d", idx); @@ -2566,8 +2571,6 @@ static int pkt_remove_dev(struct pkt_ctr blkdev_put(pd->bdev); - pkt_shrink_pktlist(pd); - remove_proc_entry(pd->name, pkt_proc); DPRINTK("pktcdvd: writer %s unmapped\n", pd->name); diff --git a/include/linux/pktcdvd.h b/include/linux/pktcdvd.h index d1c9c4a..1623da8 100644 --- a/include/linux/pktcdvd.h +++ b/include/linux/pktcdvd.h @@ -170,7 +170,7 @@ struct packet_iosched #error "PAGE_SIZE must be a multiple of CD_FRAMESIZE" #endif #define PACKET_MAX_SIZE 32 -#define PAGES_PER_PACKET (PACKET_MAX_SIZE * CD_FRAMESIZE / PAGE_SIZE) +#define FRAMES_PER_PAGE (PAGE_SIZE / CD_FRAMESIZE) #define PACKET_MAX_SECTORS (PACKET_MAX_SIZE * CD_FRAMESIZE >> 9) enum packet_data_state { @@ -219,7 +219,7 @@ struct packet_data atomic_t io_errors; /* Number of read/write errors during IO */ struct bio *r_bios[PACKET_MAX_SIZE]; /* bios to use during data gathering */ - struct page *pages[PAGES_PER_PACKET]; + struct page *pages[PACKET_MAX_SIZE / FRAMES_PER_PAGE]; int cache_valid; /* If non-zero, the data for the zone defined */ /* by the sector variable is completely cached */ -- Peter Osterlund - petero2@telia.com http://web.telia.com/~u89404340 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 5/5] pktcdvd: Allow larger packets 2006-02-03 20:20 ` [PATCH 4/5] pktcdvd: Don't waste kernel memory Peter Osterlund @ 2006-02-03 20:21 ` Peter Osterlund 0 siblings, 0 replies; 7+ messages in thread From: Peter Osterlund @ 2006-02-03 20:21 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, Phillip Susi From: Phillip Susi <psusi@cfl.rr.com> The pktcdvd driver uses a compile time macro constant to define the maximum supported packet length. I changed this from 32 sectors to 128 sectors because that allows over 100 MB of additional usable space on a 700 MB cdrw, and increases throughput. Note that you need a modified cdrwtool program that can format a CDRW disc with larger packets to benefit from this change. Signed-off-by: Peter Osterlund <petero2@telia.com> --- include/linux/pktcdvd.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/include/linux/pktcdvd.h b/include/linux/pktcdvd.h index 1623da8..8a94c71 100644 --- a/include/linux/pktcdvd.h +++ b/include/linux/pktcdvd.h @@ -169,7 +169,7 @@ struct packet_iosched #if (PAGE_SIZE % CD_FRAMESIZE) != 0 #error "PAGE_SIZE must be a multiple of CD_FRAMESIZE" #endif -#define PACKET_MAX_SIZE 32 +#define PACKET_MAX_SIZE 128 #define FRAMES_PER_PAGE (PAGE_SIZE / CD_FRAMESIZE) #define PACKET_MAX_SECTORS (PACKET_MAX_SIZE * CD_FRAMESIZE >> 9) -- Peter Osterlund - petero2@telia.com http://web.telia.com/~u89404340 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/5] pktcdvd: Remove version string 2006-02-03 20:18 ` [PATCH 2/5] pktcdvd: Remove version string Peter Osterlund 2006-02-03 20:19 ` [PATCH 3/5] Let CDROM_PKTCDVD_WCACHE depend on EXPERIMENTAL Peter Osterlund @ 2006-02-03 20:29 ` Ismail Donmez 2006-02-03 20:49 ` Peter Osterlund 1 sibling, 1 reply; 7+ messages in thread From: Ismail Donmez @ 2006-02-03 20:29 UTC (permalink / raw) To: Peter Osterlund; +Cc: linux-kernel, Andrew Morton Cuma 3 Şubat 2006 22:18 tarihinde şunları yazmıştınız: > The version information is not useful for a driver that is maintained > in Linus' kernel tree. > > Signed-off-by: Peter Osterlund <petero2@telia.com> > --- > > drivers/block/pktcdvd.c | 3 --- > 1 files changed, 0 insertions(+), 3 deletions(-) > > diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c > index f0a0ad4..01f070a 100644 > --- a/drivers/block/pktcdvd.c > +++ b/drivers/block/pktcdvd.c > @@ -43,8 +43,6 @@ > * > > *************************************************************************/ > > -#define VERSION_CODE "v0.2.0a 2004-07-14 Jens Axboe (axboe@suse.de) and > petero2@telia.com" - > #include <linux/pktcdvd.h> > #include <linux/config.h> > #include <linux/module.h> > @@ -2679,7 +2677,6 @@ static int __init pkt_init(void) > > pkt_proc = proc_mkdir("pktcdvd", proc_root_driver); > > - DPRINTK("pktcdvd: %s\n", VERSION_CODE); > return 0; > > out: Hmm this is useful to do dmesg|grep pktcdvd though when you compile it in the kernel. So I would like to keep it in. Just my 2 cents. Regards, ismail ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/5] pktcdvd: Remove version string 2006-02-03 20:29 ` [PATCH 2/5] pktcdvd: Remove version string Ismail Donmez @ 2006-02-03 20:49 ` Peter Osterlund 0 siblings, 0 replies; 7+ messages in thread From: Peter Osterlund @ 2006-02-03 20:49 UTC (permalink / raw) To: Ismail Donmez; +Cc: linux-kernel, Andrew Morton Ismail Donmez <ismail@uludag.org.tr> writes: > Cuma 3 Şubat 2006 22:18 tarihinde şunları yazmıştınız: > > The version information is not useful for a driver that is maintained > > in Linus' kernel tree. > > > > Signed-off-by: Peter Osterlund <petero2@telia.com> ... > Hmm this is useful to do dmesg|grep pktcdvd though when you compile it in the > kernel. So I would like to keep it in. Why is it useful? The actual version number can't be that useful since it hasn't been updated since 2004. If you just want to know if the driver is currently in the kernel, you can do: cat /proc/misc | grep pktcdvd -- Peter Osterlund - petero2@telia.com http://web.telia.com/~u89404340 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2006-02-03 20:50 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-02-03 20:16 [PATCH 1/5] pktcdvd: Fix overflow for discs with large packets Peter Osterlund 2006-02-03 20:18 ` [PATCH 2/5] pktcdvd: Remove version string Peter Osterlund 2006-02-03 20:19 ` [PATCH 3/5] Let CDROM_PKTCDVD_WCACHE depend on EXPERIMENTAL Peter Osterlund 2006-02-03 20:20 ` [PATCH 4/5] pktcdvd: Don't waste kernel memory Peter Osterlund 2006-02-03 20:21 ` [PATCH 5/5] pktcdvd: Allow larger packets Peter Osterlund 2006-02-03 20:29 ` [PATCH 2/5] pktcdvd: Remove version string Ismail Donmez 2006-02-03 20:49 ` Peter Osterlund
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox