From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Keith Busch <kbusch@kernel.org>,
Chandan Babu R <chandanbabu@kernel.org>,
linux-block@vger.kernel.org, linux-xfs@vger.kernel.org
Subject: Re: [BUG REPORT] General protection fault while discarding extents on XFS on next-20240305
Date: Thu, 7 Mar 2024 07:51:35 +1100 [thread overview]
Message-ID: <ZejXV1ll+sbgBP48@dread.disaster.area> (raw)
In-Reply-To: <ZeiJKmWQoE6ttn6L@infradead.org>
On Wed, Mar 06, 2024 at 07:18:02AM -0800, Christoph Hellwig wrote:
> Lookings at this a bit more I'm not sure my fix is enough as the error
> handling is really complex. Also given that some discard callers are
> from kernel threads messing with interruptibility I'm not entirely
> sure that having this check in the common helper is a good idea.
Yeah, this seems like a problem. The only places that userspace
should be issuing discards directly and hence be interruptible from
are FITRIM, BLKDISCARD and fallocate() on block devices.
Filesystems already handle fatal signals in FITRIM (e.g. see
xfs_trim_should_stop(), ext4_trim_interrupted(),
btrfs_trim_free_extents(), etc), so it seems to me that the only
non-interruptible call from userspace are operations directly on
block devices which have no higher level iteration over the range to
discard and the user controls the range directly.
Perhaps the solution is to change BLKDISCARD/fallocate() on bdev to
look more like xfs_discard_extents() where it breaks the range up
into smaller chunks and intersperses bio chaining with signal
checks.
I suspect the same solution is necessary for blkdev_issue_zeroout()
and blkdev_issue_secure_erase(), because both of them have user
controlled lengths...
-Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2024-03-06 20:51 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-06 7:19 [BUG REPORT] General protection fault while discarding extents on XFS on next-20240305 Chandan Babu R
2024-03-06 12:35 ` Christoph Hellwig
2024-03-06 14:36 ` Keith Busch
2024-03-06 14:40 ` Keith Busch
2024-03-06 14:45 ` Christoph Hellwig
2024-03-06 15:18 ` Christoph Hellwig
2024-03-06 20:51 ` Dave Chinner [this message]
2024-03-06 22:16 ` Christoph Hellwig
2024-03-07 9:21 ` Nilay Shroff
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=ZejXV1ll+sbgBP48@dread.disaster.area \
--to=david@fromorbit.com \
--cc=chandanbabu@kernel.org \
--cc=hch@infradead.org \
--cc=kbusch@kernel.org \
--cc=linux-block@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
/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