On Mon, Jan 13, 2014 at 12:06 PM, Todd E Brandt <todd.e.brandt@xxxxxxxxxxxxxxx> wrote: > On Fri, Jan 10, 2014 at 07:13:11PM -0800, Dan Williams wrote: >> On Fri, Jan 10, 2014 at 6:13 PM, Phillip Susi <psusi@xxxxxxxxxx> wrote: >> > -----BEGIN PGP SIGNED MESSAGE----- >> > Hash: SHA512 >> > >> > On 01/10/2014 06:11 PM, Brandt, Todd E wrote: >> >> Yes yours is simpler, but it also opens a potential memory issue >> >> by passing a static int as the return location for the error value. >> >> I think it's just safer to tell the callback to attempt no return >> >> value at all, and for that you need to expand it into two >> >> arguments, one for selection, the other for the output address. >> > >> > What sort of memory issue? Also isn't there a system NULL page >> > somewhere that could be used? >> > >> >> I think the static variable is ok. We can be sure that all eh threads >> are torn down before libata.ko is unloaded. > > Actually there's one other reason. In the ata_port_request_pm function it > checks to see if there's a previous resume operation pending, and if so > it calls ata_port_wait_eh in order to wait for it to complete before > issuing the new suspend. If you just use the (int*)async parameter it > will return immediately and defer to the caller to try again, like is does > with SAS. But in our case we *don't* try again, so it would result in the > resume being skipped. There needs to be a new case where the caller wants > the call to be asynchronous, and it wants ata_port_request_pm to do its > own waiting, but doesn't care about the return value. Thus the additional > parameter. I think that is specifically for the libata case of a suspend request arriving while an async resume is still in flight. Given libata suspends are synchronous I do not think we have the reverse problem of resume requests arriving while a suspend is in flight. However, it might be worth a WARN_ON_ONCE() to document that assumption. In the libsas case suspends are asynchronous, but they are flushed by libsas before any resumes are processed, so there should not be conflicts. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
