Re: libata: implement on-demand HPA unlocking

From: Phillip Susi <psusi@cfl.rr.com>
To: Tejun Heo <tj@kernel.org>
Cc: Robert Hancock <hancockrwd@gmail.com>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Ben Hutchings <ben@decadent.org.uk>,
	Jeff Garzik <jgarzik@redhat.com>,
	IDE/ATA development list <linux-ide@vger.kernel.org>
Subject: Re: libata: implement on-demand HPA unlocking
Date: Thu, 10 Feb 2011 14:11:59 -0500	[thread overview]
Message-ID: <4D54387F.3030809@cfl.rr.com> (raw)
In-Reply-To: <20110210091325.GK3770@htj.dyndns.org>

On 2/10/2011 4:13 AM, Tejun Heo wrote:
> What happens if a drive is moved to another machine?  What about
> hotplugging a drive?  What if partition detection code incorrectly
> interprets a chunk of data in a RAID5 member as a partition table
> (most partition tables don't have good protection against
> misinterpretation)?

I don't see how any of these cases are made worse by your change.  If
you move to another machine and it adds an HPA, then your change will
unlock it.  If you incorrectly interpret a chunk of data as a partition
table, then you incorrectly unlock ( this is exactly what I am to fix ),
but that isn't worse than always unlocking.

> BIOS features are designed with the mindset "If it works on this
> motherboard and windows, we're done.  Hell with all other use cases",
> which is quite different from what Linux should be aiming for.

I agree, but always unlocking seems to HURT that goal rather than help it.

> You've got it backwards.  The patch doesn't do anything for distros
> which unlock by default.  On the contrary, the patch unlocks _more_ on
> ones which didn't use to.  IIRC, debian didn't want to unlock by
> default (the rationale was that they already had both ide and libata
> userbases) and it seemed like a good enough tradeoff to avoid breaking
> ide compatibility for most people.

You can't unlock any more than always ;)

Ubuntu has dropped its patch to always unlock in favor of your upstream
change to unlock when it makes sense.  The net result is that it unlocks
less, but still sometimes unlocks when it should not with certain raid
setups.  Also this is not just a problem for fakeraid; mdadm raid setups
can run into the same problem.

> Making the condition more intricate won't fix the problem.  It's just
> gonna make it fail in more weird and convoluted ways.

If it fails less, then it is an improvement.

> Hotplugging and being able to move hard drives between different
> machines, and in general behaving consistently across different
> hardware configurations have way higher priority than trying to avoid

Your auto unlock change seems to address that issue just fine.