linux-kernel - Re: [patch 1/7] libata: check for AN support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <462DB9D1.2070106@gmail.com>
Date:	Tue, 24 Apr 2007 17:03:29 +0900
From:	Tejun Heo <htejun@...il.com>
To:	Kristen Carlson Accardi <kristen.c.accardi@...el.com>
CC:	jeff@...zik.org, linux-kernel@...r.kernel.org,
	linux-scsi@...r.kernel.org, linux-ide@...r.kernel.org
Subject: Re: [patch 1/7] libata: check for AN support

Hello,

Kristen Carlson Accardi wrote:
>  static unsigned int ata_print_id = 1;
> @@ -1744,6 +1745,23 @@ int ata_dev_configure(struct ata_device 
>  		}
>  		dev->cdb_len = (unsigned int) rc;
>  
> +		/*
> +		 * check to see if this ATAPI device supports
> +		 * Asynchronous Notification
> +		 */
> +		if ((ap->flags & ATA_FLAG_AN) && ata_id_has_AN(id))
> +		{
> +			/* issue SET feature command to turn this on */
> +			rc = ata_dev_set_AN(dev);

Please don't store err_mask into int rc.  Please store it to a separate
err_mask variable and report it when printing error message.

> +			if (rc) {
> +				ata_dev_printk(dev, KERN_ERR,
> +						"unable to set AN\n");
> +				rc = -EINVAL;

Wouldn't -EIO be more appropriate?

> +				goto err_out_nosup;
> +			}
> +			dev->flags |= ATA_DFLAG_AN;
> +		}
> +

Not NACKing.  Just notes for future improvements.  We need to be more
careful here.  ATA/ATAPI world is filled with braindamaged devices and I
bet there are devices which advertises it can do AN but chokes when AN
is enabled.

This should be handled similarly to ACPI failure.  Currently ACPI does
the following.

1. try once, if fail, record that ACPI failed.  return error to trigger
retry.
2. try again, if fail again, ignore error if possible (!FROZEN) and turn
off ACPI.

This fallback mechanism for optional features can probably be
generalized and used for both ACPI and AN.

-- 
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/