On 4/20/22 07:54, Stefan Hajnoczi wrote: > On Tue, Apr 19, 2022 at 09:10:50PM -0700, Chaitanya Kulkarni wrote: >> The function virtblk_setup_cmd() calls >> virtblk_setup_discard_write_zeroes() once we process the block layer >> request operation setup in the switch. Even though it saves duplicate >> call for REQ_OP_DISCARD and REQ_OP_WRITE_ZEROES it adds additional check >> in the fast path that is redundent since we already have a switch case >> for both REQ_OP_DISCARD and REQ_OP_WRITE_ZEROES. >> >> Move the call virtblk_setup_discard_write_zeroes() into switch case >> label of REQ_OP_DISCARD and REQ_OP_WRITE_ZEROES and avoid duplicate >> branch in the fast path. >> >> Signed-off-by: Chaitanya Kulkarni <kch@xxxxxxxxxx> >> --- >> drivers/block/virtio_blk.c | 9 ++++----- >> 1 file changed, 4 insertions(+), 5 deletions(-) > > Is there data that shows the performance effect of moving the code out > of the fast path? > I don't have a data yet but trying to minimize fast path branches as I can when I was reading the code... >> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c >> index 6ccf15253dee..b77711e73422 100644 >> --- a/drivers/block/virtio_blk.c >> +++ b/drivers/block/virtio_blk.c >> @@ -223,10 +223,14 @@ static blk_status_t virtblk_setup_cmd(struct virtio_device *vdev, >> break; >> case REQ_OP_DISCARD: >> type = VIRTIO_BLK_T_DISCARD; >> + if (virtblk_setup_discard_write_zeroes(req, unmap)) > > unmap is never true here. The variable obscures what is going on: > > s/unmap/false/ > yeah, I'll drop this patch from V2. -ck
