FW: Re. Allocation of scatter gather table in __init_io

FW: Re. Allocation of scatter gather table in __init_io

[Govindan, Thiru]

Hi,
    I'm running into an issue that seems to be caused by __init_io() in
scsi_merge.c. Apparently __init_io() is allocating the scatter gather list
before even merging the segments in the queue. In my case the request
nr_segments  has a value of 2 and the scatter gather table is allocated for
2 entries. But  then the logic merges the 2 segments into 1 and fills in one
entry, my HBA driver complains that one entry is invalid and panics. Can you
clarify how this works, I suppose getting mergeable segments in a request is
common and hence __init_io() must allocate the table after having at least
identified how may segments are really there. 

Is this a known issue and is there a patch that fixes this?

Govindan

Re: FW: Re. Allocation of scatter gather table in __init_io

[Jens Axboe]

On Mon, Aug 09 2004, Govindan, Thiru wrote:
> Hi,
>     I'm running into an issue that seems to be caused by __init_io()
>     in scsi_merge.c. Apparently __init_io() is allocating the scatter
>     gather list before even merging the segments in the queue. In my
>     case the request nr_segments  has a value of 2 and the scatter
>     gather table is allocated for 2 entries. But  then the logic
>     merges the 2 segments into 1 and fills in one entry, my HBA driver
>     complains that one entry is invalid and panics. Can you clarify
>     how this works, I suppose getting mergeable segments in a request
>     is common and hence __init_io() must allocate the table after
>     having at least identified how may segments are really there. 
> 
> Is this a known issue and is there a patch that fixes this?

It has to allocate the space for the table to hold all items, so it's
expected. Sounds like you are not using the correct return value to
check what segment count you should pass the hardware.

-- 
Jens Axboe

RE: FW: Re. Allocation of scatter gather table in __init_io

[Govindan, Thiru]

Jens,
	Ok, but __init_io() does seem to expect that the number of segments
matches the count after merging is done. As shown in the code below, if its
OK to allocate space for the max. number of items then this check is not
needed.
        /*
         * Verify that the count is correct.
         */
        if (count != SCpnt->use_sg) {
                printk("Incorrect number of segments after building
list\n");
#ifdef CONFIG_SCSI_DEBUG_QUEUES
                dump_stats(req, use_clustering, dma_host, count);
#endif
        }


How does the HBA driver know the correct number in this case? Should it use
request_bufflen in the SCSI Command in addition to use_sg, to identify the
number of segments actually there in the list? I'm using a Qlogic HBA and
its just going by the value in use_sg to identify segments in the
scatter-gather list.

Govindan

-----Original Message-----
From: Jens Axboe [mailto:axboe@suse.de] 
Sent: Monday, August 09, 2004 2:36 AM
To: Govindan, Thiru
Cc: 'linux-scsi@vger.kernel.org'
Subject: Re: FW: Re. Allocation of scatter gather table in __init_io


On Mon, Aug 09 2004, Govindan, Thiru wrote:
> Hi,
>     I'm running into an issue that seems to be caused by __init_io()
>     in scsi_merge.c. Apparently __init_io() is allocating the scatter
>     gather list before even merging the segments in the queue. In my
>     case the request nr_segments  has a value of 2 and the scatter
>     gather table is allocated for 2 entries. But  then the logic
>     merges the 2 segments into 1 and fills in one entry, my HBA driver
>     complains that one entry is invalid and panics. Can you clarify
>     how this works, I suppose getting mergeable segments in a request
>     is common and hence __init_io() must allocate the table after
>     having at least identified how may segments are really there.
> 
> Is this a known issue and is there a patch that fixes this?

It has to allocate the space for the table to hold all items, so it's
expected. Sounds like you are not using the correct return value to check
what segment count you should pass the hardware.

-- 
Jens Axboe

Re: FW: Re. Allocation of scatter gather table in __init_io

[Jens Axboe]

(don't top post, reply properly commented below the questions asked like
everybody else is doing)

On Mon, Aug 09 2004, Govindan, Thiru wrote:
> Jens,
> 	Ok, but __init_io() does seem to expect that the number of segments
> matches the count after merging is done. As shown in the code below, if its
> OK to allocate space for the max. number of items then this check is not
> needed.
>         /*
>          * Verify that the count is correct.
>          */
>         if (count != SCpnt->use_sg) {
>                 printk("Incorrect number of segments after building
> list\n");
> #ifdef CONFIG_SCSI_DEBUG_QUEUES
>                 dump_stats(req, use_clustering, dma_host, count);
> #endif
>         }

Ah, you are talking about 2.4 (in the future, remember to note which
kernel base you are talking about). Yes those counts must match, if not
something is broken in the merging code.

> How does the HBA driver know the correct number in this case? Should
> it use request_bufflen in the SCSI Command in addition to use_sg, to
> identify the number of segments actually there in the list? I'm using
> a Qlogic HBA and its just going by the value in use_sg to identify
> segments in the scatter-gather list.

I don't completely follow your question, sorry. Can you outline in
detail what happens, what values you are seeing, etc.

-- 
Jens Axboe