Re: [bug] ata subsystem related crash with latest -git

2007-10-20 Thread Torsten Kaiser
[Just catching with reading lkml to this post] On 10/18/07, Jens Axboe <[EMAIL PROTECTED]> wrote: > > Theory - ata_sg_is_last() isn't returning true for the last entry. Can > you double check that it correcly marks the last entry in mv_fill_sg()? > Alternatively, just try this patch. I "hate" to

Re: [bug] ata subsystem related crash with latest -git

2007-10-19 Thread FUJITA Tomonori
On Thu, 18 Oct 2007 19:14:29 +0200 Jens Axboe <[EMAIL PROTECTED]> wrote: > On Thu, Oct 18 2007, Arjan van de Ven wrote: > > On Thu, 18 Oct 2007 09:55:04 -0700 (PDT) > > Linus Torvalds <[EMAIL PROTECTED]> wrote: > > > > > > > > > > > On Thu, 18 Oct 2007, Jens Axboe wrote: > > > > -

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jeff Garzik
Linus Torvalds wrote: Very few drivers should care so much about the *page* itself (or the offset). That's something that the generic allocation code etc cares about, but the driver is almost bound to care mostly about the actual DMA address, so adding that helper function that abstracts the sg

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Arjan van de Ven
On Thu, 18 Oct 2007 09:55:04 -0700 (PDT) Linus Torvalds <[EMAIL PROTECTED]> wrote: > > > On Thu, 18 Oct 2007, Jens Axboe wrote: > > - unsigned long addr = page_to_phys(s->page) + > > s->offset; > > + unsigned long addr = page_to_phys(sg_page(s)) + > > s->offset; > > Umm. M

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Arjan van de Ven wrote: > On Thu, 18 Oct 2007 09:55:04 -0700 (PDT) > Linus Torvalds <[EMAIL PROTECTED]> wrote: > > > > > > > On Thu, 18 Oct 2007, Jens Axboe wrote: > > > - unsigned long addr = page_to_phys(s->page) + > > > s->offset; > > > + unsigned long ad

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Jens Axboe wrote: > On Thu, Oct 18 2007, Linus Torvalds wrote: > > > > > > On Thu, 18 Oct 2007, Jens Axboe wrote: > > > - unsigned long addr = page_to_phys(s->page) + s->offset; > > > + unsigned long addr = page_to_phys(sg_page(s)) + s->offset; > > > > Umm.

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Linus Torvalds wrote: > > > On Thu, 18 Oct 2007, Jens Axboe wrote: > > - unsigned long addr = page_to_phys(s->page) + s->offset; > > + unsigned long addr = page_to_phys(sg_page(s)) + s->offset; > > Umm. May I suggest (I haven't read the whole thread yet

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Linus Torvalds
On Thu, 18 Oct 2007, Jens Axboe wrote: > - unsigned long addr = page_to_phys(s->page) + s->offset; > + unsigned long addr = page_to_phys(sg_page(s)) + s->offset; Umm. May I suggest (I haven't read the whole thread yet, maybe somebody else already did) that sta

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Olof Johansson wrote: > On Thu, Oct 18, 2007 at 04:38:36PM +0200, Jens Axboe wrote: > > On Thu, Oct 18 2007, Benny Halevy wrote: > > > On Oct. 18, 2007, 16:05 +0200, Jens Axboe <[EMAIL PROTECTED]> wrote: > > > > On Thu, Oct 18 2007, Jens Axboe wrote: > > > >> On Thu, Oct 18 200

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Olof Johansson
On Thu, Oct 18, 2007 at 04:38:36PM +0200, Jens Axboe wrote: > On Thu, Oct 18 2007, Benny Halevy wrote: > > On Oct. 18, 2007, 16:05 +0200, Jens Axboe <[EMAIL PROTECTED]> wrote: > > > On Thu, Oct 18 2007, Jens Axboe wrote: > > >> On Thu, Oct 18 2007, Benny Halevy wrote: > > return sg; > >

Re: [PATCH] Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Olof Johansson
On Thu, Oct 18, 2007 at 06:42:46AM -0400, Jeff Garzik wrote: > Jens Axboe wrote: >> On Thu, Oct 18 2007, Jeff Garzik wrote: >>> Jens Axboe wrote: That should work as well. WRT ata_sg_is_last(), if we go ahead with my recent sg chaining updates, we can keep the test as it would be a single

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Benny Halevy
On Oct. 18, 2007, 16:05 +0200, Jens Axboe <[EMAIL PROTECTED]> wrote: > On Thu, Oct 18 2007, Jens Axboe wrote: >> On Thu, Oct 18 2007, Benny Halevy wrote: return sg; } @@ -83,6 +96,9 @@ static inline struct scatterlist *sg_last(struct scatterlist *sgl, ret =

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Mark Lord
Mark Lord wrote: On Thu, Oct 18 2007, Mark Lord wrote: > Jens wrote: >> OK, I think that covers every arch out there. I haven't been able to >> compile any of them, but it's mostly search'n replace operations. I hope >> nothing is missing linux/scatterlist.h includes... > > Patch fails on dr

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Mark Lord wrote: > Mark Lord wrote: >>> On Thu, Oct 18 2007, Mark Lord wrote: > Jens wrote: > >> OK, I think that covers every arch out there. I haven't been able to > >> compile any of them, but it's mostly search'n replace operations. I > hope > >> nothi

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Mark Lord
On Thu, Oct 18 2007, Mark Lord wrote: > Jens wrote: >> OK, I think that covers every arch out there. I haven't been able to >> compile any of them, but it's mostly search'n replace operations. I hope >> nothing is missing linux/scatterlist.h includes... > > Patch fails on drivers/scsi/scsi_lib.

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Jens Axboe wrote: > On Thu, Oct 18 2007, Benny Halevy wrote: > > > return sg; > > > } > > > @@ -83,6 +96,9 @@ static inline struct scatterlist *sg_last(struct > > > scatterlist *sgl, > > > ret = sg; > > > > > > #endif > > > +#ifdef CONFIG_DEBUG_SG > > > + BUG_O

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Mark Lord wrote: > Jens Axboe wrote: >> On Thu, Oct 18 2007, Mark Lord wrote: >>> Jens Axboe wrote: On Thu, Oct 18 2007, Mark Lord wrote: > Jens Axboe wrote: >> On Thu, Oct 18 2007, Jeff Garzik wrote: >>> Mark Lord wrote: Okay, mine is dying with EIP a

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Mark Lord
Jens Axboe wrote: On Thu, Oct 18 2007, Mark Lord wrote: Jens wrote: OK, I think that covers every arch out there. I haven't been able to compile any of them, but it's mostly search'n replace operations. I hope nothing is missing linux/scatterlist.h includes... Patch fails on drivers/scsi/scsi_

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Mark Lord
Jens Axboe wrote: On Thu, Oct 18 2007, Mark Lord wrote: Jens Axboe wrote: On Thu, Oct 18 2007, Mark Lord wrote: Jens Axboe wrote: On Thu, Oct 18 2007, Jeff Garzik wrote: Mark Lord wrote: Okay, mine is dying with EIP at blk_rq_map_sg+0xcb/0x160. Screen photo is at http://rtr.ca/recent/2.6.23

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Benny Halevy wrote: > > return sg; > > } > > @@ -83,6 +96,9 @@ static inline struct scatterlist *sg_last(struct > > scatterlist *sgl, > > ret = sg; > > > > #endif > > +#ifdef CONFIG_DEBUG_SG > > + BUG_ON(sgl[0].sg_magic != SG_MAGIC); > > can it also do B

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Benny Halevy wrote: > On Oct. 18, 2007, 15:32 +0200, Jens Axboe <[EMAIL PROTECTED]> wrote: > > static inline struct scatterlist *sg_next(struct scatterlist *sg) > > { > > - sg++; > > - > > - if (unlikely(sg_is_chain(sg))) > > +#ifdef CONFIG_DEBUG_SG > > + BUG_ON(sg->sg_

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Benny Halevy
On Oct. 18, 2007, 15:32 +0200, Jens Axboe <[EMAIL PROTECTED]> wrote: > static inline struct scatterlist *sg_next(struct scatterlist *sg) > { > - sg++; > - > - if (unlikely(sg_is_chain(sg))) > +#ifdef CONFIG_DEBUG_SG > + BUG_ON(sg->sg_magic != SG_MAGIC); > +#endif > + if (sg_is_las

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Mark Lord
Jens wrote: OK, I think that covers every arch out there. I haven't been able to compile any of them, but it's mostly search'n replace operations. I hope nothing is missing linux/scatterlist.h includes... Patch fails on drivers/scsi/scsi_lib.c. I replaced that part of the patch with this updat

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Mark Lord wrote: > Jens Axboe wrote: >> On Thu, Oct 18 2007, Mark Lord wrote: >>> Jens Axboe wrote: On Thu, Oct 18 2007, Jeff Garzik wrote: > Mark Lord wrote: >> Okay, mine is dying with EIP at blk_rq_map_sg+0xcb/0x160. >> Screen photo is at http://rtr.ca/recen

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Mark Lord
Jens Axboe wrote: On Thu, Oct 18 2007, Mark Lord wrote: Jens Axboe wrote: On Thu, Oct 18 2007, Jeff Garzik wrote: Mark Lord wrote: Okay, mine is dying with EIP at blk_rq_map_sg+0xcb/0x160. Screen photo is at http://rtr.ca/recent/2.6.23-git12-crash.jpg, but the top was cut off (isn't there a n

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Mark Lord wrote: > Jens Axboe wrote: >> On Thu, Oct 18 2007, Jeff Garzik wrote: >>> Mark Lord wrote: Okay, mine is dying with EIP at blk_rq_map_sg+0xcb/0x160. Screen photo is at http://rtr.ca/recent/2.6.23-git12-crash.jpg, but the top was cut off (isn't there a n

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Mark Lord
Jens Axboe wrote: On Thu, Oct 18 2007, Jeff Garzik wrote: Mark Lord wrote: Okay, mine is dying with EIP at blk_rq_map_sg+0xcb/0x160. Screen photo is at http://rtr.ca/recent/2.6.23-git12-crash.jpg, but the top was cut off (isn't there a new config option or patch to do double-columns or scrollba

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Benny Halevy
On Oct. 18, 2007, 14:15 +0200, Jens Axboe <[EMAIL PROTECTED]> wrote: > /** > * sg_next - return the next scatterlist entry in a list > @@ -43,10 +51,15 @@ static inline void sg_init_one(struct scatterlist *sg, > const void *buf, > */ > static inline struct scatterlist *sg_next(struct scatte

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, David Miller wrote: > From: Jens Axboe <[EMAIL PROTECTED]> > Date: Thu, 18 Oct 2007 14:15:47 +0200 > > > On Thu, Oct 18 2007, Jens Axboe wrote: > > > On Thu, Oct 18 2007, David Miller wrote: > > > > From: Jens Axboe <[EMAIL PROTECTED]> > > > > Date: Thu, 18 Oct 2007 13:57:02 +

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread David Miller
From: Jens Axboe <[EMAIL PROTECTED]> Date: Thu, 18 Oct 2007 14:15:47 +0200 > On Thu, Oct 18 2007, Jens Axboe wrote: > > On Thu, Oct 18 2007, David Miller wrote: > > > From: Jens Axboe <[EMAIL PROTECTED]> > > > Date: Thu, 18 Oct 2007 13:57:02 +0200 > > > > > > > Thanks a lot, Dave! The patch is a

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, David Miller wrote: > From: Jens Axboe <[EMAIL PROTECTED]> > Date: Thu, 18 Oct 2007 13:57:02 +0200 > > > Thanks a lot, Dave! The patch is a monster right now, I'll work on > > splitting it into a 3-step process. Any arch help is greatly > > appreciated. > > I have some other

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread David Miller
From: Jens Axboe <[EMAIL PROTECTED]> Date: Thu, 18 Oct 2007 13:57:02 +0200 > Thanks a lot, Dave! The patch is a monster right now, I'll work on > splitting it into a 3-step process. Any arch help is greatly > appreciated. I have some other bits that my compile hit, such as some things in the cryp

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, David Miller wrote: > From: Jens Axboe <[EMAIL PROTECTED]> > Date: Thu, 18 Oct 2007 10:21:45 +0200 > > > I like it. Basically the only real change is using bit 2 as a > > termination point, so we avoid going beyond the end of the sgtable. > > Here's a starting point, it actual

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread David Miller
From: Jens Axboe <[EMAIL PROTECTED]> Date: Thu, 18 Oct 2007 10:21:45 +0200 > I like it. Basically the only real change is using bit 2 as a > termination point, so we avoid going beyond the end of the sgtable. > Here's a starting point, it actually booted for me in the first go > (boggle). Only x86

Re: [PATCH] Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Ingo Molnar
* Jeff Garzik <[EMAIL PROTECTED]> wrote: > I never had to apply the changes you included, to fix problems here. perhaps because you are not running a CONFIG_DEBUG_PAGEALLOC=y kernel? I recently fixed DEBUG_PAGEALLOC (it would crash upon bootup on x86 most of the time on any real hardware - so

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Ingo Molnar wrote: > > * Ingo Molnar <[EMAIL PROTECTED]> wrote: > > > > -#define SCSI_MAX_SG_SEGMENTS 128 > > > +#define SCSI_MAX_SG_SEGMENTS 129 > > > > this one finally made the trick and it's booting fine now, without any > > crashes! > > hm, spoke too soon - i

Re: [PATCH] Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jeff Garzik
Ingo Molnar wrote: * Jeff Garzik <[EMAIL PROTECTED]> wrote: Tomo and I agreed to kill sg_last() a few days ago anyways, so this is perfectly fine with me. Yep, the [attached] patch that kills ata_sg_is_last() is working here on both machines that were previously croaking. It would be nice to

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Ingo Molnar
* Ingo Molnar <[EMAIL PROTECTED]> wrote: > > -#define SCSI_MAX_SG_SEGMENTS 128 > > +#define SCSI_MAX_SG_SEGMENTS 129 > > this one finally made the trick and it's booting fine now, without any > crashes! hm, spoke too soon - i just got the ata_qc_issue() crash below. I've attached

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Ingo Molnar wrote: > > * Jens Axboe <[EMAIL PROTECTED]> wrote: > > > > It would always be nice. For this case I don't think it's very > > > interesting, if we pursue the improved sg iteration setup. > > > > BTW, I think it's pretty clear that ata_sg_is_last() is broken. It'

Re: [PATCH] Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Ingo Molnar
* Jeff Garzik <[EMAIL PROTECTED]> wrote: > >Tomo and I agreed to kill sg_last() a few days ago anyways, so this > >is perfectly fine with me. > > Yep, the [attached] patch that kills ata_sg_is_last() is working here > on both machines that were previously croaking. > > It would be nice to get

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jeff Garzik
Ingo Molnar wrote: * Jens Axboe <[EMAIL PROTECTED]> wrote: It would always be nice. For this case I don't think it's very interesting, if we pursue the improved sg iteration setup. BTW, I think it's pretty clear that ata_sg_is_last() is broken. It's likely a one-off in the n_iter test. fixin

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Ingo Molnar
* Jens Axboe <[EMAIL PROTECTED]> wrote: > > It would always be nice. For this case I don't think it's very > > interesting, if we pursue the improved sg iteration setup. > > BTW, I think it's pretty clear that ata_sg_is_last() is broken. It's > likely a one-off in the n_iter test. fixing that

[PATCH] Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jeff Garzik
Jens Axboe wrote: On Thu, Oct 18 2007, Jeff Garzik wrote: Jens Axboe wrote: That should work as well. WRT ata_sg_is_last(), if we go ahead with my recent sg chaining updates, we can keep the test as it would be a single conditional and not require any looping. Let me know when you have tested t

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Jens Axboe wrote: > On Thu, Oct 18 2007, Ingo Molnar wrote: > > > > * Jens Axboe <[EMAIL PROTECTED]> wrote: > > > > > > Of course, this doesn't explain why ata_sg_is_last() was broken, but > > > > since it's working _and_ slightly more efficient, I don't really > > > > care

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Ingo Molnar wrote: > > * Jens Axboe <[EMAIL PROTECTED]> wrote: > > > > Of course, this doesn't explain why ata_sg_is_last() was broken, but > > > since it's working _and_ slightly more efficient, I don't really > > > care :) > > > > Tomo and I agreed to kill sg_last() a fe

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Ingo Molnar
* Jens Axboe <[EMAIL PROTECTED]> wrote: > > Of course, this doesn't explain why ata_sg_is_last() was broken, but > > since it's working _and_ slightly more efficient, I don't really > > care :) > > Tomo and I agreed to kill sg_last() a few days ago anyways, so this is > perfectly fine with me

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Jeff Garzik wrote: > Jens Axboe wrote: >> That should work as well. WRT ata_sg_is_last(), if we go ahead with my >> recent sg chaining updates, we can keep the test as it would be a single >> conditional and not require any looping. >> Let me know when you have tested this! > >

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jeff Garzik
Jens Axboe wrote: That should work as well. WRT ata_sg_is_last(), if we go ahead with my recent sg chaining updates, we can keep the test as it would be a single conditional and not require any looping. Let me know when you have tested this! The patch I attached to the last email got both sata

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Jeff Garzik wrote: > Jens Axboe wrote: >> The sata_mv construct looks a bit odd. Does this work? That last > > The sata_mv construct worked just fine before sg chaining :) Yes I know, but I'm trying to works towards getting rid of sg_last() and ata_sg_is_last() anyway :-) >>

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jeff Garzik
Jens Axboe wrote: The sata_mv construct looks a bit odd. Does this work? That last The sata_mv construct worked just fine before sg chaining :) end_mv_sg test should always be true, just being paranoid... diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c index 4df8311..5397eea 1006

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Jeff Garzik wrote: > Jens Axboe wrote: >> index 4df8311..b858183 100644 >> --- a/drivers/ata/sata_mv.c >> +++ b/drivers/ata/sata_mv.c >> @@ -1139,6 +1139,7 @@ static void mv_fill_sg(struct ata_queued_cmd *qc) >> struct mv_port_priv *pp = qc->ap->private_data; >> struc

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jeff Garzik
Benny Halevy wrote: On 10/18/07, *Jeff Garzik* <[EMAIL PROTECTED] > wrote: Jens Axboe wrote: > index 4df8311..b858183 100644 > --- a/drivers/ata/sata_mv.c > +++ b/drivers/ata/sata_mv.c > @@ -1139,6 +1139,7 @@ static void mv_fill_sg(struct a

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jeff Garzik
Jens Axboe wrote: index 4df8311..b858183 100644 --- a/drivers/ata/sata_mv.c +++ b/drivers/ata/sata_mv.c @@ -1139,6 +1139,7 @@ static void mv_fill_sg(struct ata_queued_cmd *qc) struct mv_port_priv *pp = qc->ap->private_data; struct scatterlist *sg; struct mv_sg *mv_sg; +

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jeff Garzik
Jeff Garzik wrote: I noticed that sg_tablesize in sata_mv is not LIBATA_MAX_PRD. This is expected behavior, but I wonder if that difference -- most notably being smaller than SCSI_MAX_SG_SEGMENTS -- would trigger any latent bugs. Another sata_mv difference from the rest: the chip does not su

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jeff Garzik
Jens Axboe wrote: On Thu, Oct 18 2007, Jeff Garzik wrote: Ingo Molnar wrote: * Jens Axboe <[EMAIL PROTECTED]> wrote: --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -39,7 +39,7 @@ * (unless chaining is used). Should ideally fit inside a single page, to * avoid a higher ord

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Jeff Garzik wrote: > Ingo Molnar wrote: >> * Jens Axboe <[EMAIL PROTECTED]> wrote: >>> --- a/drivers/scsi/scsi_lib.c >>> +++ b/drivers/scsi/scsi_lib.c >>> @@ -39,7 +39,7 @@ >>> * (unless chaining is used). Should ideally fit inside a single page, >>> to >>> * avoid a highe

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jeff Garzik
Ingo Molnar wrote: * Jens Axboe <[EMAIL PROTECTED]> wrote: --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -39,7 +39,7 @@ * (unless chaining is used). Should ideally fit inside a single page, to * avoid a higher order allocation. */ -#define SCSI_MAX_SG_SEGMENTS 128 +#def

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Wed, Oct 17 2007, David Miller wrote: > From: Linus Torvalds <[EMAIL PROTECTED]> > Date: Wed, 17 Oct 2007 18:07:19 -0700 (PDT) > > > sg_next() - as it stands now - never actually looks at the SG that its > > argument points to: it explicitly *only* looks at the next one. > > > > That's the bu

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jeff Garzik
Jens Axboe wrote: On Thu, Oct 18 2007, Jeff Garzik wrote: Mark Lord wrote: Mark Lord wrote: Linus Torvalds wrote: On Wed, 17 Oct 2007, Mark Lord wrote: It would be good to have something soon-ish. This "dead at boot time" issue is impacting the general ability to test patches against latest

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Ingo Molnar wrote: > > * Jens Axboe <[EMAIL PROTECTED]> wrote: > > > --- a/drivers/scsi/scsi_lib.c > > +++ b/drivers/scsi/scsi_lib.c > > @@ -39,7 +39,7 @@ > > * (unless chaining is used). Should ideally fit inside a single page, to > > * avoid a higher order allocation. >

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Jeff Garzik wrote: > Mark Lord wrote: >> Mark Lord wrote: >>> Linus Torvalds wrote: On Wed, 17 Oct 2007, Mark Lord wrote: > It would be good to have something soon-ish. > This "dead at boot time" issue is impacting the general ability to test > patches aga

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Ingo Molnar
* Jens Axboe <[EMAIL PROTECTED]> wrote: > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -39,7 +39,7 @@ > * (unless chaining is used). Should ideally fit inside a single page, to > * avoid a higher order allocation. > */ > -#define SCSI_MAX_SG_SEGMENTS 128 > +#define SCS

Re: [bug] ata subsystem related crash with latest -git

2007-10-18 Thread Jens Axboe
On Thu, Oct 18 2007, Jeff Garzik wrote: > Mark Lord wrote: >> Okay, mine is dying with EIP at blk_rq_map_sg+0xcb/0x160. >> Screen photo is at http://rtr.ca/recent/2.6.23-git12-crash.jpg, >> but the top was cut off (isn't there a new config option or patch >> to do double-columns or scrollback or so

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Jeff Garzik
Mark Lord wrote: Mark Lord wrote: Linus Torvalds wrote: On Wed, 17 Oct 2007, Mark Lord wrote: It would be good to have something soon-ish. This "dead at boot time" issue is impacting the general ability to test patches against latest -git in time for the current merge window. In the meantim

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Mark Lord
Mark Lord wrote: Linus Torvalds wrote: On Wed, 17 Oct 2007, Mark Lord wrote: It would be good to have something soon-ish. This "dead at boot time" issue is impacting the general ability to test patches against latest -git in time for the current merge window. In the meantime, does the patch

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Mark Lord
Linus Torvalds wrote: On Wed, 17 Oct 2007, Mark Lord wrote: It would be good to have something soon-ish. This "dead at boot time" issue is impacting the general ability to test patches against latest -git in time for the current merge window. In the meantime, does the patch I sent out help pe

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Mark Lord
Mark Lord wrote: Mark Lord wrote: Mark Lord wrote: Linus Torvalds wrote: On Wed, 17 Oct 2007, Mark Lord wrote: It would be good to have something soon-ish. This "dead at boot time" issue is impacting the general ability to test patches against latest -git in time for the current merge wind

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Mark Lord
Mark Lord wrote: Mark Lord wrote: Linus Torvalds wrote: On Wed, 17 Oct 2007, Mark Lord wrote: It would be good to have something soon-ish. This "dead at boot time" issue is impacting the general ability to test patches against latest -git in time for the current merge window. In the meantim

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Linus Torvalds
On Thu, 18 Oct 2007, Jeff Garzik wrote: > > Is this a sata_mv box? If so, could you try this patch? That could explain it: if the SG allocation is simply too small, the scatter-gather code will run off the end of the SG list, and encounter random uninitialized entries, and if any of those hav

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Linus Torvalds
On Thu, 18 Oct 2007, Mark Lord wrote: > > Okay, mine is dying with EIP at blk_rq_map_sg+0xcb/0x160. Ok, I think your picture cut off the last hex digits on the right, but what I can make out of the disassembly, I have to admit that it looks very much like it might be exactly the same thing In

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Mark Lord
Jeff Garzik wrote: Mark Lord wrote: Okay, mine is dying with EIP at blk_rq_map_sg+0xcb/0x160. Screen photo is at http://rtr.ca/recent/2.6.23-git12-crash.jpg, but the top was cut off (isn't there a new config option or patch to do double-columns or scrollback or something ???. Is this a sata_m

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Jeff Garzik
Mark Lord wrote: Okay, mine is dying with EIP at blk_rq_map_sg+0xcb/0x160. Screen photo is at http://rtr.ca/recent/2.6.23-git12-crash.jpg, but the top was cut off (isn't there a new config option or patch to do double-columns or scrollback or something ???. Is this a sata_mv box? If so, could

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Linus Torvalds
On Thu, 18 Oct 2007, Mark Lord wrote: > > Oh.. so this bug is supposed to only bite with DEBUG_PAGEALLOC=y ?? Yeah, this particular one really should only bite you with DEBUG_PAGEALLOC. The SG code potentially _derefences_ a field past the end of the SG array, but it should be a read, and th

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Mark Lord
Mark Lord wrote: Linus Torvalds wrote: On Wed, 17 Oct 2007, Mark Lord wrote: It would be good to have something soon-ish. This "dead at boot time" issue is impacting the general ability to test patches against latest -git in time for the current merge window. In the meantime, does the patch

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Jeff Garzik
Mark Lord wrote: Linus Torvalds wrote: On Wed, 17 Oct 2007, Mark Lord wrote: It would be good to have something soon-ish. This "dead at boot time" issue is impacting the general ability to test patches against latest -git in time for the current merge window. In the meantime, does the patch

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Mark Lord
Linus Torvalds wrote: On Wed, 17 Oct 2007, Mark Lord wrote: It would be good to have something soon-ish. This "dead at boot time" issue is impacting the general ability to test patches against latest -git in time for the current merge window. In the meantime, does the patch I sent out help pe

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Linus Torvalds
On Wed, 17 Oct 2007, Mark Lord wrote: > > It would be good to have something soon-ish. > This "dead at boot time" issue is impacting the general ability to test > patches against latest -git in time for the current merge window. In the meantime, does the patch I sent out help people? I'd like t

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Mark Lord
Linus Torvalds wrote: On Wed, 17 Oct 2007, David Miller wrote: I believe that we have enough of a limited set of accessors to sg->page that we can more aggressively encode things in the lower bits. I'm thinking of encoding the low two bits of sg->page as follows: ... Yes, that sounds sane.

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread David Miller
From: Linus Torvalds <[EMAIL PROTECTED]> Date: Wed, 17 Oct 2007 18:36:34 -0700 (PDT) > Although I also wonder whether we want one global per-arch > ARCH_HAS_SG_CHAIN It's there because the DMA mapping support code for a platform has to be converted to handle these chains and audited to make sure

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Linus Torvalds
On Wed, 17 Oct 2007, David Miller wrote: > > I believe that we have enough of a limited set of accessors to > sg->page that we can more aggressively encode things in the lower > bits. > > I'm thinking of encoding the low two bits of sg->page as > follows: > > 1) bits == 0 > >then the SG li

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread David Miller
From: Linus Torvalds <[EMAIL PROTECTED]> Date: Wed, 17 Oct 2007 18:07:19 -0700 (PDT) > sg_next() - as it stands now - never actually looks at the SG that its > argument points to: it explicitly *only* looks at the next one. > > That's the bug. If sg_next() looked at the actual *current* sg entry

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Jeff Garzik
On a related hardware note: FWIW, most ATA controllers have scatter/gather tables that terminate themselves by a bit in the final s/g entry. The 90% case needs to know the last scatterlist entry, at the end of the s/g walk. So however this all gets worked out, please make sure not to unduly

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Linus Torvalds
On Thu, 18 Oct 2007, FUJITA Tomonori wrote: > > Looks that (sglist) - 1 isn't initialized and we use sg_next for it? sg_next() - as it stands now - never actually looks at the SG that its argument points to: it explicitly *only* looks at the next one. That's the bug. If sg_next() looked at th

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread FUJITA Tomonori
On Wed, 17 Oct 2007 14:11:34 -0700 (PDT) Linus Torvalds <[EMAIL PROTECTED]> wrote: > > > On Wed, 17 Oct 2007, Jens Axboe wrote: > > > > That would hurt... Care to commit your for_each_sg() uglification fixup > > for now then? Or disable the allocation debug config entry, so that the > > sg+1 der

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Linus Torvalds
On Wed, 17 Oct 2007, Jens Axboe wrote: > > That would hurt... Care to commit your for_each_sg() uglification fixup > for now then? Or disable the allocation debug config entry, so that the > sg+1 deref wont crash? Well, in practice, it will only crash with DEBUG_PAGEALLOC, so few enough are goi

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Ingo Molnar
* Jens Axboe <[EMAIL PROTECTED]> wrote: > --- a/block/ll_rw_blk.c > +++ b/block/ll_rw_blk.c > @@ -631,7 +631,7 @@ void blk_queue_max_phys_segments(struct request_queue *q, > printk("%s: set to minimum %d\n", __FUNCTION__, max_segments); > } > > - q->max_phys_segments = m

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Jens Axboe
On Wed, Oct 17 2007, Linus Torvalds wrote: > > > On Wed, 17 Oct 2007, Jens Axboe wrote: > > > > So until that is fixed up, how about this? Should cover all block > > devices, and I don't think sg_next()/for_each_sg() has spread outside of > > that yet. > > Heh. Not good. There are drivers that

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Linus Torvalds
On Wed, 17 Oct 2007, Jens Axboe wrote: > > So until that is fixed up, how about this? Should cover all block > devices, and I don't think sg_next()/for_each_sg() has spread outside of > that yet. Heh. Not good. There are drivers that set max phys segmsnts to 1. Either through some host-specifi

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Linus Torvalds
On Wed, 17 Oct 2007, Jens Axboe wrote: > > Well, hang on - where does it end up doing sg_next() on the LAST sg > entry? They pretty much *all* do, as far as I can tell. For example, to look at the very first one: #define for_each_sg(sglist, sg, nr, __i)\ for (_

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Jens Axboe
On Wed, Oct 17 2007, Jens Axboe wrote: > On Wed, Oct 17 2007, Linus Torvalds wrote: > > > > > > On Wed, 17 Oct 2007, Jens Axboe wrote: > > > > > > OK, I think you have a very good point here. Ingo, can you verify it > > > goes away if apply the below? I have to tend to Other Life stuff but > > >

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Ingo Molnar
* Jens Axboe <[EMAIL PROTECTED]> wrote: > Ingo, if you care, can you test this one as well? hm, it still crashes - find the log below. Ingo [ 34.653682] EXT3-fs: mounted filesystem with ordered data mode. [ 34.659487] VFS: Mounted root (ext3 filesystem) readonly. [ 34.665208] Fre

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Jens Axboe
On Wed, Oct 17 2007, Linus Torvalds wrote: > > > On Wed, 17 Oct 2007, Linus Torvalds wrote: > > > > I think you'll always hit it if you have a scatter-gather list that is > > exactly filled up. > > In fact, I think you'll hit it even if you use the "for_each_sg()" > helper function. Exactly b

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Jens Axboe
On Wed, Oct 17 2007, Linus Torvalds wrote: > > > On Wed, 17 Oct 2007, Jens Axboe wrote: > > > > OK, I think you have a very good point here. Ingo, can you verify it > > goes away if apply the below? I have to tend to Other Life stuff but > > will take this up again tomorrow morning and fix the s

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Jens Axboe
On Wed, Oct 17 2007, Jens Axboe wrote: > On Wed, Oct 17 2007, Linus Torvalds wrote: > > > > > > On Wed, 17 Oct 2007, Ingo Molnar wrote: > > > > > > nope, this did not help. First bootup went fine, second bootup crashed > > > again (see below), without hitting the BUG_ON(). > > > > I think you'

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Linus Torvalds
On Wed, 17 Oct 2007, Jens Axboe wrote: > > OK, I think you have a very good point here. Ingo, can you verify it > goes away if apply the below? I have to tend to Other Life stuff but > will take this up again tomorrow morning and fix the sg_next() usage. This one will only fix the ones that use

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Linus Torvalds
On Wed, 17 Oct 2007, Linus Torvalds wrote: > > I think you'll always hit it if you have a scatter-gather list that is > exactly filled up. In fact, I think you'll hit it even if you use the "for_each_sg()" helper function. Exactly because it does the sg = sg_next(sg) at the end of the for-lo

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Jens Axboe
On Wed, Oct 17 2007, Linus Torvalds wrote: > > > On Wed, 17 Oct 2007, Ingo Molnar wrote: > > > > nope, this did not help. First bootup went fine, second bootup crashed > > again (see below), without hitting the BUG_ON(). > > I think you'll always hit it if you have a scatter-gather list that i

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Linus Torvalds
On Wed, 17 Oct 2007, Ingo Molnar wrote: > > nope, this did not help. First bootup went fine, second bootup crashed > again (see below), without hitting the BUG_ON(). I think you'll always hit it if you have a scatter-gather list that is exactly filled up. Why? Because those things do "sg_nex

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Jens Axboe
On Wed, Oct 17 2007, Ingo Molnar wrote: > > * Jens Axboe <[EMAIL PROTECTED]> wrote: > > > On Wed, Oct 17 2007, Ingo Molnar wrote: > > > > > > btw., i get the following build warning: > > > > > > drivers/scsi/scsi_lib.c: In function 'scsi_free_sgtable': > > > drivers/scsi/scsi_lib.c:708: warning

Re: [bug] ata subsystem related crash with latest -git

2007-10-17 Thread Jens Axboe
On Wed, Oct 17 2007, Ingo Molnar wrote: > > * Ingo Molnar <[EMAIL PROTECTED]> wrote: > > > > > drivers/scsi/scsi_lib.c:708: warning: 'index' may be used uninitialized > > > > in this function > > > > drivers/scsi/scsi_lib.c:708: note: 'index' was declared here > > > > > > > > not sure it matter

  1   2   >