Andrea Arcangeli (arcangeli@mbox.queen.it)
Sat, 18 Apr 1998 20:15:31 +0200 (CEST)
On Sat, 18 Apr 1998, Tim Waugh wrote:
>On Sat, 18 Apr 1998, Andrea Arcangeli wrote:
>
>> You are perfectly right. We are currently using the waitlist only for
>> device that are sleeping in parport_claim_or_block() while we can use it
>> for all device and run the parport_claim() that own the port inside
>> parport_release() for all devices (as it' s now done only for the device
>> sleeping in ..claim_or_block()). The use of the waitlist will allow a real
>> round-robin scheduling between devices.
>>
>> This patch will change the waitlist usage making it very smarter.
>
>I think this is probably the way things should be done, yes. However, it
>won't be done that way in 2.1 because of the freeze. I would like to only
>send real bug-fixes to Linus.
I don' t think so. Not using the wait list for _all_ pardevice was a flaw
in the last parport changes done in January. If you look at the patch it
doesn' t add new feature. It only make more precise and simple the
scheduling. It also fix the bug in parport_yiled_blocking() that need your
patch to work. The kernel is sure more instable without my patch applyed.
The only thing I should think a bit more is if ->request is SMP safe and
so if request should be atomic_t. I don' t like your way to fix the
problem since it' s not the right thing to do according to me. I am sure
Linus will agree with me.
BTW, the changes affect _only_ people is using the parport_sharing not
people is only printing or only using the ZIP drive.
>So, for example, if the "if (no-one claimed) schedule();" patch I sent to
>this list not long ago makes sense, I would rather send Linus that than
>introduce more complexity to the wait_queues. In 2.3 maybe we can do
>things like that.
The wait list is not more complex. The patch make all more simple and more
standard. As it is now there are two kind of pardevice and understand how
all works is more difficult. With my patch all pardevices will use the
wait list. As I just said before there was no reason to not use the wait
list for devices that are not using parport_claim_or_block().
>> For devices that are not sleeping in parport_claim_or_block() and that
>> provides a wakeup callback we could assume that the parport_claim() is run
>> from the callback, but since it' s so simple change that I updated all
>> (I hope ;-) pardevice to the new regime.
>>
>> If somebody will use the old wakeup callback style, recalling
>> parport_claim() inside the wakeup callback, this will not broke. Only the
>> guy will see a ton of messages like this:
>>
>> parport0: ppa already owner
>
>I'm less keen on this aspect. I think we should think more carefully
>about _why_ someone would be using the non-blocking interface in the first
>place.
ppa run parport_claim() and parport_release() from a task_queue that is
interrupt driven. It' s the same to be inside an interrupt so you can' t
sleep_on() or schedule(). At first ppa_pb_claim() was using a sleep_on()
if parport_claim() returned != 0. I removed it in order to avoid the reset
;-) some time ago.
We could leave the wakeup style the same, forcing the pardevice to claim
parport inside the wakeup callback, but if we will do that it' s only to
be backwards compatible with the old drivers code. Since the patch is
really _trivial_ I didn' t take the compatible way since the new way is
the obvious one. Grant do you agree with me here? Did you see the
paride.c patch inside my latest patch?
>> I also removed the sleep in parport_pc since I really don' t think it can
>> help.
>
>I've had zero feedback on this yet, so I'll leave it in for now. If
>people can compile without the sleep and let me know if they have
>success/failure, that would be good.
Nobody contacts me to report problems so I don' t know more than what you
can see on linux-kernel or linux-parport.
>> This patch also automatically fix the LP_PREEMPT() define in lp.h that
>> currently is wrong (I noticed it thinking about your last email).
>
>I've only skim-read your patch - please can you explain the problem with
>the macro, and how it is fixed?
It' s very simple. When I wrote LP_PREEMPT() I considered that all
pardevice was using the wait list. This can show you how much is more
complex the actual parport sharing scheme. To fix LP_PREEMPTING() with the
actual scheme we must reimplement a lp_preempt() callback and add a flag
to lp_struct to take care of the preempting. Then LP_PREEMPT() will check
that flag.
Andrea[s] Arcangeli
-- To unsubscribe, send mail to: linux-parport-request@torque.net --
-- with the single word "unsubscribe" in the body of the message. --
This archive was generated by hypermail 2.0b3 on Wed 30 Dec 1998 - 10:17:38 EST