From: Ming Lei <ming.lei@redhat.com>
To: Salman Qazi <sqazi@google.com>
Cc: Jens Axboe <axboe@kernel.dk>,
Bart Van Assche <bvanassche@acm.org>,
Christoph Hellwig <hch@lst.de>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-block@vger.kernel.org, Gwendal Grignou <gwendal@google.com>,
Jesse Barnes <jsbarnes@google.com>
Subject: Re: BLKSECDISCARD ioctl and hung tasks
Date: Thu, 13 Feb 2020 16:26:43 +0800 [thread overview]
Message-ID: <20200213082643.GB9144@ming.t460p> (raw)
In-Reply-To: <CAKUOC8VN5n+YnFLPbQWa1hKp+vOWH26FKS92R+h4EvS=e11jFA@mail.gmail.com>
On Wed, Feb 12, 2020 at 02:27:09PM -0800, Salman Qazi wrote:
> Hi,
>
> So, here's another issue that we are grappling with, where we have a
> root-cause but don't currently have a good fix for. BLKSECDISCARD is
> an operation used for securely destroying a subset of the data on a
> device. Unfortunately, on SSDs, this is an operation with variable
> performance. It can be O(minutes) in the worst case. The
> pathological case is when many erase blocks on the flash contain a
> small amount of data that is part of the discard and a large amount of
> data that isn't. In such cases, the erase blocks have to be copied
> almost in entirety to fresh blocks, in order to erase the sectors to
> be discarded. This can be thought of as a defragmentation operation on
> the drive and can be expected to cost in the same ballpark as
> rewriting most of the contents of the drive.
>
> Therefore, it is possible for the thread waiting in the IOCTL in
> submit_bio_wait call in blkdev_issue_discard to wait for several
> minutes. The hung task watchdog is usually configured for 2 minutes,
> and this can expire before the operation finishes.
>
> This operation is very important to the security model of Chrome OS
> devices. Therefore, we would like the kernel to survive this even if
> it takes several minutes.
>
> Three approaches come to mind:
>
> One approach is to somehow avoid waiting for a single monolithic
> operation and instead wait on bits and pieces of the operation. These
> can be sized to finish within a reasonable timeframe. The exact size
> is likely device-specific. We already split these operations before
> issuing to the device, but the IOCTL thread is waiting for the whole
> rather than the parts. The hung task watchdog only sees the total
> amount of time the thread slept and not the forward progress taking
> place quietly.
>
> Another approach might be to do something in the spirit of the write
> system call: complete the partial operation (whatever the kernel
> thinks is reasonable), adjust the IOCTL argument and have the
> userspace reissue the syscall to continue the operation. The second
> option should probably be done with a different IOCTL name to avoid
> breaking userspace.
>
> A third approach, which is perhaps more adventurous, is to have a
> notion of forward progress that a thread can export and the hung task
> watchdog can evaluate. This can take the form of a function pointer
> and an argument. The result of the function is a monotonically
> decreasing unsigned value. When this value stops changing, we can
> conclude that the thread is hung. This can be used in place of
> context switch count for tasks where this function is available. This
> can potentially solve other similar issues, there is a way to tell if
> there is forward progress, but it is not as straightforward as the
> context switch count.
>
> What are your thoughts?
The approach used in blk_execute_rq() can be borrowed for workaround the
issue, such as:
diff --git a/block/bio.c b/block/bio.c
index 94d697217887..c9ce19a86de7 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -17,6 +17,7 @@
#include <linux/cgroup.h>
#include <linux/blk-cgroup.h>
#include <linux/highmem.h>
+#include <linux/sched/sysctl.h>
#include <trace/events/block.h>
#include "blk.h"
@@ -1019,12 +1020,19 @@ static void submit_bio_wait_endio(struct bio *bio)
int submit_bio_wait(struct bio *bio)
{
DECLARE_COMPLETION_ONSTACK_MAP(done, bio->bi_disk->lockdep_map);
+ unsigned long hang_check;
bio->bi_private = &done;
bio->bi_end_io = submit_bio_wait_endio;
bio->bi_opf |= REQ_SYNC;
submit_bio(bio);
- wait_for_completion_io(&done);
+
+ /* Prevent hang_check timer from firing at us during very long I/O */
+ hang_check = sysctl_hung_task_timeout_secs;
+ if (hang_check)
+ while (!wait_for_completion_io_timeout(&done, hang_check * (HZ/2)));
+ else
+ wait_for_completion_io(&done);
return blk_status_to_errno(bio->bi_status);
}
thanks,
Ming
next prev parent reply other threads:[~2020-02-13 8:27 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-12 22:27 BLKSECDISCARD ioctl and hung tasks Salman Qazi
2020-02-12 23:06 ` Theodore Y. Ts'o
2020-02-13 1:20 ` Salman Qazi
2020-02-13 1:24 ` Jesse Barnes
2020-02-13 8:26 ` Ming Lei [this message]
2020-02-13 17:48 ` Bart Van Assche
2020-02-13 19:21 ` Salman Qazi
2020-02-13 22:08 ` Salman Qazi
2020-02-14 0:25 ` Ming Lei
2020-02-14 5:49 ` Bart Van Assche
2020-02-14 9:22 ` Ming Lei
2020-02-14 19:42 ` Salman Qazi
2020-02-15 3:46 ` Ming Lei
2020-02-18 16:11 ` Jesse Barnes
2020-02-19 1:37 ` Ming Lei
2020-02-19 2:54 ` Ming Lei
2020-02-19 17:54 ` Salman Qazi
2020-02-19 22:22 ` Ming Lei
2020-02-19 22:26 ` Salman Qazi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200213082643.GB9144@ming.t460p \
--to=ming.lei@redhat.com \
--cc=axboe@kernel.dk \
--cc=bvanassche@acm.org \
--cc=gwendal@google.com \
--cc=hch@lst.de \
--cc=jsbarnes@google.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sqazi@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).