lkml.org 
[lkml]   [2008]   [Sep]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/4 v2] libata: Implement disk shock protection support
Hello, Elias.

Looks generally good. Just a few points.

Elias Oltmanns wrote:
> +static void ata_eh_pull_action(struct ata_link *link, struct ata_device *dev,
> + unsigned int action)
> +{
> + struct ata_port *ap = link->ap;
> + struct ata_eh_info *ehi = &link->eh_info, *ehci = &link->eh_context.i;
> + struct ata_device *tdev;
> + unsigned int taction;
> + unsigned long flags;
> +
> + spin_lock_irqsave(ap->lock, flags);
> +
> + if (dev) {
> + taction = action & (ehi->action | ehi->dev_action[dev->devno]);
> + ehci->dev_action[dev->devno] |= taction & ATA_EH_PERDEV_MASK;
> + ehci->action |= taction & ~ATA_EH_PERDEV_MASK;
> + } else {
> + if (WARN_ON(action & ATA_EH_PERDEV_MASK))
> + action &= ~ATA_EH_PERDEV_MASK;
> + ata_link_for_each_dev(tdev, link)
> + taction |= ehi->dev_action[tdev->devno] & action;

taction seems to be being used uninitialized.

> + do {
> + unsigned long now;
> +
> + deadline = jiffies;
> + ata_port_for_each_link(link, ap) {
> + ata_link_for_each_dev(dev, link) {
> + struct ata_eh_info *ehi = &link->eh_context.i;
> +
> + if (dev->class != ATA_DEV_ATA)
> + continue;
> +
> + ata_eh_pull_action(link, dev, ATA_EH_PARK);
> + if (ehi->dev_action[dev->devno] & ATA_EH_PARK) {
> + unsigned long tmp =
> + dev->unpark_deadline;
> +
> + if (time_before(deadline, tmp))
> + deadline = tmp;
> + else if (time_before_eq(tmp, jiffies))
> + continue;
> + }
> +
> + ata_eh_park_issue_cmd(dev, 1);
> + }
> + }
> + now = jiffies;
> + if (time_before_eq(deadline, now))
> + break;
> + prepare_to_wait(&ata_scsi_park_wq, &wait, TASK_UNINTERRUPTIBLE);

Doesn't prepare_to_wait() have to come before pull_action and timeout
check? Which in turn means that it should be a completion instead of
wait combined with INIT_COMPLETION because thread state can't be used
to track wake up as ata_eh_park_issue_cmd() sleeps.

Thanks. :-)

--
tejun



\
 
 \ /
  Last update: 2008-09-17 20:15    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site