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    [W:0.026 / U:0.136 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site