linux-kernel - Re: [PATCH 1/2] blkdev: Submit discard bio in batches in blkdev_issue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <x49aafb4x1k.fsf@segfault.boston.devel.redhat.com>
Date:	Wed, 27 Apr 2011 09:57:11 -0400
From:	Jeff Moyer <jmoyer@...hat.com>
To:	Lukas Czerner <lczerner@...hat.com>
Cc:	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	Dmitry Monakhov <dmonakhov@...nvz.org>,
	Jens Axboe <jens.axboe@...cle.com>
Subject: Re: [PATCH 1/2] blkdev: Submit discard bio in batches in blkdev_issue_discard()

Lukas Czerner <lczerner@...hat.com> writes:

> Currently we are waiting for every submitted REQ_DISCARD bio separately,
> but it can have unwanted consequences of repeatedly flushing the queue,
> so we rather submit bios in batches and wait for the entire batch, hence
> narrowing the window of other ios going in.
>
> Use bio_batch_end_io() and struct bio_batch for that purpose, the same
> is used by blkdev_issue_zeroout(). Also change bio_batch_end_io() so we
> always set !BIO_UPTODATE in the case of error and remove the check for
> bb, since we are the only user of this function and we always set this.
>
> Remove bio_get()/bio_put() from the blkdev_issue_discard() since
> bio_alloc() and bio_batch_end_io() is doing the same thing, hence it is
> not needed anymore.
>
> I have done simple dd testing with surprising results. The script I have
> used is:
>
> for i in $(seq 10); do
>         echo $i
>         dd if=/dev/sdb1 of=/dev/sdc1 bs=4k &
>         sleep 5
> done
> /usr/bin/time -f %e ./blkdiscard /dev/sdc1
>
> Running time of BLKDISCARD on the whole device:
> with patch              without patch
> 0.95                    15.58
>
> So we can see that in this artificial test the kernel with the patch
> applied is approx 16x faster in discarding the device.

I don't see any major problems here.  I would like you to test
explicitly for queue_flag_discard before submitting any bios, though.
Previously, after the first bio returned EOPNOTSUPP, you errored out.
Now, you submit all bios that will end with the same failure.  It's
definitely suboptimal, and you already have the q, so you might as well
test for this early on.

Also, as I noted to you on irc, another fix required in this area is
submitting discard requests (the final one in the batch) that do not
honor the discard granularity exported by the device.  That's not
directly related to this patch, though.

Cheers,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/