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

From: Matthias Schwarzott <zzam@gentoo.org>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: linux-usb@vger.kernel.org, usb-storage@lists.one-eyed-alien.net,
	hirofumi@mail.parknet.co.jp,
	Lennart Poettering <lennart@poettering.net>,
	systemd-devel@lists.freedesktop.org
Subject: Re: [usb-storage] Re: Amazon Kindle disconnect after Synchronize Cache
Date: Tue, 16 Mar 2021 06:26:30 +0100	[thread overview]
Message-ID: <f2acfeef-a81a-d7c0-90a8-b290a18cb742@gentoo.org> (raw)
In-Reply-To: <20210311143912.GA257360@rowland.harvard.edu>

Am 11.03.21 um 15:39 schrieb Alan Stern:
> On Thu, Mar 11, 2021 at 07:05:00AM +0100, Matthias Schwarzott wrote:
>> Am 10.03.21 um 22:46 schrieb Alan Stern:
>>> On Wed, Mar 10, 2021 at 09:56:04PM +0100, Matthias Schwarzott wrote:
>>>>> What happens if you set the value to 1000 before running the test?
>>>>>
>>>> I tested different values. At 1000 it still disconnects. At lower values it
>>>> no longer does this.
>>>> I tested 200 up to 900. Even 900 ms is good enough to keep it connected.
>>>>
>>>> Btw. it is not a USB disconnect, but it just seems to plays medium ejected.
>>>>
>>>> Out of interest I called "sg_start -v -l /dev/sde" after one of the failing
>>>> experiments. That made the Kindle go back to connected state.
>>>>
>>>> To me the above experiments show that enough TEST UNIT READY commands are
>>>> needed in the 2 s after a SYNCHRONIZE CACHE.
>>>
>>> So you have found the solution to your problem.  Congratulations!
>>>
>> Thank you for your support.
>>
>> For longterm I think it should work automatically.
>> Some options I can think of (ordered by my preference):
>>
>> 1. Kernel sends one or more TEST UNIT READY commands after every SYNCHRONIZE
>> CACHE to a Kindle device. Regardless of triggered by kernel or by some user
>> code via ioctl.
>>
>> 2. Kernel automatically chooses a low enough value for events_poll_msecs if
>> it detects kindle.
>>
>> 3. udev rule is added that matches the Kindle and sets events_poll_msecs.
>>    3a) SUBSYSTEM=="block", ACTION=="add", ENV{DEVTYPE}=="disk",
>> ATTRS{product}=="Amazon Kindle", ATTR{events_poll_msecs}="900"
>>
>>    3b) SUBSYSTEM=="block", ACTION=="add", ENV{DEVTYPE}=="disk",
>> ATTRS{idVendor}=="1949", ATTRS{idProduct}=="0004",
>> ATTR{events_poll_msecs}="900"
>>
>> 4. Kernel sends one or more TEST UNIT READY commands after every SYNCHRONIZE
>> CACHE to a device (without matching).
>>
>>
>> I guess options 1 and 2 require a new entry in unusual_devs together with a
>> (new?) quirk.
> 
> It's not that simple.  usb-storage does not create SCSI commands; it
> merely sends commands that it receives from other parts of the kernel.
> 
>> Option 3 requires to get a new rule into udev.
>> And option 4 is ugly as it changes behaviour for usb-storage or scsi disk
>> device.
>>
>> I would prefer option 1 or 2.
> 
> I prefer option 3, although 2 would be acceptable in a pinch.  The main
> reason is because 3 is the only option that doesn't involve changing the
> kernel.  It's probably much easier to change a udev script.  (For
> example, that's something any user can quickly do themselves.)
> 
>> Do you know how high the overhead of having more TEST UNIT READY commands
>> is? (=How much better option 1 is compared to option 2?)
> 
> Options 1 and 4 would be rather difficult to implement.  2 and 3 are a
> lot simpler.
> 
> The overhead of TEST UNIT READY is pretty small.  You can get an idea
> for yourself by looking at the timestamps in the annotated extract of
> the usbmon log that I sent back to you.
> 

I implemented solution 3b. This is the pullrequest for udev (systemd 
repository):

	https://github.com/systemd/systemd/pull/19002

Now Lennart asks if udev is the best place for such hacks/work-arounds?

Well, I implemented it as suggested by Alan (see above). This was the 
simplest of the considered alternatives. Different quirks in kernel has 
been considered, but are more effort to be implemented.

Regards
Matthias