On Mon, 2008-02-25 at 15:58 +0100, Andi Kleen wrote:
> On Mon, Feb 25, 2008 at 06:46:58AM -0800, James Bottomley wrote:
> >
> > On Mon, 2008-02-25 at 00:35 +0100, Andi Kleen wrote:
> > > Should not be needed because the block layer bounces that all.
> > >
> > > Signed-off-by: Andi Kleen <ak@xxxxxxx>
> > >
> > > ---
> > > drivers/scsi/scsi_scan.c | 6 ++----
> > > 1 file changed, 2 insertions(+), 4 deletions(-)
> > >
> > > Index: linux/drivers/scsi/scsi_scan.c
> > > ===================================================================
> > > --- linux.orig/drivers/scsi/scsi_scan.c
> > > +++ linux/drivers/scsi/scsi_scan.c
> > > @@ -1010,8 +1010,7 @@ static int scsi_probe_and_add_lun(struct
> > > if (!sdev)
> > > goto out;
> > >
> > > - result = kmalloc(result_len, GFP_ATOMIC |
> > > - ((shost->unchecked_isa_dma) ? __GFP_DMA : 0));
> > > + result = kmalloc(result_len, GFP_ATOMIC);
> > > if (!result)
> > > goto out_free_sdev;
> > >
> > > @@ -1328,8 +1327,7 @@ static int scsi_report_lun_scan(struct s
> > > * prevent us from finding any LUNs on this target.
> > > */
> > > length = (max_scsi_report_luns + 1) * sizeof(struct scsi_lun);
> > > - lun_data = kmalloc(length, GFP_ATOMIC |
> > > - (sdev->host->unchecked_isa_dma ? __GFP_DMA : 0));
> > > + lun_data = kmalloc(length, GFP_ATOMIC);
> > > if (!lun_data) {
> > > printk(ALLOC_FAILURE_MSG, __FUNCTION__);
> > > goto out;
> >
> > Andi, this can't be right.
>
> You mean it is incorrect or just not optimal?
It's not optimal ... but that's sufficient an objection.
> >
> > You're removing something that's actually useful. I'm happy to
> > substitute this kmalloc for kmalloc_mask on the device dma mask which
> > will do the same thing and so junk unchecked_isa_dma() that way (and
> > actually fix us up for other weird mask devices), but just using
> > ZONE_NORMAL is wrong because we'll then bounce all the time for
> > something we knew a priori how to avoid.
>
> That would require adding a separate mask just for this to the
> template. I figured the SCSI scan was not performance critical
> so a few more copies just for this case was an ok trade off
> for simpler code.
Why? We already have the device; can't we just use its mask?
> You think it makes sense to optimize scsi scan?
It makes sense to use information we already know to optimise the path,
yes, when it's something as simple as keeping the dma buffer within the
mask bounds of the device.
James
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
