Re: [PATCH 00/29] UFS patches for kernel v5.19

[Bart Van Assche <bvanassche@acm.org>]

[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]

On 4/5/22 02:33, Bean Huo wrote:
> For performance improvement, according to my test, if we abandon SCSI
> command parsing, we can get 3%~5% performance improvement. Maybe this
> is little or no improvement? Yes, reliability issues outweigh this
> performance improvement. Error handling and UFS probes should also be
> rebuilt. But most importantly, it makes UFS more scalable. How do you
> think about adding an immature development driver to drever/staging
> first? name it driver/staging/lightweight-ufs?

I do not understand the interest in bypassing the SCSI core. The 
scsi_debug driver supports more than a million IOPS on a single CPU core 
(see also the attached script). I think this shows that the SCSI core is 
not a performance bottleneck. Additionally, I think it will take a while 
until UFS devices scale from the current performance level (100K IOPS?) 
to more than a million IOPS.

Please note that bypassing the SCSI core would make it much harder than 
necessary to introduce zoned storage support and also that this would 
lead to plenty of duplicated code.

>> For other SCSI LLDs the cost of
>> atomic operations and memory barriers in the LLD outweighs the cost
>> of
>> the operations in the SCSI core and sd drivers. I'm not sure whether
>> that's also the case for the UFS driver.
> 
> I didn't take this into account, maybe it's not a big deal, since the
> UFS driver might use its own lock/serialization lock.

Every single atomic operation in the hot path has a measurable 
performance impact. That includes locking operations.

Thanks,

Bart.

[-- Attachment #2: scsi_debug-iops --]
[-- Type: text/plain, Size: 947 bytes --]

#!/bin/bash

set -e

iodepth=${1:-1}
runtime=30
blocksize=512
numcpus=$(nproc)

modprobe -r scsi_debug
modprobe scsi_debug max_queue=128 submit_queues="$numcpus" delay=0 fake_rw=1 &&
    udevadm settle

DEVICE=$(find /sys/bus/pseudo/drivers/scsi_debug/adapter*/host*/target*/*/block/ -maxdepth 1 -type d | grep -v 'block/$' | head -1 | xargs basename) || exit $?
[ -n "$DEVICE" ] || exit $?

args=()
if [ "$iodepth" = 1 ]; then
	args+=(--ioengine=psync)
else
	args+=(--ioengine=io_uring --iodepth_batch=$((iodepth/2)) --hipri=1)
fi
args+=(
    --bs="${blocksize}"
    --direct=1
    --filename=/dev/"$DEVICE"
    --group_reporting=1
    --gtod_reduce=1
    --invalidate=1
    --iodepth="$iodepth"
    --ioscheduler=none
    --name=scsi_debug
    --numjobs=1
    --runtime="$runtime"
    --rw=read
    --thread
    --time_based=1
)
set -x
if numactl -m 0 -N 0 echo >&/dev/null; then
	numactl -m 0 -N 0 -- fio "${args[@]}"
else
	fio "${args[@]}"
fi