Re: [PATCH] pktcdvd: don't rely on bio_init() preserving bio->bi_destructor
On Thu, Sep 20 2007, Laurent Riffard wrote: > Le 14.09.2007 21:04, Laurent Riffard a écrit : > > Le 14.09.2007 13:06, Jens Axboe a écrit : > >> On Fri, Sep 14 2007, Jens Axboe wrote: > >>> On Fri, Sep 14 2007, Laurent Riffard wrote: > Le 10.09.2007 22:19, Laurent Riffard a écrit : > > Jens, > > > > git-block.patch broke pktcdvd, I've got an Oops while syncing: > > > > [snip] > I dig through git-block.patch and the culprit seems to be commit > c94f1c4ac87862675c8d70941973bc3a69aff5d8 "bio: use memset() in > bio_init()". > > Maybe the real bug is a bad bio initialization in pktcdvd driver, > which is revealed by this commit ? > >>> At least pktcdvd doesn't expect bio->bi_io_vec[] to be cleared, that's > >>> why it's oopsing now. I'll revert this bit for now, thanks for the > >>> report. > >> Rethinking this, I think bio_init() is doing the right thing, only > >> pktcdvd seems to rely on it preserving some members. So I'd rather fixup > >> pktcdvd instead. > >> > >> Does this work for you? > > > > Well, it's better: I was able to mount the DVD-RW, sync, and write data, > > but kernel oopsed when I unmounted the drive: > > Jens, > > this patch, applied on top of your previous patch, solved it. > > > > pktcdvd: don't rely on bio_init() preserving bio->bi_destructor Ah great, thanks for following up on this! Applied. -- Jens Axboe - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] pktcdvd: don't rely on bio_init() preserving bio->bi_destructor
Le 14.09.2007 21:04, Laurent Riffard a écrit : > Le 14.09.2007 13:06, Jens Axboe a écrit : >> On Fri, Sep 14 2007, Jens Axboe wrote: >>> On Fri, Sep 14 2007, Laurent Riffard wrote: Le 10.09.2007 22:19, Laurent Riffard a écrit : > Jens, > > git-block.patch broke pktcdvd, I've got an Oops while syncing: > > [snip] I dig through git-block.patch and the culprit seems to be commit c94f1c4ac87862675c8d70941973bc3a69aff5d8 "bio: use memset() in bio_init()". Maybe the real bug is a bad bio initialization in pktcdvd driver, which is revealed by this commit ? >>> At least pktcdvd doesn't expect bio->bi_io_vec[] to be cleared, that's >>> why it's oopsing now. I'll revert this bit for now, thanks for the >>> report. >> Rethinking this, I think bio_init() is doing the right thing, only >> pktcdvd seems to rely on it preserving some members. So I'd rather fixup >> pktcdvd instead. >> >> Does this work for you? > > Well, it's better: I was able to mount the DVD-RW, sync, and write data, > but kernel oopsed when I unmounted the drive: Jens, this patch, applied on top of your previous patch, solved it. pktcdvd: don't rely on bio_init() preserving bio->bi_destructor Signed-off-by: Laurent Riffard <[EMAIL PROTECTED]> --- drivers/block/pktcdvd.c |2 ++ 1 file changed, 2 insertions(+) Index: linux-2.6-mm/drivers/block/pktcdvd.c === --- linux-2.6-mm.orig/drivers/block/pktcdvd.c +++ linux-2.6-mm/drivers/block/pktcdvd.c @@ -1156,6 +1156,7 @@ static void pkt_gather_data(struct pktcd bio->bi_end_io = pkt_end_io_read; bio->bi_private = pkt; bio->bi_io_vec = vec; + bio->bi_destructor = pkt_bio_destructor; p = (f * CD_FRAMESIZE) / PAGE_SIZE; offset = (f * CD_FRAMESIZE) % PAGE_SIZE; @@ -1453,6 +1454,7 @@ static void pkt_start_write(struct pktcd pkt->w_bio->bi_end_io = pkt_end_io_packet_write; pkt->w_bio->bi_private = pkt; pkt->w_bio->bi_io_vec = bvec; + pkt->w_bio->bi_destructor = pkt_bio_destructor; for (f = 0; f < pkt->frames; f++) if (!bio_add_page(pkt->w_bio, bvec[f].bv_page, CD_FRAMESIZE, bvec[f].bv_offset)) BUG(); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] pktcdvd: don't rely on bio_init() preserving bio-bi_destructor
Le 14.09.2007 21:04, Laurent Riffard a écrit : Le 14.09.2007 13:06, Jens Axboe a écrit : On Fri, Sep 14 2007, Jens Axboe wrote: On Fri, Sep 14 2007, Laurent Riffard wrote: Le 10.09.2007 22:19, Laurent Riffard a écrit : Jens, git-block.patch broke pktcdvd, I've got an Oops while syncing: [snip] I dig through git-block.patch and the culprit seems to be commit c94f1c4ac87862675c8d70941973bc3a69aff5d8 bio: use memset() in bio_init(). Maybe the real bug is a bad bio initialization in pktcdvd driver, which is revealed by this commit ? At least pktcdvd doesn't expect bio-bi_io_vec[] to be cleared, that's why it's oopsing now. I'll revert this bit for now, thanks for the report. Rethinking this, I think bio_init() is doing the right thing, only pktcdvd seems to rely on it preserving some members. So I'd rather fixup pktcdvd instead. Does this work for you? Well, it's better: I was able to mount the DVD-RW, sync, and write data, but kernel oopsed when I unmounted the drive: Jens, this patch, applied on top of your previous patch, solved it. pktcdvd: don't rely on bio_init() preserving bio-bi_destructor Signed-off-by: Laurent Riffard [EMAIL PROTECTED] --- drivers/block/pktcdvd.c |2 ++ 1 file changed, 2 insertions(+) Index: linux-2.6-mm/drivers/block/pktcdvd.c === --- linux-2.6-mm.orig/drivers/block/pktcdvd.c +++ linux-2.6-mm/drivers/block/pktcdvd.c @@ -1156,6 +1156,7 @@ static void pkt_gather_data(struct pktcd bio-bi_end_io = pkt_end_io_read; bio-bi_private = pkt; bio-bi_io_vec = vec; + bio-bi_destructor = pkt_bio_destructor; p = (f * CD_FRAMESIZE) / PAGE_SIZE; offset = (f * CD_FRAMESIZE) % PAGE_SIZE; @@ -1453,6 +1454,7 @@ static void pkt_start_write(struct pktcd pkt-w_bio-bi_end_io = pkt_end_io_packet_write; pkt-w_bio-bi_private = pkt; pkt-w_bio-bi_io_vec = bvec; + pkt-w_bio-bi_destructor = pkt_bio_destructor; for (f = 0; f pkt-frames; f++) if (!bio_add_page(pkt-w_bio, bvec[f].bv_page, CD_FRAMESIZE, bvec[f].bv_offset)) BUG(); - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] pktcdvd: don't rely on bio_init() preserving bio-bi_destructor
On Thu, Sep 20 2007, Laurent Riffard wrote: Le 14.09.2007 21:04, Laurent Riffard a écrit : Le 14.09.2007 13:06, Jens Axboe a écrit : On Fri, Sep 14 2007, Jens Axboe wrote: On Fri, Sep 14 2007, Laurent Riffard wrote: Le 10.09.2007 22:19, Laurent Riffard a écrit : Jens, git-block.patch broke pktcdvd, I've got an Oops while syncing: [snip] I dig through git-block.patch and the culprit seems to be commit c94f1c4ac87862675c8d70941973bc3a69aff5d8 bio: use memset() in bio_init(). Maybe the real bug is a bad bio initialization in pktcdvd driver, which is revealed by this commit ? At least pktcdvd doesn't expect bio-bi_io_vec[] to be cleared, that's why it's oopsing now. I'll revert this bit for now, thanks for the report. Rethinking this, I think bio_init() is doing the right thing, only pktcdvd seems to rely on it preserving some members. So I'd rather fixup pktcdvd instead. Does this work for you? Well, it's better: I was able to mount the DVD-RW, sync, and write data, but kernel oopsed when I unmounted the drive: Jens, this patch, applied on top of your previous patch, solved it. pktcdvd: don't rely on bio_init() preserving bio-bi_destructor Ah great, thanks for following up on this! Applied. -- Jens Axboe - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
