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

2021-03-17 Thread Matthias Schwarzott

Am 17.03.21 um 16:17 schrieb Alan Stern:

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 would vote to do in kernel as that is a cleaner solution:

1. It will work out of the box.
2. It only sends one extra command when needed.
3. It makes the block-device not break if user-space adjusts the poll 
interval to higher values.


Matthias
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2021-03-16 Thread Matthias Schwarzott

Am 16.03.21 um 18:04 schrieb Alan Stern:

On Tue, Mar 16, 2021 at 05:43:34PM +0100, Hans de Goede wrote:


Thank you for this patch, yes if this works it would IMHO be
a much better solution then the udev rule.



Thank you for the patch.


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.

If it's merely a question of where to change the polling interval from
the default (automatically in the kernel or by userspace), that also
doesn't seem like a terribly important choice.  Certainly a udev rule or
hwdb entry is a lot easier to maintain than special-case code in the
kernel.


One question though, if this works to fix the undesired ejects,
will an actual eject (using e.g. the eject utility as say
"sudo eject /dev/sda") still be seen as an eject by the kindle
after this ?


It should be.  Maybe Matthias will try it and let us know.


Just for reference a direct answer here:
Yes, eject keeps working as it should.


Because that is actually kind of important for everyone using their
Kindle with Calibre, breaking that would not be good.


I also tried to upload a book using calibre (without disabling any 
workarounds in calibre).

The full process did work fine.

Regards
Matthias
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2021-03-16 Thread Matthias Schwarzott

Am 16.03.21 um 17:26 schrieb Alan Stern:

On Tue, Mar 16, 2021 at 06:26:30AM +0100, Matthias Schwarzott wrote:

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.


Lennart probably isn't aware how the usb-storage driver works.  It does
not create commands on its own; it merely sends the commands that it
gets from higher SCSI layers.

It may be possible to modify the SCSI core, to make it send a TEST UNIT
READY command immediately following any SYNCHRONIZE CACHE to a Kindle.

However, there may be an easier solution.  usb-storage does indeed send
a command of its own, REQUEST SENSE, to get error data when a command
fails.  The patch below will make it do the same thing whenever it sends
a SYNCHRONIZE CACHE to a Kindle, failure or not.

The only question is whether the Kindle will regard REQUEST SENSE as a
sufficient indication that it shouldn't do an eject.  The only way to
find out is by testing the patch.



The patch is a lot shorter than I expected it to be.

I tried it. The new udev-rule is commented, so polling interval is back 
at 2000 ms.


Testing it:
# cat /sys/block/sde/events_poll_msecs
-1
# cat /sys/module/block/parameters/events_dfl_poll_msecs
2000
# python -c "import time; import os; f = open('/dev/sde1', 'r+b'); 
os.fsync(f); time.sleep(4)"


This is wireshark output of the test:

85  4.701949host3.8.1   USBMS   95  SCSI: Test Unit Ready 
LUN: 0x00
86  4.7019723.8.1   hostUSB 64  URB_BULK out
87  4.701975host3.8.1   USB 64  URB_BULK in
88	4.702030	3.8.1	host	USBMS	77	SCSI: Response LUN: 0x00 (Test Unit 
Ready) (Good)
89	4.702043	host	3.8.1	USBMS	95	SCSI: Prevent/Allow Medium Removal LUN: 
0x00  PREVENT

90  4.7020693.8.1   hostUSB 64  URB_BULK out
91  4.702072host3.8.1   USB 64  URB_BULK in
92	4.703006	3.8.1	host	USBMS	77	SCSI: Response LUN: 0x00 (Prevent/Allow 
Medium Removal) (Good)


93	4.703052	host	3.8.1	USBMS	95	SCSI: Synchronize Cache(10) LUN: 0x00 
(LBA: 0x, Len: 0)

94  4.7030663.8.1   hostUSB 64  URB_BULK out
95  4.703067host3.8.1   USB 64  URB_BULK in
96	4.704146	3.8.1	host	USBMS	77	SCSI: Response LUN: 0x00 (Synchronize 
Cache(10)) (Good)

97  4.704149host3.8.1   USBMS   95  SCSI: Request Sense 
LUN: 0x00
98  4.7041773.8.1   hostUSB 64  URB_BULK out
99  4.704179host3.8.1   USB 64  URB_BULK in
100	4.705032	3.8.1	host	USBMS	82	SCSI: Data In LUN: 0x00 (Request Sense 
Response Data)

101 4.705035host3.8.1   USB 64  URB_BULK in
102	4.705053	3.8.1	host	USBMS	77	SCSI: Response LUN: 0x00 (Request 
Sense) (Good)

105 6.740272host3.8.1   USBMS   95  SCSI: Test Unit Ready 
LUN: 0x00
106 6.7403233.8.1   hostUSB 64  URB_BULK out
107 6.740326host3.8.1   USB 64  URB_BULK in
108	6.740410	3.8.1	host	USBMS	77	SCSI: Response LUN: 0x00 (Test Unit 
Ready) (Good)
195	8.709417	host	3.8.1	USBMS	95	SCSI: Prevent/Allow Medium Removal LUN: 
0x00  ALLOW

196 8.7094413.8.1   hostUSB 64  URB_BULK out
197 8.709445host3.8.1   USB 64  URB_BULK in
198	8.709645	3.8.1	host	USBMS	77	SCSI: Response LUN: 0x00 (Prevent/Allow 
Medium Removal) (Good)


The patch indeed works. The kindle does not disconnect.
I also tried with a sync on a mounted filesystem. The effect is the 
same: No disconnect.


Calling "eject /dev/sde" still works as it should.

Tested-by: Matthias Schwarzott 

Regards
Matthias
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2021-03-15 Thread Matthias Schwarzott

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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel