Hayes, Stuart wrote:
> Jens Axboe wrote:
>> On Thu, Mar 17 2005, [EMAIL PROTECTED] wrote:
>>> Jens Axboe wrote:
>>>> On Thu, Mar 17 2005, Jens Axboe wrote:
>>>>> On Thu, Mar 17 2005, [EMAIL PROTECTED] wrote:
>>>>>> Jens Axboe wrote:
>>>>>>> On Wed, Mar 16 2005, [EMAIL PROTECTED] wrote:
>>>>>>>> Jens Axboe wrote:
>>>>>>>>> On Wed, Mar 16 2005, [EMAIL PROTECTED] 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()
>>>
>>> Yes, I could easily implement that. I had been trying to figure out
>>> how to go about implementing what you said--I was looking at making
>>> idescsi_attach change the dma_mask that
>>> scsi_calculate_bounce_limit() currently looks at, but it wasn't
>>> looking very pretty.
>>>
>>> Unfortunately, I won't be able to do that fix with a DKMS driver on
>>> an existing kernel... but that's not really relevant to this list.
>>
>> For your existing kernel fix, just use the one you sent last using
>> kmap_atomic, it should work for you as well.
>>
>>> I'll try this out and send in a patch.
>>>
>>> Thanks!
>>
>> No problem, thanks for being persistent in getting this fixed :)
>
> OK, sorry for the delay. Here's the new patch to handle this as you
> suggested, against the 2.6.11.5 kernel. I have tested it, and it
> works.
>
> Do I need to re-send this with a new subject line, since the patch
> doesn't match the description in the subject line, or is it OK here?
>
> I'll also attach as a file, though I'm optimistic that this one won't
> be wrapped.
>
> Thanks!
> Stuart
>
>
> Signed-off-by: Stuart Hayes <[EMAIL PROTECTED]>
>
> diff -purN a/drivers/scsi/ide-scsi.c b/drivers/scsi/ide-scsi.c
> --- a/drivers/scsi/ide-scsi.c 2005-03-15 19:09:08.000000000 -0500
> +++ b/drivers/scsi/ide-scsi.c 2005-03-23 17:26:06.000000000 -0500
> @@ -1048,6 +1048,7 @@ static int idescsi_attach(ide_drive_t *d
> return 1;
>
> host->max_id = 1;
> + //host->bounce_high = 1;
>
> #if IDESCSI_DEBUG_LOG
> if (drive->id->last_lun)
> diff -purN a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> --- a/drivers/scsi/scsi_lib.c 2005-03-15 19:09:06.000000000 -0500
> +++ b/drivers/scsi/scsi_lib.c 2005-03-23 17:26:04.000000000 -0500
> @@ -1333,6 +1333,9 @@ u64 scsi_calculate_bounce_limit(struct S
>
> if (shost->unchecked_isa_dma)
> return BLK_BOUNCE_ISA;
> +
> + if (shost->bounce_high)
> + return BLK_BOUNCE_HIGH;
> /*
> * Platforms with virtual-DMA translation
> * hardware have no practical limit.
> diff -purN a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> --- a/include/scsi/scsi_host.h 2005-03-15 19:09:01.000000000
-0500
> +++ b/include/scsi/scsi_host.h 2005-03-23 17:26:54.000000000
-0500
> @@ -502,6 +502,12 @@ struct Scsi_Host {
> unsigned reverse_ordering:1;
>
> /*
> + * Host needs all high memory bounced (useful for ide-scsi,
> + * so it doesn't have to kmap when doing PIO)
> + */
> + unsigned bounce_high:1;
> +
> + /*
> * Host has rejected a command because it was busy.
> */
> unsigned int host_blocked;
It occurred to me that it might be possible for the device to have a
greater restriction on the DMA memory than BLK_BOUNCE_HIGH, so I changed
the scsi_calculate_bounce_limit to make shost->bounce_high an upper
limit on the bounce limit.
I also uncommented the "host->bounce_high=1" in ide-scsi.c... :)
I'm personally still partial to my original patch
(http://marc.theaimsgroup.com/?l=linux-scsi&m=111040058219417) to fix
this problem, but I guess it's six of one...
And, feel free to tell me if there's still something wrong with this!
Signed-off-by: Stuart Hayes <[EMAIL PROTECTED]>
diff -purN a/drivers/scsi/ide-scsi.c b/drivers/scsi/ide-scsi.c
--- a/drivers/scsi/ide-scsi.c 2005-03-25 22:28:23.000000000 -0500
+++ b/drivers/scsi/ide-scsi.c 2005-03-31 15:17:38.000000000 -0500
@@ -1048,6 +1048,7 @@ static int idescsi_attach(ide_drive_t *d
return 1;
host->max_id = 1;
+ host->bounce_high = 1;
#if IDESCSI_DEBUG_LOG
if (drive->id->last_lun)
diff -purN a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
--- a/drivers/scsi/scsi_lib.c 2005-03-25 22:28:21.000000000 -0500
+++ b/drivers/scsi/scsi_lib.c 2005-03-31 15:21:35.000000000 -0500
@@ -1344,6 +1344,9 @@ u64 scsi_calculate_bounce_limit(struct S
if (host_dev && host_dev->dma_mask)
bounce_limit = *host_dev->dma_mask;
+ if (shost->bounce_high && (bounce_limit > BLK_BOUNCE_HIGH))
+ return BLK_BOUNCE_HIGH;
+
return bounce_limit;
}
EXPORT_SYMBOL(scsi_calculate_bounce_limit);
diff -purN a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
--- a/include/scsi/scsi_host.h 2005-03-25 22:28:16.000000000 -0500
+++ b/include/scsi/scsi_host.h 2005-03-31 15:16:54.000000000 -0500
@@ -502,6 +502,12 @@ struct Scsi_Host {
unsigned reverse_ordering:1;
/*
+ * Host needs all high memory bounced (useful for ide-scsi,
+ * so it doesn't have to kmap when doing PIO)
+ */
+ unsigned bounce_high:1;
+
+ /*
* Host has rejected a command because it was busy.
*/
unsigned int host_blocked;
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
