Re: [PATCH 0/5] SATA/ACPI suspend/resume support

[Jens Axboe <axboe@suse.de>]

On Tue, Jan 03 2006, Randy Dunlap wrote:
> On Tue, 3 Jan 2006 21:13:12 +0100
> Jens Axboe <axboe@suse.de> wrote:
> 
> > On Tue, Jan 03 2006, Jens Axboe wrote:
> > > On Tue, Jan 03 2006, Randy Dunlap wrote:
> > > > 
> > > > Ugh.  So the ACPI taskfile data looks like crap??
> > > > I guess that I'll just invalidate gtf_address (set to NULL)
> > > > if gtf_length looks invalid.  Is there something better to do?
> > > 
> > > Bugger me, I'm totally acpi ignorant. But did you see my note that if I
> > > clear gtf_address before calling into the function (where you pass it by
> > > reference), the problem goes away? That smells more like return-on-error
> > > without setting it at all, hence kfree() blows up when passed a bogus
> > > stack filled variable.
> 
> Yes, I'll just init gtf_length and gtf_address to 0
> in do_drive_get_GTF().

Sounds good.

> > > > Jae-hyeon,
> > > > 
> > > > Please send me a dump of your ACPI tables (acpidump).
> > > > 'acpidump' is at
> > > > http://www.kernel.org/pub/linux/kernel/people/lenb/acpi/utils/ .
> > > > Use the latest pmtools tarball.
> > > 
> > > I've attached mine as well, in case they are useful.
> 
> At least it shows that there is no _GTF at all for SATA devices,
> only for PATA (which my patch does not address).

Aren't they the same thing? Any plans on checking that as well, I think
there's a fair share of pata-on-sata laptops out there.

> > > > Jens, could you see that <gtf_address> your system is
> > > > getting that is invalid?
> > > 
> > > Sure, will reboot it after posting this message.
> > 
> > It's just 0 (because I fill it as such). do_drive_get_GTF() looks
> > suspicious, at least with noacpi != 0 it will return success without
> > filling gtf_address. Perhaps you want to treat that as an error, or
> > check for it earlier?
> > 
> > Anyways, I'm pretty sure do_drive_get_GTF() needs some fixing. I'll
> > leave that to you, let me know if you want me to test anything.
> 
> Yes, basic error on my part.  I've updated the patch series
> and rediffed it against 2.6.15.  If one of you could test it,
> that would be great (Jae-hyeon or Jens).
> 
> New patch series is at
>   http://www.xenotime.net/linux/SATA/2.6.15/
> with a combined patch at
>   http://www.xenotime.net/linux/SATA/2.6.15/libata-combine-2615.patch
> 
> If you don't want to revert the old patchset and apply the new
> one, you can just do this:
> 
> 	*gtf_length = 0;
> 	*gtf_address = 0UL;
> at the top of do_drive_get_GTF().

Works for me (well I have my version, which logically does the same
thing).

-- 
Jens Axboe