| Message ID | 20100321215308.GB27709@Chamillionaire.breakpoint.cc |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On Sun, 21 Mar 2010 22:53:08 +0100 Sebastian Andrzej Siewior <sebastian@breakpoint.cc> wrote: > If the request has been made in user context it could be moved to a > different CPU on a SMP machine between the copy and cache flush. Thus we > could flush the dcache on the wrong CPU. This issue should be addressed in flush_dcache_page() inside? > Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc> > --- > drivers/ata/libata-sff.c | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c > index 83ecf48..dca9f90 100644 > --- a/drivers/ata/libata-sff.c > +++ b/drivers/ata/libata-sff.c > @@ -874,6 +874,9 @@ static void ata_pio_sector(struct ata_queued_cmd *qc) > > DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read"); > > +#ifdef CONFIG_SMP > + preempt_disable(); > +#endif > if (PageHighMem(page)) { > unsigned long flags; > > @@ -896,6 +899,9 @@ static void ata_pio_sector(struct ata_queued_cmd *qc) > if (!do_write && !PageSlab(page)) > flush_dcache_page(page); > > +#ifdef CONFIG_SMP > + preempt_enable(); > +#endif > qc->curbytes += qc->sect_size; > qc->cursg_ofs += qc->sect_size; > > -- > 1.6.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ide" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* FUJITA Tomonori | 2010-03-23 18:49:04 [+0900]: >On Sun, 21 Mar 2010 22:53:08 +0100 >Sebastian Andrzej Siewior <sebastian@breakpoint.cc> wrote: > >> If the request has been made in user context it could be moved to a >> different CPU on a SMP machine between the copy and cache flush. Thus we >> could flush the dcache on the wrong CPU. > >This issue should be addressed in flush_dcache_page() inside? How so? You have to remain on the same CPU on which you started the copy process. Once you get to flush_dcache_page() you may have allready switched the CPU. Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 23 Mar 2010 12:01:24 +0100 Sebastian Andrzej Siewior <sebastian@breakpoint.cc> wrote: > * FUJITA Tomonori | 2010-03-23 18:49:04 [+0900]: > > >On Sun, 21 Mar 2010 22:53:08 +0100 > >Sebastian Andrzej Siewior <sebastian@breakpoint.cc> wrote: > > > >> If the request has been made in user context it could be moved to a > >> different CPU on a SMP machine between the copy and cache flush. Thus we > >> could flush the dcache on the wrong CPU. > > > >This issue should be addressed in flush_dcache_page() inside? > > How so? You have to remain on the same CPU on which you started the copy > process. Once you get to flush_dcache_page() you may have allready > switched the CPU. Oops, I thought that you trying to address on the deferred flush_dcache_page() issue. I don't know enough about how this pio code works, but if your description is correct, then the patch is fine, I guess. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* FUJITA Tomonori | 2010-03-23 20:51:34 [+0900]: >> How so? You have to remain on the same CPU on which you started the copy >> process. Once you get to flush_dcache_page() you may have allready >> switched the CPU. > >Oops, I thought that you trying to address on the deferred >flush_dcache_page() issue. I don't know enough about how this pio code >works, but if your description is correct, then the patch is fine, I >guess. No, it is not. The dcache flush is a global one so a CPU switch does not matter. So I NAK that one myself. Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
different CPU on a SMP machine between the copy and cache flush. Thus we could flush the dcache on the wrong CPU. Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc> --- drivers/ata/libata-sff.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c index 83ecf48..dca9f90 100644 --- a/drivers/ata/libata-sff.c +++ b/drivers/ata/libata-sff.c @@ -874,6 +874,9 @@ static void ata_pio_sector(struct ata_queued_cmd *qc) DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read"); +#ifdef CONFIG_SMP + preempt_disable(); +#endif if (PageHighMem(page)) { unsigned long flags; @@ -896,6 +899,9 @@ static void ata_pio_sector(struct ata_queued_cmd *qc) if (!do_write && !PageSlab(page)) flush_dcache_page(page); +#ifdef CONFIG_SMP + preempt_enable(); +#endif qc->curbytes += qc->sect_size; qc->cursg_ofs += qc->sect_size;