* [PATCH] sg: disable interrupts inside sg_copy_buffer @ 2008-09-11 14:52 FUJITA Tomonori 2008-09-11 16:33 ` Jens Axboe 0 siblings, 1 reply; 8+ messages in thread From: FUJITA Tomonori @ 2008-09-11 14:52 UTC (permalink / raw) To: linux-scsi; +Cc: James.Bottomley, jens.axboe The callers of sg_copy_buffer must disable interrupts before calling it (since it uses kmap_atomic). Some callers use it on interrupt-disabled code but some need to take the trouble to disable interrupts just for this. No wonder they forget about it and we hit a bug like: http://bugzilla.kernel.org/show_bug.cgi?id=11529 James said that it might be better to disable interrupts inside the function rather than risk the callers getting it wrong. Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> --- lib/scatterlist.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/lib/scatterlist.c b/lib/scatterlist.c index 876ba6d..dd52cd5 100644 --- a/lib/scatterlist.c +++ b/lib/scatterlist.c @@ -422,6 +422,9 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, { unsigned int offset = 0; struct sg_mapping_iter miter; + unsigned long flags; + + local_irq_save(flags); sg_miter_start(&miter, sgl, nents, SG_MITER_ATOMIC); @@ -442,6 +445,8 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, sg_miter_stop(&miter); + local_irq_restore(flags); + return offset; } -- 1.5.4.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] sg: disable interrupts inside sg_copy_buffer 2008-09-11 14:52 [PATCH] sg: disable interrupts inside sg_copy_buffer FUJITA Tomonori @ 2008-09-11 16:33 ` Jens Axboe 2008-09-12 2:04 ` FUJITA Tomonori 0 siblings, 1 reply; 8+ messages in thread From: Jens Axboe @ 2008-09-11 16:33 UTC (permalink / raw) To: FUJITA Tomonori; +Cc: linux-scsi, James.Bottomley On Thu, Sep 11 2008, FUJITA Tomonori wrote: > The callers of sg_copy_buffer must disable interrupts before calling > it (since it uses kmap_atomic). Some callers use it on > interrupt-disabled code but some need to take the trouble to disable > interrupts just for this. No wonder they forget about it and we hit a > bug like: > > http://bugzilla.kernel.org/show_bug.cgi?id=11529 > > James said that it might be better to disable interrupts inside the > function rather than risk the callers getting it wrong. > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> > --- > lib/scatterlist.c | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/lib/scatterlist.c b/lib/scatterlist.c > index 876ba6d..dd52cd5 100644 > --- a/lib/scatterlist.c > +++ b/lib/scatterlist.c > @@ -422,6 +422,9 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > { > unsigned int offset = 0; > struct sg_mapping_iter miter; > + unsigned long flags; > + > + local_irq_save(flags); > > sg_miter_start(&miter, sgl, nents, SG_MITER_ATOMIC); > > @@ -442,6 +445,8 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > > sg_miter_stop(&miter); > > + local_irq_restore(flags); > + > return offset; > } Agreed and applied. -- Jens Axboe ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sg: disable interrupts inside sg_copy_buffer 2008-09-11 16:33 ` Jens Axboe @ 2008-09-12 2:04 ` FUJITA Tomonori 2008-09-12 6:05 ` Jens Axboe 0 siblings, 1 reply; 8+ messages in thread From: FUJITA Tomonori @ 2008-09-12 2:04 UTC (permalink / raw) To: jens.axboe; +Cc: fujita.tomonori, linux-scsi, James.Bottomley On Thu, 11 Sep 2008 18:33:03 +0200 Jens Axboe <jens.axboe@oracle.com> wrote: > On Thu, Sep 11 2008, FUJITA Tomonori wrote: > > The callers of sg_copy_buffer must disable interrupts before calling > > it (since it uses kmap_atomic). Some callers use it on > > interrupt-disabled code but some need to take the trouble to disable > > interrupts just for this. No wonder they forget about it and we hit a > > bug like: > > > > http://bugzilla.kernel.org/show_bug.cgi?id=11529 > > > > James said that it might be better to disable interrupts inside the > > function rather than risk the callers getting it wrong. > > > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> > > --- > > lib/scatterlist.c | 5 +++++ > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > diff --git a/lib/scatterlist.c b/lib/scatterlist.c > > index 876ba6d..dd52cd5 100644 > > --- a/lib/scatterlist.c > > +++ b/lib/scatterlist.c > > @@ -422,6 +422,9 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > > { > > unsigned int offset = 0; > > struct sg_mapping_iter miter; > > + unsigned long flags; > > + > > + local_irq_save(flags); > > > > sg_miter_start(&miter, sgl, nents, SG_MITER_ATOMIC); > > > > @@ -442,6 +445,8 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > > > > sg_miter_stop(&miter); > > > > + local_irq_restore(flags); > > + > > return offset; > > } > > Agreed and applied. Jens, this won't go to 2.6.27 (goes to 2.6.28)? If so, I need to ask James to apply this workaround to 2.6.27: http://marc.info/?l=linux-scsi&m=122106973807028&w=2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sg: disable interrupts inside sg_copy_buffer 2008-09-12 2:04 ` FUJITA Tomonori @ 2008-09-12 6:05 ` Jens Axboe 2008-09-12 6:26 ` FUJITA Tomonori 0 siblings, 1 reply; 8+ messages in thread From: Jens Axboe @ 2008-09-12 6:05 UTC (permalink / raw) To: FUJITA Tomonori; +Cc: linux-scsi, James.Bottomley On Fri, Sep 12 2008, FUJITA Tomonori wrote: > On Thu, 11 Sep 2008 18:33:03 +0200 > Jens Axboe <jens.axboe@oracle.com> wrote: > > > On Thu, Sep 11 2008, FUJITA Tomonori wrote: > > > The callers of sg_copy_buffer must disable interrupts before calling > > > it (since it uses kmap_atomic). Some callers use it on > > > interrupt-disabled code but some need to take the trouble to disable > > > interrupts just for this. No wonder they forget about it and we hit a > > > bug like: > > > > > > http://bugzilla.kernel.org/show_bug.cgi?id=11529 > > > > > > James said that it might be better to disable interrupts inside the > > > function rather than risk the callers getting it wrong. > > > > > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> > > > --- > > > lib/scatterlist.c | 5 +++++ > > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > > > diff --git a/lib/scatterlist.c b/lib/scatterlist.c > > > index 876ba6d..dd52cd5 100644 > > > --- a/lib/scatterlist.c > > > +++ b/lib/scatterlist.c > > > @@ -422,6 +422,9 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > > > { > > > unsigned int offset = 0; > > > struct sg_mapping_iter miter; > > > + unsigned long flags; > > > + > > > + local_irq_save(flags); > > > > > > sg_miter_start(&miter, sgl, nents, SG_MITER_ATOMIC); > > > > > > @@ -442,6 +445,8 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > > > > > > sg_miter_stop(&miter); > > > > > > + local_irq_restore(flags); > > > + > > > return offset; > > > } > > > > Agreed and applied. > > Jens, this won't go to 2.6.27 (goes to 2.6.28)? > > If so, I need to ask James to apply this workaround to 2.6.27: > > http://marc.info/?l=linux-scsi&m=122106973807028&w=2 It will go into 2.6.27, it's already upstream since ~10 hours or so. -- Jens Axboe ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sg: disable interrupts inside sg_copy_buffer 2008-09-12 6:05 ` Jens Axboe @ 2008-09-12 6:26 ` FUJITA Tomonori 2008-09-12 13:48 ` James Bottomley 0 siblings, 1 reply; 8+ messages in thread From: FUJITA Tomonori @ 2008-09-12 6:26 UTC (permalink / raw) To: jens.axboe; +Cc: fujita.tomonori, linux-scsi, James.Bottomley On Fri, 12 Sep 2008 08:05:46 +0200 Jens Axboe <jens.axboe@oracle.com> wrote: > On Fri, Sep 12 2008, FUJITA Tomonori wrote: > > On Thu, 11 Sep 2008 18:33:03 +0200 > > Jens Axboe <jens.axboe@oracle.com> wrote: > > > > > On Thu, Sep 11 2008, FUJITA Tomonori wrote: > > > > The callers of sg_copy_buffer must disable interrupts before calling > > > > it (since it uses kmap_atomic). Some callers use it on > > > > interrupt-disabled code but some need to take the trouble to disable > > > > interrupts just for this. No wonder they forget about it and we hit a > > > > bug like: > > > > > > > > http://bugzilla.kernel.org/show_bug.cgi?id=11529 > > > > > > > > James said that it might be better to disable interrupts inside the > > > > function rather than risk the callers getting it wrong. > > > > > > > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> > > > > --- > > > > lib/scatterlist.c | 5 +++++ > > > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > > > > > diff --git a/lib/scatterlist.c b/lib/scatterlist.c > > > > index 876ba6d..dd52cd5 100644 > > > > --- a/lib/scatterlist.c > > > > +++ b/lib/scatterlist.c > > > > @@ -422,6 +422,9 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > > > > { > > > > unsigned int offset = 0; > > > > struct sg_mapping_iter miter; > > > > + unsigned long flags; > > > > + > > > > + local_irq_save(flags); > > > > > > > > sg_miter_start(&miter, sgl, nents, SG_MITER_ATOMIC); > > > > > > > > @@ -442,6 +445,8 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > > > > > > > > sg_miter_stop(&miter); > > > > > > > > + local_irq_restore(flags); > > > > + > > > > return offset; > > > > } > > > > > > Agreed and applied. > > > > Jens, this won't go to 2.6.27 (goes to 2.6.28)? > > > > If so, I need to ask James to apply this workaround to 2.6.27: > > > > http://marc.info/?l=linux-scsi&m=122106973807028&w=2 > > It will go into 2.6.27, it's already upstream since ~10 hours or so. Thanks, I've just found it. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sg: disable interrupts inside sg_copy_buffer 2008-09-12 6:26 ` FUJITA Tomonori @ 2008-09-12 13:48 ` James Bottomley 2008-09-12 14:12 ` FUJITA Tomonori 0 siblings, 1 reply; 8+ messages in thread From: James Bottomley @ 2008-09-12 13:48 UTC (permalink / raw) To: FUJITA Tomonori; +Cc: jens.axboe, linux-scsi On Fri, 2008-09-12 at 15:26 +0900, FUJITA Tomonori wrote: > On Fri, 12 Sep 2008 08:05:46 +0200 > Jens Axboe <jens.axboe@oracle.com> wrote: > > > On Fri, Sep 12 2008, FUJITA Tomonori wrote: > > > On Thu, 11 Sep 2008 18:33:03 +0200 > > > Jens Axboe <jens.axboe@oracle.com> wrote: > > > > > > > On Thu, Sep 11 2008, FUJITA Tomonori wrote: > > > > > The callers of sg_copy_buffer must disable interrupts before calling > > > > > it (since it uses kmap_atomic). Some callers use it on > > > > > interrupt-disabled code but some need to take the trouble to disable > > > > > interrupts just for this. No wonder they forget about it and we hit a > > > > > bug like: > > > > > > > > > > http://bugzilla.kernel.org/show_bug.cgi?id=11529 > > > > > > > > > > James said that it might be better to disable interrupts inside the > > > > > function rather than risk the callers getting it wrong. > > > > > > > > > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> > > > > > --- > > > > > lib/scatterlist.c | 5 +++++ > > > > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > > > > > > > diff --git a/lib/scatterlist.c b/lib/scatterlist.c > > > > > index 876ba6d..dd52cd5 100644 > > > > > --- a/lib/scatterlist.c > > > > > +++ b/lib/scatterlist.c > > > > > @@ -422,6 +422,9 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > > > > > { > > > > > unsigned int offset = 0; > > > > > struct sg_mapping_iter miter; > > > > > + unsigned long flags; > > > > > + > > > > > + local_irq_save(flags); > > > > > > > > > > sg_miter_start(&miter, sgl, nents, SG_MITER_ATOMIC); > > > > > > > > > > @@ -442,6 +445,8 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > > > > > > > > > > sg_miter_stop(&miter); > > > > > > > > > > + local_irq_restore(flags); > > > > > + > > > > > return offset; > > > > > } > > > > > > > > Agreed and applied. > > > > > > Jens, this won't go to 2.6.27 (goes to 2.6.28)? > > > > > > If so, I need to ask James to apply this workaround to 2.6.27: > > > > > > http://marc.info/?l=linux-scsi&m=122106973807028&w=2 > > > > It will go into 2.6.27, it's already upstream since ~10 hours or so. > > Thanks, I've just found it. You forgot to add the cc: stable@kernel.org tag. You'll have to send this to stable manually quoting the upsream commit id. James ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sg: disable interrupts inside sg_copy_buffer 2008-09-12 13:48 ` James Bottomley @ 2008-09-12 14:12 ` FUJITA Tomonori 2008-09-12 14:11 ` James Bottomley 0 siblings, 1 reply; 8+ messages in thread From: FUJITA Tomonori @ 2008-09-12 14:12 UTC (permalink / raw) To: James.Bottomley; +Cc: fujita.tomonori, jens.axboe, linux-scsi On Fri, 12 Sep 2008 08:48:39 -0500 James Bottomley <James.Bottomley@HansenPartnership.com> wrote: > On Fri, 2008-09-12 at 15:26 +0900, FUJITA Tomonori wrote: > > On Fri, 12 Sep 2008 08:05:46 +0200 > > Jens Axboe <jens.axboe@oracle.com> wrote: > > > > > On Fri, Sep 12 2008, FUJITA Tomonori wrote: > > > > On Thu, 11 Sep 2008 18:33:03 +0200 > > > > Jens Axboe <jens.axboe@oracle.com> wrote: > > > > > > > > > On Thu, Sep 11 2008, FUJITA Tomonori wrote: > > > > > > The callers of sg_copy_buffer must disable interrupts before calling > > > > > > it (since it uses kmap_atomic). Some callers use it on > > > > > > interrupt-disabled code but some need to take the trouble to disable > > > > > > interrupts just for this. No wonder they forget about it and we hit a > > > > > > bug like: > > > > > > > > > > > > http://bugzilla.kernel.org/show_bug.cgi?id=11529 > > > > > > > > > > > > James said that it might be better to disable interrupts inside the > > > > > > function rather than risk the callers getting it wrong. > > > > > > > > > > > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> > > > > > > --- > > > > > > lib/scatterlist.c | 5 +++++ > > > > > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > > > > > > > > > diff --git a/lib/scatterlist.c b/lib/scatterlist.c > > > > > > index 876ba6d..dd52cd5 100644 > > > > > > --- a/lib/scatterlist.c > > > > > > +++ b/lib/scatterlist.c > > > > > > @@ -422,6 +422,9 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > > > > > > { > > > > > > unsigned int offset = 0; > > > > > > struct sg_mapping_iter miter; > > > > > > + unsigned long flags; > > > > > > + > > > > > > + local_irq_save(flags); > > > > > > > > > > > > sg_miter_start(&miter, sgl, nents, SG_MITER_ATOMIC); > > > > > > > > > > > > @@ -442,6 +445,8 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > > > > > > > > > > > > sg_miter_stop(&miter); > > > > > > > > > > > > + local_irq_restore(flags); > > > > > > + > > > > > > return offset; > > > > > > } > > > > > > > > > > Agreed and applied. > > > > > > > > Jens, this won't go to 2.6.27 (goes to 2.6.28)? > > > > > > > > If so, I need to ask James to apply this workaround to 2.6.27: > > > > > > > > http://marc.info/?l=linux-scsi&m=122106973807028&w=2 > > > > > > It will go into 2.6.27, it's already upstream since ~10 hours or so. > > > > Thanks, I've just found it. > > You forgot to add the cc: stable@kernel.org tag. You'll have to send > this to stable manually quoting the upsream commit id. You forgot that Tejun rewrote the sg copy code after 2.6.26. This patch can't be applied to 2.6.26.X. I need to push a patch that is not in mainline to stable trees. Can I do that ? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sg: disable interrupts inside sg_copy_buffer 2008-09-12 14:12 ` FUJITA Tomonori @ 2008-09-12 14:11 ` James Bottomley 0 siblings, 0 replies; 8+ messages in thread From: James Bottomley @ 2008-09-12 14:11 UTC (permalink / raw) To: FUJITA Tomonori; +Cc: jens.axboe, linux-scsi On Fri, 2008-09-12 at 23:12 +0900, FUJITA Tomonori wrote: > On Fri, 12 Sep 2008 08:48:39 -0500 > James Bottomley <James.Bottomley@HansenPartnership.com> wrote: > > > On Fri, 2008-09-12 at 15:26 +0900, FUJITA Tomonori wrote: > > > On Fri, 12 Sep 2008 08:05:46 +0200 > > > Jens Axboe <jens.axboe@oracle.com> wrote: > > > > > > > On Fri, Sep 12 2008, FUJITA Tomonori wrote: > > > > > On Thu, 11 Sep 2008 18:33:03 +0200 > > > > > Jens Axboe <jens.axboe@oracle.com> wrote: > > > > > > > > > > > On Thu, Sep 11 2008, FUJITA Tomonori wrote: > > > > > > > The callers of sg_copy_buffer must disable interrupts before calling > > > > > > > it (since it uses kmap_atomic). Some callers use it on > > > > > > > interrupt-disabled code but some need to take the trouble to disable > > > > > > > interrupts just for this. No wonder they forget about it and we hit a > > > > > > > bug like: > > > > > > > > > > > > > > http://bugzilla.kernel.org/show_bug.cgi?id=11529 > > > > > > > > > > > > > > James said that it might be better to disable interrupts inside the > > > > > > > function rather than risk the callers getting it wrong. > > > > > > > > > > > > > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> > > > > > > > --- > > > > > > > lib/scatterlist.c | 5 +++++ > > > > > > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > > > > > > > > > > > diff --git a/lib/scatterlist.c b/lib/scatterlist.c > > > > > > > index 876ba6d..dd52cd5 100644 > > > > > > > --- a/lib/scatterlist.c > > > > > > > +++ b/lib/scatterlist.c > > > > > > > @@ -422,6 +422,9 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > > > > > > > { > > > > > > > unsigned int offset = 0; > > > > > > > struct sg_mapping_iter miter; > > > > > > > + unsigned long flags; > > > > > > > + > > > > > > > + local_irq_save(flags); > > > > > > > > > > > > > > sg_miter_start(&miter, sgl, nents, SG_MITER_ATOMIC); > > > > > > > > > > > > > > @@ -442,6 +445,8 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > > > > > > > > > > > > > > sg_miter_stop(&miter); > > > > > > > > > > > > > > + local_irq_restore(flags); > > > > > > > + > > > > > > > return offset; > > > > > > > } > > > > > > > > > > > > Agreed and applied. > > > > > > > > > > Jens, this won't go to 2.6.27 (goes to 2.6.28)? > > > > > > > > > > If so, I need to ask James to apply this workaround to 2.6.27: > > > > > > > > > > http://marc.info/?l=linux-scsi&m=122106973807028&w=2 > > > > > > > > It will go into 2.6.27, it's already upstream since ~10 hours or so. > > > > > > Thanks, I've just found it. > > > > You forgot to add the cc: stable@kernel.org tag. You'll have to send > > this to stable manually quoting the upsream commit id. > > You forgot that Tejun rewrote the sg copy code after 2.6.26. This > patch can't be applied to 2.6.26.X. > > I need to push a patch that is not in mainline to stable trees. Can I > do that ? Yes, if we're absolutely sure it's necessary. You'll have to quote the upstream commit ID, explain why the backport has to be different and add the patch. James ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-09-12 14:11 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-09-11 14:52 [PATCH] sg: disable interrupts inside sg_copy_buffer FUJITA Tomonori 2008-09-11 16:33 ` Jens Axboe 2008-09-12 2:04 ` FUJITA Tomonori 2008-09-12 6:05 ` Jens Axboe 2008-09-12 6:26 ` FUJITA Tomonori 2008-09-12 13:48 ` James Bottomley 2008-09-12 14:12 ` FUJITA Tomonori 2008-09-12 14:11 ` James Bottomley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox