Hello, Elias.
Ah... we're so close but no ack yet. Just a few nits.
It would be nice if there's explanation why action pulling is
necessary in the first place.
> +static inline void ata_eh_pull_park_action(struct ata_port *ap)
> +{
> + struct ata_link *link;
> + struct ata_device *dev;
> + unsigned long flags;
> +
> + /*
> + * All write accesses to &ap->park_req_pending through
> + * INIT_COMPLETION() (see below) or complete_all() (see
> + * ata_scsi_park_store()) are protected by the host lock. As a
> + * result we have that park_req_pending.done is zero on exit
> + * from this function, i.e. when ATA_EH_PARK actions for *all*
> + * devices on port ap have been pulled into the respective
> + * eh_context structs. If, and only if, park_req_pending.done
> + * is non-zero by the time we reach
> + * wait_for_completion_timeout(), another ATA_EH_PARK action
> + * has been scheduled for at least one of the devices on port
> + * ap and we have to cycle over the do { } while () loop in
> + * ata_eh_recover() again.
> + */
...
> + do {
> + unsigned long now;
> +
> + ata_eh_pull_park_action(ap);
How about adding the folloiwng to the above line?
/* clears park_req_pending */
> +static ssize_t ata_scsi_park_store(struct device *device,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
...
> + complete_all(&ap->park_req_pending);
Sorry to catching this this late but calling complete_all() twice will
overflow the done counter. I think complete() should just work here,
no?
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
