Re: QUEUE_FLAG_NO_SG_MERGE and non-block-mq

[Jens Axboe]

On 11/27/2015 07:29 AM, Hannes Reinecke wrote:

On 11/26/2015 10:21 AM, Ming Lei wrote:

On Thu, Nov 26, 2015 at 4:13 PM, Hannes Reinecke  wrote:

Hi all,

while investigating the crash in scsi_lib.c I found a rather curious
behaviour for QUEUE_FLAG_NO_SG_MERGE.

While the flag is evaluated in blk_recalc_rq_segments and
blk_recount_segments (resulting in nr_phys_segments being
computed based on that flag) it is completely ignored
during blk_rq_map_sg() or the actual merging itself.


Yes, I guess Jens introduced the flag for decreasing CPU
consumption when comuputing segments, but it is still
ignored by blk_rq_map_sg(), but it may not be used
by some drivers.

After bio splitting is introduced, the flag is also ignored
when computing segments.



This typically shouldn't be an issue, seeing that with
QUEUE_FLAG_NO_SG_MERGE nr_phys_segments will always be
larger than the actual segment count.

However, it still makes me wonder:
What is the point of having a QUEUE_FLAG_NO_SG_MERGE
which doesn't work as advertised?
Or, to be precise, which only works for blk-mq?
Should we make it work for non-block-mq, too?


Thanks bio splitting, this flag has little effect on performance now,
so I think it can be removed if Jens has no objection.


As per your suggestion we've made some performance measurements,
and 4k fio showed little if no impact:

NO_SG_MERGE:
   IOPS R/W: 148097.7+-125.7 / 148124.1+-123.1
   BW   R/W: 592392.4+-502.7 / 592498.3+-492.3
SG_MERGE:
   IOPS R/W: 148054.4+-123.3 / 148082.6+-120.0
   BW   R/W: 592219.2+-493.5 / 592332.3+-479.7

So the performance benefit lies squarely within the
error margin, making me wonder if it's worth bothering
with having the NO_SG_MERGE flag at all.

Thanks to Johannes for doing the measurements :-)


150K iops is on the slow side, though. It's pointless to iterate the sg 
list if we don't have to. I can try and run a few tests next week.


--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: 'Device not ready' issue on mpt2sas since 3.1.10

[Felix Matouschek]

Hello,

I've encountered a similiar error like Matthias Prager did in his first 
mail in this thread in 2012.


I use Debian 8 Kernel 3.16 and also own a LSI 2008 card flashed to IT 
mode (firmware P20) and have problems with disks that were spun down.
Writing to them when they are spun down usually ends in the following 
errors:


[59526.359997] sd 0:0:1:0: [sdc] FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK

[59526.360003] sd 0:0:1:0: [sdc] CDB:
[59526.360006] Read(16): 88 00 00 00 00 00 31 28 fd 58 00 00 00 08 00 00
[59526.360022] blk_update_request: I/O error, dev sdc, sector 824769880
[59544.111090] sd 0:0:0:0: [sdb] FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK

[59544.111097] sd 0:0:0:0: [sdb] CDB:
[59544.00] Read(16): 88 00 00 00 00 00 31 28 fd 50 00 00 00 08 00 00
[59544.15] blk_update_request: I/O error, dev sdb, sector 824769872
[59544.114465] sd 0:0:4:0: [sdf] FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK

[59544.114468] sd 0:0:4:0: [sdf] CDB:
[59544.114469] Read(16): 88 00 00 00 00 00 31 28 fd 58 00 00 00 08 00 00
[59544.114483] blk_update_request: I/O error, dev sdf, sector 824769880
[59552.117436] sd 0:0:3:0: [sde] FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK

[59552.117443] sd 0:0:3:0: [sde] CDB:
[59552.117446] Read(16): 88 00 00 00 00 00 31 28 fd b0 00 00 00 08 00 00
[59552.117462] blk_update_request: I/O error, dev sde, sector 824769968
[59572.951158] sd 0:0:2:0: [sdd] FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK

[59572.951167] sd 0:0:2:0: [sdd] CDB:
[59572.951170] Read(16): 88 00 00 00 00 00 31 28 fd b0 00 00 00 08 00 00
[59572.951192] blk_update_request: I/O error, dev sdd, sector 824769968
[59572.955679] sd 0:0:5:0: [sdg] FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK

[59572.955695] sd 0:0:5:0: [sdg] CDB:
[59572.955701] Read(16): 88 00 00 00 00 00 31 28 fd b0 00 00 00 08 00 00
[59572.955720] blk_update_request: I/O error, dev sdg, sector 824769968
[70357.782677] sd 0:0:4:0: [sdf] FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK

[70357.782686] sd 0:0:4:0: [sdf] CDB:
[70357.782690] Read(16): 88 00 00 00 00 00 85 c1 c9 08 00 00 00 08 00 00
[70357.782712] blk_update_request: I/O error, dev sdf, sector 2244069640
[70368.087947] sd 0:0:0:0: [sdb] FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK

[70368.087953] sd 0:0:0:0: [sdb] CDB:
[70368.087955] Read(16): 88 00 00 00 00 00 85 c1 c9 00 00 00 00 08 00 00
[70368.087969] blk_update_request: I/O error, dev sdb, sector 2244069632

Notice the lack of the "Device not ready" message, otherwise these 
errors look very similiars to Matthias' errors.


I have no clue what to do to fix this problem. Any suggestions?

Greetings,
Felix


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: QUEUE_FLAG_NO_SG_MERGE and non-block-mq

[Hannes Reinecke]

On 11/26/2015 10:21 AM, Ming Lei wrote:
> On Thu, Nov 26, 2015 at 4:13 PM, Hannes Reinecke  wrote:
>> Hi all,
>>
>> while investigating the crash in scsi_lib.c I found a rather curious
>> behaviour for QUEUE_FLAG_NO_SG_MERGE.
>>
>> While the flag is evaluated in blk_recalc_rq_segments and
>> blk_recount_segments (resulting in nr_phys_segments being
>> computed based on that flag) it is completely ignored
>> during blk_rq_map_sg() or the actual merging itself.
> 
> Yes, I guess Jens introduced the flag for decreasing CPU
> consumption when comuputing segments, but it is still
> ignored by blk_rq_map_sg(), but it may not be used
> by some drivers.
> 
> After bio splitting is introduced, the flag is also ignored
> when computing segments.
> 
>>
>> This typically shouldn't be an issue, seeing that with
>> QUEUE_FLAG_NO_SG_MERGE nr_phys_segments will always be
>> larger than the actual segment count.
>>
>> However, it still makes me wonder:
>> What is the point of having a QUEUE_FLAG_NO_SG_MERGE
>> which doesn't work as advertised?
>> Or, to be precise, which only works for blk-mq?
>> Should we make it work for non-block-mq, too?
> 
> Thanks bio splitting, this flag has little effect on performance now,
> so I think it can be removed if Jens has no objection.
> 
As per your suggestion we've made some performance measurements,
and 4k fio showed little if no impact:

NO_SG_MERGE:
  IOPS R/W: 148097.7+-125.7 / 148124.1+-123.1
  BW   R/W: 592392.4+-502.7 / 592498.3+-492.3
SG_MERGE:
  IOPS R/W: 148054.4+-123.3 / 148082.6+-120.0
  BW   R/W: 592219.2+-493.5 / 592332.3+-479.7

So the performance benefit lies squarely within the
error margin, making me wonder if it's worth bothering
with having the NO_SG_MERGE flag at all.

Thanks to Johannes for doing the measurements :-)

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries & Storage
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply Me For Details

[HS09912HH3]

Good day,

I wish to contact you personally for an important proposal that might be 
of interest to you. I am sending this mail just to know if this email address
is functional.

I have something absolutely essential to discuss with you. Contact me for 
details through my private email: shu...@qq.com

Regards,
shu...@qq.com
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] arcmsr: modify codes for more readable

[Tomas Henzl]

On 26.11.2015 12:33, Ching Huang wrote:
> From: Ching Huang 
>
> modify codes for more readable
>
> Signed-of-by: Ching Huang 

Reviewed-by: Tomas Henzl 

Tomas

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] arcmsr: change driver version to v1.30.00.22-20151126

[Tomas Henzl]

On 26.11.2015 12:44, Ching Huang wrote:
> From: Ching Huang 
>
> change driver version to v1.30.00.22-20151126
>
> Signed-of-by: Ching Huang 

Reviewed-by: Tomas Henzl 

Tomas

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] arcmsr: Split dma resource allocation to a new function

[Tomas Henzl]

On 27.11.2015 03:58, Ching Huang wrote:
> On Thu, 2015-11-26 at 11:46 -0800, Joe Perches wrote:
>> On Thu, 2015-11-26 at 19:41 +0800, Ching Huang wrote:
>>> split dma resource allocation and io register assignment from get_config to 
>>> a new function arcmsr_alloc_io_queue.
>> trivia:
>>
>>> diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c 
>>> b/drivers/scsi/arcmsr/arcmsr_hba.c
>> []
>>> +static bool arcmsr_alloc_io_queue(struct AdapterControlBlock *acb)
>>> +{
>> []
>>> +   dma_coherent = dma_alloc_coherent(>dev, 
>>> acb->roundup_ccbsize,
>>> +   _coherent_handle, GFP_KERNEL);
>>> +   if (!dma_coherent){
>>> +   pr_notice("arcmsr%d: DMA allocation failed.\n", 
>>> acb->host->host_no);
>>> +   return false;
>>> +   }
>>> +   memset(dma_coherent, 0, acb->roundup_ccbsize);
>>>
>> There is a dma_zalloc_coherent
>>
>> (and even more trivially)
>>
>> Most all of your error messages don't use periods.
> Thanks Joe.
> Revised as below.
>
> Signed-of-by: Ching Huang 

Reviewed-by: Tomas Henzl 

Tomas

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND] scsi_debug: fix prevent_allow+verify regressions

[Andy Shevchenko]

On Thu, Nov 26, 2015 at 4:14 AM, Martin K. Petersen
 wrote:
>> "Andy" == Andy Shevchenko  writes:
>
> Andy,
>
> Andy> but can you pay a little attention to
> Andy> http://www.spinics.net/lists/linux-scsi/msg81778.html ? It seems
> Andy> it wasn't applied.
>
> Please resubmit to linux-scsi. We'll take a look.

Done:

http://www.spinics.net/lists/linux-scsi/msg91207.html

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 1/1] scsi_debug: check for bigger value first

[Johannes Thumshirn]

On Thu, 2015-11-26 at 20:22 +0200, Andy Shevchenko wrote:
> From: Andy Shevchenko 
> 
> Even for signed types we have to check for bigger positive value first.
> Otherwise it will be never happened.
> 
> Acked-by: Douglas Gilbert 
> Signed-off-by: Andy Shevchenko 
> ---
>  drivers/scsi/scsi_debug.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index dfcc45b..f773b34 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -4846,10 +4846,10 @@ static int __init scsi_debug_init(void)
>   /* play around with geometry, don't waste too much on track 0 */
>   sdebug_heads = 8;
>   sdebug_sectors_per = 32;
> - if (scsi_debug_dev_size_mb >= 16)
> - sdebug_heads = 32;
> - else if (scsi_debug_dev_size_mb >= 256)
> + if (scsi_debug_dev_size_mb >= 256)
>   sdebug_heads = 64;
> + else if (scsi_debug_dev_size_mb >= 16)
> + sdebug_heads = 32;
>   sdebug_cylinders_per = (unsigned long)sdebug_capacity /
>      (sdebug_sectors_per * sdebug_heads);
>   if (sdebug_cylinders_per >= 1024) {

Reviewed-by: Johannes Thumshirn 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html