__check_region in ide code?
Hi Bart,
I notice that drivers/ide/ide-probe.c's hwif_check_region()
still uses check_region(). If it really does want to use it to probe and not reserve, I think we should stop it warning there.
There's nothing inherently *wrong* with check_region, it's
just deprecated to trap the old (now racy) idiom of "if (check_region(xx)) reserve_region(xx)". There's no reason not to introduce a probe_region if IDE really wants it.
Of course, some people will start "fixing" drivers by s/check_region/probe_region/ when we do this, but that's the risk we take.
It should also allow us to easily get rid of that stupid warning in ksyms.c...
Thoughts?
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Quoted 1 line · Rusty Russell
Hi Bart,
Hi Rusty,
Quoted 8 lines
I notice that drivers/ide/ide-probe.c's
hwif_check_region()
still usescheck_region(). If it really does want to use it to probe and not reserve, I think we should stop it warning there.There's nothing inherently *wrong* with
check_region, it's
just deprecated to trap the old (now racy) idiom of "if (check_region(xx))reserve_region(xx)". There's no reason not to introduce aprobe_regionif IDE really wants it.
And ide-probe.c does exactly this racy stuff.
I did patch to convert it to request_region() some time ago, I just need to double check it and submit.
Thanks,
--
Bartlomiej
Quoted 11 lines
Of course, some people will start "fixing" drivers by s/check_region/probe_region/ when we do this, but that's the risk we take.
It should also allow us to easily get rid of that stupid warning in
ksyms.c...Thoughts?
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Quoted 9 lines · Bartlomiej Zolnierkiewicz
Quoted 4 lines
There's nothing inherently *wrong* with
check_region, it's
just deprecated to trap the old (now racy) idiom of "if (check_region(xx))reserve_region(xx)". There's no reason not to introduce aprobe_regionif IDE really wants it.And
ide-probe.cdoes exactly this racy stuff.I did patch to convert it to
request_region()some time ago, I just need to double check it and submit.
request_region at that point doesn't actually help you. For PIO devices its too late if you are handling PCMCIA, for PCI devices its too late because you want to own the PCI device properly, for MMIO its completely broken (all the mem region stuff in 2.5)
The only way I can see to fix it properly is to provide ide helpers for resource allocation that are used by the drivers when needed.
Quoted 8 lines · Alan Cox
Quoted 7 lines · Bartlomiej Zolnierkiewicz
Quoted 4 lines
There's nothing inherently *wrong* with
check_region, it's
just deprecated to trap the old (now racy) idiom of "if (check_region(xx))reserve_region(xx)". There's no reason not to introduce aprobe_regionif IDE really wants it.And
ide-probe.cdoes exactly this racy stuff.
Quoted 7 lines
Quoted 2 lines
I did patch to convert it to
request_region()some time ago, I just need to double check it and submit.
request_regionat that point doesn't actually help you. For PIO devices its too late if you are handling PCMCIA, for PCI devices its too late because you want to own the PCI device properly, for MMIO its completely broken (all the mem region stuff in 2.5)
Yes, I am aware of that.
Patch is only to fix ide-probe.c (request_region() after check_region()) not whole ide resource allocation braindamage.
Quoted 2 lines
The only way I can see to fix it properly is to provide ide helpers for resource allocation that are used by the drivers when needed.
Exactly, it is already on my todo.
--
Bartlomiej
Quoted 4 lines · Rusty Russell
I notice that drivers/ide/ide-probe.c's
hwif_check_region()
still usescheck_region(). If it really does want to use it to probe and not reserve, I think we should stop it warning there.
There's nothing inherently *wrong* withcheck_region, it's
Quoted 8 lines
just deprecated to trap the old (now racy) idiom of "if (
check_region(xx))reserve_region(xx)". There's no reason not to introduce aprobe_regionif IDE really wants it.
Of course, some people will start "fixing" drivers by s/check_region/probe_region/ when we do this, but that's the risk we take.
It should also allow us to easily get rid of that stupid warning inksyms.c...
I've actually seen IDE oops doing a racy check_region()/request_region() on 2.4.x-test*. Either the fix I brewed up never hit mainline or there's more than the one I hit.
-- wli