Re: [PATCH 3/4] ide: Implement disk shock protection support

Previous thread: [PATCH 2.6.28] tcp_ipv6: fix use of uninitialized memory by Vegard Nossum on Friday, September 12, 2008 - 12:05 am. (4 messages)

Next thread: 2.6.27-rc6: nohz + s2ram = need to press keys to get progress by Pavel Machek on Friday, September 12, 2008 - 1:31 am. (19 messages)
From: Elias Oltmanns
Date: Friday, September 12, 2008 - 2:55 am

Right.



The idea was that we only need to enqueue an unpark request if there is
no other request on the queue. The check is rather silly though because
other requests may be enqueued later anyway. In libata, we don't do a
similar check, so I dropped it here too. The assumption is, of course,
that a check power command is cheap, otherwise we should think again
whether we really should use it unconditionally.


For the sake of consistency, I've always tried to make ide and libata
behave alike (or as close to it as possible). However, the final version
of the libata patch is very hard to mimc in ide. Therefore, I wonder
whether we can do in ide what we'd really like to do in libata
eventually. The patch below is a real per-device implementation of the
unload feature. However, I'd like you to confirm the crucial assumption
underlying this patch: a port reset is the only way a device can
interfere with another device on the same port. In particular, I haven't
made an effort to understand pnp and similar stuff completely, but from
a first glance I got the impression that these things are done per-port
rather than per-device and that nothing sinister will happen behind our
back. In short, can you confirm the following:

Condition:  device A on a port is parked (implies there is at least one
            request on the queue of that device, i.e we hold a
            reference to the device and thus to the port).
Assumption: nothing will disturb the device because resets due to
            command failure / timeouts on device B are deferred (see my
            patch) and spurious commands like IDENTIFY (or whatever
            actions may be related to pnp and the like) are not 
            performed while the device is sleeping and a request is

Yes, of course, thanks for the hint.

Regards,

Elias

From: Elias Oltmanns <eo@nebensachen.de>
Subject: [PATCH] ide: Implement disk shock protection support

On user request (through sysfs), the IDLE IMMEDIATE command with ...
From: Elias Oltmanns
Date: Friday, September 12, 2008 - 4:55 am

Sorry for spamming you again, but I forgot to mention one more thing: In
my hardware environment, I cannot easily test the code in do_reset1()
which is supposed to defer resets if necessary. Since we have something
very similar in libata (which I have tested), I'm quite confident that
everything will work out nicely. Still, you may want to pay special
attention to this piece of code.

Regards,

Elias
--

From: Elias Oltmanns
Date: Monday, September 15, 2008 - 12:15 pm

This wake_up_all() has to go outside the if clause. I'll change this the
next time round to be called right after drive->sleep has been set. The
two nested if clauses will be unified and the conditions &&'ed.

Regards,

Elias
--

From: Bartlomiej Zolnierkiewicz
Date: Monday, September 15, 2008 - 4:22 pm

OK.

I've just audited your latest patch and it all seems good (the assumptions
taken are valid and all concerns were addressed) so you may add my ACK to
the next-time-round patch.  Big thanks for patiently improving this patch,
the final version looks so much better than the initial/draft one. :)

Thanks,
Bart
--

Previous thread: [PATCH 2.6.28] tcp_ipv6: fix use of uninitialized memory by Vegard Nossum on Friday, September 12, 2008 - 12:05 am. (4 messages)

Next thread: 2.6.27-rc6: nohz + s2ram = need to press keys to get progress by Pavel Machek on Friday, September 12, 2008 - 1:31 am. (19 messages)