Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   bugzilla-daemon@kernel.org
To:     linux-ext4@vger.kernel.org
Subject: [Bug 216322] Freezing of tasks failed after 60.004 seconds (1 tasks
 refusing to freeze... task:fstrim  ext4_trim_fs - Dell XPS 13 9310
Date:   Thu, 04 Aug 2022 11:47:47 +0000
Message-ID: <bug-216322-13602-2MvUDlAfJU@https.bugzilla.kernel.org/>
In-Reply-To: <bug-216322-13602@https.bugzilla.kernel.org/>
References: <bug-216322-13602@https.bugzilla.kernel.org/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Auto-Submitted: auto-generated
MIME-Version: 1.0
Precedence: bulk

https://bugzilla.kernel.org/show_bug.cgi?id=3D216322

--- Comment #4 from Lukas Czerner (lczerner@redhat.com) ---
On Thu, Aug 04, 2022 at 12:44:45AM +0000, bugzilla-daemon@kernel.org wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=3D216322
>=20
> Theodore Tso (tytso@mit.edu) changed:
>=20
>            What    |Removed                     |Added
> -------------------------------------------------------------------------=
---
>                  CC|                            |tytso@mit.edu
>=20
> --- Comment #2 from Theodore Tso (tytso@mit.edu) ---
> So the problem is that the FITRIM ioctl does not check if a signal is
> pending,
> and so if the fstrim program requests that the entire SSD (len=3DULLONG_M=
AX),
> like the broomstick set off by Mickey Mouse in Fantasia's "Sorcerer's
> Apprentive", it will mindlessly send discard requests for any blocks not =
in
> use
> by the file system until it is done.   Or to put it another way, "Neither
> rain,
> nor snow, or a request to freeze the OS, shall stop the FITRIM ioctl from=
 its
> appointed task."  :-)
>=20
> The question is how to fix things.   The problem is that the FITRIM ioctl
> interface is pretty horrible.   The fstrim_range.len variable is an IN/OUT
> field where on the input it is the number of bytes that should be trimmed
> (from
> start to start+len) and when the ioctl returns fstrm_range.len is the num=
ber
> of
> bytes that were actually trimmed.   So this is not really amenable for
> -ERESTARTSYS.
>=20
> Worse, the fstrim program in util-linux doesn't handle an EAGAIN error re=
turn
> code, so if it gets the EAGAIN after try_to_freeze_tasks send the fake si=
gnal
> to the process, fstrim will print to stderr "fstrim: FITRIM ioctl failed"=
 and
> the rest of the file system trim operation will be aborted.
>=20
> It might be that the only way we can fix this is to have FITRIM return
> EAGAIN,
> which will stop the fstrim in its tracks.  This is... not great, but
> typically
> fstrim is run out of crontab or a systemd timer once a month, so if the u=
ser
> tries to suspend right as the fstrim is running, hopefully we'll get lucky
> next
> month.    We can then try teach fstrim to do the right thing, and so this
> lossage mode would only happen in the combination of a new kernel and an
> older
> version of util-linux.
>=20
> I'm not happy with that solution, but the alternative of creating a new
> FITRIM2
> ioctl that has a sane interface means that you need an new kernel and a n=
ew
> util-linux package, and if you don't, the user will have to deal with a h=
ot
> laptop bag and a drained battery.   And not changing FITRIM's behaviour w=
ill
> have the same potential end result, if the user gets unlucky and tries to
> suspend the laptop when there is more than 60 seconds left before FITRIM =
to
> complete.   :-/
>=20
> The other thing I'll note is that every file system has its own FITRIM
> implementation, and I suspect they all have this issue, because the FITRIM
> interface is fundamentally flawed.

I agree that the FITRIM interface is flawed in this way. But
ext4_try_to_trim_range() actually does have fatal_signal_pending() and
will return -ERESTARTSYS if that's true. Or did you have something else in
mind?

Also in that case, I see no reason why we would not be able to adjust
the fstrim_range to make it easier to re-start where we left off if
we're going to return -ERESTARTSYS. I am missing something?

I have not had time to look deeply into the traces, but are you actually
sure that we're not stuck in blkdev_issue_discard() instead?

-Lukas

--=20
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.=