Re: [systemd-devel] [usb-storage] Re: Amazon Kindle disconnect after Synchronize Cache

From: Lennart Poettering <lennart@poettering.net>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Hans de Goede <hdegoede@redhat.com>,
	usb-storage@lists.one-eyed-alien.net,
	Matthias Schwarzott <zzam@gentoo.org>,
	linux-usb@vger.kernel.org, systemd-devel@lists.freedesktop.org,
	hirofumi@mail.parknet.co.jp
Subject: Re: [systemd-devel] [usb-storage] Re: Amazon Kindle disconnect after Synchronize Cache
Date: Wed, 17 Mar 2021 17:47:05 +0100	[thread overview]
Message-ID: <YFIyidaZZmDoTevB@gardel-login> (raw)
In-Reply-To: <20210317151746.GB488655@rowland.harvard.edu>

On Mi, 17.03.21 11:17, Alan Stern (stern@rowland.harvard.edu) wrote:

> On Wed, Mar 17, 2021 at 01:21:50PM +0100, Hans de Goede wrote:
> > Hi,
> >
> > On 3/16/21 6:04 PM, Alan Stern wrote:
> > > I think it would be mildly better, but not a whole lot.  Since the
> > > Kindle describes itself as having removable media, the kernel normally
> > > probes it periodically to make sure the media remains present.  The
> > > default probing interval is 2 seconds.  Reducing it to 0.9 seconds
> > > doesn't represent an exorbitant additional load IMO -- especially since
> > > Kindles don't tend to spend huge amounts of time connected to computers.
> >
> > Ah, I did not know that the default polling interval was that low(ish),
> > given that the default indeed is already that low, then changing it to
> > 0.8 seconds would not be a big change.  And we probably have a lot of
> > lower hanging fruit for unnecessary wakeups then that.
>
> So we need to make a decision: Should the patch be merged, or should we
> punt the issue to userspace tools?
>
> On the plus side, the patch is rather small and non-invasive (although
> it does allocate the last remaining bit in the 32-bit fflags field).
> There's also the advantage of sending the extra command only when it is
> needed, as opposed to increasing the overall frequency of TUR polling.
>
> Any opinions?

I'd argue that this should be done in the kernel.

IIUC the issue can actually lead to data corruption, no? i.e. some
program writes 25 different files to different places on such an fs,
then calls fsync() on one of them but not the others. Then quite
possibly the fsync() will trigger a device disconnect sooner or later
at which point the one file surely hit the disk, but there's no
guarantee for the other 24, they might remain cached in memory and are
never written out.

I'd say quirks that are necessary to avoid data corruption should
better be done in the kernel and udev's hwdb stuff is only for stuff
that "fills in gaps", i.e. adds additional tweaks that make things
prettier, cleaner, nicer, more efficient but not things that make the
basic things work, and data integrity sounds pretty basic to me.

Or to give a counter example: the device advertises it can do media
change, but actually cannot, right, it's not a floppy drive or cdrom
driver after all? maybe hwdb would thus actually be the place for the
opposite of the suggested fix: turn off the media change polling to
reduce needless wakeups.

I mean, I'd be more open to this if this was a frequent thing and the
database for devices like this was just tooo large for the kernel to
carry, but that's not the case here either: it's two devices afaik,
and such an issue wasn't seen elswhere.

Lennart

--
Lennart Poettering, Berlin