Re: [PATCH 2.6.11] ide-scsi: map sg buffers in idescsi_input/output_buffers() to kernel virtual address

[Jens Axboe <axboe@suse.de>]

On Thu, Mar 17 2005, Jens Axboe wrote:
> On Thu, Mar 17 2005, Stuart_Hayes@Dell.com wrote:
> > Jens Axboe wrote:
> > > On Wed, Mar 16 2005, Stuart_Hayes@Dell.com wrote:
> > >> Jens Axboe wrote:
> > >>> On Wed, Mar 16 2005, Stuart_Hayes@Dell.com wrote:
> > >>>> Hayes, Stuart wrote:
> > >>>>> This patch will map the sg buffers to kernel virtual memory space
> > >>>>> in the functions idescsi_input_buffers() and
> > >>>>> idescsi_output_buffers(). Without this patch, idescsi passes a
> > >>>>> null pointer to atapi_input_bytes() and atapi_output_bytes() when
> > >>>>> sg pages are in high memory (i686 architecture).
> > >>>>> 
> > >>>>> I'm attaching as a file, too, as the text will certainly be
> > >>>>> wrapped. 
> > >>>>> 
> > >>>>> (Sorry for the subject rename--I'm trying to use the correct
> > >>>>> format for patch emails.)
> > >>>>> 
> > >>>>> Thanks
> > >>>>> Stuart
> > >>>> 
> > >>>> And, while there's another high memory/kmap patch question on this
> > >>>> list... 
> > >>>> 
> > >>>> Is there some reason nobody seems interested in this patch (except
> > >>>> Jens--thanks for the help!)?  I'm kind of new to sending in
> > >>>> patches, and I'm not sure if I'm just not waiting long enough, or
> > >>>> if there is a problem with this patch...
> > >>>> 
> > >>>> But really, we're getting a null pointer dereference oops when
> > >>>> using ATAPI tape drives (with ide-scsi) without this patch...
> > >>> 
> > >>> Sorry, that did seem to get dropped on the floor. Actually I'm
> > >>> wondering why you are seeing highmem pages there in the first place,
> > >>> it would be easier/better just to limit ide-scsi to non-highmem
> > >>> pages. That would remove the need to add any work arounds in the
> > >>> driver.
> > >> 
> > >> I think we're seeing highmem pages in the sg list because that's
> > >> where the user memory was when st_write() was called.
> > >> 
> > >> The sg list is set up when st_write(), which calls setup_buffering(),
> > >> which calls st_map_user_pages()... this just sets up the sg pages to
> > >> point directly to the user memory.  So, by the time ide-scsi comes
> > >> into the picture, the sg list is already set up to point to high
> > >> memory pages. 
> > >> 
> > >> Are you suggesting that ide-scsi should change the dma_mask for the
> > >> device so that st_map_user_pages() won't let sg pages point to high
> > >> memory?  Or is there something else I'm missing?
> > > 
> > > I think that is a bug, this effectively bypasses whatever
> > > restrictions the scsi host adapter has said about memory limits. It
> > > is a problem because the request doesn't come from the block layer
> > > (which handles all of this).   
> > > 
> > > I would much prefer fixing that real issue!
> > 
> > By "whatever restrictions the scsi host adapter has said about memory
> > limits", are you referring to the dma_boundary or the dma_mask?  I'm
> > don't know any other ways a host adapter can specify memory limits.
> > 
> > I wasn't complete in my statement above, though--st_map_user_pages()
> > does prevent the sg list from pointing to pages that are above the
> > "max_pfn" for the device (it gets max_pfn from scsi_calculate_bounce_
> > limit()), and that appears to be working ok.  But, even though
> > max_pfn is 0xfffff (so that the maximum physical address of any sg
> > page won't be over 4G (0xffffffff)), there are apparently high 
> > memory pages that are below physical address of 4G (0xffffffff), but
> > are still considered high memory.
> > 
> > So the entire first 4G of memory isn't low memory... for example,
> > on my box, pfn 0xfbc3c, with physical address 0xfbc3c000, has the
> > PG_highmem bit set in &(page)->flags.  Because of this, when ide-scsi
> > needs to do PIO, it needs to do a kmap_atomic().
> > 
> > Am I completely missing your point?
> 
> Ok good, so st isn't broken at least. You are not missing my point, but
> maybe you don't realize that highmem pages doesn't refer to any specific
> address in memory - it refers to pages that don't have a virtual
> mapping, so depending on the kernel config that can be anywhere between
> ~900MB and 2GB (typicall). ide-scsi should just use blk_max_low_pfn as
> the bounce limit, that will work all the time.

IOW, one way to solve this would be to add an shost->bounce_high flag
and add something ala

        if (shost->bounce_high)
                return BLK_BOUNCE_HIGH;

to scsi_calculate_bounce_limit()

-- 
Jens Axboe