pktcdvd for > 2.4.19-pre4
Is there a pktcdvd patch for > 2.4.19-pre4 in the works? the 2.4.19-pre4 patch causes 2.4.19-pre6 and pre7 to hang when I try to run pktsetup. I would try the one for 2.5.10 if IDE wasn't broken in 2.5 kernels since 2.5.7 (I think). -John
On Tue, 30 Apr 2002, Johnathan Hicks wrote:
Is there a pktcdvd patch for > 2.4.19-pre4 in the works? the 2.4.19-pre4 patch causes 2.4.19-pre6 and pre7 to hang when I try to run pktsetup.
I generated a new patch for 2.4.19-pre7, but it ended up being equal to the -pre4 patch, since nothing has changed between -pre4 and -pre7 that affects the packet patch. I have tested the -pre4 packet patch with both -pre6 and -pre7 kernels, and it works ok on my systems. Is the -pre5 kernel working for you? Is the whole kernel locking up or just the pktsetup command? Are there any messages on the console before the lockup? Is magic sysrq still working after the lockup? Does setting PACKET_DEBUG to 2 in linux/include/linux/pktcdvd.h reveal any additional info?
I would try the one for 2.5.10 if IDE wasn't broken in 2.5 kernels since 2.5.7 (I think).
IDE works on my laptop in 2.5.10. I have not experienced any filesystem corruption, but I would not use it on a system with important data anyway. -- Peter Osterlund - petero2@telia.com http://w1.894.telia.com/~u89404340
Peter Osterlund wrote:
On Tue, 30 Apr 2002, Johnathan Hicks wrote:
Is there a pktcdvd patch for > 2.4.19-pre4 in the works? the 2.4.19-pre4 patch causes 2.4.19-pre6 and pre7 to hang when I try to run pktsetup.
I generated a new patch for 2.4.19-pre7, but it ended up being equal to the -pre4 patch, since nothing has changed between -pre4 and -pre7 that affects the packet patch.
Hrm... That's interesting, 'patch' tells me the same thing. :)
I have tested the -pre4 packet patch with both -pre6 and -pre7 kernels, and it works ok on my systems. Is the -pre5 kernel working for you? Is the whole kernel locking up or just the pktsetup command? Are there any messages on the console before the lockup? Is magic sysrq still working after the lockup? Does setting PACKET_DEBUG to 2 in linux/include/linux/pktcdvd.h reveal any additional info?
I haven't tried pre5, but I did set the PACKET_DEBUG option for both pre4 and pre7 as you suggested. I booted the pre4 first to see what normal output would be, and then booted pre7 to find out where the problem was. The last line that the pre7 printed was: grow_bhlist: count=32
From there I could not switch VCs, but sysrq appeared responsive until I tried to use it to reboot as which point I had to hard reset.
After many VPRINTK() insertions into pktcdvd.c I finally tracked the freeze point down to a blk_init_queue() call from pkt_init_queue(). From that it would appear the the problem is not with pktcdvd after all. Do you think I should hunt down blk_init_queue() and stuff it with printk() or would I end up with too much output for it to be useful?
I would try the one for 2.5.10 if IDE wasn't broken in 2.5 kernels since 2.5.7 (I think).
IDE works on my laptop in 2.5.10. I have not experienced any filesystem corruption, but I would not use it on a system with important data anyway.
I just tried 2.5.12 and it worked but that might be something to do with me disabling ACPI. I also massaged the 2.5.10 pktcdvd patch onto it which basically involved keeping up with BIO changes in pktcdvd.c. There was also a single reject from ide-cd.c that would be fixed by regenerating the pktcdvd patch. The module compiled fine except for compaining about the pointers passed to the spinlock functions. Since the module appeared to load fine I've attached the patch for your inspection. I noticed when I loaded the module that the version string hasn't been updated for a while, did you miss updating it? I tried to use the linux-2.5 UDF from the udf CVS of a few days ago. It required some header file modifications, but the resulting module had some unresolved symbols. The UDF that ships with 2.5.12 seems to cause an oops that befuddles the cd-rw something aweful... --John --- pktcdvd.c.org Thu May 2 16:34:10 2002 +++ pktcdvd.c Thu May 2 15:07:12 2002 @@ -161,7 +161,7 @@ static int pkt_front_merge_fn(request_queue_t *q, struct request *rq, struct bio *bio) { - struct pktcdvd_device *pd = &pkt_devs[minor(bio->bi_dev)]; + struct pktcdvd_device *pd = &pkt_devs[minor(bio->bi_bdev->bd_inode->i_rdev)]; return ZONE(rq->sector, pd) == ZONE(bio->bi_sector, pd); } @@ -169,7 +169,7 @@ static int pkt_back_merge_fn(request_queue_t *q, struct request *rq, struct bio *bio) { - struct pktcdvd_device *pd = &pkt_devs[minor(bio->bi_dev)]; + struct pktcdvd_device *pd = &pkt_devs[minor(bio->bi_bdev->bd_inode->i_rdev)]; return ZONE(rq->sector, pd) == ZONE(bio->bi_sector, pd); } @@ -192,7 +192,7 @@ static int pkt_lowlevel_elv_merge_fn(request_queue_t *q, struct request **rq, struct bio *bio) { - struct pktcdvd_device *pd = pkt_find_dev(bio->bi_dev); + struct pktcdvd_device *pd = pkt_find_dev(bio->bi_bdev->bd_inode->i_rdev); if (test_bit(BIO_RW, &bio->bi_rw)) return ELEVATOR_NO_MERGE; @@ -547,7 +547,7 @@ static void pkt_end_io_read(struct bio *bio) { struct packet_data *pkt = bio->bi_private; - struct pktcdvd_device *pd = pkt_find_dev(bio->bi_dev); + struct pktcdvd_device *pd = pkt_find_dev(bio->bi_bdev->bd_inode->i_rdev); VPRINTK("pkt_end_io_read: bio=%p sec0=%ld sec=%ld\n", bio, pkt->sector, bio->bi_sector); @@ -565,7 +565,7 @@ static void pkt_end_io_packet_write(struct bio *bio) { struct packet_data *pkt = bio->bi_private; - struct pktcdvd_device *pd = pkt_find_dev(bio->bi_dev); + struct pktcdvd_device *pd = pkt_find_dev(bio->bi_bdev->bd_inode->i_rdev); VPRINTK("pkt_end_io_packet_write: rq=%p, rw=%ld, q=%p\n", pkt->rq, rq_data_dir(pkt->rq), pkt->rq->q); @@ -607,7 +607,7 @@ /* Initialize pkt->bio */ pkt->bio->bi_sector = start_s; pkt->bio->bi_next = NULL; - pkt->bio->bi_dev = rq->rq_dev; + pkt->bio->bi_bdev->bd_inode->i_rdev = rq->rq_dev; pkt->bio->bi_flags = 0; pkt->bio->bi_rw = (1 << BIO_RW); pkt->bio->bi_vcnt = pkt->frames; @@ -678,7 +678,7 @@ rd_bio = pkt_get_read_bio(pkt); BUG_ON(!rd_bio); rd_bio->bi_sector = start_s + f * (CD_FRAMESIZE >> 9); - rd_bio->bi_dev = rq->rq_dev; + rd_bio->bi_bdev->bd_inode->i_rdev = rq->rq_dev; rd_bio->bi_io_vec[0].bv_page = frame_bvl->bv_page; rd_bio->bi_io_vec[0].bv_len = frame_bvl->bv_len; rd_bio->bi_io_vec[0].bv_offset = frame_bvl->bv_offset; @@ -765,7 +765,7 @@ pkt->bio->bi_flags = 0; pkt->bio->bi_idx = 0; - BUG_ON(!kdev_same(pkt->bio->bi_dev, rq->rq_dev)); + BUG_ON(!kdev_same(pkt->bio->bi_bdev->bd_inode->i_rdev, rq->rq_dev)); BUG_ON(pkt->bio->bi_rw != (1 << BIO_RW)); BUG_ON(pkt->bio->bi_vcnt != pkt->frames); BUG_ON(pkt->bio->bi_size != pkt->frames * CD_FRAMESIZE); @@ -1939,12 +1939,12 @@ int src_idx, src_offs; - if (minor(bio->bi_dev) >= MAX_WRITERS) { - printk("pktcdvd: %s out of range\n", kdevname(bio->bi_dev)); + if (minor(bio->bi_bdev->bd_inode->i_rdev) >= MAX_WRITERS) { + printk("pktcdvd: %s out of range\n", kdevname(bio->bi_bdev->bd_inode->i_rdev)); goto end_io; } - pd = &pkt_devs[minor(bio->bi_dev)]; + pd = &pkt_devs[minor(bio->bi_bdev->bd_inode->i_rdev)]; if (kdev_none(pd->dev)) { printk("pktcdvd: request received for non-active pd\n"); goto end_io; @@ -1954,7 +1954,7 @@ * quick remap a READ */ if (!test_bit(BIO_RW, &bio->bi_rw)) { - bio->bi_dev = pd->dev; + bio->bi_bdev->bd_inode->i_rdev = pd->dev; return 1; } @@ -2012,7 +2012,7 @@ BUG_ON(!new_bio); new_bio->bi_sector = start_s; new_bio->bi_next = NULL; - new_bio->bi_dev = bio->bi_dev; + new_bio->bi_bdev = bio->bi_bdev; new_bio->bi_flags = 0; new_bio->bi_rw = (1 << BIO_RW); new_bio->bi_vcnt = 0;
On Thu, May 02 2002, Johnathan Hicks wrote:
--- pktcdvd.c.org Thu May 2 16:34:10 2002 +++ pktcdvd.c Thu May 2 15:07:12 2002 @@ -161,7 +161,7 @@ static int pkt_front_merge_fn(request_queue_t *q, struct request *rq, struct bio *bio) { - struct pktcdvd_device *pd = &pkt_devs[minor(bio->bi_dev)]; + struct pktcdvd_device *pd = &pkt_devs[minor(bio->bi_bdev->bd_inode->i_rdev)];
make that struct pktcdvd_device *pd = &pkt_devs[minor(to_kdev_t(bio->bi_bdev))]; Did you just go digging for where to find a kdev_t? :-) You must always use bi_bdev, since the bio could have been remapped xx times before we receive it. Did you just go digging for where to find a kdev_t? :-) You must always use bi_bdev, since the bio could have been remapped xx times before we receive it. -- Jens Axboe
On Fri, 3 May 2002, Jens Axboe wrote:
On Thu, May 02 2002, Johnathan Hicks wrote:
--- pktcdvd.c.org Thu May 2 16:34:10 2002 +++ pktcdvd.c Thu May 2 15:07:12 2002 @@ -161,7 +161,7 @@ static int pkt_front_merge_fn(request_queue_t *q, struct request *rq, struct bio *bio) { - struct pktcdvd_device *pd = &pkt_devs[minor(bio->bi_dev)]; + struct pktcdvd_device *pd = &pkt_devs[minor(bio->bi_bdev->bd_inode->i_rdev)];
make that
struct pktcdvd_device *pd = &pkt_devs[minor(to_kdev_t(bio->bi_bdev))];
Or, in this particular case, struct pktcdvd_device *pd = (struct pktcdvd_device *) q->queuedata; which is much simpler. (This is included in the 2.5.12 packet patch.) -- Peter Osterlund - petero2@telia.com http://w1.894.telia.com/~u89404340
Jens Axboe wrote:
On Thu, May 02 2002, Johnathan Hicks wrote:
--- pktcdvd.c.org Thu May 2 16:34:10 2002 +++ pktcdvd.c Thu May 2 15:07:12 2002 @@ -161,7 +161,7 @@ static int pkt_front_merge_fn(request_queue_t *q, struct request *rq, struct bio *bio) { - struct pktcdvd_device *pd = &pkt_devs[minor(bio->bi_dev)]; + struct pktcdvd_device *pd = &pkt_devs[minor(bio->bi_bdev->bd_inode->i_rdev)];
make that
struct pktcdvd_device *pd = &pkt_devs[minor(to_kdev_t(bio->bi_bdev))];
Did you just go digging for where to find a kdev_t? :-)
Sort of. I tried to do it as logically as possible but the lack of comments on the structures prevented this. I'm sure someone will document them eventually. :)
You must always use bi_bdev, since the bio could have been remapped xx times before we receive it.
Yeah, I did see a whole bunch of remappings in the code there. --John
On Thu, 2 May 2002, Johnathan Hicks wrote:
After many VPRINTK() insertions into pktcdvd.c I finally tracked the freeze point down to a blk_init_queue() call from pkt_init_queue(). From that it would appear the the problem is not with pktcdvd after all. Do you think I should hunt down blk_init_queue() and stuff it with printk() or would I end up with too much output for it to be useful?
I think you are using an SMP kernel. The packet driver grabs the io_request_lock before calling blk_init_queue, which then tries to grab the lock again, causing a deadlock. It works on UP kernels because spinlocks are NOPs there. It worked before 2.4.19-pre5 because then blk_init_queue didn't grab the io_request_lock. I don't know if it was ever legal to call blk_init_queue with the lock held, or if the packet driver just worked by accident before kernel 2.4.19-pre5. The patch below fixes the freeze, but I suspect there are SMP races in the open/setup/teardown paths in the packet driver. (Based on a quick comparision with the loop device driver.) --- pktcdvd.c.old Fri May 3 22:09:02 2002 +++ pktcdvd.c Fri May 3 22:03:06 2002 @@ -2303,13 +2303,11 @@ * scatter-gather tables) */ q = blk_get_queue(dev); - spin_lock_irq(&io_request_lock); pkt_init_queue(pd); pd->cdrw.front_merge_fn = q->front_merge_fn; pd->cdrw.back_merge_fn = q->back_merge_fn; pd->cdrw.merge_requests_fn = q->merge_requests_fn; pd->cdrw.queuedata = q->queuedata; - spin_unlock_irq(&io_request_lock); pd->cdrw.pid = kernel_thread(kcdrwd, pd, CLONE_FS | CLONE_FILES | CLONE_SIGHAND); if (pd->cdrw.pid < 0) { -- Peter Osterlund - petero2@telia.com http://w1.894.telia.com/~u89404340
Peter Osterlund wrote:
On Thu, 2 May 2002, Johnathan Hicks wrote:
After many VPRINTK() insertions into pktcdvd.c I finally tracked the freeze point down to a blk_init_queue() call from pkt_init_queue(). From that it would appear the the problem is not with pktcdvd after all. Do you think I should hunt down blk_init_queue() and stuff it with printk() or would I end up with too much output for it to be useful?
I think you are using an SMP kernel. The packet driver grabs the io_request_lock before calling blk_init_queue, which then tries to grab the lock again, causing a deadlock. It works on UP kernels because spinlocks are NOPs there. It worked before 2.4.19-pre5 because then blk_init_queue didn't grab the io_request_lock.
Correct, I am using an SMP kernel. I think that the 2.5 version has a similar problem as I'm seeing 2 overlapping appearently identical calls and then a freeze.
I don't know if it was ever legal to call blk_init_queue with the lock held, or if the packet driver just worked by accident before kernel 2.4.19-pre5.
The patch below fixes the freeze, but I suspect there are SMP races in the open/setup/teardown paths in the packet driver. (Based on a quick comparision with the loop device driver.)
<PATCH SNIPPED> With any luck I'll run into them too. :) --John
On Sun, 5 May 2002, Johnathan Hicks wrote:
Peter Osterlund wrote:
I think you are using an SMP kernel. The packet driver grabs the io_request_lock before calling blk_init_queue, which then tries to grab the lock again, causing a deadlock. It works on UP kernels because spinlocks are NOPs there. It worked before 2.4.19-pre5 because then blk_init_queue didn't grab the io_request_lock.
Correct, I am using an SMP kernel. I think that the 2.5 version has a similar problem as I'm seeing 2 overlapping appearently identical calls and then a freeze.
I don't see this when running an SMP kernel on a UP machine. Can you provide more details please? Turning on spinlock debugging and the NMI watchdog might help. I don't think the SMP races I mentioned previously will trigger unless you actively try to make them happen, for instance by running pktsetup and mount in parallel. -- Peter Osterlund - petero2@telia.com http://w1.894.telia.com/~u89404340
participants (3)
-
Jens Axboe -
Johnathan Hicks -
Peter Osterlund