Linux-2.5.17

Linux-2.5.17

[Linus Torvalds]

Various FS updates (including merges of quota and iget_locked), and
Makefile cleanups from Kai.

And yet more TLB shootdown stuff.

		Linus

-----

Summary of changes from v2.5.16 to v2.5.17
============================================

<acme@conectiva.com.br>
	copy_from/to_user checking in
	 o drivers/sound/*.c
	 o fs/intermezzo/ext_attr.c
	 o drivers/isdn/*.c
	 o drivers/usr/*.c
	 o sound/{core,pci}/*.c
	 o drivers/char/*
	 o drivers/block/*.c

Andrew Morton <akpm@zip.com.au>
	o check for dirtying of non-uptodate buffers
	o reduce lock contention in do_pagecache_readahead
	o larger b_size, and misc fixlets
	o fix dirty page management
	o reiserfs locking fix
	o pdflush exclusion infrastructure
	o dirty inode management
	o i_dirty_buffers locking fix
	o ext2: preread inode backing blocks
	o pdflush exclusion
	o fix ext3 race with writeback
	o fix ext3 buffer-stealing
	o writeback tuning
	o remove PG_launder
	o improved I/O scheduling for indirect blocks

<david@gibson.dropbear.id.au>
	o Missing init.h in drivers/pci/power.c

<dmccr@us.ibm.com>
	o Thread group exit problem reappeared

Christoph Hellwig <hch@infradead.org>
	o cleanup read/write
	o Small cleanup of nfsd export checks
	o kNFSd cleanup of nfsd_open
	o get rid of <linux/locks.h>

<jack@suse.cz>
	o quota-1-newlocks
	o quota-2-formats
	o quota-3-register
	o quota-4-getstats
	o quota-5-space
	o quota-6-bytes
	o quota-7-quotactl
	o quota-8-format1
	o quota-9-format2
	o quota-10-inttype
	o quota-11-sync
	o quota-12-compat
	o quota-13-ioctl

<jaharkes@cs.cmu.edu>
	o iget_locked  [1-6]

<jhammer@us.ibm.com>
	o ips for 2.5

<kai@tp1.ruhr-uni-bochum.de>
	o Rules.make cleanup: introduce c_flags, a_flags
	o Remuve some cruft from top-level Makefile
	o Move DocBook stuff out of top-level Makefile
	o Move arch specific options to their Makefile
	o Don't implicitly export all symbols
	o top-level Makefile cleanup
	o Remove assembler rules from top-level Makefile
	o Add scripts to generate include/linux/{version,compile}.h
	o Rules.make: Use variables for commands
	o Small Rules.make cleanup
	o Rules.make: check for changed command line
	o Makefile cleanup: Don't rebuild init/version.o on each build
	o IA64: Use standard AS rule
	o x86_64: Use standard AS rule
	o Rules.make: Remove special rule for $(export-objs)
	o Fix a typo in drivers/pcmcia/Makefile
	o Fix arch/alpha/boot AS rule
	o Makefile: fix merge
	o ISDN: Export CAPI user interface directly
	o ISDN: Remove remaining MOD_{INC,DEC}_USE_COUNT from CAPI drivers
	o Make AFLAGS_KERNEL use consistent with CFLAGS_KERNEL
	o ISDN: CAPI: Remove duplicate statistics
	o ISDN: CAPI: Remove capi_interface_user etc.
	o ISDN: CAPI: Move the notification callback
	o ISDN: Have the CAPI application alloc struct capi_appl
	o ISDN: CAPI: Pass struct capi_appl * instead of index
	o ISDN: CAPI use struct capi20_appl * in signal callback
	o ISDN: CAPI: Get rid of capi_signal mechanism
	o ISDN: AVM T1 ISA CAPI controller fix
	o Update /BitKeeper/etc/ignore
	o kbuild: Use $(CURDIR)
	o kbuild: Suppress printing of '$(MAKE) -C command' line
	o kbuild: Fix object-specific CFLAGS_foo.o
	o Small fix for net/irda/Makefile
	o Fix ext2 compilation
	o Fix some compiler warnings
	o kbuild: Remove generated .<object>.cmd files on 'make clean'
	o kbuild: Standardize building of init/*
	o kbuild: Speed up vmlinux build

<mason@suse.com>
	o reiserfs bitops warnings
	o reiserfs iput deadlock fix

Neil Brown <neilb@cse.unsw.edu.au>
	o Increase snd buffer size for UDP
	o Change MD Superblock IO to go straight to submit_bio
	o Tidy up raid5 code
	o Initial md/raid5 support for 2.5 (with bio)

<torvalds@transmeta.com>
	o Clean up %cr3 loading on x86, fix lazy TLB problem
	o Fix double i_writecount handling (Tony Luck)
	o Make generic TLB shootdown friendlier to non-x86 architectures
	o Fix OSS API emulation when sound is compiled as a module
	o Update kernel version to 2.5.17
	o New makefiles generate .*.cmd files, not .*.flags files

Re: Linux-2.5.17

[Roman Zippel]

Hi,

On Mon, 20 May 2002, Linus Torvalds wrote:

> And yet more TLB shootdown stuff.

I'm a bit puzzled, how you want to do proper rss accounting, you put now a
"tlb->freed++;" into zap_pte_range(). mmu_gather_t is supposed to be an
opaque type and this access violates this.

bye, Roman

Re: Linux-2.5.17

[Linus Torvalds]

On Tue, 21 May 2002, Roman Zippel wrote:
> On Mon, 20 May 2002, Linus Torvalds wrote:
>
> > And yet more TLB shootdown stuff.
>
> I'm a bit puzzled, how you want to do proper rss accounting, you put now a
> "tlb->freed++;" into zap_pte_range(). mmu_gather_t is supposed to be an
> opaque type and this access violates this.

I don't think there is any validity any more in the "opaque type" comment,
and I'd rather expose the fact that it _has_ to have the rss computations
inside of it than have more made-up interfaces to hide it.

The fact is, the rss cannot be computed anywhere else any more, so why
play games about it?

		Linus

Re: Linux-2.5.17

[Roman Zippel]

Hi,

Linus Torvalds wrote:

> I don't think there is any validity any more in the "opaque type" comment,
> and I'd rather expose the fact that it _has_ to have the rss computations
> inside of it than have more made-up interfaces to hide it.
> 
> The fact is, the rss cannot be computed anywhere else any more, so why
> play games about it?

Basically I could agree with it, but something looks wrong. Why exactly
is pte_free_tlb() needed in first place? Why does it call
tlb_remove_page()? A page mapped into user space has little to do with a
page used as page table. Latter is never in the user tlb, so it doesn't
need to be removed from it, so calling tlb_remove_page() is just a more
complicated way of calling __free_page() or am I missing something?

bye, Roman

Re: Linux-2.5.17

[Linus Torvalds]

On Tue, 21 May 2002, Roman Zippel wrote:
> 
> Basically I could agree with it, but something looks wrong. Why exactly
> is pte_free_tlb() needed in first place? Why does it call
> tlb_remove_page()?

That is a x86-specific thing, not aarchitected.

The _architected_ thing is

 - pte_free() does the physical free of a pte pointer that was allocated 
   but never inserted into the page tables due to optimistic locking (see 
   pte_alloc_map() in mm/memory.c).

 - pte_free_tlb() does the same BUT it is also an architecture-specific 
   hook to allow the architecture to also some way shoot down whatever TLB 
   contents that might depend on the pmd_page in question.

   On x86, we do that by just adding it as another page to teh tlb flush 
   stuff, but other architectures might just make it be the same as 
   pte_free() if there are no TLB issues involved.

If you care, the reason we need to do this on x86 is that the TLB walker
is speculative and almost totally asynchronous wrt the rest of the CPU
core, so we may have a CPU "TLB lookup thread" goin on in parallel with 
the TLB cleaning - and that TLB lookup may have looked up the pmd contents 
already but not resolved the entry yet. Which is why we have to 
synchronize the PMD freeing with the TLB flush - the same way we already 
have to do it for the regular data pages.

Other architectures may not have this issue (or you can fix it with
alternative approaches, like using the pmd quicklists etc to avoid freeing
the pmd before the TLB flush, which is likely to be the fix in the 2.4.x
tree).

		Linus

Re: Linux-2.5.17

[Roman Zippel]

Hi,

Linus Torvalds wrote:

> If you care, the reason we need to do this on x86 is that the TLB walker
> is speculative and almost totally asynchronous wrt the rest of the CPU
> core, so we may have a CPU "TLB lookup thread" goin on in parallel with
> the TLB cleaning - and that TLB lookup may have looked up the pmd contents
> already but not resolved the entry yet. Which is why we have to
> synchronize the PMD freeing with the TLB flush - the same way we already
> have to do it for the regular data pages.

Alternative suggestion: remove the present bit from the pgd/pmd entry.
After you flushed the tlb, you can clean up the page tables without a
hurry. That will work on any sane system and you don't have to force
data and table pages into the same interface.

bye, Roman

Re: Linux-2.5.17

[Linus Torvalds]

On Wed, 22 May 2002, Roman Zippel wrote:
>
> Alternative suggestion: remove the present bit from the pgd/pmd entry.
> After you flushed the tlb, you can clean up the page tables without a
> hurry. That will work on any sane system and you don't have to force
> data and table pages into the same interface.

Sounds sane, except for the fact that some architectures do not actually
care about the "Present" bit in the pgd at all.

x86, to be exact ;(

		Linus

Re: Linux-2.5.17

[Roman Zippel]

Hi,

On Tue, 21 May 2002, Linus Torvalds wrote:

> > Alternative suggestion: remove the present bit from the pgd/pmd entry.
> > After you flushed the tlb, you can clean up the page tables without a
> > hurry. That will work on any sane system and you don't have to force
> > data and table pages into the same interface.
> 
> Sounds sane, except for the fact that some architectures do not actually
> care about the "Present" bit in the pgd at all.
> 
> x86, to be exact ;(

IMO that's not really problem, the pmd tables are created and destroyed
with the pgd table.

bye, Roman

Re: Linux-2.5.17

[Linus Torvalds]

On Wed, 22 May 2002, Roman Zippel wrote:
> >
> > x86, to be exact ;(
>
> IMO that's not really problem, the pmd tables are created and destroyed
> with the pgd table.

unmap()?

That's the big one, actually. The exit case we _could_ do very differently
anyway, and there are reasons that we probably should try to.

(When we exit, we could flush the TLB and at the same time do a
"speculative" switch to the mm of the next process on the run-queue of
this CPU, so that when we actually tear down the MM we would have no TLB
issues at all any more).

		Linus

Re: Linux-2.5.17

[David S. Miller]

   From: Linus Torvalds <torvalds@transmeta.com>
   Date: Tue, 21 May 2002 17:54:18 -0700 (PDT)

   That's the big one, actually. The exit case we _could_ do very differently
   anyway, and there are reasons that we probably should try to.
   
   (When we exit, we could flush the TLB and at the same time do a
   "speculative" switch to the mm of the next process on the run-queue of
   this CPU, so that when we actually tear down the MM we would have no TLB
   issues at all any more).

I think deferring this to the lazy TLB end at the next task switch is
worth pursuing.

I always wanted to also explore way to speed up these pieces of code
we have which walk the page table tree to kill everything off.
Something simple like a very small bitmap in the mm_struct.  It would
work by keeping track of which areas of the address space actually
have some mappings present.  The set bits would be kept track of
pessimistically, to keep it fast and simple.

So when you add a page mapping somewhere you'd go:

	set_mapping_bit(mm, address);

Then exit_mmap() would only traverse into parts of the page
tables where mappings actually existed.

Similarly for copy_page_range when dup'ing an address space.

This stuff shows up clearly on the fork/exit/exec microbenchmark
profiles.

Like I said, keep the bitmap very small, perhaps 4 unsigned longs
at the most.

Actually, what this suggests is that we blow away the page table
flushing guts of exit_mmap() and just have this
"anihilate_address_space()" thing that is %100 arch-specific and can
be used to optimize this as much as a platform wants to.  We can even
provide a "boring" generic implementation protected by
HAVE_ARCH_ANIHILATE_ADDRESS_SPACE that basically looks like what we
have there today.  (The interface name sucks, I know, sorry, we'll
will have to come up with a nicer name :-)

Re: Linux-2.5.17

[Linus Torvalds]

On Tue, 21 May 2002, David S. Miller wrote:
>
> I think deferring this to the lazy TLB end at the next task switch is
> worth pursuing.

No can do.

If we tear down the page tables, we _have_ to flush the TLB on x86,
because even if we don't touch them later on, speculative execution may
end up causing TLB fills, and if we don't tell the TLB fill hw that we've
torn down the pages (by invalidating the TLB), you can get all the same
nasty behaviour.

And we cannot just defer the TLB flush to a later date ("who cares if we
get crap in the TLB, we'll flush it anyway"), because some of the bogus
TLB contents might get the "Global" bit set too. Which would mean that
those bogus entries wouldn't be flushed at all.

In short:
 - if we tear down the page tables, we _have_ to flush the TLB, even if we
   turn it into a lazy TLB.
 - At least on x86, once you flush the TLB, the incremental cost of doing
   a full mm switch is basically zero. The TLB flush is, after all, the
   real cost of the mm switch (this is likely to be true on other CPU's
   too).
 - so we can choose between just flushing the TLB (and leaving it lazy),
   and then on the next switch_mm() we flush it again when we switch into
   the next process, _OR_ we could try to opportunistically switch mm's
   "early".

The early switch would at least on x86 be likely to result in the minimal
amount of TLB flushing theoretically possible. Which I kind of like (if
you can _prove_ that you cannot do better, you're in a good position ;).

But the "just flush the TLB" approach certainly also works.

		Linus

Re: Linux-2.5.17

[David S. Miller]

   From: Linus Torvalds <torvalds@transmeta.com>
   Date: Tue, 21 May 2002 19:40:08 -0700 (PDT)
   
   The early switch would at least on x86 be likely to result in the minimal
   amount of TLB flushing theoretically possible. Which I kind of like (if
   you can _prove_ that you cannot do better, you're in a good position ;).
   
Probably on sparc64 too.  The simplest way to kill off a TLB context
on sparc64 at exit_mmap() is to just mark it invalid (this means just
clearing the cpu_vm_mask of the mm_struct using that context PID).

It is even simpler than that, at exit_mmap() time we are destroying
the mm_struct anyways, nobody references it, and thus destroy_context
does all of the work.

Unfortunately, today mmdrop() (which is where destroy_context is
invoked) happens after exit_mmap().

Maybe some kind of "switch_from_dead_context()" type of thing?

Re: Linux-2.5.17

[Linus Torvalds]

On Tue, 21 May 2002, David S. Miller wrote:
>
> Unfortunately, today mmdrop() (which is where destroy_context is
> invoked) happens after exit_mmap().
>
> Maybe some kind of "switch_from_dead_context()" type of thing?

Yes, I was thinking of an extra step like that.

The problem is just finding a _good_ context to switch to. We can do this
two different ways:

 - actually doing a real context switch, but with a magic
   "schedule_tail()" that ends up being the rest of do_exit(). This is
   _really_ hard to get right, and implies that everything after the
   context switch has to be non-blocking (since we'd block in the "wrong"
   process context at that point.

 - my preferred solution: speculatively find _some_ process (preferably
   one that we are likely to schedule next), and use that process's
   "active_mm" to do a "switch_mm()" into (and set that to "current->mm")

   This is kind of like the lazy TLB thing, except going the other way.

The speculative thing has the problem of finding a good process, but I
would suggest something along the lines of:

 - take the first process in the run-queue on the current CPU.
 - if there is no process on th erun-queue, take our parent

The "parent" fallback is nice because (a) we're guaranteed to have a
parent and it is easily found and (b) we're going to wake our parent up
soon enough in "notify_parent()", so if the current runqueue is empty, the
parent is one of the likelier processes to end up there..

But no, I've not looked into the details. We've never stolen a mm from
anybody else before (lazy TLB _gives_ a mm to the next process, it doesn't
take it from anybody), so it might have nasty locking issues or something.

			Linus

Re: Linux-2.5.17

[David Lang]

what about SMP where you may have multiple children hit this at the same
time on different CPUs?

David Lang

On Tue, 21 May 2002, Linus Torvalds wrote:
>  - if there is no process on th erun-queue, take our parent
>
> The "parent" fallback is nice because (a) we're guaranteed to have a
> parent and it is easily found and (b) we're going to wake our parent up
> soon enough in "notify_parent()", so if the current runqueue is empty, the
> parent is one of the likelier processes to end up there..
>
> But no, I've not looked into the details. We've never stolen a mm from
> anybody else before (lazy TLB _gives_ a mm to the next process, it doesn't
> take it from anybody), so it might have nasty locking issues or something.
>
> 			Linus

Re: Linux-2.5.17

[Dave McCracken]

--On Tuesday, May 21, 2002 08:21:56 PM -0700 Linus Torvalds
<torvalds@transmeta.com> wrote:

> The problem is just finding a _good_ context to switch to. We can do this
> two different ways:
> 
> (...)
> 
>  - my preferred solution: speculatively find _some_ process (preferably
>    one that we are likely to schedule next), and use that process's
>    "active_mm" to do a "switch_mm()" into (and set that to "current->mm")
> 
> The speculative thing has the problem of finding a good process, but I
> would suggest something along the lines of:
> 
>  - take the first process in the run-queue on the current CPU.
>  - if there is no process on th erun-queue, take our parent

What would be the incremental cost of just switching to init_mm?  Granted,
it's likely to require switching again when you schedule, but this is the
exit path.  It could be a fallback if nothing else looks good.

Dave McCracken

======================================================================
Dave McCracken          IBM Linux Base Kernel Team      1-512-838-3059
dmccr@us.ibm.com                                        T/L   678-3059

Re: Linux-2.5.17

[Linus Torvalds]

On Wed, 22 May 2002, Dave McCracken wrote:
>
> What would be the incremental cost of just switching to init_mm?

Pretty much zero.

Switching to init_mm is the easy approach with no real downside, it just
has the downside that it's also guaranteed to have no upside (ie there is
no win on the _next_ context switch).

			Linus

Re: Linux-2.5.17

[Roman Zippel]

Hi,

On Tue, 21 May 2002, Linus Torvalds wrote:

> > > x86, to be exact ;(
> >
> > IMO that's not really problem, the pmd tables are created and destroyed
> > with the pgd table.
> 
> unmap()?

We already don't let the general vm touch the pgd entries for the same
reason, so I don't think that's really a big problem.
Using the present bit has another consequence. unmap() had to be done in
two phases:
1. Disable the table entries at the highest possible level. Using the
previous and following vma avoids scanning the tables (something like
free_pgtables already does, only more accurate).
2. Scan the tables and free all the disabled entries. At this point we
don't have worry about any tlb issues anymore.

I can see a few advantages doing it this way. The first phase could be
quite fast even for large unmaps and so reducing the time holding the
page_table_lock. It avoids the race mentioned by Paul (although a
ptep_clear_present() would still be needed). It would also free up more
unused tables. The tlb shootdown stuff would be simpler as well.
On the other hand it's a rather rough idea and I don't know how feasible
it really is, but without the exit case it should become easier and IMO
worth a try. 

bye, Roman

Re: Linux-2.5.17

[Linus Torvalds]

On Wed, 22 May 2002, Roman Zippel wrote:
>
> We already don't let the general vm touch the pgd entries for the same
> reason, so I don't think that's really a big problem.
> Using the present bit has another consequence. unmap() had to be done in
> two phases:

I don't disagree. Are you interested in trying to write it up? It sounds
like a potentially good idea, with few downsides (but I can imagine some:
it does bad things to threads that just happen to share the same 4M area
for other stuff, and that start getting spurious page faults on another
CPU because _their_ area temporarily went away from under them).

I also suspect that it might simplify the TLB shootdown enough that we
wouldn't _have_ to split out the exit case and could use the shared
zapping. But I'm kind of worried about the potential threading issues.

(Rule of thumb: it's always a bad idea to cut down on parallelism, and
we'll _really_ be up shit creek if some threaded app comes along later
where munmap() ends up serializing threads too much).

			Linus

[PATCH] 2.5.17 IDE 65

[Martin Dalecki]

[-- Attachment #1: Type: text/plain, Size: 1077 bytes --]

Wed May 15 15:04:58 CEST 2002 ide-clean-65:

- Apply cleanup of host chip drivers by Bartomiej Zonierkiewicz:

     affected drivers - aec62xx.c, alim15x3.c, cmd64x.c, hpt34x.c, sis5513.c

     new tuning scheme (wip) part 1:
	- introduce ratemask()
	- use ata_timing_mode()
	- use ide_config_drive_speed() return value

     forward port from convert.10:
	- support for AEC6280, AEC6280R
	- misc cleanups

     I had to fix a small typo in sis5513 code...

- Add a new entry for an old VIA cell hiding as something new.
   (Pointed out by Kees Bakker.)

- Make the synchronization token active resident on the same level as the
   spin lock. They interact with each other until the generic queue handling
   gets sanitized to not attach hardware properties like the hard sector size
   to the queue entities. This is a design mistake in ll_rw_blk biting everybody
   out there.

- Synchronize with linux-2.5.17.

- HPT366 driver typo fix by Andries Brouwer.

- Export udma_tcq_enable() symbol right now. The blk_get_request() is undefined
   as well.

Time for a patch round.

[-- Attachment #2: ide-clean-65.diff.gz --]
[-- Type: application/x-gzip, Size: 21873 bytes --]

Re: [PATCH] 2.5.17 IDE 65

[Linus Torvalds]

On Tue, 21 May 2002, Martin Dalecki wrote:

> - Make the synchronization token active resident on the same level as the
>    spin lock. They interact with each other until the generic queue handling
>    gets sanitized to not attach hardware properties like the hard sector size
>    to the queue entities. This is a design mistake in ll_rw_blk biting everybody
>    out there.

This does not parse. It is _not_ a design mistake in ll_rw_blk - if it
bites you, you're doing something wrong.

The queue should be a per-device thing. If you have multiple devices with
different hard-sector-sizes (or other queue attributes) on the same queue,
that's _your_ problem, not the problem of ll_rw_block.

Sure, ll_rw_block _allows_ you to use the same queue for multiple devices,
but if you do that you only have yourself to blame, and you get:

 - shared values (like the hardsector-size above)
 - worse elevator performance (longer queues to traverse)
 - worse elevator schedules (mixing devices will caused mixed queue
   contents, which makes it basically impossible to do a good ordering)

I thought the IDE layer already did the "one queue per device" thing, is
there somewhere where this isn't true?

In short, I think whatever synchronization token problem there is is
completely an internal IDE problem, and no blame should be laid at anybody
else.

		Linus

Re: [PATCH] 2.5.17 IDE 65

[Martin Dalecki]

Użytkownik Linus Torvalds napisał:
> 
> On Tue, 21 May 2002, Martin Dalecki wrote:
> 
> 
>>- Make the synchronization token active resident on the same level as the
>>   spin lock. They interact with each other until the generic queue handling
>>   gets sanitized to not attach hardware properties like the hard sector size
>>   to the queue entities. This is a design mistake in ll_rw_blk biting everybody
>>   out there.
> 
> This does not parse. It is _not_ a design mistake in ll_rw_blk - if it
> bites you, you're doing something wrong.
> 
> The queue should be a per-device thing. If you have multiple devices with
> different hard-sector-sizes (or other queue attributes) on the same queue,
> that's _your_ problem, not the problem of ll_rw_block.

I didn't change the behaviour with respect to this.

But please consider the following points:

1. Layered devices. md. lvm raid  and so on, over mutliple physical
disks with possible different properties. Due to they nature
they have to handle the case of multiple block sizes.
The only reason this isn't biting us here right now is the
simple fact that most disks just stick with the dreaded 512 byte sector 
addressing, but there are people out there esp. from Maxtor
complaining about this... which would love to expand the smallest
transfer unit. It's just natural that this will be a subject
to change. Please note that most file systems out there are
already acting on expanded sector sizes indeed.

2. Removable media like CD-ROM contain already somehow inherently
the property of needing to deal with different sector sizes.
There are for example 1024 and 2048 byte modes for CD-ROMs.

3. I would love it to handle multiple sector transfers just as
big hard-sector sizes :-). Not quite perfectsolution ins some
corner cases but it would simplify many things inside the driver.
However this is just an idea I'm thinking about it and I didn't
came to any final conclusion about this.

> Sure, ll_rw_block _allows_ you to use the same queue for multiple devices,
> but if you do that you only have yourself to blame, and you get:

Linus - recycling the same request queue would simplify the
serialization issues by a significant amount. We have already
a mechanism for passing the spin lock to use to the upper
layer (blk_init_queue()). It would be just natural if it was
acting as kind of a true semaphore for overall request serialization. But
currently it's just used to serialize the access to the corresponding
queue lists, which doesn't buy us *anything*,
since request finish ACK happens asynchronously. And therefore
it doesn't make much sense indeed. It would really make only sense
if it would be kind of a semaphore-token shared between the queues in
question - which it isn't right now.

>  - shared values (like the hardsector-size above)
>  - worse elevator performance (longer queues to traverse)
>  - worse elevator schedules (mixing devices will caused mixed queue
>    contents, which makes it basically impossible to do a good ordering)

This wouldn't happen, since the intention would be to use
them only in case of the devices which need to be serialized
with each other.

> I thought the IDE layer already did the "one queue per device" thing, is
> there somewhere where this isn't true?

Yes it does it this way and this isn't subject to change
any time soon on sane hardware. After looking at the ll_rw_blk
code close enough I have already dropped the idea of different
queues for ATA and ATAPI reuqests. The point which I'm spinning
around are just the few "insane" cases where total
request serialization between different devices would help
to assert physical register access requirements.
A case where the above performance issues are not
prohibitive in my opinnion.

> In short, I think whatever synchronization token problem there is is
> completely an internal IDE problem, and no blame should be laid at anybody
> else.

Well if you are looking at it. Please have a look at the following
struct request memebers as well:

[root@kozaczek linux]# find ./ -name "*.[ch]" -exec grep hard_nr_sectors 
/dev/null {} \;

./include/linux/blkdev.h:       unsigned long hard_nr_sectors;
./drivers/ide/ide-cd.c: rq->hard_nr_sectors = rq->nr_sectors;
./drivers/ide/ide-taskfile.c:           __ide_end_request(drive, rq, 1, 
rq->hard_nr_sectors);
./drivers/block/ll_rw_blk.c:    unsigned long blocks = rq->hard_nr_sectors / 
(hard_sect >> 9);
./drivers/block/ll_rw_blk.c:            req->nr_sectors = req->hard_nr_sectors 
+= next->hard_nr_sectors;
./drivers/block/ll_rw_blk.c:                    req->nr_sectors = 
req->hard_nr_sectors += nr_sectors;
./drivers/block/ll_rw_blk.c:                    req->nr_sectors = 
req->hard_nr_sectors += nr_sectors;
./drivers/block/ll_rw_blk.c:    req->hard_nr_sectors = req->nr_sectors = nr_sectors;
./drivers/block/ll_rw_blk.c:            rq->hard_nr_sectors -= nsect;
./drivers/block/ll_rw_blk.c:            rq->nr_sectors = rq->hard_nr_sectors;

You will notice that the "other" hard_blah members are used
inside the IDE layer only as well for the running request and much
in pair with the normal members of struct request...
I'm planing right now how to push them down to the only
palce where they are used - namely the IDE layer next.
But I see that this has to be done *very carefully*.

The other things which ll_rw_blk.c doesn't get right is the
fact that the current merge functions don't respect hard sector
sizes, despite of attaching them to every single request out there.
Since we have the requests in device context you can hardly
argue that this is at least smelling funny since this information
is already accessible through the device context and shouldn't
be duplicated...

*Those* are the (minor) things which did lead me to the conclusion
that something isn't (quite) right there. OK?

Re: [PATCH] 2.5.17 IDE 65

[Linus Torvalds]

On Tue, 21 May 2002, Martin Dalecki wrote:
>
> But please consider the following points:
>
> 1. Layered devices.

Sure. But to the upper layers they _do_ have a single hardsector-size.
They look (by definition) like one device, and th eupper layers must not
know that the physical devices underneath may end up having different
sector sizes.

Which is why a md queue must have a sector size that is at least as large
as the largest sector size of any of the devices underneath it.

> The only reason this isn't biting us here right now is the
> simple fact that most disks just stick with the dreaded 512 byte sector
> addressing, but there are people out there esp. from Maxtor
> complaining about this...

Absolutely not. Even if Maxtor were to do a 2kB-sector disk, that only
means that the md layer would have to make a 2kB-sector md device.

We have the support for all of this already, as many (most?) SCSI CD-ROM's
are 2kB-only.

> 2. Removable media like CD-ROM contain already somehow inherently
> the property of needing to deal with different sector sizes.
> There are for example 1024 and 2048 byte modes for CD-ROMs.

Sure. We have allt he support for this, and the queue sector size can (and
does) change when you change a removable media.

I repeat: there is no design limitation in ll_rw_block().

> 3. I would love it to handle multiple sector transfers just as
> big hard-sector sizes :-)

That's up to _you_, the driver.

The driver can choose to consider 2 512-byte sectors to be equivalent to 1
1024-byte sector. The _only_ thing the "hardsector size" means is that the
driver _guarantees_ that it can handle any IO request of that size. It's
really a promise upwards toward higher layers, not a limitation on the
driver.

> Linus - recycling the same request queue would simplify the
> serialization issues by a significant amount.

I don't see that at ALL.

The ll_rw_blk code _explicitly_ has a spinlock pointer (instead of a plain
spinlock) in the queue structure _exactly_ because the code knows (and
expects) different devices to want to have common synchronization.

Any other synchronization is totally up to the driver: a driver can, at
any point, just stop doing requests and waiting until all of its requests
are done, if it wants to find some "idle point".

> We have already
> a mechanism for passing the spin lock to use to the upper
> layer (blk_init_queue()). It would be just natural if it was
> acting as kind of a true semaphore for overall request serialization. But
> currently it's just used to serialize the access to the corresponding
> queue lists, which doesn't buy us *anything*,

That's a load of baloney.

If you want a semaphore in the driver, you just add one. There is
absolutely _nothing_ in the upper layers that wants to have a semaphore,
never has been, and never will be. It's a internal driver issue, and as
such no such semaphore should ever be exposed to upper layers.

The upper layers do not care, and CANNOT care. They put requests on the
queue, and if the lower layer is serializing itself for some reason, the
upper layer has no reason to know - except by the fact that the results
don't come back, of course.

What would the upper layers do with the semaphore? Nothing.

> The other things which ll_rw_blk.c doesn't get right is the
> fact that the current merge functions don't respect hard sector
> sizes

They aren't there to be respected by the ll_rw_blk layer - if some layer
above it has created a request larger than the hard sector size, THAT is
the problem, and there is nothing ll_rw_blk can do (except maybe BUG() on
it, but I don't think we've ever really seen those kinds of bugs).

		Linus

Re: [PATCH] 2.5.17 IDE 65

[Alan Cox]

> Absolutely not. Even if Maxtor were to do a 2kB-sector disk, that only
> means that the md layer would have to make a 2kB-sector md device.
> 
> We have the support for all of this already, as many (most?) SCSI CD-ROM's
> are 2kB-only.

We also support M/O disks. Ext2 fs with a block size >= the block size of
the media works well. 512byte FATfs needs loop. I've been using 2K media
on and off for a long time. Our design limit is page size.

It all works fine in 2.2 and 2.4

Re: [PATCH] 2.5.17 IDE 65

[Vojtech Pavlik]

On Tue, May 21, 2002 at 10:56:14AM -0700, Linus Torvalds wrote:

> > The other things which ll_rw_blk.c doesn't get right is the
> > fact that the current merge functions don't respect hard sector
> > sizes
> 
> They aren't there to be respected by the ll_rw_blk layer - if some layer
> above it has created a request larger than the hard sector size, THAT is
> the problem, and there is nothing ll_rw_blk can do (except maybe BUG() on
> it, but I don't think we've ever really seen those kinds of bugs).

Hum, I'm confused here - shouldn't that be "if some layer above it has
created a request SMALLER than the hard sector size"? Or better a
request that is not a multiple of hard sector size?

-- 
Vojtech Pavlik
SuSE Labs

Re: [PATCH] 2.5.17 IDE 65

[Linus Torvalds]

On Tue, 21 May 2002, Vojtech Pavlik wrote:
> > 
> > They aren't there to be respected by the ll_rw_blk layer - if some layer
> > above it has created a request larger than the hard sector size, THAT is
> > the problem, and there is nothing ll_rw_blk can do (except maybe BUG() on
> > it, but I don't think we've ever really seen those kinds of bugs).
> 
> Hum, I'm confused here - shouldn't that be "if some layer above it has
> created a request SMALLER than the hard sector size"? Or better a
> request that is not a multiple of hard sector size?

Yes, yes, you're obviously right, and I just had a brainfart when writing
it. It should be basically: "higher levels must make sure on their own
that all requests are nice integer multiples of the hw sector-size", and 
ll_rw_blk should never have to care.

		Linus "neurons dying left and right" Torvalds

Re: [PATCH] 2.5.17 IDE 65

[Martin Dalecki]

Uz.ytkownik Linus Torvalds napisa?:
> On Tue, 21 May 2002, Vojtech Pavlik wrote:
> 
>>>They aren't there to be respected by the ll_rw_blk layer - if some layer
>>>above it has created a request larger than the hard sector size, THAT is
>>>the problem, and there is nothing ll_rw_blk can do (except maybe BUG() on
>>>it, but I don't think we've ever really seen those kinds of bugs).
>>
>>Hum, I'm confused here - shouldn't that be "if some layer above it has
>>created a request SMALLER than the hard sector size"? Or better a
>>request that is not a multiple of hard sector size?
> 
> 
> Yes, yes, you're obviously right, and I just had a brainfart when writing
> it. It should be basically: "higher levels must make sure on their own
> that all requests are nice integer multiples of the hw sector-size", and 
> ll_rw_blk should never have to care.

Please add the following to the bag:
"We never saw a filesystem with less then 512 byte sectors,
so let's assume this is our request size unit." (CP/M uses 256...)
Not that pretty at all.

Re: [PATCH] 2.5.17 IDE 65

[Kai Henningsen]

dalecki@evision-ventures.com (Martin Dalecki)  wrote on 22.05.02 in <3CEB4084.90806@evision-ventures.com>:

> Uz.ytkownik Linus Torvalds napisa?:
> > On Tue, 21 May 2002, Vojtech Pavlik wrote:
> >
> >>>They aren't there to be respected by the ll_rw_blk layer - if some layer
> >>>above it has created a request larger than the hard sector size, THAT is
> >>>the problem, and there is nothing ll_rw_blk can do (except maybe BUG() on
> >>>it, but I don't think we've ever really seen those kinds of bugs).
> >>
> >>Hum, I'm confused here - shouldn't that be "if some layer above it has
> >>created a request SMALLER than the hard sector size"? Or better a
> >>request that is not a multiple of hard sector size?
> >
> >
> > Yes, yes, you're obviously right, and I just had a brainfart when writing
> > it. It should be basically: "higher levels must make sure on their own
> > that all requests are nice integer multiples of the hw sector-size", and
> > ll_rw_blk should never have to care.
>
> Please add the following to the bag:
> "We never saw a filesystem with less then 512 byte sectors,
> so let's assume this is our request size unit." (CP/M uses 256...)
> Not that pretty at all.

That's why Alan said 512-byte FAT on 2k MO needs loop.

Of course, way back when, I used 2k FAT on MO and it "just worked" ... no  
idea if that would still work today, but FAT *can* at least in principle  
do larger sizes.

MfG Kai

[PATCH] 2.5.17 IDE 66

[Martin Dalecki]

[-- Attachment #1: Type: text/plain, Size: 1496 bytes --]

Tue May 21 18:36:04 CEST 2002 ide-clean-66

- Move ll_10byte_cmd_build to the only place where it's used: ide-cd.  The SCSI
   layer does have it's own implementation which additionally it's messing
   around with the hard_nr_sectors struct request value.  One should *not*
   provide "infrastructure" until its really used as such.

   If anywhere this should reside in a file called ATAPI.

- Unfold the INIT_REQUEST macro from blk.h. This showed up plenty of duplicate
   checks for QUEUE_EMPTY. Clean them as well. Remove the over cautious
   major(CURRENT->rq_dev != MAJOR_NR) checks. During the last several years I
   never saw any report about it. Looking at the !CURRENT->bio it is clear that
   dereferencing NULL will provide the same kind of panic as the check.  Some
   comments around the code in question show nicely that indeed INIT_REQUEST
   was a good example of code obfuscation.

- A short look at RQ_INACTIVE shows that it is only used inside the scsi.c file
   and during the removal of devices. This shows that the many checks for
   RQ_INACTIVE are not necessary. Looking closer even shows that some of them
   did happen before checks for an empty queue. Plenty of drivers didn't care
   about it and the CD-ROM ones should be handled properly, because the
   most common drivers would fail as well. Comments indicate that this
   was an leftover from 1.3 days...

Well indeed this patch turned out to be more about blkdev handling then
ATA device handling, but anyway.

[-- Attachment #2: ide-clean-66.diff --]
[-- Type: text/plain, Size: 16717 bytes --]

diff -urN linux-2.5.17/arch/m68k/atari/stram.c linux/arch/m68k/atari/stram.c
--- linux-2.5.17/arch/m68k/atari/stram.c	2002-05-21 07:07:33.000000000 +0200
+++ linux/arch/m68k/atari/stram.c	2002-05-22 01:03:39.000000000 +0200
@@ -984,8 +984,11 @@
 	unsigned long len;
 
 	while (1) {
-		INIT_REQUEST;
-		
+		if (blk_queue_empty(QUEUE)) {
+			CLEAR_INTR;
+			return;
+		}
+
 		start = swap_start + (CURRENT->sector << 9);
 		len   = CURRENT->current_nr_sectors << 9;
 		if ((start + len) > swap_end) {
diff -urN linux-2.5.17/drivers/acorn/block/mfmhd.c linux/drivers/acorn/block/mfmhd.c
--- linux-2.5.17/drivers/acorn/block/mfmhd.c	2002-05-21 07:07:33.000000000 +0200
+++ linux/drivers/acorn/block/mfmhd.c	2002-05-22 01:12:52.000000000 +0200
@@ -889,17 +889,6 @@
 {
 	DBG("mfm_request CURRENT=%p Busy=%d\n", CURRENT, Busy);
 
-	if (QUEUE_EMPTY) {
-		DBG("mfm_request: Exited due to NULL Current 1\n");
-		return;
-	}
-
-	if (CURRENT->rq_status == RQ_INACTIVE) {
-		/* Hmm - seems to be happening a lot on 1.3.45 */
-		/*console_printf("mfm_request: Exited due to INACTIVE Current\n"); */
-		return;
-	}
-
 	/* If we are still processing then return; we will get called again */
 	if (Busy) {
 		/* Again seems to be common in 1.3.45 */
@@ -914,16 +903,14 @@
 		DBG("mfm_request: loop start\n");
 		sti();
 
-		DBG("mfm_request: before INIT_REQUEST\n");
+		DBG("mfm_request: before blk_queue_empty\n");
 
-		if (QUEUE_EMPTY) {
-			printk("mfm_request: Exiting due to !CURRENT (pre)\n");
+		if (blk_queue_empty(QUEUE)) {
+			printk("mfm_request: Exiting due to empty queue (pre)\n");
 			CLEAR_INTR;
 			Busy = 0;
 			return;
-		};
-
-		INIT_REQUEST;
+		}
 
 		DBG("mfm_request:                 before arg extraction\n");
 
diff -urN linux-2.5.17/drivers/block/ll_rw_blk.c linux/drivers/block/ll_rw_blk.c
--- linux-2.5.17/drivers/block/ll_rw_blk.c	2002-05-21 07:07:31.000000000 +0200
+++ linux/drivers/block/ll_rw_blk.c	2002-05-22 00:01:44.000000000 +0200
@@ -523,42 +523,6 @@
 	printk("\n");
 }
 
-/*
- * standard prep_rq_fn that builds 10 byte cmds
- */
-int ll_10byte_cmd_build(request_queue_t *q, struct request *rq)
-{
-	int hard_sect = queue_hardsect_size(q);
-	sector_t block = rq->hard_sector / (hard_sect >> 9);
-	unsigned long blocks = rq->hard_nr_sectors / (hard_sect >> 9);
-
-	if (!(rq->flags & REQ_CMD))
-		return 0;
-
-	memset(rq->cmd, 0, sizeof(rq->cmd));
-
-	if (rq_data_dir(rq) == READ)
-		rq->cmd[0] = READ_10;
-	else 
-		rq->cmd[0] = WRITE_10;
-
-	/*
-	 * fill in lba
-	 */
-	rq->cmd[2] = (block >> 24) & 0xff;
-	rq->cmd[3] = (block >> 16) & 0xff;
-	rq->cmd[4] = (block >>  8) & 0xff;
-	rq->cmd[5] = block & 0xff;
-
-	/*
-	 * and transfer length
-	 */
-	rq->cmd[7] = (blocks >> 8) & 0xff;
-	rq->cmd[8] = blocks & 0xff;
-
-	return 0;
-}
-
 void blk_recount_segments(request_queue_t *q, struct bio *bio)
 {
 	struct bio_vec *bv, *bvprv = NULL;
@@ -1763,9 +1727,8 @@
 {
 	if (rq->flags & REQ_CMD) {
 		rq->hard_sector += nsect;
-		rq->hard_nr_sectors -= nsect;
+		rq->nr_sectors = rq->hard_nr_sectors -= nsect;
 		rq->sector = rq->hard_sector;
-		rq->nr_sectors = rq->hard_nr_sectors;
 
 		rq->current_nr_sectors = bio_iovec(rq->bio)->bv_len >> 9;
 		rq->hard_cur_sectors = rq->current_nr_sectors;
@@ -1942,7 +1905,6 @@
 EXPORT_SYMBOL(blk_phys_contig_segment);
 EXPORT_SYMBOL(blk_hw_contig_segment);
 
-EXPORT_SYMBOL(ll_10byte_cmd_build);
 EXPORT_SYMBOL(blk_queue_prep_rq);
 
 EXPORT_SYMBOL(blk_queue_init_tags);
diff -urN linux-2.5.17/drivers/block/paride/pcd.c linux/drivers/block/paride/pcd.c
--- linux-2.5.17/drivers/block/paride/pcd.c	2002-05-21 07:07:38.000000000 +0200
+++ linux/drivers/block/paride/pcd.c	2002-05-22 01:13:05.000000000 +0200
@@ -760,8 +760,11 @@
 
 	if (pcd_busy) return;
         while (1) {
-	    if (QUEUE_EMPTY || (CURRENT->rq_status == RQ_INACTIVE)) return;
-	    INIT_REQUEST;
+	    if (blk_queue_empty(QUEUE)) {
+		    CLEAR_INTR;
+		    return;
+	    }
+
 	    if (rq_data_dir(CURRENT) == READ) {
 		unit = minor(CURRENT->rq_dev);
 		if (unit != pcd_unit) {
diff -urN linux-2.5.17/drivers/block/paride/pd.c linux/drivers/block/paride/pd.c
--- linux-2.5.17/drivers/block/paride/pd.c	2002-05-21 07:07:34.000000000 +0200
+++ linux/drivers/block/paride/pd.c	2002-05-22 01:12:02.000000000 +0200
@@ -835,8 +835,10 @@
 
         if (pd_busy) return;
 repeat:
-        if (QUEUE_EMPTY || (CURRENT->rq_status == RQ_INACTIVE)) return;
-        INIT_REQUEST;
+	if (blk_queue_empty(QUEUE)) {
+		CLEAR_INTR;
+		return;
+	}
 
         pd_dev = minor(CURRENT->rq_dev);
 	pd_unit = unit = DEVICE_NR(CURRENT->rq_dev);
diff -urN linux-2.5.17/drivers/block/paride/pf.c linux/drivers/block/paride/pf.c
--- linux-2.5.17/drivers/block/paride/pf.c	2002-05-21 07:07:32.000000000 +0200
+++ linux/drivers/block/paride/pf.c	2002-05-22 01:11:45.000000000 +0200
@@ -841,8 +841,10 @@
 
         if (pf_busy) return;
 repeat:
-        if (QUEUE_EMPTY || (CURRENT->rq_status == RQ_INACTIVE)) return;
-        INIT_REQUEST;
+	if (blk_queue_empty(QUEUE)) {
+		CLEAR_INTR;
+		return;
+	}
 
         pf_unit = unit = DEVICE_NR(CURRENT->rq_dev);
         pf_block = CURRENT->sector;
diff -urN linux-2.5.17/drivers/block/ps2esdi.c linux/drivers/block/ps2esdi.c
--- linux-2.5.17/drivers/block/ps2esdi.c	2002-05-21 07:07:29.000000000 +0200
+++ linux/drivers/block/ps2esdi.c	2002-05-22 01:02:12.000000000 +0200
@@ -471,9 +471,12 @@
 	       CURRENT->current_nr_sectors, CURRENT->buffer);
 #endif
 
-	/* standard macro that ensures that requests are really on the
+	/* standard procedure to ensure that requests are really on the
 	   list + sanity checks.                     */
-	INIT_REQUEST;
+	if (blk_queue_empty(QUEUE)) {
+		CLEAR_INTR;
+		return;
+	}
 
 	if (isa_virt_to_bus(CURRENT->buffer + CURRENT->current_nr_sectors * 512) > 16 * MB) {
 		printk("%s: DMA above 16MB not supported\n", DEVICE_NAME);
diff -urN linux-2.5.17/drivers/block/xd.c linux/drivers/block/xd.c
--- linux-2.5.17/drivers/block/xd.c	2002-05-21 07:07:34.000000000 +0200
+++ linux/drivers/block/xd.c	2002-05-22 01:04:21.000000000 +0200
@@ -279,8 +279,13 @@
 	sti();
 	if (xdc_busy)
 		return;
-	while (code = 0, !QUEUE_EMPTY) {
-		INIT_REQUEST;	/* do some checking on the request structure */
+	while (1) {
+		code = 0;
+		/* do some checking on the request structure */
+		if (blk_queue_empty(QUEUE)) {
+			CLEAR_INTR;
+			return;
+		}
 
 		if (CURRENT_DEV < xd_drives
 		    && (CURRENT->flags & REQ_CMD)
diff -urN linux-2.5.17/drivers/block/z2ram.c linux/drivers/block/z2ram.c
--- linux-2.5.17/drivers/block/z2ram.c	2002-05-21 07:07:41.000000000 +0200
+++ linux/drivers/block/z2ram.c	2002-05-22 00:59:50.000000000 +0200
@@ -76,7 +76,10 @@
 
     while ( TRUE )
     {
-	INIT_REQUEST;
+	if (blk_queue_empty(QUEUE)) {
+		CLEAR_INTR;
+		return;
+	}
 
 	start = CURRENT->sector << 9;
 	len  = CURRENT->current_nr_sectors << 9;
diff -urN linux-2.5.17/drivers/cdrom/cdu31a.c linux/drivers/cdrom/cdu31a.c
--- linux-2.5.17/drivers/cdrom/cdu31a.c	2002-05-21 07:07:32.000000000 +0200
+++ linux/drivers/cdrom/cdu31a.c	2002-05-22 01:11:00.000000000 +0200
@@ -1598,7 +1598,8 @@
 		 * The beginning here is stolen from the hard disk driver.  I hope
 		 * it's right.
 		 */
-		if (QUEUE_EMPTY || CURRENT->rq_status == RQ_INACTIVE) {
+		if (blk_queue_empty(QUEUE)) {
+			CLEAR_INTR;
 			goto end_do_cdu31a_request;
 		}
 
@@ -1606,8 +1607,6 @@
 			scd_spinup();
 		}
 
-		INIT_REQUEST;
-
 		block = CURRENT->sector;
 		nblock = CURRENT->nr_sectors;
 
diff -urN linux-2.5.17/drivers/cdrom/cm206.c linux/drivers/cdrom/cm206.c
--- linux-2.5.17/drivers/cdrom/cm206.c	2002-05-21 07:07:36.000000000 +0200
+++ linux/drivers/cdrom/cm206.c	2002-05-22 01:10:17.000000000 +0200
@@ -856,9 +856,11 @@
 	uch *source, *dest;
 
 	while (1) {		/* repeat until all requests have been satisfied */
-		INIT_REQUEST;
-		if (QUEUE_EMPTY || CURRENT->rq_status == RQ_INACTIVE)
+		if (blk_queue_empty(QUEUE)) {
+			CLEAR_INTR;
 			return;
+		}
+
 		if (CURRENT->cmd != READ) {
 			debug(("Non-read command %d on cdrom\n",
 			       CURRENT->cmd));
diff -urN linux-2.5.17/drivers/cdrom/gscd.c linux/drivers/cdrom/gscd.c
--- linux-2.5.17/drivers/cdrom/gscd.c	2002-05-21 07:07:30.000000000 +0200
+++ linux/drivers/cdrom/gscd.c	2002-05-22 01:11:10.000000000 +0200
@@ -279,9 +279,11 @@
 	unsigned int nsect;
 
       repeat:
-	if (QUEUE_EMPTY || CURRENT->rq_status == RQ_INACTIVE)
-		goto out;
-	INIT_REQUEST;
+	if (blk_queue_empty(QUEUE)) {
+		CLEAR_INTR;
+		return;
+	}
+
 	dev = minor(CURRENT->rq_dev);
 	block = CURRENT->sector;
 	nsect = CURRENT->nr_sectors;
diff -urN linux-2.5.17/drivers/cdrom/mcdx.c linux/drivers/cdrom/mcdx.c
--- linux-2.5.17/drivers/cdrom/mcdx.c	2002-05-21 07:07:38.000000000 +0200
+++ linux/drivers/cdrom/mcdx.c	2002-05-22 01:09:54.000000000 +0200
@@ -562,19 +562,11 @@
 
       again:
 
-	if (QUEUE_EMPTY) {
-		xtrace(REQUEST, "end_request(0): CURRENT == NULL\n");
+	if (blk_queue_empty(QUEUE)) {
+		CLEAR_INTR;
 		return;
 	}
 
-	if (CURRENT->rq_status == RQ_INACTIVE) {
-		xtrace(REQUEST,
-		       "end_request(0): rq_status == RQ_INACTIVE\n");
-		return;
-	}
-
-	INIT_REQUEST;
-
 	dev = minor(CURRENT->rq_dev);
 	stuffp = mcdx_stuffp[dev];
 
diff -urN linux-2.5.17/drivers/cdrom/sbpcd.c linux/drivers/cdrom/sbpcd.c
--- linux-2.5.17/drivers/cdrom/sbpcd.c	2002-05-21 07:07:33.000000000 +0200
+++ linux/drivers/cdrom/sbpcd.c	2002-05-22 01:09:26.000000000 +0200
@@ -4915,12 +4915,15 @@
 	printk(" do_sbpcd_request[%di](%p:%ld+%ld), Pid:%d, Time:%li\n",
 		xnr, CURRENT, CURRENT->sector, CURRENT->nr_sectors, current->pid, jiffies);
 #endif
-	INIT_REQUEST;
+
+	if (blk_queue_empty(QUEUE)) {
+		CLEAR_INTR;
+		return;
+	}
+
 	req=CURRENT;		/* take out our request so no other */
 	blkdev_dequeue_request(req);	/* task can fuck it up         GTL  */
-	
-	if (req->rq_status == RQ_INACTIVE)
-		sbpcd_end_request(req, 0);
+
 	if (req -> sector == -1)
 		sbpcd_end_request(req, 0);
 	spin_unlock_irq(q->queue_lock);
diff -urN linux-2.5.17/drivers/cdrom/sonycd535.c linux/drivers/cdrom/sonycd535.c
--- linux-2.5.17/drivers/cdrom/sonycd535.c	2002-05-21 07:07:36.000000000 +0200
+++ linux/drivers/cdrom/sonycd535.c	2002-05-22 01:10:43.000000000 +0200
@@ -806,10 +806,11 @@
 		 * The beginning here is stolen from the hard disk driver.  I hope
 		 * it's right.
 		 */
-		if (QUEUE_EMPTY || CURRENT->rq_status == RQ_INACTIVE) {
+		if (blk_queue_empty(QUEUE)) {
+			CLEAR_INTR;
 			return;
 		}
-		INIT_REQUEST;
+
 		dev = minor(CURRENT->rq_dev);
 		block = CURRENT->sector;
 		nsect = CURRENT->nr_sectors;
diff -urN linux-2.5.17/drivers/ide/hd.c linux/drivers/ide/hd.c
--- linux-2.5.17/drivers/ide/hd.c	2002-05-21 07:07:32.000000000 +0200
+++ linux/drivers/ide/hd.c	2002-05-22 01:08:50.000000000 +0200
@@ -544,13 +544,17 @@
 {
 	unsigned int dev, block, nsect, sec, track, head, cyl;
 
-	if (!QUEUE_EMPTY && CURRENT->rq_status == RQ_INACTIVE) return;
 	if (DEVICE_INTR)
 		return;
 repeat:
 	del_timer(&device_timer);
 	sti();
-	INIT_REQUEST;
+
+	if (blk_queue_empty(QUEUE)) {
+		CLEAR_INTR;
+		return;
+	}
+
 	if (reset) {
 		cli();
 		reset_hd();
diff -urN linux-2.5.17/drivers/ide/ide-cd.c linux/drivers/ide/ide-cd.c
--- linux-2.5.17/drivers/ide/ide-cd.c	2002-05-22 01:35:08.000000000 +0200
+++ linux/drivers/ide/ide-cd.c	2002-05-22 01:21:25.000000000 +0200
@@ -2658,6 +2658,47 @@
 	return nslots;
 }
 
+
+/*
+ * standard prep_rq_fn that builds 10 byte cmds
+ */
+static int ll_10byte_cmd_build(request_queue_t *q, struct request *rq)
+{
+	int hard_sect = queue_hardsect_size(q);
+	sector_t block = rq->hard_sector / (hard_sect >> 9);
+	unsigned long blocks = rq->hard_nr_sectors / (hard_sect >> 9);
+
+	if (!(rq->flags & REQ_CMD))
+		return 0;
+
+	if (rq->hard_nr_sectors != rq->nr_sectors) {
+		printk(KERN_ERR "ide-cd: hard_nr_sectors differs from nr_sectors! %lu %lu\n",
+				rq->nr_sectors, rq->hard_nr_sectors);
+	}
+	memset(rq->cmd, 0, sizeof(rq->cmd));
+
+	if (rq_data_dir(rq) == READ)
+		rq->cmd[0] = GPCMD_READ_10;
+	else
+		rq->cmd[0] = GPCMD_WRITE_10;
+
+	/*
+	 * fill in lba
+	 */
+	rq->cmd[2] = (block >> 24) & 0xff;
+	rq->cmd[3] = (block >> 16) & 0xff;
+	rq->cmd[4] = (block >>  8) & 0xff;
+	rq->cmd[5] = block & 0xff;
+
+	/*
+	 * and transfer length
+	 */
+	rq->cmd[7] = (blocks >> 8) & 0xff;
+	rq->cmd[8] = blocks & 0xff;
+
+	return 0;
+}
+
 static int ide_cdrom_setup(struct ata_device *drive)
 {
 	struct cdrom_info *info = drive->driver_data;
diff -urN linux-2.5.17/drivers/mtd/ftl.c linux/drivers/mtd/ftl.c
--- linux-2.5.17/drivers/mtd/ftl.c	2002-05-21 07:07:35.000000000 +0200
+++ linux/drivers/mtd/ftl.c	2002-05-22 00:59:41.000000000 +0200
@@ -1191,11 +1191,14 @@
     partition_t *part;
 
     do {
-      //	    sti();
-	INIT_REQUEST;
+	//	    sti();
+	if (blk_queue_empty(QUEUE)) {
+		CLEAR_INTR;
+		return;
+	}
 
 	minor = minor(CURRENT->rq_dev);
-	
+
 	part = myparts[minor >> 4];
 	if (part) {
 	  ret = 0;
diff -urN linux-2.5.17/drivers/mtd/mtdblock.c linux/drivers/mtd/mtdblock.c
--- linux-2.5.17/drivers/mtd/mtdblock.c	2002-05-21 07:07:37.000000000 +0200
+++ linux/drivers/mtd/mtdblock.c	2002-05-22 01:00:09.000000000 +0200
@@ -405,7 +405,11 @@
 	unsigned int res;
 
 	for (;;) {
-		INIT_REQUEST;
+		if (blk_queue_empty(QUEUE)) {
+			CLEAR_INTR;
+			return;
+		}
+
 		req = CURRENT;
 		spin_unlock_irq(QUEUE->queue_lock);
 		mtdblk = mtdblks[minor(req->rq_dev)];
diff -urN linux-2.5.17/drivers/mtd/mtdblock_ro.c linux/drivers/mtd/mtdblock_ro.c
--- linux-2.5.17/drivers/mtd/mtdblock_ro.c	2002-05-21 07:07:39.000000000 +0200
+++ linux/drivers/mtd/mtdblock_ro.c	2002-05-22 01:04:34.000000000 +0200
@@ -112,11 +112,15 @@
 
    while (1)
    {
-      /* Grab the Request and unlink it from the request list, INIT_REQUEST
-       	 will execute a return if we are done. */
-      INIT_REQUEST;
+      /* Grab the Request and unlink it from the request list, we
+	 will execute a return if we are done. */
+	if (blk_queue_empty(QUEUE)) {
+		CLEAR_INTR;
+		return;
+	}
+
       current_request = CURRENT;
-   
+
       if (minor(current_request->rq_dev) >= MAX_MTD_DEVICES)
       {
 	 printk("mtd: Unsupported device!\n");
diff -urN linux-2.5.17/drivers/mtd/nftlcore.c linux/drivers/mtd/nftlcore.c
--- linux-2.5.17/drivers/mtd/nftlcore.c	2002-05-21 07:07:30.000000000 +0200
+++ linux/drivers/mtd/nftlcore.c	2002-05-22 00:59:11.000000000 +0200
@@ -837,7 +837,11 @@
 	int res;
 
 	while (1) {
-		INIT_REQUEST;	/* blk.h */
+		if (blk_queue_empty(QUEUE)) {
+			CLEAR_INTR;
+			return;
+		}
+
 		req = CURRENT;
 		
 		/* We can do this because the generic code knows not to
diff -urN linux-2.5.17/drivers/s390/block/xpram.c linux/drivers/s390/block/xpram.c
--- linux-2.5.17/drivers/s390/block/xpram.c	2002-05-21 07:07:41.000000000 +0200
+++ linux/drivers/s390/block/xpram.c	2002-05-22 01:03:03.000000000 +0200
@@ -747,7 +747,10 @@
 #endif /* V24 */
 
 	while(1) {
-		INIT_REQUEST;
+		if (blk_queue_empty(QUEUE)) {
+			CLEAR_INTR;
+			return;
+		}
 
 		fault=0;
 #if ( XPRAM_VERSION == 24 )
diff -urN linux-2.5.17/drivers/sbus/char/jsflash.c linux/drivers/sbus/char/jsflash.c
--- linux-2.5.17/drivers/sbus/char/jsflash.c	2002-05-21 07:07:38.000000000 +0200
+++ linux/drivers/sbus/char/jsflash.c	2002-05-22 01:04:14.000000000 +0200
@@ -207,7 +207,11 @@
 	size_t len;
 
 	for (;;) {
-		INIT_REQUEST;	/* if (QUEUE_EMPTY) return; */
+		if (blk_queue_empty(QUEUE)) {
+			CLEAR_INTR;
+			return;
+		}
+
 		req = CURRENT;
 
 		dev = MINOR(req->rq_dev);
diff -urN linux-2.5.17/include/linux/blkdev.h linux/include/linux/blkdev.h
--- linux-2.5.17/include/linux/blkdev.h	2002-05-21 07:07:34.000000000 +0200
+++ linux/include/linux/blkdev.h	2002-05-21 23:57:17.000000000 +0200
@@ -303,7 +303,6 @@
 extern inline int blk_phys_contig_segment(request_queue_t *q, struct bio *, struct bio *);
 extern inline int blk_hw_contig_segment(request_queue_t *q, struct bio *, struct bio *);
 extern int block_ioctl(struct block_device *, unsigned int, unsigned long);
-extern int ll_10byte_cmd_build(request_queue_t *, struct request *);
 
 /*
  * get ready for proper ref counting
diff -urN linux-2.5.17/include/linux/blk.h linux/include/linux/blk.h
--- linux-2.5.17/include/linux/blk.h	2002-05-21 07:07:35.000000000 +0200
+++ linux/include/linux/blk.h	2002-05-22 01:17:31.000000000 +0200
@@ -313,22 +313,11 @@
 
 #define SET_INTR(x) (DEVICE_INTR = (x))
 
-#ifdef DEVICE_INTR
-#define CLEAR_INTR SET_INTR(NULL)
-#else
-#define CLEAR_INTR
-#endif
-
-#define INIT_REQUEST						\
-	if (QUEUE_EMPTY) {					\
-		CLEAR_INTR;					\
-		return;						\
-	}							\
-	if (major(CURRENT->rq_dev) != MAJOR_NR) 			\
-		panic(DEVICE_NAME ": request list destroyed");	\
-	if (!CURRENT->bio)					\
-		panic(DEVICE_NAME ": no bio");			\
-
+# ifdef DEVICE_INTR
+#  define CLEAR_INTR SET_INTR(NULL)
+# else
+#  define CLEAR_INTR
+# endif
 #endif /* !defined(IDE_DRIVER) */
 
 /*

[PATCH] 2.5.17 IDE 67

[Martin Dalecki]

[-- Attachment #1: Type: text/plain, Size: 235 bytes --]

Wed May 22 01:43:54 CEST 2002 ide-clean-67

- Nuke COMMERIAL and similar spurious configuration options...
   The fact that every single default configuration option contained
   those bits makes this trivial patch appear rather big.


[-- Attachment #2: ide-clean-67.diff.gz --]
[-- Type: application/x-gzip, Size: 5752 bytes --]

Re: [PATCH] 2.5.17 IDE 67

[Tom Rini]

On Wed, May 22, 2002 at 09:19:07AM +0200, Martin Dalecki wrote:

> Wed May 22 01:43:54 CEST 2002 ide-clean-67
> 
> - Nuke COMMERIAL and similar spurious configuration options...
>   The fact that every single default configuration option contained
>   those bits makes this trivial patch appear rather big.

This also nukes CONFIG_DMA_NONPCI.  While this probably shouldn't have
been define_bool'ed to 'n' all of the time, there are cases where this
seems to be properly used.  I know PPC4xx uses it (or will be using it
once the driver is ready to be submitted) and it looks like cris uses it
as well.

-- 
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

Re: [PATCH] 2.5.17 IDE 67

[Martin Dalecki]

Uz.ytkownik Tom Rini napisa?:
> On Wed, May 22, 2002 at 09:19:07AM +0200, Martin Dalecki wrote:
> 
> 
>>Wed May 22 01:43:54 CEST 2002 ide-clean-67
>>
>>- Nuke COMMERIAL and similar spurious configuration options...
>>  The fact that every single default configuration option contained
>>  those bits makes this trivial patch appear rather big.
> 
> 
> This also nukes CONFIG_DMA_NONPCI.  While this probably shouldn't have
> been define_bool'ed to 'n' all of the time, there are cases where this
> seems to be properly used.  I know PPC4xx uses it (or will be using it
> once the driver is ready to be submitted) and it looks like cris uses it
> as well.

No I checked. PPC4xx had no functional code for the case
of CONFIG_DMA_NONPCI. It just looked like it had.
Look at the patch and see that it is removing the two
nonpci_xxx functions which are nowhere defined!

And the portability layer provides better mechanisms for the purpose
the serve to serve by the neas of the udma_ interface now.
Please look closer. The __CRIS__ behaviour is preserved and BTW.
not pretty as well.
But this can be fixed a bit later...

Re: [PATCH] 2.5.17 IDE 67

[Tom Rini]

On Wed, May 22, 2002 at 06:21:10PM +0200, Martin Dalecki wrote:
> Uz.ytkownik Tom Rini napisa?:
> >On Wed, May 22, 2002 at 09:19:07AM +0200, Martin Dalecki wrote:
> >
> >
> >>Wed May 22 01:43:54 CEST 2002 ide-clean-67
> >>
> >>- Nuke COMMERIAL and similar spurious configuration options...
> >> The fact that every single default configuration option contained
> >> those bits makes this trivial patch appear rather big.
> >
> >
> >This also nukes CONFIG_DMA_NONPCI.  While this probably shouldn't have
> >been define_bool'ed to 'n' all of the time, there are cases where this
> >seems to be properly used.  I know PPC4xx uses it (or will be using it
> >once the driver is ready to be submitted) and it looks like cris uses it
> >as well.
> 
> No I checked. PPC4xx had no functional code for the case
> of CONFIG_DMA_NONPCI. It just looked like it had.

You checked the kernel.org tree, not the PPC tree.

> Look at the patch and see that it is removing the two
> nonpci_xxx functions which are nowhere defined!

Because the driver isn't quite ready to be submitted just yet.

> The __CRIS__ behaviour is preserved and BTW.
> not pretty as well.
> But this can be fixed a bit later...

And when the PPC4xx drivers are ready to be submitted we'll need to add
in __powerpc__ tests too.  Can't we just keep CONFIG_DMA_NONPCI for now?

-- 
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

Re: [PATCH] 2.5.17 IDE 67

[Martin Dalecki]

Uz.ytkownik Tom Rini napisa?:
>
> And when the PPC4xx drivers are ready to be submitted we'll need to add
> in __powerpc__ tests too.  Can't we just keep CONFIG_DMA_NONPCI for now?
> 

Plese feel free to add them when and where they are needed.
It's no problem to clean this all up
then again if a true usage pattern emerges.
And I have no problem with patches toching "core" driver stuff as long
as the changes are justified I will integrate them with pleasure at time.

In the time between I tend to code for reality and not for vapour
(no offence intendid...) :-).

Re: [PATCH] 2.5.17 IDE 67

[Tom Rini]

On Wed, May 22, 2002 at 06:40:14PM +0200, Martin Dalecki wrote:
> Uz.ytkownik Tom Rini napisa?:
> >
> >And when the PPC4xx drivers are ready to be submitted we'll need to add
> >in __powerpc__ tests too.  Can't we just keep CONFIG_DMA_NONPCI for now?
> 
> Plese feel free to add them when and where they are needed.
> It's no problem to clean this all up
> then again if a true usage pattern emerges.
> And I have no problem with patches toching "core" driver stuff as long
> as the changes are justified I will integrate them with pleasure at time.

Okay. :)

BTW, maybe it's part of the great IDE rewrite and all, but the
CONFIG_DMA_NONPCI part of drivers/ide/ide.c didn't get a __CRIS__ added
back in.

-- 
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

Re: [PATCH] 2.5.17 IDE 67

[Martin Dalecki]

Uz.ytkownik Tom Rini napisa?:
> On Wed, May 22, 2002 at 06:40:14PM +0200, Martin Dalecki wrote:
> 
>>Uz.ytkownik Tom Rini napisa?:
>>
>>>And when the PPC4xx drivers are ready to be submitted we'll need to add
>>>in __powerpc__ tests too.  Can't we just keep CONFIG_DMA_NONPCI for now?
>>
>>Plese feel free to add them when and where they are needed.
>>It's no problem to clean this all up
>>then again if a true usage pattern emerges.
>>And I have no problem with patches toching "core" driver stuff as long
>>as the changes are justified I will integrate them with pleasure at time.
> 
> 
> Okay. :)
> 
> BTW, maybe it's part of the great IDE rewrite and all, but the
> CONFIG_DMA_NONPCI part of drivers/ide/ide.c didn't get a __CRIS__ added
> back in.

Please look at ide-features.c there is should be there.

Re: [PATCH] 2.5.17 IDE 67

[Tom Rini]

On Thu, May 23, 2002 at 08:08:40AM +0200, Martin Dalecki wrote:
> Uz.ytkownik Tom Rini napisa?:
> >On Wed, May 22, 2002 at 06:40:14PM +0200, Martin Dalecki wrote:
> >
> >>Uz.ytkownik Tom Rini napisa?:
> >>
> >>>And when the PPC4xx drivers are ready to be submitted we'll need to add
> >>>in __powerpc__ tests too.  Can't we just keep CONFIG_DMA_NONPCI for now?
> >>
> >>Plese feel free to add them when and where they are needed.
> >>It's no problem to clean this all up
> >>then again if a true usage pattern emerges.
> >>And I have no problem with patches toching "core" driver stuff as long
> >>as the changes are justified I will integrate them with pleasure at time.
> >
> >
> >Okay. :)
> >
> >BTW, maybe it's part of the great IDE rewrite and all, but the
> >CONFIG_DMA_NONPCI part of drivers/ide/ide.c didn't get a __CRIS__ added
> >back in.
> 
> Please look at ide-features.c there is should be there.

Er, you've lost me.  The call to ide_release_dma(ch) used to be guarded
by (CONFIG_BLK_DEV_IDEDMA) && !CONFIG_DMA_NONPCI) but is now just
guarded by CONFIG_BLK_DEV_IDEDMA.  Am I missing something (should all
drivers be implementing ide_release_dma()?)

-- 
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

Re: [PATCH] 2.5.17 IDE 67

[Martin Dalecki]

Uz.ytkownik Tom Rini napisa?:

> 
> Er, you've lost me.  The call to ide_release_dma(ch) used to be guarded
> by (CONFIG_BLK_DEV_IDEDMA) && !CONFIG_DMA_NONPCI) but is now just
> guarded by CONFIG_BLK_DEV_IDEDMA.  Am I missing something (should all
> drivers be implementing ide_release_dma()?)

Ahh this case. Well please look even closer:

./include/linux/ide.h:extern void ide_release_dma(struct ata_channel *);
./arch/cris/drivers/ide.c: * - added ide_release_dma
./arch/cris/drivers/ide.c:void ide_release_dma(struct ata_channel *ch)

All right?

This was actually fixing a bug/mission :-)

Re: [PATCH] 2.5.17 IDE 67

[Tom Rini]

On Thu, May 23, 2002 at 04:32:30PM +0200, Martin Dalecki wrote:
> Uz.ytkownik Tom Rini napisa?:
> 
> >
> >Er, you've lost me.  The call to ide_release_dma(ch) used to be guarded
> >by (CONFIG_BLK_DEV_IDEDMA) && !CONFIG_DMA_NONPCI) but is now just
> >guarded by CONFIG_BLK_DEV_IDEDMA.  Am I missing something (should all
> >drivers be implementing ide_release_dma()?)
> 
> Ahh this case. Well please look even closer:
> 
> ./include/linux/ide.h:extern void ide_release_dma(struct ata_channel *);
> ./arch/cris/drivers/ide.c: * - added ide_release_dma
> ./arch/cris/drivers/ide.c:void ide_release_dma(struct ata_channel *ch)
> 
> All right?

Ah, so everyone has to implement ide_release_dma() now then.  Thanks.

-- 
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

[PATCH] 2.5.16 IDE 68

[Martin Dalecki]

[-- Attachment #1: Type: text/plain, Size: 323 bytes --]

Wed May 22 02:56:45 CEST 2002 ide-clean-68

- Make the different ATAPI device type drivers use a unified packet command
   structure. We have to start to push them together.

This patch is rather trivial in itself, but the plentora of code duplication it
is trying to fight against is making it unfortunately rather big...

[-- Attachment #2: ide-clean-68.diff.gz --]
[-- Type: application/x-gzip, Size: 21757 bytes --]

Re: [PATCH] 2.5.16 IDE 68

[Juan Quintela]

>>>>> "martin" == Martin Dalecki <dalecki@evision-ventures.com> writes:

martin> Wed May 22 02:56:45 CEST 2002 ide-clean-68
martin> - Make the different ATAPI device type drivers use a unified packet command
martin> structure. We have to start to push them together.

martin> This patch is rather trivial in itself, but the plentora of code duplication it
martin> is trying to fight against is making it unfortunately rather big...

Hi

atapi.c is not a module, then it don't make sense to have a
MODULE_LICENSE :(

Notice that I don't like that kind of trap:

disable IDECD
compile kernel
enable IDECD as module
compile module

your module don't work :(

And we have had already that problem with NFS, please, make that file
_always_ compiled in, or make it a real module.

Later, Juan.


-- 
In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy

Re: [PATCH] 2.5.16 IDE 68

[Martin Dalecki]

Uz.ytkownik Juan Quintela napisa?:
>>>>>>"martin" == Martin Dalecki <dalecki@evision-ventures.com> writes:
>>>>>
> 
> martin> Wed May 22 02:56:45 CEST 2002 ide-clean-68
> martin> - Make the different ATAPI device type drivers use a unified packet command
> martin> structure. We have to start to push them together.
> 
> martin> This patch is rather trivial in itself, but the plentora of code duplication it
> martin> is trying to fight against is making it unfortunately rather big...
> 
> Hi
> 
> atapi.c is not a module, then it don't make sense to have a
> MODULE_LICENSE :(
> 
> Notice that I don't like that kind of trap:
> 
> disable IDECD
> compile kernel
> enable IDECD as module
> compile module
> 
> your module don't work :(
> 
> And we have had already that problem with NFS, please, make that file
> _always_ compiled in, or make it a real module.

Yes I'm aware of this, but some time soon it is going to become
a real module :-). It just has to came over the "triviality" age.

Re: [PATCH] 2.5.16 IDE 68

[Linus Torvalds]

On Wed, 22 May 2002, Martin Dalecki wrote:
>
> - Make the different ATAPI device type drivers use a unified packet command
>    structure. We have to start to push them together.

Good.

However, I suggest you really look at the big picture, and realize that
this is NOT an ATA-only issue.

The unified packet command thing should be started at a higher level,
where the command creation itself should create them into "rq->cmd[]", and
the interfaces to the _user_ should also be unified.

If you think it's stupid to have different packet mechanisms for IDE CD vs
disk, imagine how stupid it is to have different packet interfaces for
SCSI CD vs IDE CD and expose those different interfaces to user mode.

This is why I think you made a mistake to move ll_10byte_cmd_build() into
the IDE layer. We want to move the packet building _up_, not down.

And you should put the command into "rq->cmd[]", _not_ into "pc->c[]" like
the curren code does. And strive to standardize on a command set (where
the obvious target is a very SCSI-like one - for all the same reasons
that ATAPI commands themselves tend to look like SCSI commands).

So please don't lose sight of the fact that there is more than just IDE
out there, and _please_ get rid of the atapi-specific command. Think ahead
a bit, and notice how you have the "scsi" parts in the
"atapi_packet_command", and realize that you shouldn't need to have that,
if you were just to share the same request cmd layout.

		Linus

Re: [PATCH] 2.5.16 IDE 68

[Martin Dalecki]

Uz.ytkownik Linus Torvalds napisa?:
> 
> On Wed, 22 May 2002, Martin Dalecki wrote:
> 
>>- Make the different ATAPI device type drivers use a unified packet command
>>   structure. We have to start to push them together.
> 
> 
> Good.
> 
> However, I suggest you really look at the big picture, and realize that
> this is NOT an ATA-only issue.

Linus I agree fully with you!

> The unified packet command thing should be started at a higher level,
> where the command creation itself should create them into "rq->cmd[]", and
> the interfaces to the _user_ should also be unified.

Well you should have seen me already hopping between differnt
layers with stuff like this quite frequently. I'm just not going
to "jump down" (well in this case up) this cliff in one big step.
I will try to push it step but step where it belongs. OK?

> If you think it's stupid to have different packet mechanisms for IDE CD vs
> disk, imagine how stupid it is to have different packet interfaces for
> SCSI CD vs IDE CD and expose those different interfaces to user mode.
> 
> This is why I think you made a mistake to move ll_10byte_cmd_build() into
> the IDE layer. We want to move the packet building _up_, not down.

No I didn't.
It's nearly always a mistake to push the methods (well functions),
before the objects (well structs) are in place.
Pushing methods before objects makes no semantical anvances, becouse
it's just tossing code lines around and obfuscating the code by adding
#include <> interdependancies.

> And you should put the command into "rq->cmd[]", _not_ into "pc->c[]" like
> the curren code does. And strive to standardize on a command set (where

Applying the same cure to struct atapi_packet_command as has been
applyed (well by me - who else) to the struct atapi_packed_command
is of course the next "cheap" goal! It has been exerciesed once in ide-cd.c
and the problems are well known. But even by way of this simple
example one can see immediately that it is better to first target
the structural part (structs) and then the code. Not the other way around.
(Next thing should be to ake rq->buffer and rq->special in line too for
example.)

> the obvious target is a very SCSI-like one - for all the same reasons
> that ATAPI commands themselves tend to look like SCSI commands).
> 
> So please don't lose sight of the fact that there is more than just IDE
> out there, and _please_ get rid of the atapi-specific command. Think ahead
> a bit, and notice how you have the "scsi" parts in the
> "atapi_packet_command", and realize that you shouldn't need to have that,
> if you were just to share the same request cmd layout.

Sure I realize it. ide-cd.c should make it clean that
struct atapi_packet_comman will be "melted down" step by step
the same way the ide-cd stuff got metled down if possible :-).

I hope the "hawkeye" view layered out above makes clear why I'm
doing the stuff in the order I do and why it has more
chances to succeed. At least there are no dissagreements about
the goals. OK?

[PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

[-- Attachment #1: Type: text/plain, Size: 795 bytes --]

Remove support for /dev/port altogether.

1. It is not usable with ports which require 4 byte access.

2. The same can be achieved by using capabilities and su bits and so on.

3. __m68000__ doesn't even implement it and most other non i386 archs
"implement" it but apparently don't even care about endianess issues.

4. It's not standard.

5. seek() + port access is "racy" with respect to multiple usage.

6. Nothing is using it.

... and so on and so on ...

And finally, kernel size with it:

    text    data     bss     dec     hex filename
1480587  243280  259628 1983495  1e4407 vmlinux

kernel size without it:

[root@kozaczek linux]# size vmlinux
    text    data     bss     dec     hex filename
1480229  243184  259628 1983041  1e4241 vmlinux

Which means a saving of 454 bytes :-).

[-- Attachment #2: kill-ports-2.5.17.patch --]
[-- Type: text/plain, Size: 3101 bytes --]

diff -urN linux-old/Documentation/devices.txt linux/Documentation/devices.txt
--- linux-old/Documentation/devices.txt	2002-05-21 07:07:37.000000000 +0200
+++ linux/Documentation/devices.txt	2002-05-22 11:38:47.000000000 +0200
@@ -90,7 +90,7 @@
 		  1 = /dev/mem		Physical memory access
 		  2 = /dev/kmem		Kernel virtual memory access
 		  3 = /dev/null		Null device
-		  4 = /dev/port		I/O port access
+		  4 = /dev/port		OBSOLETE - since 2.5.18
 		  5 = /dev/zero		Null byte source
 		  6 = /dev/core		OBSOLETE - replaced by /proc/kcore
 		  7 = /dev/full		Returns ENOSPC on write
diff -urN linux-old/drivers/char/mem.c linux/drivers/char/mem.c
--- linux-old/drivers/char/mem.c	2002-05-21 07:07:40.000000000 +0200
+++ linux/drivers/char/mem.c	2002-05-22 11:26:13.000000000 +0200
@@ -324,46 +324,6 @@
  	return virtr + wrote;
 }
 
-#if !defined(__mc68000__)
-static ssize_t read_port(struct file * file, char * buf,
-			 size_t count, loff_t *ppos)
-{
-	unsigned long i = *ppos;
-	char *tmp = buf;
-
-	if (verify_area(VERIFY_WRITE,buf,count))
-		return -EFAULT; 
-	while (count-- > 0 && i < 65536) {
-		if (__put_user(inb(i),tmp) < 0) 
-			return -EFAULT;  
-		i++;
-		tmp++;
-	}
-	*ppos = i;
-	return tmp-buf;
-}
-
-static ssize_t write_port(struct file * file, const char * buf,
-			  size_t count, loff_t *ppos)
-{
-	unsigned long i = *ppos;
-	const char * tmp = buf;
-
-	if (verify_area(VERIFY_READ,buf,count))
-		return -EFAULT;
-	while (count-- > 0 && i < 65536) {
-		char c;
-		if (__get_user(c, tmp)) 
-			return -EFAULT; 
-		outb(c,i);
-		i++;
-		tmp++;
-	}
-	*ppos = i;
-	return tmp-buf;
-}
-#endif
-
 static ssize_t read_null(struct file * file, char * buf,
 			 size_t count, loff_t *ppos)
 {
@@ -522,7 +482,7 @@
 	return ret;
 }
 
-static int open_port(struct inode * inode, struct file * filp)
+static int open_mem(struct inode * inode, struct file * filp)
 {
 	return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
 }
@@ -532,7 +492,6 @@
 #define full_lseek      null_lseek
 #define write_zero	write_null
 #define read_full       read_zero
-#define open_mem	open_port
 #define open_kmem	open_mem
 
 static struct file_operations mem_fops = {
@@ -557,15 +516,6 @@
 	write:		write_null,
 };
 
-#if !defined(__mc68000__)
-static struct file_operations port_fops = {
-	llseek:		memory_lseek,
-	read:		read_port,
-	write:		write_port,
-	open:		open_port,
-};
-#endif
-
 static struct file_operations zero_fops = {
 	llseek:		zero_lseek,
 	read:		read_zero,
@@ -591,11 +541,6 @@
 		case 3:
 			filp->f_op = &null_fops;
 			break;
-#if !defined(__mc68000__)
-		case 4:
-			filp->f_op = &port_fops;
-			break;
-#endif
 		case 5:
 			filp->f_op = &zero_fops;
 			break;
@@ -628,7 +573,6 @@
 	{1, "mem",     S_IRUSR | S_IWUSR | S_IRGRP, &mem_fops},
 	{2, "kmem",    S_IRUSR | S_IWUSR | S_IRGRP, &kmem_fops},
 	{3, "null",    S_IRUGO | S_IWUGO,           &null_fops},
-	{4, "port",    S_IRUSR | S_IWUSR | S_IRGRP, &port_fops},
 	{5, "zero",    S_IRUGO | S_IWUGO,           &zero_fops},
 	{7, "full",    S_IRUGO | S_IWUGO,           &full_fops},
 	{8, "random",  S_IRUGO | S_IWUSR,           &random_fops},

Re: [PATCH] 2.5.17 /dev/ports

[Paul Mackerras]

Martin Dalecki writes:

> Remove support for /dev/port altogether.

Great idea!

Paul.

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Paul Mackerras napisa?:
> Martin Dalecki writes:
> 
> 
>>Remove support for /dev/port altogether.
> 
> 
> Great idea!

I learned already that PPC people hate outb() and inb() :-).

Re: [PATCH] 2.5.17 /dev/ports

[David S. Miller]

   From: Paul Mackerras <paulus@samba.org>
   Date: Wed, 22 May 2002 20:42:47 +1000 (EST)

   Martin Dalecki writes:
   
   > Remove support for /dev/port altogether.
   
   Great idea!
   
You have my blessing too :-)

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Użytkownik David S. Miller napisał:
>    From: Paul Mackerras <paulus@samba.org>
>    Date: Wed, 22 May 2002 20:42:47 +1000 (EST)
> 
>    Martin Dalecki writes:
>    
>    > Remove support for /dev/port altogether.
>    
>    Great idea!
>    
> You have my blessing too :-)

And now I'm just eagerly awaiting the first clueless
l^Huser lurking on this list, who will flame me as usuall...
But that's no problem - I got already used to it :-).

Re: [PATCH] 2.5.17 /dev/ports

[Russell King]

On Wed, May 22, 2002 at 12:13:05PM +0200, Martin Dalecki wrote:
> And now I'm just eagerly awaiting the first clueless
> l^Huser lurking on this list, who will flame me as usuall...
> But that's no problem - I got already used to it :-).

I'm waiting on Phil Blundell to notice - I think /dev/port may get used
on ARM to emulate inb() and outb() from userspace; I don't look after
glibc so shrug.

I agree however that /dev/port is a rotten interface that needs to go.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Russell King napisa?:
> On Wed, May 22, 2002 at 12:13:05PM +0200, Martin Dalecki wrote:
> 
>>And now I'm just eagerly awaiting the first clueless
>>l^Huser lurking on this list, who will flame me as usuall...
>>But that's no problem - I got already used to it :-).
> 
> 
> I'm waiting on Phil Blundell to notice - I think /dev/port may get used
> on ARM to emulate inb() and outb() from userspace; I don't look after
> glibc so shrug.
> 
> I agree however that /dev/port is a rotten interface that needs to go.
> 

Hmm still not flames? Do they all sleep right now?

- Should I perhaps tell what I think about the glibc bloat^W coding style?

- Should I perhaps tell how "usefull" the GNU extensions to the POSIX
   standards in question are?

- Or a side note about RH's slang and popt and other useless "required"
   shared libraries?

- Is there maybe some Python module using /dev/port for precisely
   the purpose you mention. (This is actually a good candidate.)

Anyway, dear Russell (plese note the double ll!):

[root@kozaczek glibc-2.2.5]# find ./ -name "*.[ch]" -exec grep \/dev\/port 
/dev/null {} \;
[root@kozaczek glibc-2.2.5]#

[root@kozaczek glibc-2.2.5]# find ./ -name "*.[ch]" -exec grep \"port\" 
/dev/null {} \;
./hesiod/nss_hesiod/hesiod-service.c:  return lookup (portstr, "port", protocol, 
serv, buffer, buflen, errnop);
[root@kozaczek glibc-2.2.5]#
[root@kozaczek glibc-2.2.5]# find ./ -name "*.[ch]" -exec grep outb\( /dev/null 
{} \;
[root@kozaczek glibc-2.2.5]#

So I rather think that glibc may be bloated but it's not idiotic and
we have nothing to fear from it ;-)... well this time at least...
As far as I know (and I know little about ARM). It would be anwyay
unnatural to use /dev/port for the purpose you mention.
ARM io space is memmory mapped, so if any file you would
rather use /dev/kmem...

Still no flames? This silence makes me suspicious....

Re: [PATCH] 2.5.17 /dev/ports

[Richard B. Johnson]

On Wed, 22 May 2002, Martin Dalecki wrote:

> Uz.ytkownik Russell King napisa?:
> > On Wed, May 22, 2002 at 12:13:05PM +0200, Martin Dalecki wrote:
> > 
> >>And now I'm just eagerly awaiting the first clueless
> >>l^Huser lurking on this list, who will flame me as usuall...
> >>But that's no problem - I got already used to it :-).
> > 
> > 
> > I'm waiting on Phil Blundell to notice - I think /dev/port may get used
> > on ARM to emulate inb() and outb() from userspace; I don't look after
> > glibc so shrug.
> > 
> > I agree however that /dev/port is a rotten interface that needs to go.
> > 
> 
> Hmm still not flames? Do they all sleep right now?
> 
> - Should I perhaps tell what I think about the glibc bloat^W coding style?
> 
> - Should I perhaps tell how "usefull" the GNU extensions to the POSIX
>    standards in question are?
> 
> - Or a side note about RH's slang and popt and other useless "required"
>    shared libraries?
> 
> - Is there maybe some Python module using /dev/port for precisely
>    the purpose you mention. (This is actually a good candidate.)
> 
> Anyway, dear Russell (plese note the double ll!):
> 
> [root@kozaczek glibc-2.2.5]# find ./ -name "*.[ch]" -exec grep \/dev\/port 
> /dev/null {} \;
> [root@kozaczek glibc-2.2.5]#
> 
> [root@kozaczek glibc-2.2.5]# find ./ -name "*.[ch]" -exec grep \"port\" 
> /dev/null {} \;
> ./hesiod/nss_hesiod/hesiod-service.c:  return lookup (portstr, "port", protocol, 
> serv, buffer, buflen, errnop);
> [root@kozaczek glibc-2.2.5]#
> [root@kozaczek glibc-2.2.5]# find ./ -name "*.[ch]" -exec grep outb\( /dev/null 
> {} \;
> [root@kozaczek glibc-2.2.5]#
> 
> So I rather think that glibc may be bloated but it's not idiotic and
> we have nothing to fear from it ;-)... well this time at least...
> As far as I know (and I know little about ARM). It would be anwyay
> unnatural to use /dev/port for the purpose you mention.
> ARM io space is memmory mapped, so if any file you would
> rather use /dev/kmem...
> 
> Still no flames? This silence makes me suspicious....
> 

Yawn...


Cheers,
Dick Johnson

Penguin : Linux version 2.4.18 on an i686 machine (797.90 BogoMips).

                 Windows-2000/Professional isn't.

Re: [PATCH] 2.5.17 /dev/ports

[Russell King]

On Wed, May 22, 2002 at 12:40:11PM +0200, Martin Dalecki wrote:
> ARM io space is memmory mapped, so if any file you would
> rather use /dev/kmem...

kmem = kernel memory.  That may not be the same as the physical
memory (the fact that it is at present I find mostly irrelevant here).
/dev/mem is the more correct device to use for this purpose.

> Still no flames? This silence makes me suspicious....

They're in deep sleep at the moment; try getting a metal dustbin lid and
banging it hard - maybe you'll wake the dragons from hell... 8)

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: [PATCH] 2.5.17 /dev/ports

[Alan Cox]

> kmem = kernel memory.  That may not be the same as the physical
> memory (the fact that it is at present I find mostly irrelevant here).
> /dev/mem is the more correct device to use for this purpose.

/dev/mem is also not strictly correct. Linux in/out space is operated as
synchronous I/O operations. A dumb map of /dev/mem areas can lead to
differences if the platform concerned has to do the I/O post and wait
completion handling in software. (O_SYNC is also not enough since thats
memory caching not PCI posting)

Alan

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Alan Cox napisa?:
>>kmem = kernel memory.  That may not be the same as the physical
>>memory (the fact that it is at present I find mostly irrelevant here).
>>/dev/mem is the more correct device to use for this purpose.
> 
> 
> /dev/mem is also not strictly correct. Linux in/out space is operated as
> synchronous I/O operations. A dumb map of /dev/mem areas can lead to
> differences if the platform concerned has to do the I/O post and wait
> completion handling in software. (O_SYNC is also not enough since thats
> memory caching not PCI posting)
> 

I wisper only - memzone...

Re: [PATCH] 2.5.17 /dev/ports

[Alan Cox]

> I'm waiting on Phil Blundell to notice - I think /dev/port may get used
> on ARM to emulate inb() and outb() from userspace; I don't look after
> glibc so shrug.
> 
> I agree however that /dev/port is a rotten interface that needs to go.

The /dev/port interface is used by various apps and its a traditional
x86 in paticular unix thing. For platforms like ARM its poorly implemented
since it ought to turn into a fraction of /dev/mem and support mmap for
speedier user space in/out emulation..

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Alan Cox napisa?:
>>I'm waiting on Phil Blundell to notice - I think /dev/port may get used
>>on ARM to emulate inb() and outb() from userspace; I don't look after
>>glibc so shrug.
>>
>>I agree however that /dev/port is a rotten interface that needs to go.
> 
> 
> The /dev/port interface is used by various apps and its a traditional
> x86 in paticular unix thing. For platforms like ARM its poorly implemented

Erm... unix thing? I see it only in Linux...
BTW. Just recently someone has found out that it is indeed
*poorly* implemented.


> since it ought to turn into a fraction of /dev/mem and support mmap for
> speedier user space in/out emulation..

Re: [PATCH] 2.5.17 /dev/ports

[Alan Cox]

> > The /dev/port interface is used by various apps and its a traditional
> > x86 in paticular unix thing. For platforms like ARM its poorly implemented
> 
> Erm... unix thing? I see it only in Linux...
> BTW. Just recently someone has found out that it is indeed
> *poorly* implemented.

Well you should learn to use grep, google and a library then. The /dev/port
interface is in a whole variety of older Unixen for x86, and also in systems
like Minix.

Re: [PATCH] 2.5.17 /dev/ports

[Alan Cox]

>    From: Paul Mackerras <paulus@samba.org>
>    Date: Wed, 22 May 2002 20:42:47 +1000 (EST)
> 
>    Martin Dalecki writes:
>    
>    > Remove support for /dev/port altogether.
>    
>    Great idea!
>    
> You have my blessing too :-)

I'd rather you didn't break too much application code. How do you think the
perl people and the python folks do hardware control ? Or for that matter
java people trying to avoid the crawling horrors of JNI. Its also used by
libraries like libieee1284. Clock(8) uses it on some systems if the 
/dev/rtc isn't available or built in.

Xfree86 much to my suprise seems completely clean. Non Linux stuff uses it
extensively but not Linux.

ALan

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Alan Cox napisa?:
>>   From: Paul Mackerras <paulus@samba.org>
>>   Date: Wed, 22 May 2002 20:42:47 +1000 (EST)
>>
>>   Martin Dalecki writes:
>>   
>>   > Remove support for /dev/port altogether.
>>   
>>   Great idea!
>>   
>>You have my blessing too :-)
> 
> 
> I'd rather you didn't break too much application code. How do you think the
> perl people and the python folks do hardware control ? Or for that matter
> java people trying to avoid the crawling horrors of JNI. Its also used by
> libraries like libieee1284. Clock(8) uses it on some systems if the 
> /dev/rtc isn't available or built in.
> 
> Xfree86 much to my suprise seems completely clean. Non Linux stuff uses it
> extensively but not Linux.

The Xfree86 people are actually sane and hold up the BSD tradition.
They don't even use /proc/bus and killed early /proc/agpgart usage!
Quite the reverse is true.

Re: [PATCH] 2.5.17 /dev/ports

[Alan Cox]

> The Xfree86 people are actually sane and hold up the BSD tradition.
> They don't even use /proc/bus and killed early /proc/agpgart usage!
> Quite the reverse is true.

XFree86 uses /proc/cpuinfo, /proc/bus/pci, /proc/mtrr, /proc/fb, /proc/dri
and even such goodies as /proc/sys/dev/mac_hid/keyboard_sends_linux_keycodes

Alan

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Alan Cox napisa?:
>>The Xfree86 people are actually sane and hold up the BSD tradition.
>>They don't even use /proc/bus and killed early /proc/agpgart usage!
>>Quite the reverse is true.
> 
> 
> XFree86 uses /proc/cpuinfo, /proc/bus/pci, /proc/mtrr, /proc/fb, /proc/dri
> and even such goodies as /proc/sys/dev/mac_hid/keyboard_sends_linux_keycodes
> 
> Alan

All the cases you encounter above are cases where Linux
leaks a more palatable interface.

/proc/cpuinfo for one could be replaced by dropping syslog
messages at a fixed file in /etc/ during boot - it's static after
all!.

DRI is one of the few XFree86 things which indeed
got born in the linux context. It should in fact run on
top  of either the /dev/agpgard ir /dev/fb device.
/proc/dri is a war... similar to the former /proc agp stuff..

/proc/bus - is not as bad in my opinnion :-).

Re: [PATCH] 2.5.17 /dev/ports

[Lars Marowsky-Bree]

On 2002-05-22T15:53:24,
   Martin Dalecki <dalecki@evision-ventures.com> said:

> /proc/cpuinfo for one could be replaced by dropping syslog
> messages at a fixed file in /etc/ during boot - it's static after
> all!.

It is not.

Sincerely,
    Lars Marowsky-Brée <lmb@suse.de>

-- 
Immortality is an adequate definition of high availability for me.
	--- Gregory F. Pfister

Re: [PATCH] 2.5.17 /dev/ports

[Padraig Brady]

Martin Dalecki wrote:
> /proc/cpuinfo for one could be replaced by dropping syslog
> messages at a fixed file in /etc/ during boot - it's static after
> all!.

The new cpufreq dynamic frequency scaling
stuff changes "cpu MHz" and "bogomips" at least.

Padraig.

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Padraig Brady napisa?:
> Martin Dalecki wrote:
> 
>> /proc/cpuinfo for one could be replaced by dropping syslog
>> messages at a fixed file in /etc/ during boot - it's static after
>> all!.
> 
> 
> The new cpufreq dynamic frequency scaling
> stuff changes "cpu MHz" and "bogomips" at least.

Both are sysctl stuff -> /proc/sys/kernel/cpu/
But it's a point the data can change indeed.

Re: [PATCH] 2.5.17 /dev/ports

[Dave Jones]

On Wed, May 22, 2002 at 04:07:25PM +0200, Martin Dalecki wrote:
 > > The new cpufreq dynamic frequency scaling
 > > stuff changes "cpu MHz" and "bogomips" at least.
 > Both are sysctl stuff -> /proc/sys/kernel/cpu/

It also changes /proc/cpuinfo to remain coherent.

-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

Re: [PATCH] 2.5.17 /dev/ports

[Dave Jones]

On Wed, May 22, 2002 at 03:53:24PM +0200, Martin Dalecki wrote:

 > > XFree86 uses /proc/cpuinfo, /proc/bus/pci, /proc/mtrr, /proc/fb, /proc/dri
 > > and even such goodies as /proc/sys/dev/mac_hid/keyboard_sends_linux_keycodes
 > All the cases you encounter above are cases where Linux
 > leaks a more palatable interface.

proc/cpuinfo -> /dev/cpu/n/cpuid driver
proc/mtrr -> /dev/cpu/mtrr if devfs is around, but given thats not the
 common case, there is no alternative.

 > /proc/cpuinfo for one could be replaced by dropping syslog
 > messages at a fixed file in /etc/ during boot 

userspace really should be using the per-cpu drivers.

 > it's static after all!.

In most cses, yes. However this is not going to stay the same forever..
1. hotplug CPUs
2. speedscaling work like cpufreq
3. enabling/disabling of features by poking /dev/cpu/n/msr is possible, and
   cpuinfo should reflect that (but currently doesn't)


    Dave.

-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

Re: [PATCH] 2.5.17 /dev/ports

[Alan Cox]

> /proc/cpuinfo for one could be replaced by dropping syslog
> messages at a fixed file in /etc/ during boot - it's static after
> all!.

Wrong on that one too

Re: [PATCH] 2.5.17 /dev/ports

[Pavel Machek]

Jo!

> /proc/cpuinfo for one could be replaced by dropping syslog
> messages at a fixed file in /etc/ during boot - it's static after
> all!.

cpuinfo may change at runtime -- think hot-plugging CPUs.
								Pavel
-- 
Philips Velo 1: 1"x4"x8", 300gram, 60, 12MB, 40bogomips, linux, mutt,
details at http://atrey.karlin.mff.cuni.cz/~pavel/velo/index.html.

Re: [PATCH] 2.5.17 /dev/ports

[Alexander Viro]

On Wed, 22 May 2002, Alan Cox wrote:

> > The Xfree86 people are actually sane and hold up the BSD tradition.
> > They don't even use /proc/bus and killed early /proc/agpgart usage!
> > Quite the reverse is true.
> 
> XFree86 uses /proc/cpuinfo, /proc/bus/pci, /proc/mtrr, /proc/fb, /proc/dri
> and even such goodies as /proc/sys/dev/mac_hid/keyboard_sends_linux_keycodes

... and while we are at flamewar-mongering, none of these files have any
business being in procfs.

Re: [PATCH] 2.5.17 /dev/ports

[Alan Cox]

> On Wed, 22 May 2002, Alan Cox wrote:
> 
> > > The Xfree86 people are actually sane and hold up the BSD tradition.
> > > They don't even use /proc/bus and killed early /proc/agpgart usage!
> > > Quite the reverse is true.
> > 
> > XFree86 uses /proc/cpuinfo, /proc/bus/pci, /proc/mtrr, /proc/fb, /proc/dri
> > and even such goodies as /proc/sys/dev/mac_hid/keyboard_sends_linux_keycodes
> 
> ... and while we are at flamewar-mongering, none of these files have any
> business being in procfs.

That depends on how you define procfs. Linux is not Plan 9. A lot of it 
certainly is going to cleaner with a devicefs and sysctlfs

Re: [PATCH] 2.5.17 /dev/ports

[Alexander Viro]

On Wed, 22 May 2002, Alan Cox wrote:

> > ... and while we are at flamewar-mongering, none of these files have any
> > business being in procfs.
> 
> That depends on how you define procfs. Linux is not Plan 9. A lot of it 
> certainly is going to cleaner with a devicefs and sysctlfs

OK, let me put it that way:

none of these files has any business bringing the rest of procfs along
for a ride and none of them has any reason to use any code from fs/proc/*.c

Re: [PATCH] 2.5.17 /dev/ports

[Ruth Ivimey-Cook]

On Wednesday 22 May 2002 16:10, Alexander Viro wrote:
> On Wed, 22 May 2002, Alan Cox wrote:
> > > ... and while we are at flamewar-mongering, none of these files have
> > > any business being in procfs.
> >
> > That depends on how you define procfs. Linux is not Plan 9. A lot of it
> > certainly is going to cleaner with a devicefs and sysctlfs
>
> OK, let me put it that way:
>
> none of these files has any business bringing the rest of procfs along
> for a ride and none of them has any reason to use any code from fs/proc/*.c

Ok, I'll bite. Why? Note: I'm not necessarily saying I disagree, just that I 
don't know what "test" you are applying to determine whether stuf should be 
in or out?

Personally, my test is "does it provide useful information to a program or to 
a user that is not available in other ways, or where the other ways require 
an ioctl interface". 

I insert the second phrase because I do like the general principle that in 
Unix you read and write stuff to files, and procfs does provide a lot of 
extra scope for that to happen.

There is an obvious problem with formats, but that is a specification and 
discipline issue between the various developers, and not something that is 
wrong with procfs as such.

Ruth

Re: [PATCH] 2.5.17 /dev/ports

[Rusty Russell]

On Wed, 22 May 2002 10:54:25 -0400 (EDT)
Alexander Viro <viro@math.psu.edu> wrote:
> On Wed, 22 May 2002, Alan Cox wrote:
> > XFree86 uses /proc/cpuinfo, /proc/bus/pci, /proc/mtrr, /proc/fb, /proc/dri
> > and even such goodies as /proc/sys/dev/mac_hid/keyboard_sends_linux_keycodes
> 
> ... and while we are at flamewar-mongering, none of these files have any
> business being in procfs.

Let it never be said that you lack courage 8)

Let's sort this out at the kernel summit:
 dev vs. driverfs. vs proc vs sysctl vs boot params vs. module params vs netlink

Rusty.
-- 
   there are those who do and those who hang on and you don't see too
   many doers quoting their contemporaries.  -- Larry McVoy

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Rusty Russell napisa?:
> On Wed, 22 May 2002 10:54:25 -0400 (EDT)
> Alexander Viro <viro@math.psu.edu> wrote:
> 
>>On Wed, 22 May 2002, Alan Cox wrote:
>>
>>>XFree86 uses /proc/cpuinfo, /proc/bus/pci, /proc/mtrr, /proc/fb, /proc/dri
>>>and even such goodies as /proc/sys/dev/mac_hid/keyboard_sends_linux_keycodes
>>
>>... and while we are at flamewar-mongering, none of these files have any
>>business being in procfs.
> 
> 
> Let it never be said that you lack courage 8)
> 
> Let's sort this out at the kernel summit:
>  dev vs. driverfs. vs proc vs sysctl vs boot params vs. module params vs netlink

There isn't that much to be discussed there.

1. /proc is for process data becouse this has inherently a tree structure and
the FS abstractis is fitting this nicely. (We can share code with the VFS layer
therefore.)

2. /proc/sys is justifyed by the fact that sysctl can share a significant
amount of code with the procfs implementation. Note this *could* be changed
by abstracting the common code out of *both* procfs and sysctl instead
of stacking sysctl on to of procfs.

3. /proc/bus is superceeded by driverfs but has a tree struct and one can life
with it.

The rest is utter crap and legacy. Technically at least.

In particular the stuff listed above is looking like things which are in
reality device access abstractions and which belongs therefore to /dev/.

The only problem here is - people without taste for the implementation,
apparently love to look at files in /proc becouse this is giving them some
feelings I can't share...

PS. Did I mention that uniformity of interfaces is quite common in good
design practice? And something beeing ASCII simple doesn't imply that
it is an uniform interface.

Re: [PATCH] 2.5.17 /dev/ports

[Rusty Russell]

In message <3CEC8FB2.8090900@evision-ventures.com> you write:
> Uz.ytkownik Rusty Russell napisa?:
> >  dev vs. driverfs. vs proc vs sysctl vs boot params vs. module params vs netlink
> 
> There isn't that much to be discussed there.

[ one page of opinionated rant deleted 8) ]

I think you've proven my point that this needs discussion, and at the
same time proven my point that this is not a place it can be discussed
8)

Thanks!
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] 2.5.17 /dev/ports

[Padraig Brady]

Martin Dalecki wrote:
> Remove support for /dev/port altogether.

FYI:

[root@pixelbeat padraig]# find /bin /usr/bin /lib /sbin /usr/sbin 
/usr/lib -maxdepth 1 -type f -perm +111 | xargs grep -l "/dev/port"
/sbin/hwclock: util-linux
/sbin/kbdrate: util-linux
/bin/watchdog: ;-)

Padraig.

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Padraig Brady napisa?:
> Martin Dalecki wrote:
> 
>> Remove support for /dev/port altogether.
> 
> 
> FYI:
> 
> [root@pixelbeat padraig]# find /bin /usr/bin /lib /sbin /usr/sbin 
> /usr/lib -maxdepth 1 -type f -perm +111 | xargs grep -l "/dev/port"
> /sbin/hwclock: util-linux
> /sbin/kbdrate: util-linux
> /bin/watchdog: ;-)

Let's have a closer look.

[root@kozaczek sbin]# rpm -qf /sbin/kbdrate
util-linux-2.11n-12
[root@kozaczek sbin]# rpm -qf /sbin/hwclock
util-linux-2.11n-12
[root@kozaczek sbin]#

/dev/null {} \;util-linux-2.11r]# find ./ -name "*.[ch]" -exec grep \/dev\/port
./po/cat-id-tbl.c:  {"Cannot open /dev/port: %s", 971},
./hwclock/cmos.c:    if ((dev_port_fd = open("/dev/port", O_RDWR)) < 0) {
./hwclock/cmos.c:      fprintf(stderr, _("Cannot open /dev/port: %s"), 
strerror(errsv));
./hwclock/clock-ppc.c: *  code and not via /dev/port (still possible via #undef 
...)."

static int
get_permissions_cmos(void) {
   int rc;

   if (use_dev_port) {
     if ((dev_port_fd = open("/dev/port", O_RDWR)) < 0) {
       int errsv = errno;
       fprintf(stderr, _("Cannot open /dev/port: %s"), strerror(errsv));
       rc = 1;
     } else
       rc = 0;
   } else {
     rc = i386_iopl(3);
     if (rc == -2) {

./hwclock/cmos.c:int use_dev_port = 0;          /* 1 for Jensen */
./hwclock/cmos.c:    use_dev_port = 1;
./hwclock/cmos.c:  if (use_dev_port) {
./hwclock/cmos.c:  if (use_dev_port) {
./hwclock/cmos.c:  if (use_dev_port) {
[root@kozaczek util-linux-2.11r]#


void
set_cmos_access(int Jensen, int funky_toy) {

   /* See whether we're dealing with a Jensen---it has a weird I/O
      system.  DEC was just learning how to build Alpha PCs.  */
   if (Jensen || is_in_cpuinfo("system type", "Jensen")) {
     use_dev_port = 1;
     clock_ctl_addr = 0x170;
     clock_data_addr = 0x171;
     if (debug) printf (_("clockport adjusted to 0x%x\n"), clock_ctl_addr);
   }

You can see from the above that the code in question
is accessing /dev/port only for the Jensen architecture...
which is:

1. Obsolete by a bright margin.

2. Very rare.

3. Should be fixed anyway.

4. Most possibly not correct anylonger.

So both of the above aplications in fact don't access /dev/port
at all at 99.9% of the systems.
Since they belong in to the util-linux category - well
we require even new versions of mount for new kernels.

Still no problem at all.

Re: [PATCH] 2.5.17 /dev/ports

[Sebastian Droege]

[-- Attachment #1: Type: text/plain, Size: 791 bytes --]

On Wed, 22 May 2002 14:30:44 +0200
Martin Dalecki <dalecki@evision-ventures.com> wrote:

> Uz.ytkownik Padraig Brady napisa?:
> > Martin Dalecki wrote:
> > 
> >> Remove support for /dev/port altogether.
> > 
> > 
> > FYI:
> > 
> > [root@pixelbeat padraig]# find /bin /usr/bin /lib /sbin /usr/sbin 
> > /usr/lib -maxdepth 1 -type f -perm +111 | xargs grep -l "/dev/port"
> > /sbin/hwclock: util-linux
> > /sbin/kbdrate: util-linux
> > /bin/watchdog: ;-)
> 
> Let's have a closer look.
> 
[snip]
> 
> You can see from the above that the code in question
> is accessing /dev/port only for the Jensen architecture...
This is util-linux 2.11n on x86:

slomosnail:~# kbdrate 
Cannot open /dev/port: No such file or directory

slomosnail:~# hwclock 
Wed May 22 15:48:50 2002  -0.460607 seconds

Bye

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] 2.5.17 /dev/ports

[Alan Cox]

> /sbin/kbdrate: util-linux

I'd hope kbrate is using the ioctls nowdays, otherwise it will break on USB

Re: [PATCH] 2.5.17 /dev/ports

[Vojtech Pavlik]

On Wed, May 22, 2002 at 02:52:19PM +0100, Alan Cox wrote:

> > /sbin/kbdrate: util-linux
> 
> I'd hope kbrate is using the ioctls nowdays, otherwise it will break on USB

Actually, the ioctls won't work on USB, because they try to output data
to the traditional i/o ports anyway.

-- 
Vojtech Pavlik
SuSE Labs

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Vojtech Pavlik napisa?:
> On Wed, May 22, 2002 at 02:52:19PM +0100, Alan Cox wrote:
> 
> 
>>>/sbin/kbdrate: util-linux
>>
>>I'd hope kbrate is using the ioctls nowdays, otherwise it will break on USB
> 
> 
> Actually, the ioctls won't work on USB, because they try to output data
> to the traditional i/o ports anyway.


So at least we know now:

1. Kernel is bogous.
2. util-linux is bogous.

IOCTL is ineed the way to go to implement such functionality...

Re: [PATCH] 2.5.17 /dev/ports

[Vojtech Pavlik]

On Wed, May 22, 2002 at 02:51:49PM +0200, Martin Dalecki wrote:
> Uz.ytkownik Vojtech Pavlik napisa?:
> > On Wed, May 22, 2002 at 02:52:19PM +0100, Alan Cox wrote:
> > 
> > 
> >>>/sbin/kbdrate: util-linux
> >>
> >>I'd hope kbrate is using the ioctls nowdays, otherwise it will break on USB
> > 
> > 
> > Actually, the ioctls won't work on USB, because they try to output data
> > to the traditional i/o ports anyway.
> 
> 
> So at least we know now:
> 
> 1. Kernel is bogous.
> 2. util-linux is bogous.
> 
> IOCTL is ineed the way to go to implement such functionality...

Yes, the EVIOCSREP ioctl will be the one soon (works for USB keyboards
now).

-- 
Vojtech Pavlik
SuSE Labs

Re: [PATCH] 2.5.17 /dev/ports

[Alan Cox]

> > IOCTL is ineed the way to go to implement such functionality...
> 
> Yes, the EVIOCSREP ioctl will be the one soon (works for USB keyboards
> now).

The KBDRATE ioctl is already supported by all other keyboard drivers and
used by XFree86....

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Alan Cox napisa?:
>>>IOCTL is ineed the way to go to implement such functionality...
>>
>>Yes, the EVIOCSREP ioctl will be the one soon (works for USB keyboards
>>now).
> 
> 
> The KBDRATE ioctl is already supported by all other keyboard drivers and
> used by XFree86....


Yeep. Now I even remember from the dust of my mind,
that there is a *direct mapping*
between terminal settings and ioctl numbers present in
the line discipine code. So ioctl is definitively the way to
go.

BTW.> Did I mention today that the XFree86 people are
indeed the sane ones and always choose the most
UNIX a like and proper interfaces apparently?
!Marvelously good work over a long time out there!
(Well not for font issues, in esp. in an i18n context,
but that's another story...)

Re: [PATCH] 2.5.17 /dev/ports

[Vojtech Pavlik]

On Wed, May 22, 2002 at 03:58:38PM +0100, Alan Cox wrote:

> > > IOCTL is ineed the way to go to implement such functionality...
> > 
> > Yes, the EVIOCSREP ioctl will be the one soon (works for USB keyboards
> > now).
> 
> The KBDRATE ioctl is already supported by all other keyboard drivers and
> used by XFree86....

Correct. And it'll work on USB as well once the console code is
interfaced to USB better than just by injecting scancodes into
pc_keyb.c.

KBDRATE will work on console, while EVIOCSREP will work if you open the
keyboard as an event device.

-- 
Vojtech Pavlik
SuSE Labs

Re: [PATCH] 2.5.17 /dev/ports

[Alexander Viro]

On Wed, 22 May 2002, Martin Dalecki wrote:

> So at least we know now:
> 
> 1. Kernel is bogous.
> 2. util-linux is bogous.
> 
> IOCTL is ineed the way to go to implement such functionality...

For kbdrate???  sysctl I might see - after all, we are talking about
setting two numbers.  ioctl() to pass a couple of integers to the kernel?
No, thanks.

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Alexander Viro napisa?:
> 
> On Wed, 22 May 2002, Martin Dalecki wrote:
> 
> 
>>So at least we know now:
>>
>>1. Kernel is bogous.
>>2. util-linux is bogous.
>>
>>IOCTL is ineed the way to go to implement such functionality...
> 
> 
> For kbdrate???  sysctl I might see - after all, we are talking about
> setting two numbers.  ioctl() to pass a couple of integers to the kernel?
> No, thanks.

Portable along architectures - no thanks?
Portbale along different devices and device driver implementations - no thanks?
Not to mess with hardware with preassumtptions how it works - no thanks?
Giving PC vendors a chance to get rid of silly legacy hardware - no thanks?
Abviously documented by beeing there - no thanks?
Just one case in switch statement + few bytes for copy from user and stuff - no 
thanks?
Actual hardware functionality abstraction - no thanks?
Operating system - no thanks?

*BUT* filesystems attached to /dev/ nodes - NO THANKS indeed!

Re: [PATCH] 2.5.17 /dev/ports

[Alexander Viro]

On Wed, 22 May 2002, Martin Dalecki wrote:

> > For kbdrate???  sysctl I might see - after all, we are talking about
> > setting two numbers.  ioctl() to pass a couple of integers to the kernel?
> > No, thanks.

If you are complaining about use of /dev/port - I completely agree that it's
crap...
 
> Portable along architectures - no thanks?
> Portbale along different devices and device driver implementations - no thanks?
> Not to mess with hardware with preassumtptions how it works - no thanks?
> Giving PC vendors a chance to get rid of silly legacy hardware - no thanks?
> Abviously documented by beeing there - no thanks?

... but WTF does it have to ioctl vs. saner mechanisms?

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Alexander Viro napisa?:
> 
> On Wed, 22 May 2002, Martin Dalecki wrote:
> 
> 
>>>For kbdrate???  sysctl I might see - after all, we are talking about
>>>setting two numbers.  ioctl() to pass a couple of integers to the kernel?
>>>No, thanks.
>>
> 
> If you are complaining about use of /dev/port - I completely agree that it's
> crap...

Oh then sorry we must have missed the point.
Maybe I was just too eager for a nice flame-war which doesn't
contain the magical word BitKeeper or similar ;-)...
Anyway I'm satisfied now ;-))))...


>>Portable along architectures - no thanks?
>>Portbale along different devices and device driver implementations - no thanks?
>>Not to mess with hardware with preassumtptions how it works - no thanks?
>>Giving PC vendors a chance to get rid of silly legacy hardware - no thanks?
>>Abviously documented by beeing there - no thanks?
> 
> 
> ... but WTF does it have to ioctl vs. saner mechanisms?

It's all about the basic principle that operating systems
serve the principal purpose of abstracting hardware inferfaces
out of the actual hardware implementation and provide
*abstract* interfaces to the user space programmer. Not
more.

In this particular case it's just that ioctl fits it best.
Becouse:

1. It's already there.

2. What is there should only be fixed.

3. It's at the right level (input hardware).

4. It's more easy to implement then every thing else.

5. It's easiest to use by the application programmer.

6. I don't think that there is something saner out there.

7. Did I mention that it's already there?

But now I see that you have just certainly missed only
some previous points in this "discussion" and I wouldn't
of couse at no single second think seriously,
that the former triviallity isn't abvious to you ;-).

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Alexander Viro napisa?:
> 
> On Wed, 22 May 2002, Martin Dalecki wrote:
> 
> 
>>So at least we know now:
>>
>>1. Kernel is bogous.
>>2. util-linux is bogous.
>>
>>IOCTL is ineed the way to go to implement such functionality...
> 
> 
> For kbdrate???  sysctl I might see - after all, we are talking about
> setting two numbers.  ioctl() to pass a couple of integers to the kernel?
> No, thanks.

Ahhh and just another note - we are talking about a property of a
*device* not a property of the kernel - so ioctl (read io as device)
and certainly not sysctl (read sys as kernel).

What could be sonsidered as an *serious* alternative would
be to abstract it out even further and implement it on
the tset (terminal settings) levels. But *certainly* not sysctl.

Re: [PATCH] 2.5.17 /dev/ports

[Alexander Viro]

On Wed, 22 May 2002, Martin Dalecki wrote:

> > For kbdrate???  sysctl I might see - after all, we are talking about
> > setting two numbers.  ioctl() to pass a couple of integers to the kernel?
> > No, thanks.
> 
> Ahhh and just another note - we are talking about a property of a
> *device* not a property of the kernel - so ioctl (read io as device)
> and certainly not sysctl (read sys as kernel).
> 
> What could be sonsidered as an *serious* alternative would
> be to abstract it out even further and implement it on
> the tset (terminal settings) levels. But *certainly* not sysctl.

Well...  Point about per-device taken, but strictly speaking we
do have e.g. per-interface sysctls, etc.

Re: [PATCH] 2.5.17 /dev/ports

[James Simmons]

> > For kbdrate???  sysctl I might see - after all, we are talking about
> > setting two numbers.  ioctl() to pass a couple of integers to the kernel?
> > No, thanks.
> 
> Ahhh and just another note - we are talking about a property of a
> *device* not a property of the kernel - so ioctl (read io as device)
> and certainly not sysctl (read sys as kernel).
> 
> What could be sonsidered as an *serious* alternative would
> be to abstract it out even further and implement it on
> the tset (terminal settings) levels. But *certainly* not sysctl.

Hm. That is a interesting idea. I like it.

Re: [PATCH] 2.5.17 /dev/ports

[Vojtech Pavlik]

On Wed, May 22, 2002 at 09:59:41AM -0400, Alexander Viro wrote:

> On Wed, 22 May 2002, Martin Dalecki wrote:
> 
> > So at least we know now:
> > 
> > 1. Kernel is bogous.
> > 2. util-linux is bogous.
> > 
> > IOCTL is ineed the way to go to implement such functionality...
> 
> For kbdrate???  sysctl I might see - after all, we are talking about
> setting two numbers.  ioctl() to pass a couple of integers to the kernel?
> No, thanks.

Sysctl won't be too good an idea if you have more than one keyboard.
It needs to be device-related, hence an ioctl.

-- 
Vojtech Pavlik
SuSE Labs

Re: [PATCH] 2.5.17 /dev/ports

[Pavel Machek]

Hi

> > So at least we know now:
> > 
> > 1. Kernel is bogous.
> > 2. util-linux is bogous.
> > 
> > IOCTL is ineed the way to go to implement such functionality...
> 
> For kbdrate???  sysctl I might see - after all, we are talking about
> setting two numbers.  ioctl() to pass a couple of integers to the kernel?
> No, thanks.

There may be more than one keyboard in the system, so sysctl() is not 
suitable.
								Pavel
-- 
Philips Velo 1: 1"x4"x8", 300gram, 60, 12MB, 40bogomips, linux, mutt,
details at http://atrey.karlin.mff.cuni.cz/~pavel/velo/index.html.

Re: [PATCH] 2.5.17 /dev/ports

[Alan Cox]

> On Wed, May 22, 2002 at 02:52:19PM +0100, Alan Cox wrote:
> 
> > > /sbin/kbdrate: util-linux
> > 
> > I'd hope kbrate is using the ioctls nowdays, otherwise it will break on USB
> 
> Actually, the ioctls won't work on USB, because they try to output data
> to the traditional i/o ports anyway.

The ioctl goes to the keyboard driver. They keyboard driver either
implements it, errors it or is buggy. No other path. I seem to be able to
portably set my keyboard rate on everything but USB 8)

Re: [PATCH] 2.5.17 /dev/ports

[Vojtech Pavlik]

On Wed, May 22, 2002 at 04:00:23PM +0100, Alan Cox wrote:

> > On Wed, May 22, 2002 at 02:52:19PM +0100, Alan Cox wrote:
> > 
> > > > /sbin/kbdrate: util-linux
> > > 
> > > I'd hope kbrate is using the ioctls nowdays, otherwise it will break on USB
> > 
> > Actually, the ioctls won't work on USB, because they try to output data
> > to the traditional i/o ports anyway.
> 
> The ioctl goes to the keyboard driver. They keyboard driver either
> implements it, errors it or is buggy. No other path. I seem to be able to
> portably set my keyboard rate on everything but USB 8)

Agreed, current method of interfacing USB keyboards to keyboard.c (as
implemented by keybdev.c) is a bug in itself.

-- 
Vojtech Pavlik
SuSE Labs

Re: [PATCH] 2.5.17 /dev/ports

[Linus Torvalds]

On Wed, 22 May 2002, Martin Dalecki wrote:
>
> Remove support for /dev/port altogether.

Yes, I don't think it has actually ever been used.

It was done purely because Minix did it that way, and it wasn't even
compatible with Minix (I think Minix actually supoorted 2- and 4-byte
accesses by just doign 2- and 4-byte read/write calls, the Linux code
never did).

Everybody always used the iobitmap/iopl interfaces under Linux as far as I
know.

Anybody: if you've ever used /dev/ports, holler _now_.

		Linus

Re: [PATCH] 2.5.17 /dev/ports

[Alan Cox]

> Anybody: if you've ever used /dev/ports, holler _now_.

Holler. I posted a list of examples to linux-kernel already. iopl and ioperm
are not portable in the way /dev/port is. ioperm/iopl also doesnt work
with most scripting languages, java tools trying to avoid JNI etc

I've seen it used in tools written in java, python, perl, even tcl

Other examples include libieee1284, the pic 16x84 programmer, hwclock,
older kbdrate, /sbin/clock on machines that don't have /dev/rtc.

Not everything in the world is an x86, and not every app wants to be Linux/x86
specific or use weird syscalls

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Alan Cox napisa?:
>>Anybody: if you've ever used /dev/ports, holler _now_.
> 
> 
> Holler. I posted a list of examples to linux-kernel already. iopl and ioperm
> are not portable in the way /dev/port is. ioperm/iopl also doesnt work
> with most scripting languages, java tools trying to avoid JNI etc

What?

#include <linux/io.h>
#include <stdio.h>
#include <stdlib.h>

int main(char *argv[], int argc)
{
	int port = aoit(argv[0]);
	 int byte = aoit(argv[1]);

	if (port > 0)
		return inb(port);		
	 else
			outb(port, byte);


		return 0;
}

and then syscall("/sbin/doportio")

Is certainly *NOT IMPOSSIBLE*. But it's of course too
much of a burden...

BTW> Under java it's rather hard to get around
CAP_RAWIO if you ask me without going down to JNI.

> I've seen it used in tools written in java, python, perl, even tcl
> 
> Other examples include libieee1284, the pic 16x84 programmer, hwclock,
> older kbdrate, /sbin/clock on machines that don't have /dev/rtc.

All the examples above are samples of bad coding practice - I have
uncovered already here in C what can be expected inside there!

> Not everything in the world is an x86, and not every app wants to be Linux/x86
> specific or use weird syscalls

Yes and in esp. everything in the world is a __m68000__!

Re: [PATCH] 2.5.17 /dev/ports

[Russell King]

On Wed, May 22, 2002 at 06:17:26PM +0200, Martin Dalecki wrote:
> #include <linux/io.h>
> #include <stdio.h>
> #include <stdlib.h>
> 
> int main(char *argv[], int argc)
> {
> 	int port = aoit(argv[0]);
> 	 int byte = aoit(argv[1]);
> 
> 	if (port > 0)
> 		return inb(port);		
> 	 else
> 			outb(port, byte);
> 
> 
> 		return 0;
> }

Erm:

1. not checking number of arguments passed.
2. thinking argv[0] is the first arg.
3. wrong test for in/out (port > 0 -> inb, port <= 0 -> outb)
4. returning the read byte via the program status code.
5. aoit is an undefined function.
6. including linux/*.h is fundamentally wrong for any user space
   program.

That's one bug every 2 lines.  Should I continue? 8)

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: [PATCH] 2.5.17 /dev/ports

[Martin Dalecki]

Uz.ytkownik Russell King napisa?:
> On Wed, May 22, 2002 at 06:17:26PM +0200, Martin Dalecki wrote:
> 
>>#include <linux/io.h>
>>#include <stdio.h>
>>#include <stdlib.h>
>>
>>int main(char *argv[], int argc)
>>{
>>	int port = aoit(argv[0]);
>>	 int byte = aoit(argv[1]);
>>
>>	if (port > 0)
>>		return inb(port);		
>>	 else
>>			outb(port, byte);
>>
>>
>>		return 0;
>>}
> 
> 
> Erm:
> 
> 1. not checking number of arguments passed.
> 2. thinking argv[0] is the first arg.
> 3. wrong test for in/out (port > 0 -> inb, port <= 0 -> outb)
> 4. returning the read byte via the program status code.
> 5. aoit is an undefined function.
> 6. including linux/*.h is fundamentally wrong for any user space
>    program.
> 
> That's one bug every 2 lines.  Should I continue? 8)

Where the *hell* - did I say that the above program was supposed
to be ANSI C ?#$@#! It's in my own
advanced/object oriented/intuitive/component/virtual machine
based mdcki-c-alike-python commercial closed source language
I'm developing to provide a replacement base for the next
generation of Rudy102 distro configuration utilities if you ask. OK?

BTW.> I'm sure it will bite the pants out of C# in the year 2100 where I
have scheduled it for the first public release.

Any questions remaining?

Re: [PATCH] 2.5.17 /dev/ports

[Alexander Viro]

On Wed, 22 May 2002, Russell King wrote:

> > int main(char *argv[], int argc)

> 1. not checking number of arguments passed.
> 2. thinking argv[0] is the first arg.

Sod that; thinking that _argv_ is the first argument of main(). ;-)

Re: [PATCH] 2.5.17 /dev/ports

[Alan Cox]

> BTW> Under java it's rather hard to get around
> CAP_RAWIO if you ask me without going down to JNI.

People run them as root. Thats not rocket science

> > I've seen it used in tools written in java, python, perl, even tcl
> > 
> > Other examples include libieee1284, the pic 16x84 programmer, hwclock,
> > older kbdrate, /sbin/clock on machines that don't have /dev/rtc.
> 
> All the examples above are samples of bad coding practice - I have
> uncovered already here in C what can be expected inside there!

Portable code is good practice.

> > Not everything in the world is an x86, and not every app wants to be Linux/x86
> > specific or use weird syscalls
> 
> Yes and in esp. everything in the world is a __m68000__!

When you look at the kernel codebase its abundantly obvious that its much
more "if its not 32bit with hardware managed caches, little endian, with
no more than one root pci bridge, has a 32/64bit machine word, is byte 
addressed, has sane store ordering rules, conventional not reverse mapped
mmu, machine word sized cache coherency, and no more than 3 level page
tables" and so forth its going to be hard to get it to work well.
 
A lot of that /dev/port using code is portable to everything from Minix through
most BSD's. It works on pretty much anything with PC style devices.

Alan

Re: [PATCH] 2.5.17 /dev/ports

[Riley Williams]

Hi Martin.

> #include <linux/io.h>
> #include <stdio.h>
> #include <stdlib.h>
> 
> int main(char *argv[], int argc)
> {
>	int port = aoit(argv[0]);
>	int byte = aoit(argv[1]);
>
>	if (port > 0)
>		return inb(port);		
>	else {
>		outb(port, byte);
>		return 0;
>	}
> }

Interesting code. Did you really mean to have the program name as the
port number, or should that be argv[1] and argv[2] instead? Also, what
is it supposed to do? A quick analysis makes no sense of it.

Would the following be what you had intended?

	#include <linux/io.h>
	#include <stdio.h>
	#include <stdlib.h>

	int main(int argc, char **argv) {
	    int byte, port, result = 0;

	    switch (argc) {
		case 2:
		    port = atoi(argv[1]);
		    result = inb(port);
		    break;

		case 3:
		    port = atoi(argv[1]);
		    byte = atoi(argv[2]);
		    outb(port, byte);
		    break;

		default:
		    fprintf(stderr, "Usage: %s port [byte]\n", argv[0]);
		    break;
	    }
	    return result;
	}

Best wishes from Riley.

Re: [PATCH] 2.5.17 /dev/ports

[Vojtech Pavlik]

On Sun, May 26, 2002 at 02:53:37PM +0100, Riley Williams wrote:

> Would the following be what you had intended?
> 
> 	#include <linux/io.h>
> 	#include <stdio.h>
> 	#include <stdlib.h>
> 
> 	int main(int argc, char **argv) {
> 	    int byte, port, result = 0;

	    iopl(3);
> 
> 	    switch (argc) {
> 		case 2:
> 		    port = atoi(argv[1]);
> 		    result = inb(port);
> 		    break;
> 
> 		case 3:
> 		    port = atoi(argv[1]);
> 		    byte = atoi(argv[2]);
> 		    outb(port, byte);
> 		    break;
> 
> 		default:
> 		    fprintf(stderr, "Usage: %s port [byte]\n", argv[0]);
> 		    break;
> 	    }
> 	    return result;
> 	}

-- 
Vojtech Pavlik
SuSE Labs

Re: [PATCH] 2.5.17 /dev/ports

[Riley Williams]

Hi Vojtech, Martin.

>> Would the following be what you had intended?
>>
>>	#include <linux/io.h>
>>	#include <stdio.h>
>>	#include <stdlib.h>
>>
>>	int main(int argc, char **argv) {
>>	    int byte, port, result = 0;
>> 
>	    iopl(3);
>>	    switch (argc) {
>>		case 2:
>>		    port = atoi(argv[1]);
>>		    result = inb(port);
>>		    break;
>>
>>		case 3:
>>		    port = atoi(argv[1]);
>>		    byte = atoi(argv[2]);
>>		    outb(port, byte);
>>		    break;
>>
>>		default:
>>		    fprintf(stderr, "Usage: %s port [byte]\n",
>>			    argv[0]);
		    result = -1;
>>		    break;
>>	    }
	    exit(result);
>>	}

Wonder how many bugs are left now those two have been squished...

Best wishes from Riley.

Re: [PATCH] 2.5.17 /dev/ports

[Martin Diehl]

On Wed, 22 May 2002, Linus Torvalds wrote:

> Anybody: if you've ever used /dev/ports, holler _now_.

For me, "hexedit /dev/port" turned out being a pretty helpful tool for
simple hardware analysis. Besides playing what-if games it's also
sometimes useful for driver testing to trigger certain paths for example.

Wouldn't it be worth keeping it opt-in (like CONFIG_NVRAM e.g.)?

Martin

Re: Linux-2.5.17

[Martin Dalecki]

Uz.ytkownik Linus Torvalds napisa?:

> 
> Summary of changes from v2.5.16 to v2.5.17
> ============================================

> 
> <jack@suse.cz>
> 	o quota-1-newlocks
> 	o quota-2-formats
> 	o quota-3-register
> 	o quota-4-getstats
> 	o quota-5-space
> 	o quota-6-bytes
> 	o quota-7-quotactl
> 	o quota-8-format1
> 	o quota-9-format2
> 	o quota-10-inttype
> 	o quota-11-sync
> 	o quota-12-compat
> 	o quota-13-ioctl

Please put the following crap under /proc/sys/fs,
where it belongs. OK?

[root@kozaczek fs]# pwd
/proc/fs
[root@kozaczek fs]# cat quota
Version 60501
Formats
0 0 0 0 0 0 0 8
[root@kozaczek fs]#

Or are are you going to reinvent just enother
case of /proc/ formatting compatibility problems?!
And the requirement to have /proc mounted for quoate usage?!

I hate /proc/my/random/sandbox/becouse/I/dont/knwo/unix/and/have/no/taste
interfaces more and more...

(PS. Hah! I found finally someone today who deserves flames! :-).)

Re: Linux-2.5.17

[Alexander Viro]

On Wed, 22 May 2002, Martin Dalecki wrote:
 
> Or are are you going to reinvent just enother
> case of /proc/ formatting compatibility problems?!
> And the requirement to have /proc mounted for quoate usage?!
> 
> I hate /proc/my/random/sandbox/becouse/I/dont/knwo/unix/and/have/no/taste
> interfaces more and more...
> 
> (PS. Hah! I found finally someone today who deserves flames! :-).)

Gives the phrase "finding yourself" a whole new meaning, doesn't it?

Al, deeply PO'd by assorted cretinisms _not_ related to the kernel.
Sigh...

Re: Linux-2.5.17

[Martin Dalecki]

Uz.ytkownik Alexander Viro napisa?:
> 
> On Wed, 22 May 2002, Martin Dalecki wrote:
>  
> 
>>Or are are you going to reinvent just enother
>>case of /proc/ formatting compatibility problems?!
>>And the requirement to have /proc mounted for quoate usage?!
>>
>>I hate /proc/my/random/sandbox/becouse/I/dont/knwo/unix/and/have/no/taste
>>interfaces more and more...
>>
>>(PS. Hah! I found finally someone today who deserves flames! :-).)
> 
> 
> Gives the phrase "finding yourself" a whole new meaning, doesn't it?
> 
> Al, deeply PO'd by assorted cretinisms _not_ related to the kernel.
> Sigh...

Lokking at 2.5.17 I see the following:

-#define QUOTAFILENAME "quota"
-#define QUOTAGROUP "staff"


As usuall we can see what goes to /proc is apparently
random bulls*it as always. I love in esp. the assumption about
some group name on a system!
But it get's removed this time. So let's peer where
it get's reintroduced:

Ah... yes, patch-2.5.17, here it is:

+#ifdef CONFIG_PROC_FS
+static int read_stats(char *buffer, char **start, off_t offset, int count, int 
*eof, void *data)
+{
+ 
int len;
+ 
struct quota_format_type *actqf;
+
+ 
dqstats.allocated_dquots = nr_dquots;
+ 
dqstats.free_dquots = nr_free_dquots;
+
+ 
len = sprintf(buffer, "Version %u\n", __DQUOT_NUM_VERSION__);
+ 
len += sprintf(buffer + len, "Formats");
+ 
lock_kernel();
+ 
for (actqf = quota_formats; actqf; actqf = actqf->qf_next)
+ 
	len += sprintf(buffer + len, " %u", actqf->qf_fmt_id);
  	unlock_kernel();
- 
return ret;
+ 
len += sprintf(buffer + len, "\n%u %u %u %u %u %u %u %u\n",
+ 
		dqstats.lookups, dqstats.drops,
+ 
		dqstats.reads, dqstats.writes,
+ 
		dqstats.cache_hits, dqstats.allocated_dquots,
+ 
		dqstats.free_dquots, dqstats.syncs);
+
+ 
if (offset >= len) {
+ 
	*start = buffer;
+ 
	*eof = 1;
+ 
	return 0;
+ 
}
+ 
*start = buffer + offset;
+ 
if ((len -= offset) > count)
+ 
	return count;
+ 
*eof = 1;
+
+ 
return len;
+}
+#endif

What can we see in the above:

1. Those are first grade candidates for sysctl read-only entires, since they
    are system global statistics which should belong to /proc/sys/fs/
    We even have already fs.dquot-nr there! Why the hell don't put them
    alongside?

2. Typical string formating and value copy and termination
     problems inherent to string stuff...

3. The futile hope that tools using it will even bother to check the
    Version... gtop just *right today* showed that user space programmers
    won't care about it, so it gains us literally *nothing*.

If it where sysctl numbers they would just vanish beneath them if something
changed semantincally and they *would have no chance* to do it wrong.

Re: Linux-2.5.17

[Alexander Viro]

On Wed, 22 May 2002, Martin Dalecki wrote:

> 2. Typical string formating and value copy and termination
>      problems inherent to string stuff...

s/inherent to/inherent to incompetently written/

BTW, quoted code should've used seq_file helpers - that would both
cut the code size way down and fix the damn thing.

Re: Linux-2.5.17

[Martin Dalecki]

Uz.ytkownik Alexander Viro napisa?:
> 
> On Wed, 22 May 2002, Martin Dalecki wrote:
> 
> 
>>2. Typical string formating and value copy and termination
>>     problems inherent to string stuff...
> 
> 
> s/inherent to/inherent to incompetently written/
> 
> BTW, quoted code should've used seq_file helpers - that would both
> cut the code size way down and fix the damn thing.

Ah... I think I will just provide the step toward the
/proc/sys/fs. Code talks best I think in this case.

jack would you mind it?
Are there any user land tool issues I should keep an eye
on?

Re: Linux-2.5.17

[Jan Kara]

  Hello,
  
> Uz.ytkownik Alexander Viro napisa?:
> >
> >On Wed, 22 May 2002, Martin Dalecki wrote:
> > 
> >
> >>Or are are you going to reinvent just enother
> >>case of /proc/ formatting compatibility problems?!
> >>And the requirement to have /proc mounted for quoate usage?!
> >>
> >>I hate /proc/my/random/sandbox/becouse/I/dont/knwo/unix/and/have/no/taste
> >>interfaces more and more...
> >>
> >>(PS. Hah! I found finally someone today who deserves flames! :-).)
> >
> >
> >Gives the phrase "finding yourself" a whole new meaning, doesn't it?
> >
> >Al, deeply PO'd by assorted cretinisms _not_ related to the kernel.
> >Sigh...
> 
> Lokking at 2.5.17 I see the following:
> 
> -#define QUOTAFILENAME "quota"
> -#define QUOTAGROUP "staff"
> 
> 
> As usuall we can see what goes to /proc is apparently
> random bulls*it as always. I love in esp. the assumption about
> some group name on a system!
> But it get's removed this time. So let's peer where
> it get's reintroduced:
  gets reintroduced? I think I removed QUOTAGROUP forever...
  
> Ah... yes, patch-2.5.17, here it is:
> 
> +#ifdef CONFIG_PROC_FS
> +static int read_stats(char *buffer, char **start, off_t offset, int count, 
> int *eof, void *data)
> +{
> + 
  <snip>
  
> return len;
> +}
> +#endif
> 
> What can we see in the above:
> 
> 1. Those are first grade candidates for sysctl read-only entires, since they
>    are system global statistics which should belong to /proc/sys/fs/
>    We even have already fs.dquot-nr there! Why the hell don't put them
>    alongside?
> 
> 2. Typical string formating and value copy and termination
>     problems inherent to string stuff...
  I agree that the proc code isn't good (maybe you missed the mail from
Christoph Hellwing and my answer to it...) and should be replaced.

> 3. The futile hope that tools using it will even bother to check the
>    Version... gtop just *right today* showed that user space programmers
>    won't care about it, so it gains us literally *nothing*.
  The hope isn't futile I think. At least quota tools (which are
IMHO the most interesting) are checking the version and warning user
about too new kernel.

> If it where sysctl numbers they would just vanish beneath them if something
> changed semantincally and they *would have no chance* to do it wrong.
  The version isn't there only for format of that quota file in proc.
It's *mainly* used for detection of kernel interface to use. Previously
tools had to try a few quotactl()s and from their results they had to
guess the quota format etc. With version somewhere it's a bit easier...
  Looking forward to next flame from you ;)

								Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

Re: Linux-2.5.17

[Russell King]

On Wed, May 22, 2002 at 12:54:15PM +0200, Martin Dalecki wrote:
> Please put the following crap under /proc/sys/fs,
> where it belongs. OK?

/proc/sys is for sysctls, not random proc junk.  Therefore, putting the
random crap you point out that's currently in /proc/fs in /proc/sys/fs:

> [root@kozaczek fs]# pwd
> /proc/fs
> [root@kozaczek fs]# cat quota
> Version 60501
> Formats
> 0 0 0 0 0 0 0 8
> [root@kozaczek fs]#

is even worse.

/proc/sys has a clean and clear purpose.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: Linux-2.5.17

[Martin Dalecki]

Uz.ytkownik Russell King napisa?:
> On Wed, May 22, 2002 at 12:54:15PM +0200, Martin Dalecki wrote:
> 
>>Please put the following crap under /proc/sys/fs,
>>where it belongs. OK?
> 
> 
> /proc/sys is for sysctls, not random proc junk.  Therefore, putting the
> random crap you point out that's currently in /proc/fs in /proc/sys/fs:
> 
> 
>>[root@kozaczek fs]# pwd
>>/proc/fs
>>[root@kozaczek fs]# cat quota
>>Version 60501
>>Formats
>>0 0 0 0 0 0 0 8
>>[root@kozaczek fs]#
> 
> 
> is even worse.
> 
> /proc/sys has a clean and clear purpose.

sysctl is for adjusting global system parameters.
So apparently it's even worser, becouse the above
doesn't even serve this purpose?
I tought 0 0 0 0 0 0 8 where random configuration parameters.

Re: Linux-2.5.17

[Linus Torvalds]

On Wed, 22 May 2002, Russell King wrote:
>
> /proc/sys has a clean and clear purpose.

Yes, but it _:would_ be good to make the quota stuff use the existign
helper functions to make it much cleaner.

And some of those helper functions are definitely from sysctl's: splitting
up the quota file into multiple sysctls (_and_ moving it to /proc/sys/fs)
sounds like a good idea to me.

		Linus

Re: Linux-2.5.17

[Martin Dalecki]

Uz.ytkownik Linus Torvalds napisa?:
> 
> On Wed, 22 May 2002, Russell King wrote:
> 
>>/proc/sys has a clean and clear purpose.
> 
> 
> Yes, but it _:would_ be good to make the quota stuff use the existign
> helper functions to make it much cleaner.
> 
> And some of those helper functions are definitely from sysctl's: splitting
> up the quota file into multiple sysctls (_and_ moving it to /proc/sys/fs)
> sounds like a good idea to me.

Well I'm actually coding this right now :-).

Re: Linux-2.5.17

[Jan Kara]

> Uz.ytkownik Linus Torvalds napisa?:
> >
> >On Wed, 22 May 2002, Russell King wrote:
> >
> >>/proc/sys has a clean and clear purpose.
> >
> >
> >Yes, but it _:would_ be good to make the quota stuff use the existign
> >helper functions to make it much cleaner.
> >
> >And some of those helper functions are definitely from sysctl's: splitting
> >up the quota file into multiple sysctls (_and_ moving it to /proc/sys/fs)
> >sounds like a good idea to me.
> 
> Well I'm actually coding this right now :-).
  Thanks. I'll update quota tools to use your new files if you send me
new layout of interface...

								Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

Re: Linux-2.5.17

[Martin Dalecki]

Uz.ytkownik Jan Kara napisa?:
>>Uz.ytkownik Linus Torvalds napisa?:
>>
>>>On Wed, 22 May 2002, Russell King wrote:
>>>
>>>
>>>>/proc/sys has a clean and clear purpose.
>>>
>>>
>>>Yes, but it _:would_ be good to make the quota stuff use the existign
>>>helper functions to make it much cleaner.
>>>
>>>And some of those helper functions are definitely from sysctl's: splitting
>>>up the quota file into multiple sysctls (_and_ moving it to /proc/sys/fs)
>>>sounds like a good idea to me.
>>
>>Well I'm actually coding this right now :-).
> 
>   Thanks. I'll update quota tools to use your new files if you send me
> new layout of interface...

I'm not ready right now but...
Well actually I went the cheapest way possible:


Here is the layout of the /proc/sys/fs/dquotas array:

/*
  * Statistics about disc quota.
  */
enum {
	DQSTATS_LOOKUPS,
	DQSTATS_DROPS,
	DQSTATS_READS,
	DQSTATS_WRITES,
	DQSTATS_CACHE_HITS,
	DQSTATS_ALLOCATED, // formerly known as nr_dquts inside kernel.
	DQSTATS_FREE,       // formerly known as nr_free_dquots inside kernel.
	DQSTATS_SYNCS,
	DQSTATS_SIZE
};

extern __u32 dqstats_array[DQSTATS_SIZE];

And here is the allocated sysctl id number:

	FS_DQSTATS=16,	/* int: disc quota suage statistics *

All of this appears under:

static ctl_table fs_table[] = {
	{FS_DQSTATS, "dqstats", dqstats_array, sizeof(dqstats_array), 0444, NULL, 
&proc_dointvec},
	{},
};

inside /proc/sys/fs/dqstats

I dodn't think the particular fields are subject to change soon
so I wen't for the array.
If yes - please feel rather free to complain :-).
Switch over to sysctl() and see the client code
melting down :-).

BTW> Since I got already my "required flame dosis" for toady I would
rather like to express that the rest of the new quota
handling code is, well, quite nice IMHO of course :-).

Re: Linux-2.5.17

[Jan Kara]

> Uz.ytkownik Jan Kara napisa?:
> >>Uz.ytkownik Linus Torvalds napisa?:
> >>
> >>>On Wed, 22 May 2002, Russell King wrote:
> >>>
> >>>
> >>>>/proc/sys has a clean and clear purpose.
> >>>
> >>>
> >>>Yes, but it _:would_ be good to make the quota stuff use the existign
> >>>helper functions to make it much cleaner.
> >>>
> >>>And some of those helper functions are definitely from sysctl's: 
> >>>splitting
> >>>up the quota file into multiple sysctls (_and_ moving it to /proc/sys/fs)
> >>>sounds like a good idea to me.
> >>
> >>Well I'm actually coding this right now :-).
> >
> >  Thanks. I'll update quota tools to use your new files if you send me
> >new layout of interface...
> 
> I'm not ready right now but...
> Well actually I went the cheapest way possible:
> 
> 
> Here is the layout of the /proc/sys/fs/dquotas array:
> 
> /*
>  * Statistics about disc quota.
>  */
> enum {
> 	DQSTATS_LOOKUPS,
> 	DQSTATS_DROPS,
> 	DQSTATS_READS,
> 	DQSTATS_WRITES,
> 	DQSTATS_CACHE_HITS,
> 	DQSTATS_ALLOCATED, // formerly known as nr_dquts inside kernel.
> 	DQSTATS_FREE,       // formerly known as nr_free_dquots inside 
> 	kernel.
> 	DQSTATS_SYNCS,
> 	DQSTATS_SIZE
> };
> 
> extern __u32 dqstats_array[DQSTATS_SIZE];
> 
> And here is the allocated sysctl id number:
> 
> 	FS_DQSTATS=16,	/* int: disc quota suage statistics *
> 
> All of this appears under:
> 
> static ctl_table fs_table[] = {
> 	{FS_DQSTATS, "dqstats", dqstats_array, sizeof(dqstats_array), 0444, 
> 	NULL, &proc_dointvec},
> 	{},
> };
> 
> inside /proc/sys/fs/dqstats
> 
> I dodn't think the particular fields are subject to change soon
> so I wen't for the array.
> If yes - please feel rather free to complain :-).
> Switch over to sysctl() and see the client code
> melting down :-).
  The array is OK (I don't expect any changes in statistics too).
I'd just like to have that 'version' and 'formats' fields somewhere.
Otherwise it's rather hard for quota tools to recognize quota
interface...

> BTW> Since I got already my "required flame dosis" for toady I would
> rather like to express that the rest of the new quota
> handling code is, well, quite nice IMHO of course :-).
  Thanks :).

  								Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

Re: Linux-2.5.17

[Martin Dalecki]

Uz.ytkownik Jan Kara napisa?:
>>Uz.ytkownik Jan Kara napisa?:
>>
>>>>Uz.ytkownik Linus Torvalds napisa?:
>>>>
>>>>
>>>>>On Wed, 22 May 2002, Russell King wrote:
>>>>>
>>>>>
>>>>>
>>>>>>/proc/sys has a clean and clear purpose.
>>>>>
>>>>>
>>>>>Yes, but it _:would_ be good to make the quota stuff use the existign
>>>>>helper functions to make it much cleaner.
>>>>>
>>>>>And some of those helper functions are definitely from sysctl's: 
>>>>>splitting
>>>>>up the quota file into multiple sysctls (_and_ moving it to /proc/sys/fs)
>>>>>sounds like a good idea to me.
>>>>
>>>>Well I'm actually coding this right now :-).
>>>
>>> Thanks. I'll update quota tools to use your new files if you send me
>>>new layout of interface...
>>
>>I'm not ready right now but...
>>Well actually I went the cheapest way possible:
>>
>>
>>Here is the layout of the /proc/sys/fs/dquotas array:
>>
>>/*
>> * Statistics about disc quota.
>> */
>>enum {
>>	DQSTATS_LOOKUPS,
>>	DQSTATS_DROPS,
>>	DQSTATS_READS,
>>	DQSTATS_WRITES,
>>	DQSTATS_CACHE_HITS,
>>	DQSTATS_ALLOCATED, // formerly known as nr_dquts inside kernel.
>>	DQSTATS_FREE,       // formerly known as nr_free_dquots inside 
>>	kernel.
>>	DQSTATS_SYNCS,
>>	DQSTATS_SIZE
>>};
>>
>>extern __u32 dqstats_array[DQSTATS_SIZE];
>>
>>And here is the allocated sysctl id number:
>>
>>	FS_DQSTATS=16,	/* int: disc quota suage statistics *
>>
>>All of this appears under:
>>
>>static ctl_table fs_table[] = {
>>	{FS_DQSTATS, "dqstats", dqstats_array, sizeof(dqstats_array), 0444, 
>>	NULL, &proc_dointvec},
>>	{},
>>};
>>
>>inside /proc/sys/fs/dqstats
>>
>>I dodn't think the particular fields are subject to change soon
>>so I wen't for the array.
>>If yes - please feel rather free to complain :-).
>>Switch over to sysctl() and see the client code
>>melting down :-).
> 
>   The array is OK (I don't expect any changes in statistics too).
> I'd just like to have that 'version' and 'formats' fields somewhere.
> Otherwise it's rather hard for quota tools to recognize quota
> interface...

You have the sysctl id number for this purpose and the /proc/sys/fs file
name is right now unique. So there is no need for more
treatment here then just trying to stick to what we get once it's there.
The versioning of syscall returns I will just preserve.

Going through sysctl *will be much easier* in code
then fs lookup of the file above.

Re: Linux-2.5.17

[Jan Kara]

> Uz.ytkownik Jan Kara napisa?:
> >>Uz.ytkownik Jan Kara napisa?:
> >>
> >>>>Uz.ytkownik Linus Torvalds napisa?:
> >>>>
> >>>>
> >>>>>On Wed, 22 May 2002, Russell King wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>>>/proc/sys has a clean and clear purpose.
> >>>>>
> >>>>>
> >>>>>Yes, but it _:would_ be good to make the quota stuff use the existign
> >>>>>helper functions to make it much cleaner.
> >>>>>
> >>>>>And some of those helper functions are definitely from sysctl's: 
> >>>>>splitting
> >>>>>up the quota file into multiple sysctls (_and_ moving it to 
> >>>>>/proc/sys/fs)
> >>>>>sounds like a good idea to me.
> >>>>
> >>>>Well I'm actually coding this right now :-).
> >>>
> >>>Thanks. I'll update quota tools to use your new files if you send me
> >>>new layout of interface...
> >>
> >>I'm not ready right now but...
> >>Well actually I went the cheapest way possible:
> >>
> >>
> >>Here is the layout of the /proc/sys/fs/dquotas array:
> >>
> >>/*
> >>* Statistics about disc quota.
> >>*/
> >>enum {
> >>	DQSTATS_LOOKUPS,
> >>	DQSTATS_DROPS,
> >>	DQSTATS_READS,
> >>	DQSTATS_WRITES,
> >>	DQSTATS_CACHE_HITS,
> >>	DQSTATS_ALLOCATED, // formerly known as nr_dquts inside kernel.
> >>	DQSTATS_FREE,       // formerly known as nr_free_dquots inside 
> >>	kernel.
> >>	DQSTATS_SYNCS,
> >>	DQSTATS_SIZE
> >>};
> >>
> >>extern __u32 dqstats_array[DQSTATS_SIZE];
> >>
> >>And here is the allocated sysctl id number:
> >>
> >>	FS_DQSTATS=16,	/* int: disc quota suage statistics *
> >>
> >>All of this appears under:
> >>
> >>static ctl_table fs_table[] = {
> >>	{FS_DQSTATS, "dqstats", dqstats_array, sizeof(dqstats_array), 0444, 
> >>	NULL, &proc_dointvec},
> >>	{},
> >>};
> >>
> >>inside /proc/sys/fs/dqstats
> >>
> >>I dodn't think the particular fields are subject to change soon
> >>so I wen't for the array.
> >>If yes - please feel rather free to complain :-).
> >>Switch over to sysctl() and see the client code
> >>melting down :-).
> >
> >  The array is OK (I don't expect any changes in statistics too).
> >I'd just like to have that 'version' and 'formats' fields somewhere.
> >Otherwise it's rather hard for quota tools to recognize quota
> >interface...
> 
> You have the sysctl id number for this purpose and the /proc/sys/fs file
> name is right now unique. So there is no need for more
> treatment here then just trying to stick to what we get once it's there.
> The versioning of syscall returns I will just preserve.
> 
> Going through sysctl *will be much easier* in code
> then fs lookup of the file above.
  OK. You convinced me that 'version' isn't needed. But how about
'formats'? Currently quotaon(8) uses this field to check which format it
should try to turn on... I can live without it as quotaon(8) might try
new format and if it doesn't succeed it will try the old one but
anyway...

							Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

Re: Linux-2.5.17

[Russell King]

On Wed, May 22, 2002 at 08:17:53PM +0200, Jan Kara wrote:
>   OK. You convinced me that 'version' isn't needed. But how about
> 'formats'? Currently quotaon(8) uses this field to check which format it
> should try to turn on... I can live without it as quotaon(8) might try
> new format and if it doesn't succeed it will try the old one but
> anyway...

Each sysctl file is only supposed to carry one bit of information - one
number, one string.  There should be no formatting of data.

Have a look at /proc/sys/net/ipv4/* as an example.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: Linux-2.5.17

[Alan Cox]

> Please put the following crap under /proc/sys/fs,
> where it belongs. OK?
> 
> [root@kozaczek fs]# pwd
> /proc/fs
> [root@kozaczek fs]# cat quota
> Version 60501
> Formats
> 0 0 0 0 0 0 0 8
> [root@kozaczek fs]#
> 
> Or are are you going to reinvent just enother
> case of /proc/ formatting compatibility problems?!
> And the requirement to have /proc mounted for quoate usage?!

/proc/sys/ is sysctl space. 

	/proc/sys/fs/quota/version
	/proc/sys/fs/quota/format/0,1,2,3..

maybe

> (PS. Hah! I found finally someone today who deserves flames! :-).)

You looked in the mirror ?

Re: Linux-2.5.17

[Russell King]

On Mon, May 20, 2002 at 10:16:35PM -0700, Linus Torvalds wrote:
> Various FS updates (including merges of quota and iget_locked), and
> Makefile cleanups from Kai.
> 
> And yet more TLB shootdown stuff.

We seem to have inconsistent cache handling in the new TLB shootdown stuff.
Or maybe its just my misunderstanding of what's going on; whatever it is,
the new TLB shootdown stuff appears to be quite messy.

Lets look at the flow of the 3 places where tlb_gather_mmu is used:

zap_page_range		unmap_region		exit_mmap
  flush_cache_range	  tlb_gather_mmu	  tlb_gather_mmu
  tlb_gather_mmu	  unmap_page_range	  flush_cache_mm
  unmap_page_range	  free_pgtables		  unmap_page_range
  tlb_finish_mmu	  tlb_finish_mmu	  clear_page_tables
						  tlb_finish_mmu

So we have 3 different functions, 2 different orders of gather_mmu
and cache handling, and one with no cache handling what so ever.

I think we have two options - either leave the cache handling up to
tlb_start_vma() (in which case, flush_cache_range and flush_cache_mm
are redundant and should be removed) or let it be up to the caller
of tlb_gather_mmu to call the right cache handling function.

I think which is actually function dependent - in zap_page_range,
we're only removing one vma.  In exit_mmap, we're removing all vmas.
In unmap_region, we're removing an unspecified number of vmas.
Depending on which option we choose, we'll either end up calling
flush_cache_range() many times, or flush_cache_mm() and flushing
the cache for a munmap of a small area.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: Linux-2.5.17

[David S. Miller]

   From: Russell King <rmk@arm.linux.org.uk>
   Date: Wed, 22 May 2002 12:19:29 +0100

   So we have 3 different functions, 2 different orders of gather_mmu
   and cache handling, and one with no cache handling what so ever.
   
   I think we have two options - either leave the cache handling up to
   tlb_start_vma() (in which case, flush_cache_range and flush_cache_mm
   are redundant and should be removed) or let it be up to the caller
   of tlb_gather_mmu to call the right cache handling function.
   
We're in very much agreement with you, that is why we are
still hashing out how to make this thing as optimal as
possible.

The idea currently is that the tlb_vma_{start,end}() handle
cache and tlb flush respectively.

Ignore the exit_mmap() case, it will be optimized to shreds :-)

Re: Linux-2.5.17

[Linus Torvalds]

On Wed, 22 May 2002, Russell King wrote:
>
> We seem to have inconsistent cache handling in the new TLB shootdown stuff.

Not surprising - I've worried only about changing the TLB architecture on
x86, where the caches do not matter.

> I think we have two options - either leave the cache handling up to
> tlb_start_vma() (in which case, flush_cache_range and flush_cache_mm
> are redundant and should be removed) or let it be up to the caller
> of tlb_gather_mmu to call the right cache handling function.

I think I'd prefer the "let the tlb functions handle caches too" approach.

For many architectures, that means "tlb_start/end_vma()". Others can do it
in "tlb_remove_tlb_entry()".

There's another issue: I think we should aim to get rid of the old
"flush_tlb_xxxx()" functions, and aim to rely entirely on the TLB
gathering. vmalloc/vfree might be the one special case (and I suspect
vfree() is going to get a lot slower to make sure it does the right thing
wrt TLB's).

		Linus

[PATCH] 2.5.17 IDE 69

[Martin Dalecki]

[-- Attachment #1: Type: text/plain, Size: 695 bytes --]

Basically accumulated stuff from other people.
Thank you all again!

Wed May 22 10:26:29 CEST 2002 ide-clean-69

- Apply small host chip driver cosmetics by Andrej xxx Panin
   and Vojtech Pavlik.

- Remove support for "disc recovery time". It could only supposedly
   help with really simplistic broken devices from the past,
   which didn't have moderately sophisticated controllers.
   And finally Vojtech voted for it as well... so I just trust him.

- Apply icside host chip driver and other ARM related updates by Russell King,
   which finally settle the "portability" work a bit, well hopefully.

Still Adam Richters draft patches on my agenda remaining... But
well it's only one day...

[-- Attachment #2: ide-clean-69.diff --]
[-- Type: text/plain, Size: 61413 bytes --]

diff -urN linux-2.5.17/drivers/ide/aec62xx.c linux/drivers/ide/aec62xx.c
--- linux-2.5.17/drivers/ide/aec62xx.c	2002-05-22 20:11:38.000000000 +0200
+++ linux/drivers/ide/aec62xx.c	2002-05-22 13:21:11.000000000 +0200
@@ -259,7 +259,7 @@
 {
 	for ( ; chipset_table->xfer_speed ; chipset_table++)
 		if (chipset_table->xfer_speed == speed) {
-			return ((byte) ((system_bus_speed <= 33) ? chipset_table->chipset_settings_33 : chipset_table->chipset_settings_34));
+			return ((byte) ((system_bus_speed <= 33333) ? chipset_table->chipset_settings_33 : chipset_table->chipset_settings_34));
 		}
 	return 0x00;
 }
@@ -268,7 +268,7 @@
 {
 	for ( ; chipset_table->xfer_speed ; chipset_table++)
 		if (chipset_table->xfer_speed == speed) {
-			return ((byte) ((system_bus_speed <= 33) ? chipset_table->ultra_settings_33 : chipset_table->ultra_settings_34));
+			return ((byte) ((system_bus_speed <= 33333) ? chipset_table->ultra_settings_33 : chipset_table->ultra_settings_34));
 		}
 	return 0x00;
 }
diff -urN linux-2.5.17/drivers/ide/ali14xx.c linux/drivers/ide/ali14xx.c
--- linux-2.5.17/drivers/ide/ali14xx.c	2002-05-22 20:11:38.000000000 +0200
+++ linux/drivers/ide/ali14xx.c	2002-05-22 13:21:11.000000000 +0200
@@ -124,8 +124,8 @@
 	/* calculate timing, according to PIO mode */
 	time1 = t->cycle;
 	time2 = t->active;
-	param3 = param1 = (time2 * system_bus_speed + 999) / 1000;
-	param4 = param2 = (time1 * system_bus_speed + 999) / 1000 - param1;
+	param3 = param1 = (time2 * system_bus_speed + 999999) / 1000000;
+	param4 = param2 = (time1 * system_bus_speed + 999999) / 1000000 - param1;
 	if (pio < XFER_PIO_3) {
 		param3 += 8;
 		param4 += 8;
diff -urN linux-2.5.17/drivers/ide/alim15x3.c linux/drivers/ide/alim15x3.c
--- linux-2.5.17/drivers/ide/alim15x3.c	2002-05-22 20:11:38.000000000 +0200
+++ linux/drivers/ide/alim15x3.c	2002-05-22 13:21:11.000000000 +0200
@@ -261,18 +261,18 @@
 
 	s_time = t->setup;
 	a_time = t->active;
-	if ((s_clc = (s_time * system_bus_speed + 999) / 1000) >= 8)
+	if ((s_clc = (s_time * system_bus_speed + 999999) / 1000000) >= 8)
 		s_clc = 0;
-	if ((a_clc = (a_time * system_bus_speed + 999) / 1000) >= 8)
+	if ((a_clc = (a_time * system_bus_speed + 999999) / 1000000) >= 8)
 		a_clc = 0;
 	c_time = t->cycle;
 
 #if 0
-	if ((r_clc = ((c_time - s_time - a_time) * system_bus_speed + 999) / 1000) >= 16)
+	if ((r_clc = ((c_time - s_time - a_time) * system_bus_speed + 999999) / 1000000) >= 16)
 		r_clc = 0;
 #endif
 
-	if (!(r_clc = (c_time * system_bus_speed + 999) / 1000 - a_clc - s_clc)) {
+	if (!(r_clc = (c_time * system_bus_speed + 999999) / 1000000 - a_clc - s_clc)) {
 		r_clc = 1;
 	} else {
 		if (r_clc >= 16)
diff -urN linux-2.5.17/drivers/ide/amd74xx.c linux/drivers/ide/amd74xx.c
--- linux-2.5.17/drivers/ide/amd74xx.c	2002-05-21 07:07:40.000000000 +0200
+++ linux/drivers/ide/amd74xx.c	2002-05-22 13:21:11.000000000 +0200
@@ -87,7 +87,6 @@
 static struct amd_ide_chip *amd_config;
 static unsigned char amd_enabled;
 static unsigned int amd_80w;
-static unsigned int amd_clock;
 
 static unsigned char amd_cyc2udma[] = { 6, 6, 5, 4, 0, 1, 1, 2, 2, 3, 3 };
 static unsigned char amd_udma2cyc[] = { 4, 6, 8, 10, 3, 2, 1, 1 };
@@ -131,7 +130,7 @@
 	amd_print("Highest DMA rate:                   %s", amd_dma[amd_config->flags & AMD_UDMA]);
 
 	amd_print("BM-DMA base:                        %#x", amd_base);
-	amd_print("PCI clock:                          %d.%dMHz", amd_clock / 1000, amd_clock / 100 % 10);
+	amd_print("PCI clock:                          %d.%dMHz", system_bus_speed / 1000, system_bus_speed / 100 % 10);
 	
 	amd_print("-----------------------Primary IDE-------Secondary IDE------");
 
@@ -147,7 +146,7 @@
 
 	amd_print("Cable Type:            %10s%20s", (amd_80w & 1) ? "80w" : "40w", (amd_80w & 2) ? "80w" : "40w");
 
-	if (!amd_clock)
+	if (!system_bus_speed)
                 return p - buffer;
 
 	amd_print("-------------------drive0----drive1----drive2----drive3-----");
@@ -169,22 +168,22 @@
 		den[i]  = (c & ((i & 1) ? 0x40 : 0x20) << ((i & 2) << 2));
 
 		if (den[i] && uen[i] && udma[i] == 1) {
-			speed[i] = amd_clock * 3;
-			cycle[i] = 666666 / amd_clock;
+			speed[i] = system_bus_speed * 3;
+			cycle[i] = 666666 / system_bus_speed;
 			continue;
 		}
 
-		speed[i] = 4 * amd_clock / ((den[i] && uen[i]) ? udma[i] : (active[i] + recover[i]) * 2);
-		cycle[i] = 1000000 * ((den[i] && uen[i]) ? udma[i] : (active[i] + recover[i]) * 2) / amd_clock / 2;
+		speed[i] = 4 * system_bus_speed / ((den[i] && uen[i]) ? udma[i] : (active[i] + recover[i]) * 2);
+		cycle[i] = 1000000 * ((den[i] && uen[i]) ? udma[i] : (active[i] + recover[i]) * 2) / system_bus_speed / 2;
 	}
 
 	amd_print_drive("Transfer Mode: ", "%10s", den[i] ? (uen[i] ? "UDMA" : "DMA") : "PIO");
 
-	amd_print_drive("Address Setup: ", "%8dns", 1000000 * setup[i] / amd_clock);
-	amd_print_drive("Cmd Active:    ", "%8dns", 1000000 * active8b[i] / amd_clock);
-	amd_print_drive("Cmd Recovery:  ", "%8dns", 1000000 * recover8b[i] / amd_clock);
-	amd_print_drive("Data Active:   ", "%8dns", 1000000 * active[i] / amd_clock);
-	amd_print_drive("Data Recovery: ", "%8dns", 1000000 * recover[i] / amd_clock);
+	amd_print_drive("Address Setup: ", "%8dns", 1000000 * setup[i] / system_bus_speed);
+	amd_print_drive("Cmd Active:    ", "%8dns", 1000000 * active8b[i] / system_bus_speed);
+	amd_print_drive("Cmd Recovery:  ", "%8dns", 1000000 * recover8b[i] / system_bus_speed);
+	amd_print_drive("Data Active:   ", "%8dns", 1000000 * active[i] / system_bus_speed);
+	amd_print_drive("Data Recovery: ", "%8dns", 1000000 * recover[i] / system_bus_speed);
 	amd_print_drive("Cycle Time:    ", "%8dns", cycle[i]);
 	amd_print_drive("Transfer Rate: ", "%4d.%dMB/s", speed[i] / 1000, speed[i] / 100 % 10);
 
@@ -238,7 +237,7 @@
 			printk(KERN_WARNING "ide%d: Drive %d didn't accept speed setting. Oh, well.\n",
 				drive->dn >> 1, drive->dn & 1);
 
-	T = 1000000000 / amd_clock;
+	T = 1000000000 / system_bus_speed;
 	UT = T / min_t(int, max_t(int, amd_config->flags & AMD_UDMA, 1), 2);
 
 	ata_timing_compute(drive, speed, &t, T, UT);
@@ -248,7 +247,7 @@
 		ata_timing_merge(&p, &t, &t, IDE_TIMING_8BIT);
 	}
 
-	if (speed == XFER_UDMA_5 && amd_clock <= 33333) t.udma = 1;
+	if (speed == XFER_UDMA_5 && system_bus_speed <= 33333) t.udma = 1;
 
 	amd_set_speed(drive->channel->pci_dev, drive->dn, &t);
 
@@ -358,24 +357,6 @@
 		(amd_config->flags & AMD_BAD_FIFO) ? (t & 0x0f) : (t | 0xf0));
 
 /*
- * Determine the system bus clock.
- */
-
-	amd_clock = system_bus_speed * 1000;
-
-	switch (amd_clock) {
-		case 33000: amd_clock = 33333; break;
-		case 37000: amd_clock = 37500; break;
-		case 41000: amd_clock = 41666; break;
-	}
-
-	if (amd_clock < 20000 || amd_clock > 50000) {
-		printk(KERN_WARNING "AMD_IDE: User given PCI clock speed impossible (%d), using 33 MHz instead.\n", amd_clock);
-		printk(KERN_WARNING "AMD_IDE: Use ide0=ata66 if you want to assume 80-wire cable\n");
-		amd_clock = 33333;
-	}
-
-/*
  * Print the boot message.
  */
 
diff -urN linux-2.5.17/drivers/ide/cmd640.c linux/drivers/ide/cmd640.c
--- linux-2.5.17/drivers/ide/cmd640.c	2002-05-21 07:07:38.000000000 +0200
+++ linux/drivers/ide/cmd640.c	2002-05-22 13:21:11.000000000 +0200
@@ -603,7 +603,7 @@
 	u8 cycle_count;
 
 	recovery_time = cycle_time - (setup_time + active_time);
-	clock_time = 1000 / system_bus_speed;
+	clock_time = 1000000 / system_bus_speed;
 	cycle_count = (cycle_time + clock_time - 1) / clock_time;
 
 	setup_count = (setup_time + clock_time - 1) / clock_time;
diff -urN linux-2.5.17/drivers/ide/cmd64x.c linux/drivers/ide/cmd64x.c
--- linux-2.5.17/drivers/ide/cmd64x.c	2002-05-22 20:11:38.000000000 +0200
+++ linux/drivers/ide/cmd64x.c	2002-05-22 13:21:11.000000000 +0200
@@ -305,7 +305,7 @@
 	 */
 
 	recovery_time = t->cycle - (t->setup + t->active);
-	clock_time = 1000 / system_bus_speed;
+	clock_time = 1000000 / system_bus_speed;
 	cycle_count = (t->cycle + clock_time - 1) / clock_time;
 	setup_count = (t->setup + clock_time - 1) / clock_time;
 	active_count = (t->active + clock_time - 1) / clock_time;
diff -urN linux-2.5.17/drivers/ide/cy82c693.c linux/drivers/ide/cy82c693.c
--- linux-2.5.17/drivers/ide/cy82c693.c	2002-05-21 07:07:34.000000000 +0200
+++ linux/drivers/ide/cy82c693.c	2002-05-22 13:21:11.000000000 +0200
@@ -121,7 +121,7 @@
 {
 	int clocks;
 
-	clocks = (time*bus_speed+999)/1000 -1;
+	clocks = (time*bus_speed+999999)/1000000 -1;
 
 	if (clocks < 0)
 		clocks = 0;
diff -urN linux-2.5.17/drivers/ide/ht6560b.c linux/drivers/ide/ht6560b.c
--- linux-2.5.17/drivers/ide/ht6560b.c	2002-05-21 07:07:33.000000000 +0200
+++ linux/drivers/ide/ht6560b.c	2002-05-22 13:21:11.000000000 +0200
@@ -219,8 +219,8 @@
 		/*
 		 *  Cycle times should be Vesa bus cycles
 		 */
-		active_cycles   = (active_time   * system_bus_speed + 999) / 1000;
-		recovery_cycles = (recovery_time * system_bus_speed + 999) / 1000;
+		active_cycles   = (active_time   * system_bus_speed + 999999) / 1000000;
+		recovery_cycles = (recovery_time * system_bus_speed + 999999) / 1000000;
 		/*
 		 *  Upper and lower limits
 		 */
diff -urN linux-2.5.17/drivers/ide/icside.c linux/drivers/ide/icside.c
--- linux-2.5.17/drivers/ide/icside.c	2002-05-21 07:07:31.000000000 +0200
+++ linux/drivers/ide/icside.c	2002-05-22 17:06:50.000000000 +0200
@@ -1,7 +1,7 @@
 /*
  * linux/drivers/ide/icside.c
  *
- * Copyright (c) 1996,1997 Russell King.
+ * Copyright (c) 1996-2002 Russell King.
  *
  * Changelog:
  *  08-Jun-1996	RMK	Created
@@ -25,6 +25,7 @@
 #include <asm/dma.h>
 #include <asm/ecard.h>
 #include <asm/io.h>
+#include <asm/page.h>
 
 extern char *ide_xfer_verbose (byte xfer_rate);
 
@@ -75,6 +76,12 @@
 	ICS_ARCIN_V6_IDESTEPPING
 };
 
+struct icside_state {
+	unsigned int channel;
+	unsigned int enabled;
+	unsigned int irq_port;
+};
+
 static const card_ids icside_cids[] = {
 	{ MANU_ICS,  PROD_ICS_IDE  },
 	{ MANU_ICS2, PROD_ICS2_IDE },
@@ -122,10 +129,21 @@
  */
 static void icside_irqenable_arcin_v6 (struct expansion_card *ec, int irqnr)
 {
-	unsigned int ide_base_port = (unsigned int)ec->irq_data;
+	struct icside_state *state = ec->irq_data;
+	unsigned int base = state->irq_port;
+
+	state->enabled = 1;
 
-	outb (0, ide_base_port + ICS_ARCIN_V6_INTROFFSET_1);
-	outb (0, ide_base_port + ICS_ARCIN_V6_INTROFFSET_2);
+	switch (state->channel) {
+	case 0:
+		outb(0, base + ICS_ARCIN_V6_INTROFFSET_1);
+		inb(base + ICS_ARCIN_V6_INTROFFSET_2);
+		break;
+	case 1:
+		outb(0, base + ICS_ARCIN_V6_INTROFFSET_2);
+		inb(base + ICS_ARCIN_V6_INTROFFSET_1);
+		break;
+	}
 }
 
 /* Prototype: icside_irqdisable_arcin_v6 (struct expansion_card *ec, int irqnr)
@@ -133,10 +151,12 @@
  */
 static void icside_irqdisable_arcin_v6 (struct expansion_card *ec, int irqnr)
 {
-	unsigned int ide_base_port = (unsigned int)ec->irq_data;
+	struct icside_state *state = ec->irq_data;
+
+	state->enabled = 0;
 
-	inb (ide_base_port + ICS_ARCIN_V6_INTROFFSET_1);
-	inb (ide_base_port + ICS_ARCIN_V6_INTROFFSET_2);
+	inb (state->irq_port + ICS_ARCIN_V6_INTROFFSET_1);
+	inb (state->irq_port + ICS_ARCIN_V6_INTROFFSET_2);
 }
 
 /* Prototype: icside_irqprobe(struct expansion_card *ec)
@@ -144,10 +164,10 @@
  */
 static int icside_irqpending_arcin_v6(struct expansion_card *ec)
 {
-	unsigned int ide_base_port = (unsigned int)ec->irq_data;
+	struct icside_state *state = ec->irq_data;
 
-	return inb(ide_base_port + ICS_ARCIN_V6_INTRSTAT_1) & 1 ||
-	       inb(ide_base_port + ICS_ARCIN_V6_INTRSTAT_2) & 1;
+	return inb(state->irq_port + ICS_ARCIN_V6_INTRSTAT_1) & 1 ||
+	       inb(state->irq_port + ICS_ARCIN_V6_INTRSTAT_2) & 1;
 }
 
 static const expansioncard_ops_t icside_ops_arcin_v6 = {
@@ -210,6 +230,39 @@
 	return iftype;
 }
 
+/*
+ * Handle routing of interrupts.  This is called before
+ * we write the command to the drive.
+ */
+static void icside_maskproc(struct ata_device *drive, int mask)
+{
+	struct ata_channel *ch = drive->channel;
+	struct icside_state *state = ch->hw.priv;
+	unsigned long flags;
+
+	local_irq_save(flags);
+
+	state->channel = ch->unit;
+
+	if (state->enabled && !mask) {
+		switch (ch->unit) {
+		case 0:
+			outb(0, state->irq_port + ICS_ARCIN_V6_INTROFFSET_1);
+			inb(state->irq_port + ICS_ARCIN_V6_INTROFFSET_2);
+			break;
+		case 1:
+			outb(0, state->irq_port + ICS_ARCIN_V6_INTROFFSET_2);
+			inb(state->irq_port + ICS_ARCIN_V6_INTROFFSET_1);
+			break;
+		}
+	} else {
+		inb(state->irq_port + ICS_ARCIN_V6_INTROFFSET_2);
+		inb(state->irq_port + ICS_ARCIN_V6_INTROFFSET_1);
+	}
+
+	local_irq_restore(flags);
+}
+
 #ifdef CONFIG_BLK_DEV_IDEDMA_ICS
 /*
  * SG-DMA support.
@@ -223,32 +276,42 @@
 #define NR_ENTRIES 256
 #define TABLE_SIZE (NR_ENTRIES * 8)
 
-static int ide_build_sglist(struct ata_channel *hwif, struct request *rq)
+static int ide_build_sglist(struct ata_channel *ch, struct request *rq)
 {
-	request_queue_t *q = &hwif->drives[DEVICE_NR(rq->rq_dev) & 1].queue;
-	struct scatterlist *sg = hwif->sg_table;
-	int nents = blk_rq_map_sg(q, rq, sg);
-
-	if (rq->q && nents > rq->nr_phys_segments)
-		printk("icside: received %d segments, build %d\n",
-			rq->nr_phys_segments, nents);
+	struct scatterlist *sg = ch->sg_table;
+	int nents;
 
-	if (rq_data_dir(rq) == READ)
-		hwif->sg_dma_direction = PCI_DMA_FROMDEVICE;
-	else
-		hwif->sg_dma_direction = PCI_DMA_TODEVICE;
+	if (rq->flags & REQ_DRIVE_ACB) {
+		struct ata_taskfile *args = rq->special;
 
-	return pci_map_sg(NULL, sg, nents, hwif->sg_dma_direction);
-}
+		if (args->command_type == IDE_DRIVE_TASK_RAW_WRITE)
+			ch->sg_dma_direction = PCI_DMA_TODEVICE;
+		else
+			ch->sg_dma_direction = PCI_DMA_FROMDEVICE;
+
+		memset(sg, 0, sizeof(*sg));
+		sg->page   = virt_to_page(rq->buffer);
+		sg->offset = ((unsigned long)rq->buffer) & ~PAGE_MASK;
+		sg->length = rq->nr_sectors * SECTOR_SIZE;
+		nents = 1;
+	} else {
+		nents = blk_rq_map_sg(rq->q, rq, sg);
+
+		if (rq->q && nents > rq->nr_phys_segments)
+			printk("icside: received %d segments, build %d\n",
+				rq->nr_phys_segments, nents);
+
+		if (rq_data_dir(rq) == READ)
+			ch->sg_dma_direction = PCI_DMA_FROMDEVICE;
+		else
+			ch->sg_dma_direction = PCI_DMA_TODEVICE;
+	}
 
-static int
-icside_udma_new_table(struct ata_channel *ch, struct request *rq)
-{
-	return ch->sg_nents = ide_build_sglist(ch, rq);
+	return pci_map_sg(NULL, sg, nents, ch->sg_dma_direction);
 }
 
 /* Teardown mappings after DMA has completed.  */
-static void icside_destroy_dmatable(ide_drive_t *drive)
+static void icside_destroy_dmatable(struct ata_device *drive)
 {
 	struct scatterlist *sg = drive->channel->sg_table;
 	int nents = drive->channel->sg_nents;
@@ -282,10 +345,9 @@
  *	MW2	70	25	25	120	C
  */
 static int
-icside_config_if(ide_drive_t *drive, int xfer_mode)
+icside_config_if(struct ata_device *drive, int xfer_mode)
 {
-	int func = ide_dma_off;
-	int cycle_time = 0, use_dma_info = 0;
+	int on = 0, cycle_time = 0, use_dma_info = 0;
 
 	switch (xfer_mode) {
 	case XFER_MW_DMA_2: cycle_time = 250; use_dma_info = 1;	break;
@@ -306,7 +368,7 @@
 		drive->init_speed = xfer_mode;
 
 	if (cycle_time && ide_config_drive_speed(drive, xfer_mode) == 0)
-		func = ide_dma_on;
+		on = 1;
 	else
 		drive->drive_data = 480;
 
@@ -315,50 +377,45 @@
 
 	drive->current_speed = xfer_mode;
 
-	return func;
+	return on;
 }
 
-static int
-icside_set_speed(ide_drive_t *drive, byte speed)
+static int icside_set_speed(struct ata_device *drive, byte speed)
 {
 	return icside_config_if(drive, speed);
 }
 
-/*
- * dma_intr() is the handler for disk read/write DMA interrupts
- */
-static ide_startstop_t icside_dmaintr(struct ata_device *drive, struct request *rq)
+static void icside_dma_enable(struct ata_device *drive, int on, int verbose)
 {
-	int i;
-	byte stat, dma_stat;
-
-	dma_stat = drive->channel->udma(ide_dma_end, drive, rq);
-	stat = GET_STAT();			/* get drive status */
-	if (OK_STAT(stat,DRIVE_READY,drive->bad_wstat|DRQ_STAT)) {
-		if (!dma_stat) {
-			for (i = rq->nr_sectors; i > 0;) {
-				i -= rq->current_nr_sectors;
-				ide_end_request(drive, 1);
-			}
-			return ide_stopped;
-		}
-		printk("%s: dma_intr: bad DMA status (dma_stat=%x)\n",
-		       drive->name, dma_stat);
+	if (!on) {
+		if (verbose)
+			printk("%s: DMA disabled\n", drive->name);
+#ifdef CONFIG_BLK_DEV_IDE_TCQ
+		udma_tcq_enable(drive, 0);
+#endif
 	}
-	return ide_error(drive, "dma_intr", stat);
-}
+	
+	/*
+	 * We don't need any bouncing.  Yes, this looks the
+	 * wrong way around, but it is correct.
+	 */
+	blk_queue_bounce_limit(&drive->queue, BLK_BOUNCE_ANY);
+	drive->using_dma = on;
 
+#ifdef CONFIG_CLK_DEV_IDE_TCQ_DEFAULT
+	if (on)
+		udma_tcq_enable(drive, 1);
+#endif
+}
 
-static int
-icside_dma_check(struct ata_device *drive, struct request *rq)
+static int icside_dma_check(struct ata_device *drive)
 {
 	struct hd_driveid *id = drive->id;
-	struct ata_channel *hwif = drive->channel;
-	int autodma = hwif->autodma;
+	struct ata_channel *ch = drive->channel;
 	int xfer_mode = XFER_PIO_2;
-	int func = ide_dma_off_quietly;
+	int on;
 
-	if (!id || !(id->capability & 1) || !autodma)
+	if (!id || !(id->capability & 1) || !ch->autodma)
 		goto out;
 
 	/*
@@ -367,188 +424,284 @@
 	if (id->field_valid & 2) {
 		if (id->dma_mword & 4) {
 			xfer_mode = XFER_MW_DMA_2;
-			func = ide_dma_on;
 		} else if (id->dma_mword & 2) {
 			xfer_mode = XFER_MW_DMA_1;
-			func = ide_dma_on;
 		} else if (id->dma_mword & 1) {
 			xfer_mode = XFER_MW_DMA_0;
-			func = ide_dma_on;
 		}
-		goto out;
 	}
 
 out:
-	func = icside_config_if(drive, xfer_mode);
+	on = icside_config_if(drive, xfer_mode);
+
+	icside_dma_enable(drive, on, 0);
+
+	return 0;
+}
+
+static int icside_dma_stop(struct ata_device *drive)
+{
+	struct ata_channel *ch = drive->channel;
+
+	drive->waiting_for_dma = 0;
+	disable_dma(ch->hw.dma);
+	icside_destroy_dmatable(drive);
+
+	return get_dma_residue(ch->hw.dma) != 0;
+}
+
+static int icside_dma_start(struct ata_device *drive, struct request *rq)
+{
+	struct ata_channel *ch = drive->channel;
+
+	/*
+	 * We can not enable DMA on both channels.
+	 */
+	BUG_ON(dma_channel_active(ch->hw.dma));
+
+	enable_dma(ch->hw.dma);
+
+	return 0;
+}
+
+/*
+ * dma_intr() is the handler for disk read/write DMA interrupts
+ */
+static ide_startstop_t icside_dmaintr(struct ata_device *drive, struct request *rq)
+{
+	int dma_stat;
+	byte stat;
+
+	dma_stat = icside_dma_stop(drive);
 
-	return hwif->udma(func, drive, rq);
+	stat = GET_STAT();			/* get drive status */
+	if (OK_STAT(stat,DRIVE_READY,drive->bad_wstat|DRQ_STAT)) {
+		if (!dma_stat) {
+			__ide_end_request(drive, rq, 1, rq->nr_sectors);
+			return ide_stopped;
+		}
+		printk("%s: dma_intr: bad DMA status (dma_stat=%x)\n",
+		       drive->name, dma_stat);
+	}
+	return ide_error(drive, rq, "dma_intr", stat);
 }
 
 static int
-icside_dmaproc(ide_dma_action_t func, ide_drive_t *drive)
+icside_dma_common(struct ata_device *drive, struct request *rq,
+		  unsigned int dma_mode)
+{
+	struct ata_channel *ch = drive->channel;
+	unsigned int count;
+
+	/*
+	 * We can not enable DMA on both channels.
+	 */
+	BUG_ON(dma_channel_active(ch->hw.dma));
+
+	count = ch->sg_nents = ide_build_sglist(ch, rq);
+	if (!count)
+		return 1;
+
+	/*
+	 * Route the DMA signals to to this channel.
+	 */
+	outb(ch->select_data, ch->config_data);
+
+	/*
+	 * Select the correct timing for this drive.
+	 */
+	set_dma_speed(ch->hw.dma, drive->drive_data);
+
+	/*
+	 * Tell the DMA engine about the SG table and
+	 * data direction.
+	 */
+	set_dma_sg(ch->hw.dma, ch->sg_table, count);
+	set_dma_mode(ch->hw.dma, dma_mode);
+
+	drive->waiting_for_dma = 1;
+
+	return 0;
+}
+
+static int icside_dma_read(struct ata_device *drive, struct request *rq)
 {
-	struct ata_channel *hwif = drive->channel;
-	int count, reading = 0;
+	struct ata_channel *ch = drive->channel;
+	unsigned int cmd;
 
-	switch (func) {
-	case ide_dma_off:
-		printk("%s: DMA disabled\n", drive->name);
-		/*FALLTHROUGH*/
-
-	case ide_dma_off_quietly:
-	case ide_dma_on:
-		/*
-		 * We don't need any bouncing.  Yes, this looks the
-		 * wrong way around, but it is correct.
-		 */
-		blk_queue_bounce_limit(&drive->queue, BLK_BOUNCE_ANY);
-		drive->using_dma = (func == ide_dma_on);
+	if (icside_dma_common(drive, rq, DMA_MODE_READ))
+		return 1;
+
+	if (drive->type != ATA_DISK)
 		return 0;
 
-	case ide_dma_check:
-		return icside_dma_check(drive, rq);
+	ide_set_handler(drive, icside_dmaintr, WAIT_CMD, NULL);
+
+	if ((rq->flags & REQ_DRIVE_ACB) && drive->addressing == 1) {
+		struct ata_taskfile *args = rq->special;
+		cmd = args->taskfile.command;
+	} else if (drive->addressing) {
+		cmd = WIN_READDMA_EXT;
+	} else {
+		cmd = WIN_READDMA;
+	}
 
-	case ide_dma_read:
-		reading = 1;
-	case ide_dma_write:
-		count = icside_udma_new_table(hwif, rq);
-		if (!count)
-			return 1;
-		disable_dma(hwif->hw.dma);
-
-		/* Route the DMA signals to
-		 * to the correct interface.
-		 */
-		outb(hwif->select_data, hwif->config_data);
-
-		/* Select the correct timing
-		 * for this drive
-		 */
-		set_dma_speed(hwif->hw.dma, drive->drive_data);
-
-		set_dma_sg(hwif->hw.dma, drive->channel->sg_table, count);
-		set_dma_mode(hwif->hw.dma, reading ? DMA_MODE_READ
-			     : DMA_MODE_WRITE);
-
-		drive->waiting_for_dma = 1;
-		if (drive->type != ATA_DISK)
-			return 0;
-
-		ide_set_handler(drive, icside_dmaintr, WAIT_CMD, NULL);
-		OUT_BYTE(reading ? WIN_READDMA : WIN_WRITEDMA,
-			 IDE_COMMAND_REG);
+	OUT_BYTE(cmd, IDE_COMMAND_REG);
+	enable_dma(ch->hw.dma);
+	return 0;
+}
 
-	case ide_dma_begin:
-		enable_dma(hwif->hw.dma);
+static int icside_dma_write(struct ata_device *drive, struct request *rq)
+{
+	struct ata_channel *ch = drive->channel;
+	unsigned int cmd;
+
+	if (icside_dma_common(drive, rq, DMA_MODE_WRITE))
+		return 1;
+
+	if (drive->type != ATA_DISK)
 		return 0;
 
-	case ide_dma_end:
-		drive->waiting_for_dma = 0;
-		disable_dma(hwif->hw.dma);
-		icside_destroy_dmatable(drive);
-		return get_dma_residue(hwif->hw.dma) != 0;
-
-	case ide_dma_test_irq:
-		return inb((unsigned long)hwif->hw.priv) & 1;
-
-	case ide_dma_timeout:
-	default:
-		printk("icside_dmaproc: unsupported function: %d\n", func);
+	ide_set_handler(drive, icside_dmaintr, WAIT_CMD, NULL);
+
+	if ((rq->flags & REQ_DRIVE_ACB) && drive->addressing == 1) {
+		struct ata_taskfile *args = rq->special;
+		cmd = args->taskfile.command;
+	} else if (drive->addressing) {
+		cmd = WIN_WRITEDMA_EXT;
+	} else {
+		cmd = WIN_WRITEDMA;
 	}
-	return 1;
+	OUT_BYTE(cmd, IDE_COMMAND_REG);
+
+	enable_dma(ch->hw.dma);
+	return 0;
 }
 
-static int
-icside_setup_dma(struct ata_channel *hwif, int autodma)
+static int icside_irq_status(struct ata_device *drive)
+{
+	struct ata_channel *ch = drive->channel;
+	struct icside_state *state = ch->hw.priv;
+
+	return inb(state->irq_port +
+		   (ch->unit ?
+			ICS_ARCIN_V6_INTRSTAT_2 :
+			ICS_ARCIN_V6_INTRSTAT_1)) & 1;
+}
+
+static void icside_dma_timeout(struct ata_device *drive)
+{
+	printk(KERN_ERR "ATA: %s: UDMA timeout occured:", drive->name);
+	ide_dump_status(drive, NULL, "UDMA timeout", GET_STAT());
+}
+
+static void icside_irq_lost(struct ata_device *drive)
 {
-	printk("    %s: SG-DMA", hwif->name);
+	printk(KERN_ERR "ATA: %s: IRQ lost\n", drive->name);
+}
 
-	hwif->sg_table = kmalloc(sizeof(struct scatterlist) * NR_ENTRIES,
+static int icside_setup_dma(struct ata_channel *ch)
+{
+	int autodma = 0;
+
+#ifdef CONFIG_IDEDMA_ICS_AUTO
+	autodma = 1;
+#endif
+
+	printk("    %s: SG-DMA", ch->name);
+
+	ch->sg_table = kmalloc(sizeof(struct scatterlist) * NR_ENTRIES,
 				 GFP_KERNEL);
-	if (!hwif->sg_table)
+	if (!ch->sg_table)
 		goto failed;
 
-	hwif->dmatable_cpu = NULL;
-	hwif->dmatable_dma = 0;
-	hwif->speedproc = icside_set_speed;
-	hwif->dmaproc = icside_dmaproc;
-	hwif->autodma = autodma;
+	ch->dmatable_cpu    = NULL;
+	ch->dmatable_dma    = 0;
+	ch->speedproc       = icside_set_speed;
+	ch->XXX_udma        = icside_dma_check;
+	ch->udma_enable     = icside_dma_enable;
+	ch->udma_start      = icside_dma_start;
+	ch->udma_stop       = icside_dma_stop;
+	ch->udma_read       = icside_dma_read;
+	ch->udma_write      = icside_dma_write;
+	ch->udma_irq_status = icside_irq_status;
+	ch->udma_timeout    = icside_dma_timeout;
+	ch->udma_irq_lost   = icside_irq_lost;
+	ch->autodma         = autodma;
 
-	printk(" capable%s\n", autodma ?
-		", auto-enable" : "");
+	printk(" capable%s\n", autodma ? ", auto-enable" : "");
 
 	return 1;
 
 failed:
-	printk(" -- ERROR, unable to allocate DMA table\n");
+	printk(" disabled, unable to allocate DMA table\n");
 	return 0;
 }
 
-void ide_release_dma(struct ata_channel *hwif)
+void ide_release_dma(struct ata_channel *ch)
 {
-	if (hwif->sg_table) {
-		kfree(hwif->sg_table);
-		hwif->sg_table = NULL;
+	if (ch->sg_table) {
+		kfree(ch->sg_table);
+		ch->sg_table = NULL;
 	}
 }
 #endif
 
-static struct ata_channel *
-icside_find_hwif(unsigned long dataport)
+static struct ata_channel *icside_find_hwif(unsigned long dataport)
 {
-	struct ata_channel *hwif;
+	struct ata_channel *ch;
 	int index;
 
 	for (index = 0; index < MAX_HWIFS; ++index) {
-		hwif = &ide_hwifs[index];
-		if (hwif->io_ports[IDE_DATA_OFFSET] == (ide_ioreg_t)dataport)
+		ch = &ide_hwifs[index];
+		if (ch->io_ports[IDE_DATA_OFFSET] == (ide_ioreg_t)dataport)
 			goto found;
 	}
 
 	for (index = 0; index < MAX_HWIFS; ++index) {
-		hwif = &ide_hwifs[index];
-		if (!hwif->io_ports[IDE_DATA_OFFSET])
+		ch = &ide_hwifs[index];
+		if (!ch->io_ports[IDE_DATA_OFFSET])
 			goto found;
 	}
 
 	return NULL;
 found:
-	return hwif;
+	return ch;
 }
 
 static struct ata_channel *
 icside_setup(unsigned long base, struct cardinfo *info, int irq)
 {
 	unsigned long port = base + info->dataoffset;
-	struct ata_channel *hwif;
+	struct ata_channel *ch;
 
-	hwif = icside_find_hwif(base);
-	if (hwif) {
+	ch = icside_find_hwif(base);
+	if (ch) {
 		int i;
 
-		memset(&hwif->hw, 0, sizeof(hw_regs_t));
+		memset(&ch->hw, 0, sizeof(hw_regs_t));
 
 		for (i = IDE_DATA_OFFSET; i <= IDE_STATUS_OFFSET; i++) {
-			hwif->hw.io_ports[i] = (ide_ioreg_t)port;
-			hwif->io_ports[i] = (ide_ioreg_t)port;
+			ch->hw.io_ports[i] = (ide_ioreg_t)port;
+			ch->io_ports[i] = (ide_ioreg_t)port;
 			port += 1 << info->stepping;
 		}
-		hwif->hw.io_ports[IDE_CONTROL_OFFSET] = base + info->ctrloffset;
-		hwif->io_ports[IDE_CONTROL_OFFSET] = base + info->ctrloffset;
-		hwif->hw.irq  = irq;
-		hwif->irq     = irq;
-		hwif->hw.dma  = NO_DMA;
-		hwif->noprobe = 0;
-		hwif->chipset = ide_acorn;
+		ch->hw.io_ports[IDE_CONTROL_OFFSET] = base + info->ctrloffset;
+		ch->io_ports[IDE_CONTROL_OFFSET] = base + info->ctrloffset;
+		ch->hw.irq  = irq;
+		ch->irq     = irq;
+		ch->hw.dma  = NO_DMA;
+		ch->noprobe = 0;
+		ch->chipset = ide_acorn;
 	}
 
-	return hwif;
+	return ch;
 }
 
-static int __init icside_register_v5(struct expansion_card *ec, int autodma)
+static int __init icside_register_v5(struct expansion_card *ec)
 {
 	unsigned long slot_port;
-	struct ata_channel *hwif;
+	struct ata_channel *ch;
 
 	slot_port = ecard_address(ec, ECARD_MEMC, 0);
 
@@ -562,17 +715,17 @@
 	 */
 	inb(slot_port + ICS_ARCIN_V5_INTROFFSET);
 
-	hwif = icside_setup(slot_port, &icside_cardinfo_v5, ec->irq);
+	ch = icside_setup(slot_port, &icside_cardinfo_v5, ec->irq);
 
-	return hwif ? 0 : -1;
+	return ch ? 0 : -1;
 }
 
-static int __init icside_register_v6(struct expansion_card *ec, int autodma)
+static int __init icside_register_v6(struct expansion_card *ec)
 {
 	unsigned long slot_port, port;
-	struct ata_channel *hwif;
-	struct ata_channel *mate;
-	int sel = 0;
+	struct icside_state *state;
+	struct ata_channel *ch0, *ch1;
+	unsigned int sel = 0;
 
 	slot_port = ecard_address(ec, ECARD_IOC, ECARD_FAST);
 	port      = ecard_address(ec, ECARD_EASI, ECARD_FAST);
@@ -584,55 +737,57 @@
 
 	outb(sel, slot_port);
 
-	ec->irq_data = (void *)port;
-	ec->ops      = (expansioncard_ops_t *)&icside_ops_arcin_v6;
-
 	/*
 	 * Be on the safe side - disable interrupts
 	 */
 	inb(port + ICS_ARCIN_V6_INTROFFSET_1);
 	inb(port + ICS_ARCIN_V6_INTROFFSET_2);
 
-	hwif = icside_setup(port, &icside_cardinfo_v6_1, ec->irq);
-	mate = icside_setup(port, &icside_cardinfo_v6_2, ec->irq);
+	/*
+	 * Find and register the interfaces.
+	 */
+	ch0 = icside_setup(port, &icside_cardinfo_v6_1, ec->irq);
+	ch1 = icside_setup(port, &icside_cardinfo_v6_2, ec->irq);
+
+	if (!ch0 || !ch1)
+		return -ENODEV;
+
+	state = kmalloc(sizeof(struct icside_state), GFP_KERNEL);
+	if (!state)
+		return -ENOMEM;
+
+	state->channel    = 0;
+	state->enabled    = 0;
+	state->irq_port   = port;
+
+	ec->irq_data      = state;
+	ec->ops           = (expansioncard_ops_t *)&icside_ops_arcin_v6;
+
+	ch0->maskproc     = icside_maskproc;
+	ch0->unit         = 0;
+	ch0->hw.priv      = state;
+	ch0->config_data  = slot_port;
+	ch0->select_data  = sel;
+	ch0->hw.dma       = ec->dma;
+
+	ch1->maskproc     = icside_maskproc;
+	ch1->unit         = 1;
+	ch1->hw.priv      = state;
+	ch1->config_data  = slot_port;
+	ch1->select_data  = sel | 1;
+	ch1->hw.dma       = ec->dma;
 
 #ifdef CONFIG_BLK_DEV_IDEDMA_ICS
-	if (ec->dma != NO_DMA) {
-		if (request_dma(ec->dma, hwif->name))
-			goto no_dma;
-
-		if (hwif) {
-			hwif->config_data = slot_port;
-			hwif->select_data = sel;
-			hwif->hw.dma  = ec->dma;
-			hwif->hw.priv = (void *)
-					(port + ICS_ARCIN_V6_INTRSTAT_1);
-			hwif->unit = 0;
-			icside_setup_dma(hwif, autodma);
-		}
-		if (mate) {
-			mate->config_data = slot_port;
-			mate->select_data = sel | 1;
-			mate->hw.dma  = ec->dma;
-			mate->hw.priv = (void *)
-					(port + ICS_ARCIN_V6_INTRSTAT_2);
-			mate->unit = 1;
-			icside_setup_dma(mate, autodma);
-		}
+	if (ec->dma != NO_DMA && !request_dma(ec->dma, ch0->name)) {
+		icside_setup_dma(ch0);
+		icside_setup_dma(ch1);
 	}
-no_dma:
 #endif
-	return hwif || mate ? 0 : -1;
+	return 0;
 }
 
 int __init icside_init(void)
 {
-	int autodma = 0;
-
-#ifdef CONFIG_IDEDMA_ICS_AUTO
-	autodma = 1;
-#endif
-
 	ecard_startfind ();
 
 	do {
@@ -647,11 +802,11 @@
 
 		switch (icside_identifyif(ec)) {
 		case ics_if_arcin_v5:
-			result = icside_register_v5(ec, autodma);
+			result = icside_register_v5(ec);
 			break;
 
 		case ics_if_arcin_v6:
-			result = icside_register_v6(ec, autodma);
+			result = icside_register_v6(ec);
 			break;
 
 		default:
diff -urN linux-2.5.17/drivers/ide/ide.c linux/drivers/ide/ide.c
--- linux-2.5.17/drivers/ide/ide.c	2002-05-22 20:11:43.000000000 +0200
+++ linux/drivers/ide/ide.c	2002-05-22 17:06:50.000000000 +0200
@@ -139,34 +139,6 @@
  */
 struct ata_channel ide_hwifs[MAX_HWIFS];	/* master data repository */
 
-#if (DISK_RECOVERY_TIME > 0)
-/*
- * For really screwed hardware (hey, at least it *can* be used with Linux)
- * we can enforce a minimum delay time between successive operations.
- */
-static unsigned long read_timer (void)
-{
-	unsigned long t, flags;
-	int i;
-
-	__save_flags(flags);	/* local CPU only */
-	__cli();		/* local CPU only */
-	t = jiffies * 11932;
-	outb_p(0, 0x43);
-	i = inb_p(0x40);
-	i |= inb(0x40) << 8;
-	__restore_flags(flags);	/* local CPU only */
-	return (t - i);
-}
-#endif
-
-static inline void set_recovery_timer(struct ata_channel *channel)
-{
-#if (DISK_RECOVERY_TIME > 0)
-	channel->last_time = read_timer();
-#endif
-}
-
 static void init_hwif_data(struct ata_channel *ch, unsigned int index)
 {
 	static const byte ide_major[] = {
@@ -241,8 +213,6 @@
 
 	/* Add default hw interfaces */
 	ide_init_default_hwifs();
-
-	idebus_parameter = 0;
 }
 
 /*
@@ -667,6 +637,50 @@
 	end_that_request_last(rq);
 }
 
+#if FANCY_STATUS_DUMPS
+struct ata_bit_messages {
+	u8 mask;
+	u8 match;
+	const char *msg;
+};
+
+static struct ata_bit_messages ata_status_msgs[] = {
+	{ BUSY_STAT,  BUSY_STAT,  "Busy"           },
+	{ READY_STAT, READY_STAT, "DriveReady"     },
+	{ WRERR_STAT, WRERR_STAT, "DeviceFault"    },
+	{ SEEK_STAT,  SEEK_STAT,  "SeekComplete"   },
+	{ DRQ_STAT,   DRQ_STAT,   "DataRequest"    },
+	{ ECC_STAT,   ECC_STAT,   "CorrectedError" },
+	{ INDEX_STAT, INDEX_STAT, "Index"          },
+	{ ERR_STAT,   ERR_STAT,   "Error"          }
+};
+
+static struct ata_bit_messages ata_error_msgs[] = {
+	{ ICRC_ERR|ABRT_ERR,	ABRT_ERR,		"DriveStatusError"   },
+	{ ICRC_ERR|ABRT_ERR,	ICRC_ERR,		"BadSector"	     },
+	{ ICRC_ERR|ABRT_ERR,	ICRC_ERR|ABRT_ERR,	"BadCRC"	     },
+	{ ECC_ERR,		ECC_ERR,		"UncorrectableError" },
+	{ ID_ERR,		ID_ERR,			"SectorIdNotFound"   },
+	{ TRK0_ERR,		TRK0_ERR,		"TrackZeroNotFound"  },
+	{ MARK_ERR,		MARK_ERR,		"AddrMarkNotFound"   }
+};
+
+static void ata_dump_bits(struct ata_bit_messages *msgs, int nr, byte bits)
+{
+	int i;
+
+	printk(" { ");
+
+	for (i = 0; i < nr; i++, msgs++)
+		if ((bits & msgs->mask) == msgs->match)
+			printk("%s ", msgs->msg);
+
+	printk("} ");
+}
+#else
+#define ata_dump_bits(msgs,nr,bits) do { } while (0)
+#endif
+
 /*
  * Error reporting, in human readable form (luxurious, but a memory hog).
  */
@@ -674,49 +688,22 @@
 {
 	unsigned long flags;
 	byte err = 0;
+	int i;
 
 	__save_flags (flags);	/* local CPU only */
 	ide__sti();		/* local CPU only */
-#if !(FANCY_STATUS_DUMPS)
-	printk("%s: %s: status=0x%02x\n", drive->name, msg, stat);
-#else
-	printk(" { ");
-	{
-		char *msg = "";
-		if (stat & BUSY_STAT)
-			msg = "Busy";
-		else {
-			if (stat & READY_STAT)
-				msg = "DriveReady";
-			if (stat & WRERR_STAT)
-				msg = "DeviceFault";
-			if (stat & SEEK_STAT)
-				msg = "SeekComplete";
-			if (stat & DRQ_STAT)
-				msg = "DataRequest";
-			if (stat & ECC_STAT)
-				msg = "CorrectedError";
-			if (stat & INDEX_STAT)
-				msg = "Index";
-			if (stat & ERR_STAT)
-				msg = "Error";
-		}
-	}
-	printk("%s }\n", msg);
-#endif
+
+	printk("%s: %s: status=0x%02x", drive->name, msg, stat);
+	ata_dump_bits(ata_status_msgs, ARRAY_SIZE(ata_status_msgs), stat);
+	printk("\n");
+
 	if ((stat & (BUSY_STAT|ERR_STAT)) == ERR_STAT) {
 		err = GET_ERR();
 		printk("%s: %s: error=0x%02x", drive->name, msg, err);
 #if FANCY_STATUS_DUMPS
 		if (drive->type == ATA_DISK) {
-			printk(" { ");
-			if (err & ABRT_ERR)	printk("DriveStatusError ");
-			if (err & ICRC_ERR)	printk("%s", (err & ABRT_ERR) ? "BadCRC " : "BadSector ");
-			if (err & ECC_ERR)	printk("UncorrectableError ");
-			if (err & ID_ERR)	printk("SectorIdNotFound ");
-			if (err & TRK0_ERR)	printk("TrackZeroNotFound ");
-			if (err & MARK_ERR)	printk("AddrMarkNotFound ");
-			printk("}");
+			ata_dump_bits(ata_error_msgs, ARRAY_SIZE(ata_error_msgs), err);
+
 			if ((err & (BBD_ERR | ABRT_ERR)) == BBD_ERR || (err & (ECC_ERR|ID_ERR|MARK_ERR))) {
 				if ((drive->id->command_set_2 & 0x0400) &&
 				    (drive->id->cfs_enable_2 & 0x0400) &&
@@ -988,11 +975,6 @@
 	 */
 	if (block == 0 && drive->remap_0_to_1 == 1)
 		block = 1;  /* redirect MBR access to EZ-Drive partn table */
-
-#if (DISK_RECOVERY_TIME > 0)
-	while ((read_timer() - ch->last_time) < DISK_RECOVERY_TIME);
-#endif
-
 	{
 		ide_startstop_t res;
 
@@ -1464,7 +1446,6 @@
 				} else
 					startstop = ide_error(drive, drive->rq, "irq timeout", GET_STAT());
 			}
-			set_recovery_timer(ch);
 			enable_irq(ch->irq);
 
 			spin_lock_irq(ch->lock);
@@ -1614,7 +1595,6 @@
 	 * same irq as is currently being serviced here, and Linux
 	 * won't allow another of the same (on any CPU) until we return.
 	 */
-	set_recovery_timer(drive->channel);
 	if (startstop == ide_stopped) {
 		if (!ch->handler) {	/* paranoia */
 			clear_bit(IDE_BUSY, ch->active);
@@ -2787,10 +2767,7 @@
 	if (!strncmp(s, "idebus", 6)) {
 		if (match_parm(&s[6], NULL, vals, 1) != 1)
 			goto bad_option;
-		if (vals[0] >= 20 && vals[0] <= 66) {
-			idebus_parameter = vals[0];
-		} else
-			printk(" -- BAD BUS SPEED! Expected value from 20 to 66");
+		idebus_parameter = vals[0];
 		goto done;
 	}
 
@@ -3219,27 +3196,40 @@
 
 	ide_devfs_handle = devfs_mk_dir (NULL, "ide", NULL);
 
-	/* Initialize system bus speed.
-	 *
-	 * This can be changed by a particular chipse initialization module.
-	 * Otherwise we assume 33MHz as a safe value for PCI bus based systems.
-	 * 50MHz will be assumed for abolitions like VESA, since higher values
-	 * result in more conservative timing setups.
-	 *
-	 * The kernel parameter idebus=XX overrides the default settings.
-	 */
+/*
+ * Because most of the ATA adapters represent the timings in unit of bus
+ * clocks, and there is no known reliable way to detect the bus clock
+ * frequency, we assume 50 MHz for non-PCI (VLB, EISA) and 33 MHz for PCI based
+ * systems. Since assuming only hurts performance and not stability, this is
+ * OK. The user can change this on the command line by using the "idebus=XX"
+ * parameter. While the system_bus_speed variable is in kHz units, we accept
+ * both MHz and kHz entry on the command line for backward compatibility.
+ */
 
-	system_bus_speed = 50;
-	if (idebus_parameter)
-	    system_bus_speed = idebus_parameter;
-#ifdef CONFIG_PCI
-	else if (pci_present())
-	    system_bus_speed = 33;
-#endif
+	system_bus_speed = 50000;
 
-	printk(KERN_INFO "ATA: system bus speed %dMHz\n", system_bus_speed);
+	if (pci_present())
+	    system_bus_speed = 33333;
 
-	init_ide_data ();
+	if (idebus_parameter >= 20 && idebus_parameter <= 80) {
+
+		system_bus_speed = idebus_parameter * 1000;
+
+		switch (system_bus_speed) {
+			case 33000: system_bus_speed = 33333; break;
+			case 37000: system_bus_speed = 37500; break;
+			case 41000: system_bus_speed = 41666; break;
+			case 66000: system_bus_speed = 66666; break;
+		}
+	}
+
+	if (idebus_parameter >= 20000 && idebus_parameter <= 80000)
+	    system_bus_speed = idebus_parameter;
+	
+	printk(KERN_INFO "ATA: %s bus speed %d.%dMHz\n",
+		pci_present() ? "PCI" : "System", system_bus_speed / 1000, system_bus_speed / 100 % 10);
+
+	init_ide_data();
 
 	initializing = 1;
 
diff -urN linux-2.5.17/drivers/ide/opti621.c linux/drivers/ide/opti621.c
--- linux-2.5.17/drivers/ide/opti621.c	2002-05-21 07:07:32.000000000 +0200
+++ linux/drivers/ide/opti621.c	2002-05-22 13:21:11.000000000 +0200
@@ -175,7 +175,7 @@
  * Use idebus=xx to select right frequency.
  */
 {
-	return ((time*bus_speed+999)/1000);
+	return ((time*bus_speed+999999)/1000000);
 }
 
 static void write_reg(byte value, int reg)
diff -urN linux-2.5.17/drivers/ide/pcidma.c linux/drivers/ide/pcidma.c
--- linux-2.5.17/drivers/ide/pcidma.c	2002-05-21 07:07:33.000000000 +0200
+++ linux/drivers/ide/pcidma.c	2002-05-22 17:06:50.000000000 +0200
@@ -258,7 +258,7 @@
  * recovery" in the ATAPI drivers. This was just plain wrong before, in esp.
  * not portable, and just got uncovered now.
  */
-static void udma_pci_enable(struct ata_device *drive, int on, int verbose)
+void udma_pci_enable(struct ata_device *drive, int on, int verbose)
 {
 	struct ata_channel *ch = drive->channel;
 	int set_high = 1;
@@ -396,7 +396,7 @@
  * about addressing modes.
  */
 
-static int udma_pci_start(struct ata_device *drive, struct request *rq)
+int udma_pci_start(struct ata_device *drive, struct request *rq)
 {
 	struct ata_channel *ch = drive->channel;
 	unsigned long dma_base = ch->dma_base;
@@ -410,7 +410,7 @@
 	return 0;
 }
 
-static int udma_pci_stop(struct ata_device *drive)
+int udma_pci_stop(struct ata_device *drive)
 {
 	struct ata_channel *ch = drive->channel;
 	unsigned long dma_base = ch->dma_base;
@@ -425,12 +425,12 @@
 	return (dma_stat & 7) != 4 ? (0x10 | dma_stat) : 0;	/* verify good DMA status */
 }
 
-static int udma_pci_read(struct ata_device *drive, struct request *rq)
+int udma_pci_read(struct ata_device *drive, struct request *rq)
 {
 	return ata_do_udma(1, drive, rq);
 }
 
-static int udma_pci_write(struct ata_device *drive, struct request *rq)
+int udma_pci_write(struct ata_device *drive, struct request *rq)
 {
 	return ata_do_udma(0, drive, rq);
 }
@@ -438,7 +438,7 @@
 /*
  * FIXME: This should be attached to a channel as we can see now!
  */
-static int udma_pci_irq_status(struct ata_device *drive)
+int udma_pci_irq_status(struct ata_device *drive)
 {
 	struct ata_channel *ch = drive->channel;
 	u8 dma_stat;
@@ -449,12 +449,12 @@
 	return (dma_stat & 4) == 4;	/* return 1 if INTR asserted */
 }
 
-static void udma_pci_timeout(struct ata_device *drive)
+void udma_pci_timeout(struct ata_device *drive)
 {
 	printk(KERN_ERR "ATA: UDMA timeout occured %s!\n", drive->name);
 }
 
-static void udma_pci_irq_lost(struct ata_device *drive)
+void udma_pci_irq_lost(struct ata_device *drive)
 {
 }
 
@@ -553,3 +553,11 @@
 
 EXPORT_SYMBOL(ata_do_udma);
 EXPORT_SYMBOL(ide_dma_intr);
+EXPORT_SYMBOL(udma_pci_enable);
+EXPORT_SYMBOL(udma_pci_start);
+EXPORT_SYMBOL(udma_pci_stop);
+EXPORT_SYMBOL(udma_pci_read);
+EXPORT_SYMBOL(udma_pci_write);
+EXPORT_SYMBOL(udma_pci_irq_status);
+EXPORT_SYMBOL(udma_pci_timeout);
+EXPORT_SYMBOL(udma_pci_irq_lost);
diff -urN linux-2.5.17/drivers/ide/pdc202xx.c linux/drivers/ide/pdc202xx.c
--- linux-2.5.17/drivers/ide/pdc202xx.c	2002-05-21 07:07:42.000000000 +0200
+++ linux/drivers/ide/pdc202xx.c	2002-05-22 11:47:10.000000000 +0200
@@ -166,10 +166,12 @@
 
 #endif /* PDC202XX_DECODE_REGISTER_INFO */
 
-int check_in_drive_lists(struct ata_device *drive) {
-	const char *pdc_quirk_drives[] = {
+int check_in_drive_lists(struct ata_device *drive)
+{
+	static const char *pdc_quirk_drives[] = {
 		"QUANTUM FIREBALLlct08 08",
 		"QUANTUM FIREBALLP KA6.4",
+		"QUANTUM FIREBALLP KA9.1",
 		"QUANTUM FIREBALLP LM20.4",
 		"QUANTUM FIREBALLP KX20.5",
 		"QUANTUM FIREBALLP KX27.3",
diff -urN linux-2.5.17/drivers/ide/piix.c linux/drivers/ide/piix.c
--- linux-2.5.17/drivers/ide/piix.c	2002-05-22 20:11:38.000000000 +0200
+++ linux/drivers/ide/piix.c	2002-05-22 13:21:11.000000000 +0200
@@ -105,7 +105,6 @@
 static struct piix_ide_chip *piix_config;
 static unsigned char piix_enabled;
 static unsigned int piix_80w;
-static unsigned int piix_clock;
 
 static char *piix_dma[] = { "MWDMA16", "UDMA33", "UDMA66", "UDMA100", "UDMA133" };
 
@@ -147,7 +146,7 @@
 								: piix_dma[piix_config->flags & PIIX_UDMA]);
 
 	piix_print("BM-DMA base:                        %#x", piix_base);
-	piix_print("PCI clock:                          %d.%dMHz", piix_clock / 1000, piix_clock / 100 % 10);
+	piix_print("PCI clock:                          %d.%dMHz", system_bus_speed / 1000, system_bus_speed / 100 % 10);
 
 	piix_print("-----------------------Primary IDE-------Secondary IDE------");
 
@@ -160,7 +159,7 @@
 
 	piix_print("Cable Type:            %10s%20s", (piix_80w & 1) ? "80w" : "40w", (piix_80w & 2) ? "80w" : "40w");
 
-	if (!piix_clock)
+	if (!system_bus_speed)
                 return p - buffer;
 
 	piix_print("-------------------drive0----drive1----drive2----drive3-----");
@@ -192,8 +191,8 @@
 		}
 
 		dmaen[i] = (c & ((i & 1) ? 0x40 : 0x20) << ((i & 2) << 2));
-		cycle[i] = 1000000 / piix_clock * (active[i] + recover[i]);
-		speed[i] = 2 * piix_clock / (active[i] + recover[i]);
+		cycle[i] = 1000000 / system_bus_speed * (active[i] + recover[i]);
+		speed[i] = 2 * system_bus_speed / (active[i] + recover[i]);
 
 		if (!(piix_config->flags & PIIX_UDMA))
 			continue;
@@ -213,17 +212,17 @@
 			udma[i] = (4 - ((e >> (i << 2)) & 3)) * umul;
 		} else  udma[i] = (8 - ((e >> (i << 2)) & 7)) * 2;
 
-		speed[i] = 8 * piix_clock / udma[i];
-		cycle[i] = 250000 * udma[i] / piix_clock;
+		speed[i] = 8 * system_bus_speed / udma[i];
+		cycle[i] = 250000 * udma[i] / system_bus_speed;
 	}
 
 	piix_print_drive("Transfer Mode: ", "%10s", dmaen[i] ? (uen[i] ? "UDMA" : "DMA") : "PIO");
 
-	piix_print_drive("Address Setup: ", "%8dns", (1000000 / piix_clock) * 3);
-	piix_print_drive("Cmd Active:    ", "%8dns", (1000000 / piix_clock) * 12);
-	piix_print_drive("Cmd Recovery:  ", "%8dns", (1000000 / piix_clock) * 18);
-	piix_print_drive("Data Active:   ", "%8dns", (1000000 / piix_clock) * active[i]);
-	piix_print_drive("Data Recovery: ", "%8dns", (1000000 / piix_clock) * recover[i]);
+	piix_print_drive("Address Setup: ", "%8dns", (1000000 / system_bus_speed) * 3);
+	piix_print_drive("Cmd Active:    ", "%8dns", (1000000 / system_bus_speed) * 12);
+	piix_print_drive("Cmd Recovery:  ", "%8dns", (1000000 / system_bus_speed) * 18);
+	piix_print_drive("Data Active:   ", "%8dns", (1000000 / system_bus_speed) * active[i]);
+	piix_print_drive("Data Recovery: ", "%8dns", (1000000 / system_bus_speed) * recover[i]);
 	piix_print_drive("Cycle Time:    ", "%8dns", cycle[i]);
 	piix_print_drive("Transfer Rate: ", "%4d.%dMB/s", speed[i] / 1000, speed[i] / 100 % 10);
 
@@ -339,7 +338,7 @@
 	if (speed > XFER_UDMA_4 && (piix_config->flags & PIIX_UDMA) >= PIIX_UDMA_100)
 		umul = 4;
 	
-	T = 1000000000 / piix_clock;
+	T = 1000000000 / system_bus_speed;
 	UT = T / umul;
 
 	ata_timing_compute(drive, speed, &t, T, UT);
@@ -494,24 +493,6 @@
 	}
 
 /*
- * Determine the system bus clock.
- */
-
-	piix_clock = system_bus_speed * 1000;
-
-	switch (piix_clock) {
-		case 33000: piix_clock = 33333; break;
-		case 37000: piix_clock = 37500; break;
-		case 41000: piix_clock = 41666; break;
-	}
-
-	if (piix_clock < 20000 || piix_clock > 50000) {
-		printk(KERN_WARNING "PIIX: User given PCI clock speed impossible (%d), using 33 MHz instead.\n", piix_clock);
-		printk(KERN_WARNING "PIIX: Use ide0=ata66 if you want to assume 80-wire cable\n");
-		piix_clock = 33333;
-	}
-
-/*
  * Print the boot message.
  */
 
diff -urN linux-2.5.17/drivers/ide/qd65xx.c linux/drivers/ide/qd65xx.c
--- linux-2.5.17/drivers/ide/qd65xx.c	2002-05-21 07:07:35.000000000 +0200
+++ linux/drivers/ide/qd65xx.c	2002-05-22 13:21:11.000000000 +0200
@@ -133,12 +133,12 @@
 {
 	byte active_cycle,recovery_cycle;
 
-	if (system_bus_speed <= 33) {
-		active_cycle =   9  - IDE_IN(active_time   * system_bus_speed / 1000 + 1, 2, 9);
-		recovery_cycle = 15 - IDE_IN(recovery_time * system_bus_speed / 1000 + 1, 0, 15);
+	if (system_bus_speed <= 33333) {
+		active_cycle =   9  - IDE_IN(active_time   * system_bus_speed / 1000000 + 1, 2, 9);
+		recovery_cycle = 15 - IDE_IN(recovery_time * system_bus_speed / 1000000 + 1, 0, 15);
 	} else {
-		active_cycle =   8  - IDE_IN(active_time   * system_bus_speed / 1000 + 1, 1, 8);
-		recovery_cycle = 18 - IDE_IN(recovery_time * system_bus_speed / 1000 + 1, 3, 18);
+		active_cycle =   8  - IDE_IN(active_time   * system_bus_speed / 1000000 + 1, 1, 8);
+		recovery_cycle = 18 - IDE_IN(recovery_time * system_bus_speed / 1000000 + 1, 3, 18);
 	}
 
 	return((recovery_cycle<<4) | 0x08 | active_cycle);
@@ -152,8 +152,8 @@
 
 static byte qd6580_compute_timing (int active_time, int recovery_time)
 {
-	byte active_cycle   = 17-IDE_IN(active_time   * system_bus_speed / 1000 + 1, 2, 17);
-	byte recovery_cycle = 15-IDE_IN(recovery_time * system_bus_speed / 1000 + 1, 2, 15);
+	byte active_cycle   = 17-IDE_IN(active_time   * system_bus_speed / 1000000 + 1, 2, 17);
+	byte recovery_cycle = 15-IDE_IN(recovery_time * system_bus_speed / 1000000 + 1, 2, 15);
 
 	return((recovery_cycle<<4) | active_cycle);
 }
diff -urN linux-2.5.17/drivers/ide/serverworks.c linux/drivers/ide/serverworks.c
--- linux-2.5.17/drivers/ide/serverworks.c	2002-05-21 07:07:40.000000000 +0200
+++ linux/drivers/ide/serverworks.c	2002-05-22 11:10:00.000000000 +0200
@@ -242,9 +242,9 @@
 
 static int svwks_tune_chipset(struct ata_device *drive, byte speed)
 {
-	byte udma_modes[]	= { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 };
-	byte dma_modes[]	= { 0x77, 0x21, 0x20 };
-	byte pio_modes[]	= { 0x5d, 0x47, 0x34, 0x22, 0x20 };
+	static u8 udma_modes[]	= { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 };
+	static u8 dma_modes[]	= { 0x77, 0x21, 0x20 };
+	static u8 pio_modes[]	= { 0x5d, 0x47, 0x34, 0x22, 0x20 };
 
 	struct ata_channel *hwif = drive->channel;
 	struct pci_dev *dev	= hwif->pci_dev;
@@ -253,7 +253,7 @@
 
 #ifdef CONFIG_BLK_DEV_IDEDMA
 	unsigned long dma_base	= hwif->dma_base;
-#endif /* CONFIG_BLK_DEV_IDEDMA */
+#endif
 	int err;
 
 	byte drive_pci		= 0x00;
diff -urN linux-2.5.17/drivers/ide/sl82c105.c linux/drivers/ide/sl82c105.c
--- linux-2.5.17/drivers/ide/sl82c105.c	2002-05-21 07:07:35.000000000 +0200
+++ linux/drivers/ide/sl82c105.c	2002-05-22 17:09:02.000000000 +0200
@@ -5,8 +5,12 @@
  *
  * Maintainer unknown.
  *
- * Drive tuning added from Rebel.com's kernel sources
- *  -- Russell King (15/11/98) linux@arm.linux.org.uk
+ * Changelog:
+ *
+ * 15/11/1998	RMK	Drive tuning added from Rebel.com's kernel
+ *			sources
+ * 30/03/2002	RMK	Add fixes specified in W83C553F errata.
+ *			(with special thanks to Todd Inglett)
  */
 #include <linux/types.h>
 #include <linux/kernel.h>
@@ -28,6 +32,17 @@
 extern char *ide_xfer_verbose (byte xfer_rate);
 
 /*
+ * SL82C105 PCI config register 0x40 bits.
+ */
+#define CTRL_IDE_IRQB	(1 << 30)
+#define CTRL_IDE_IRQA	(1 << 28)
+#define CTRL_LEGIRQ	(1 << 11)
+#define CTRL_P1F16	(1 << 5)
+#define CTRL_P1EN	(1 << 4)
+#define CTRL_P0F16	(1 << 1)
+#define	CTRL_P0EN	(1 << 0)
+
+/*
  * Convert a PIO mode and cycle time to the required on/off
  * times for the interface.  This has protection against run-away
  * timings.
@@ -111,6 +126,7 @@
 	return 0;
 }
 
+
 /*
  * Check to see if the drive and
  * chipset is capable of DMA mode
@@ -146,6 +162,7 @@
 			break;
 		}
 	} while (0);
+
 	if (on)
 		config_for_dma(drive);
 	else
@@ -153,12 +170,12 @@
 
 	udma_enable(drive, on, 0);
 
-
 	return 0;
 }
 
 /*
- * Our own dmaproc, only to intercept ide_dma_check
+ * Our very own dmaproc.  We need to intercept various calls
+ * to fix up the SL82C105 specific behaviour.
  */
 static int sl82c105_dmaproc(struct ata_device *drive)
 {
@@ -166,6 +183,93 @@
 }
 
 /*
+ * The SL82C105 holds off all IDE interrupts while in DMA mode until
+ * all DMA activity is completed.  Sometimes this causes problems (eg,
+ * when the drive wants to report an error condition).
+ *
+ * 0x7e is a "chip testing" register.  Bit 2 resets the DMA controller
+ * state machine.  We need to kick this to work around various bugs.
+ */
+static inline void sl82c105_reset_host(struct pci_dev *dev)
+{
+	u16 val;
+
+	pci_read_config_word(dev, 0x7e, &val);
+	pci_write_config_word(dev, 0x7e, val | (1 << 2));
+	pci_write_config_word(dev, 0x7e, val & ~(1 << 2));
+}
+
+static void sl82c105_dma_enable(struct ata_device *drive, int on, int verbose)
+{
+	if (!on || config_for_dma(drive)) {
+		config_for_pio(drive, 4, 0);
+		on = 0;
+	}
+	udma_pci_enable(drive, on, verbose);
+}
+
+/*
+ * ATAPI devices can cause the SL82C105 DMA state machine to go gaga.
+ * Winbond recommend that the DMA state machine is reset prior to
+ * setting the bus master DMA enable bit.
+ *
+ * The generic IDE core will have disabled the BMEN bit before this
+ * function is called.
+ */
+static int sl82c105_dma_read(struct ata_device *drive, struct request *rq)
+{
+	sl82c105_reset_host(drive->channel->pci_dev);
+	return udma_pci_read(drive, rq);
+}
+
+static int sl82c105_dma_write(struct ata_device *drive, struct request *rq)
+{
+	sl82c105_reset_host(drive->channel->pci_dev);
+	return udma_pci_write(drive, rq);
+}
+
+static void sl82c105_timeout(struct ata_device *drive)
+{
+	sl82c105_reset_host(drive->channel->pci_dev);
+}
+
+/*
+ * If we get an IRQ timeout, it might be that the DMA state machine
+ * got confused.  Fix from Todd Inglett.  Details from Winbond.
+ *
+ * This function is called when the IDE timer expires, the drive
+ * indicates that it is READY, and we were waiting for DMA to complete.
+ */
+static void sl82c105_lostirq(ide_drive_t *drive)
+{
+	struct ata_channel *ch = drive->channel;
+	struct pci_dev *dev = ch->pci_dev;
+	u32 val, mask = ch->unit ? CTRL_IDE_IRQB : CTRL_IDE_IRQA;
+	unsigned long dma_base = ch->dma_base;
+
+	printk("sl82c105: lost IRQ: resetting host\n");
+
+	/*
+	 * Check the raw interrupt from the drive.
+	 */
+	pci_read_config_dword(dev, 0x40, &val);
+	if (val & mask)
+		printk("sl82c105: drive was requesting IRQ, but host lost it\n");
+
+	/*
+	 * Was DMA enabled?  If so, disable it - we're resetting the
+	 * host.  The IDE layer will be handling the drive for us.
+	 */
+	val = inb(dma_base);
+	if (val & 1) {
+		outb(val & ~1, dma_base);
+		printk("sl82c105: DMA was enabled\n");
+	}
+
+	sl82c105_reset_host(dev);
+}
+
+/*
  * We only deal with PIO mode here - DMA mode 'using_dma' is not
  * initialised at the point that this function is called.
  */
@@ -185,21 +289,25 @@
  * Return the revision of the Winbond bridge
  * which this function is part of.
  */
-static unsigned int sl82c105_bridge_revision(struct pci_dev *dev)
+static __init unsigned int sl82c105_bridge_revision(struct pci_dev *dev)
 {
 	struct pci_dev *bridge;
 	unsigned char rev;
 
-	bridge = pci_find_device(PCI_VENDOR_ID_WINBOND, PCI_DEVICE_ID_WINBOND_83C553, NULL);
-
 	/*
-	 * If we are part of a Winbond 553
+	 * The bridge should be part of the same device, but function 0.
 	 */
-	if (!bridge || bridge->class >> 8 != PCI_CLASS_BRIDGE_ISA)
+	bridge = pci_find_slot(dev->bus->number,
+			       PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
+	if (!bridge)
 		return -1;
 
-	if (bridge->bus != dev->bus ||
-	    PCI_SLOT(bridge->devfn) != PCI_SLOT(dev->devfn))
+	/*
+	 * Make sure it is a Winbond 553 and is an ISA bridge.
+	 */
+	if (bridge->vendor != PCI_VENDOR_ID_WINBOND ||
+	    bridge->device != PCI_DEVICE_ID_WINBOND_83C553 ||
+	    bridge->class >> 8 != PCI_CLASS_BRIDGE_ISA)
 		return -1;
 
 	/*
@@ -215,49 +323,55 @@
  */
 static unsigned int __init sl82c105_init_chipset(struct pci_dev *dev)
 {
-	unsigned char ctrl_stat;
+	u32 val;
 
-	/*
-	 * Enable the ports
-	 */
-	pci_read_config_byte(dev, 0x40, &ctrl_stat);
-	pci_write_config_byte(dev, 0x40, ctrl_stat | 0x33);
+	pci_read_config_dword(dev, 0x40, &val);
+	val |= CTRL_P0EN | CTRL_P0F16 | CTRL_P1EN | CTRL_P1F16;
+	pci_write_config_dword(dev, 0x40, val);
 
 	return dev->irq;
 }
 
-static void __init sl82c105_init_dma(struct ata_channel *hwif, unsigned long dma_base)
+static void __init sl82c105_init_dma(struct ata_channel *ch, unsigned long dma_base)
 {
-	unsigned int rev;
+	unsigned int bridge_rev;
 	byte dma_state;
 
 	dma_state = inb(dma_base + 2);
-	rev = sl82c105_bridge_revision(hwif->pci_dev);
-	if (rev <= 5) {
-		hwif->autodma = 0;
-		hwif->drives[0].autotune = 1;
-		hwif->drives[1].autotune = 1;
+	bridge_rev = sl82c105_bridge_revision(ch->pci_dev);
+	if (bridge_rev <= 5) {
+		ch->autodma = 0;
+		ch->drives[0].autotune = 1;
+		ch->drives[1].autotune = 1;
 		printk("    %s: Winbond 553 bridge revision %d, BM-DMA disabled\n",
-		       hwif->name, rev);
+		       ch->name, bridge_rev);
 		dma_state &= ~0x60;
 	} else {
 		dma_state |= 0x60;
-		hwif->autodma = 1;
+		ch->autodma = 1;
 	}
 	outb(dma_state, dma_base + 2);
 
-	hwif->XXX_udma = NULL;
-	ata_init_dma(hwif, dma_base);
-	if (hwif->XXX_udma)
-		hwif->XXX_udma = sl82c105_dmaproc;
+	ata_init_dma(ch, dma_base);
+
+	if (bridge_rev <= 5)
+		ch->XXX_udma = NULL;
+	else {
+		ch->XXX_udma      = sl82c105_dmaproc;
+		ch->udma_enable   = sl82c105_dma_enable;
+		ch->udma_read     = sl82c105_dma_read;
+		ch->udma_write    = sl82c105_dma_write;
+		ch->udma_timeout  = sl82c105_timeout;
+		ch->udma_irq_lost = sl82c105_lostirq;
+	}
 }
 
 /*
  * Initialise the chip
  */
-static void __init sl82c105_init_channel(struct ata_channel *hwif)
+static void __init sl82c105_init_channel(struct ata_channel *ch)
 {
-	hwif->tuneproc = tune_sl82c105;
+	ch->tuneproc = tune_sl82c105;
 }
 
 
diff -urN linux-2.5.17/drivers/ide/via82cxxx.c linux/drivers/ide/via82cxxx.c
--- linux-2.5.17/drivers/ide/via82cxxx.c	2002-05-22 20:11:38.000000000 +0200
+++ linux/drivers/ide/via82cxxx.c	2002-05-22 13:21:11.000000000 +0200
@@ -105,7 +105,7 @@
 	unsigned char rev_min;
 	unsigned char rev_max;
 	unsigned short flags;
-} via_isa_bridges[] = {
+} via_isa_bridges [] __initdata = {
 #ifdef FUTURE_BRIDGES
 	{ "vt8237",	PCI_DEVICE_ID_VIA_8237,     0x00, 0x2f, VIA_UDMA_133 },
 	{ "vt8235",	PCI_DEVICE_ID_VIA_8235,     0x00, 0x2f, VIA_UDMA_133 },
@@ -132,7 +132,6 @@
 static struct via_isa_bridge *via_config;
 static unsigned char via_enabled;
 static unsigned int via_80w;
-static unsigned int via_clock;
 static char *via_dma[] = { "MWDMA16", "UDMA33", "UDMA66", "UDMA100", "UDMA133" };
 
 /*
@@ -175,7 +174,7 @@
 	via_print("Highest DMA rate:                   %s", via_dma[via_config->flags & VIA_UDMA]);
 
 	via_print("BM-DMA base:                        %#x", via_base);
-	via_print("PCI clock:                          %d.%dMHz", via_clock / 1000, via_clock / 100 % 10);
+	via_print("PCI clock:                          %d.%dMHz", system_bus_speed / 1000, system_bus_speed / 100 % 10);
 
 	pci_read_config_byte(dev, VIA_MISC_1, &t);
 	via_print("Master Read  Cycle IRDY:            %dws", (t & 64) >> 6);
@@ -223,8 +222,8 @@
 		uen[i]       = ((u >> ((3 - i) << 3)) & 0x20);
 		den[i]       = (c & ((i & 1) ? 0x40 : 0x20) << ((i & 2) << 2));
 
-		speed[i] = 2 * via_clock / (active[i] + recover[i]);
-		cycle[i] = 1000000 * (active[i] + recover[i]) / via_clock;
+		speed[i] = 2 * system_bus_speed / (active[i] + recover[i]);
+		cycle[i] = 1000000 * (active[i] + recover[i]) / system_bus_speed;
 
 		if (!uen[i] || !den[i])
 			continue;
@@ -232,34 +231,34 @@
 		switch (via_config->flags & VIA_UDMA) {
 
 			case VIA_UDMA_33:
-				speed[i] = 2 * via_clock / udma[i];
-				cycle[i] = 1000000 * udma[i] / via_clock;
+				speed[i] = 2 * system_bus_speed / udma[i];
+				cycle[i] = 1000000 * udma[i] / system_bus_speed;
 				break;
 
 			case VIA_UDMA_66:
-				speed[i] = 4 * via_clock / (udma[i] * umul[i]);
-				cycle[i] = 500000 * (udma[i] * umul[i]) / via_clock;
+				speed[i] = 4 * system_bus_speed / (udma[i] * umul[i]);
+				cycle[i] = 500000 * (udma[i] * umul[i]) / system_bus_speed;
 				break;
 
 			case VIA_UDMA_100:
-				speed[i] = 6 * via_clock / udma[i];
-				cycle[i] = 333333 * udma[i] / via_clock;
+				speed[i] = 6 * system_bus_speed / udma[i];
+				cycle[i] = 333333 * udma[i] / system_bus_speed;
 				break;
 
 			case VIA_UDMA_133:
-				speed[i] = 8 * via_clock / udma[i];
-				cycle[i] = 250000 * udma[i] / via_clock;
+				speed[i] = 8 * system_bus_speed / udma[i];
+				cycle[i] = 250000 * udma[i] / system_bus_speed;
 				break;
 		}
 	}
 
 	via_print_drive("Transfer Mode: ", "%10s", den[i] ? (uen[i] ? "UDMA" : "DMA") : "PIO");
 
-	via_print_drive("Address Setup: ", "%8dns", 1000000 * setup[i] / via_clock);
-	via_print_drive("Cmd Active:    ", "%8dns", 1000000 * active8b[i] / via_clock);
-	via_print_drive("Cmd Recovery:  ", "%8dns", 1000000 * recover8b[i] / via_clock);
-	via_print_drive("Data Active:   ", "%8dns", 1000000 * active[i] / via_clock);
-	via_print_drive("Data Recovery: ", "%8dns", 1000000 * recover[i] / via_clock);
+	via_print_drive("Address Setup: ", "%8dns", 1000000 * setup[i] / system_bus_speed);
+	via_print_drive("Cmd Active:    ", "%8dns", 1000000 * active8b[i] / system_bus_speed);
+	via_print_drive("Cmd Recovery:  ", "%8dns", 1000000 * recover8b[i] / system_bus_speed);
+	via_print_drive("Data Active:   ", "%8dns", 1000000 * active[i] / system_bus_speed);
+	via_print_drive("Data Recovery: ", "%8dns", 1000000 * recover[i] / system_bus_speed);
 	via_print_drive("Cycle Time:    ", "%8dns", cycle[i]);
 	via_print_drive("Transfer Rate: ", "%4d.%dMB/s", speed[i] / 1000, speed[i] / 100 % 10);
 
@@ -314,7 +313,7 @@
 			printk(KERN_WARNING "ide%d: Drive %d didn't accept speed setting. Oh, well.\n",
 				drive->dn >> 1, drive->dn & 1);
 
-	T = 1000000000 / via_clock;
+	T = 1000000000 / system_bus_speed;
 
 	switch (via_config->flags & VIA_UDMA) {
 		case VIA_UDMA_33:   UT = T;   break;
@@ -471,24 +470,6 @@
 	pci_write_config_byte(dev, VIA_FIFO_CONFIG, t);
 
 /*
- * Determine system bus clock.
- */
-
-	via_clock = system_bus_speed * 1000;
-
-	switch (via_clock) {
-		case 33000: via_clock = 33333; break;
-		case 37000: via_clock = 37500; break;
-		case 41000: via_clock = 41666; break;
-	}
-
-	if (via_clock < 20000 || via_clock > 50000) {
-		printk(KERN_WARNING "VP_IDE: User given PCI clock speed impossible (%d), using 33 MHz instead.\n", via_clock);
-		printk(KERN_WARNING "VP_IDE: Use ide0=ata66 if you want to assume 80-wire cable.\n");
-		via_clock = 33333;
-	}
-
-/*
  * Print the boot message.
  */
 
diff -urN linux-2.5.17/include/linux/ide.h linux/include/linux/ide.h
--- linux-2.5.17/include/linux/ide.h	2002-05-22 20:11:38.000000000 +0200
+++ linux/include/linux/ide.h	2002-05-22 19:28:27.000000000 +0200
@@ -40,9 +40,6 @@
 
 /* Right now this is only needed by a promise controlled.
  */
-#ifndef DISK_RECOVERY_TIME		/* off=0; on=access_delay_time */
-# define DISK_RECOVERY_TIME	0	/*  for hardware that needs it */
-#endif
 #ifndef OK_TO_RESET_CONTROLLER		/* 1 needed for good error recovery */
 # define OK_TO_RESET_CONTROLLER	0	/* 0 for use with AH2372A/B interface */
 #endif
@@ -202,7 +199,8 @@
 	ide_cmd646,
 	ide_cy82c693,
 	ide_pmac,
-	ide_etrax100
+	ide_etrax100,
+	ide_acorn
 } hwif_chipset_t;
 
 
@@ -547,10 +545,6 @@
 	unsigned slow		: 1;	/* flag: slow data port */
 	unsigned io_32bit	: 1;	/* 0=16-bit, 1=32-bit */
 	unsigned char bus_state;	/* power state of the IDE bus */
-
-#if (DISK_RECOVERY_TIME > 0)
-	unsigned long last_time;	/* time when previous rq was done */
-#endif
 };
 
 /*
@@ -861,6 +855,15 @@
 
 #ifdef CONFIG_BLK_DEV_IDEDMA
 
+void udma_pci_enable(struct ata_device *drive, int on, int verbose);
+int udma_pci_start(struct ata_device *drive, struct request *rq);
+int udma_pci_stop(struct ata_device *drive);
+int udma_pci_read(struct ata_device *drive, struct request *rq);
+int udma_pci_write(struct ata_device *drive, struct request *rq);
+int udma_pci_irq_status(struct ata_device *drive);
+void udma_pci_timeout(struct ata_device *drive);
+void udma_pci_irq_lost(struct ata_device *);
+
 extern int udma_new_table(struct ata_channel *, struct request *);
 extern void udma_destroy_table(struct ata_channel *);
 extern void udma_print(struct ata_device *);

[PATCH] 2.5.17 sysvipc (AKA: spoiling oil in to the flames)

[Martin Dalecki]

[-- Attachment #1: Type: text/plain, Size: 1018 bytes --]

Kill /proc/sysvipc and friends. This is a pure case of interface duplication,
since we have the ipcs command and nice fine /proc/sys/kernel entries for the
relevant stuff... Fortunately apparently nothing is using it.

Fix improper extern inline usage in ipc/utils.h as well as add
some static attributes to local functions found there.

If some "embedded" system user starts to holler about
memory issues. Well:

[root@domek linux]# size /usr/bin/ipcs
    text    data     bss     dec     hex filename
   13433     324      32   13789    35dd /usr/bin/ipcs

And now count the pages used to implement /proc/sysvipc entires +
the few kilbytes of actual code removed from the kernel.

As an added bonus the functions implementing /proc/sysvipc
where accessible by everyone and getting the msg_ide.sem and similar
semaphores for quite a significant amount of time...

Perhaps it is time as well to look at the two different
IPC structures carried around there ("64 bit" and 32 bit)
and the double liked list usage there?

[-- Attachment #2: kill-sysvipc-2.5.17.patch --]
[-- Type: text/plain, Size: 11986 bytes --]

diff -urN linux-2.5.17/fs/proc/root.c linux/fs/proc/root.c
--- linux-2.5.17/fs/proc/root.c	2002-05-21 07:07:42.000000000 +0200
+++ linux/fs/proc/root.c	2002-05-23 07:07:38.000000000 +0200
@@ -53,9 +53,6 @@
 	}
 	proc_misc_init();
 	proc_net = proc_mkdir("net", 0);
-#ifdef CONFIG_SYSVIPC
-	proc_mkdir("sysvipc", 0);
-#endif
 #ifdef CONFIG_SYSCTL
 	proc_sys_root = proc_mkdir("sys", 0);
 #endif
Binary files linux-2.5.17/fs/proc/.root.c.swp and linux/fs/proc/.root.c.swp differ
diff -urN linux-2.5.17/ipc/msg.c linux/ipc/msg.c
--- linux-2.5.17/ipc/msg.c	2002-05-21 07:07:29.000000000 +0200
+++ linux/ipc/msg.c	2002-05-22 22:37:32.000000000 +0200
@@ -8,8 +8,6 @@
  * Fixed up the unchecked user space derefs
  * Copyright (C) 1998 Alan Cox & Andi Kleen
  *
- * /proc/sysvipc/msg support (c) 1999 Dragos Acostachioaie <dragos@iname.com>
- *
  * mostly rewritten, threaded and wake-one semantics added
  * MSGMAX limit removed, sysctl's added
  * (c) 1999 Manfred Spraul <manfreds@colorfullife.com>
@@ -20,9 +18,12 @@
 #include <linux/msg.h>
 #include <linux/spinlock.h>
 #include <linux/init.h>
-#include <linux/proc_fs.h>
 #include <linux/list.h>
+#include <linux/stat.h>
+#include <linux/err.h>
+
 #include <asm/uaccess.h>
+
 #include "util.h"
 
 /* sysctl: */
@@ -54,8 +55,8 @@
 };
 /* one msg_msg structure for each message */
 struct msg_msg {
-	struct list_head m_list; 
-	long  m_type;          
+	struct list_head m_list;
+	long  m_type;
 	int m_ts;           /* message text size */
 	struct msg_msgseg* next;
 	/* the actual message follows immediately */
@@ -101,17 +102,10 @@
 
 static void freeque (int id);
 static int newque (key_t key, int msgflg);
-#ifdef CONFIG_PROC_FS
-static int sysvipc_msg_read_proc(char *buffer, char **start, off_t offset, int length, int *eof, void *data);
-#endif
 
 void __init msg_init (void)
 {
 	ipc_init_ids(&msg_ids,msg_ctlmni);
-
-#ifdef CONFIG_PROC_FS
-	create_proc_read_entry("sysvipc/msg", 0, 0, sysvipc_msg_read_proc, NULL);
-#endif
 }
 
 static int newque (key_t key, int msgflg)
@@ -847,57 +841,3 @@
 		msg_unlock(msqid);
 	return err;
 }
-
-#ifdef CONFIG_PROC_FS
-static int sysvipc_msg_read_proc(char *buffer, char **start, off_t offset, int length, int *eof, void *data)
-{
-	off_t pos = 0;
-	off_t begin = 0;
-	int i, len = 0;
-
-	down(&msg_ids.sem);
-	len += sprintf(buffer, "       key      msqid perms      cbytes       qnum lspid lrpid   uid   gid  cuid  cgid      stime      rtime      ctime\n");
-
-	for(i = 0; i <= msg_ids.max_id; i++) {
-		struct msg_queue * msq;
-		msq = msg_lock(i);
-		if(msq != NULL) {
-			len += sprintf(buffer + len, "%10d %10d  %4o  %10lu %10lu %5u %5u %5u %5u %5u %5u %10lu %10lu %10lu\n",
-				msq->q_perm.key,
-				msg_buildid(i,msq->q_perm.seq),
-				msq->q_perm.mode,
-				msq->q_cbytes,
-				msq->q_qnum,
-				msq->q_lspid,
-				msq->q_lrpid,
-				msq->q_perm.uid,
-				msq->q_perm.gid,
-				msq->q_perm.cuid,
-				msq->q_perm.cgid,
-				msq->q_stime,
-				msq->q_rtime,
-				msq->q_ctime);
-			msg_unlock(i);
-
-			pos += len;
-			if(pos < offset) {
-				len = 0;
-				begin = pos;
-			}
-			if(pos > offset + length)
-				goto done;
-		}
-
-	}
-	*eof = 1;
-done:
-	up(&msg_ids.sem);
-	*start = buffer + (offset - begin);
-	len -= (offset - begin);
-	if(len > length)
-		len = length;
-	if(len < 0)
-		len = 0;
-	return len;
-}
-#endif
diff -urN linux-2.5.17/ipc/sem.c linux/ipc/sem.c
--- linux-2.5.17/ipc/sem.c	2002-05-21 07:07:37.000000000 +0200
+++ linux/ipc/sem.c	2002-05-22 23:01:02.000000000 +0200
@@ -49,8 +49,6 @@
  *      increase. If there are decrement operations in the operations
  *      array we do the same as before.
  *
- * /proc/sysvipc/sem support (c) 1999 Dragos Acostachioaie <dragos@iname.com>
- *
  * SMP-threaded, sysctl's added
  * (c) 1999 Manfred Spraul <manfreds@colorfullife.com>
  * Enforced range limit on SEM_UNDO
@@ -61,9 +59,12 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/init.h>
-#include <linux/proc_fs.h>
 #include <linux/smp_lock.h>
+#include <linux/stat.h>
+#include <linux/err.h>
+
 #include <asm/uaccess.h>
+
 #include "util.h"
 
 
@@ -78,9 +79,6 @@
 
 static int newary (key_t, int, int);
 static void freeary (int id);
-#ifdef CONFIG_PROC_FS
-static int sysvipc_sem_read_proc(char *buffer, char **start, off_t offset, int length, int *eof, void *data);
-#endif
 
 #define SEMMSL_FAST	256 /* 512 bytes on stack */
 #define SEMOPM_FAST	64  /* ~ 372 bytes on stack */
@@ -91,7 +89,7 @@
  *	sem_array.sem_pending{,last},
  *	sem_array.sem_undo: sem_lock() for read/write
  *	sem_undo.proc_next: only "current" is allowed to read/write that field.
- *	
+ *
  */
 
 int sem_ctls[4] = {SEMMSL, SEMMNS, SEMOPM, SEMMNI};
@@ -106,10 +104,6 @@
 {
 	used_sems = 0;
 	ipc_init_ids(&sem_ids,sc_semmni);
-
-#ifdef CONFIG_PROC_FS
-	create_proc_read_entry("sysvipc/sem", 0, 0, sysvipc_sem_read_proc, NULL);
-#endif
 }
 
 static int newary (key_t key, int nsems, int semflg)
@@ -158,7 +152,7 @@
 	if (nsems < 0 || nsems > sc_semmsl)
 		return -EINVAL;
 	down(&sem_ids.sem);
-	
+
 	if (key == IPC_PRIVATE) {
 		err = newary(key, nsems, semflg);
 	} else if ((id = ipc_findkey(&sem_ids, key)) == -1) {  /* key not used */
@@ -321,7 +315,7 @@
 	struct sem_queue * q;
 
 	for (q = sma->sem_pending; q; q = q->next) {
-			
+
 		if (q->status == 1)
 			continue;	/* this one was woken up before */
 
@@ -370,6 +364,7 @@
 	}
 	return semncnt;
 }
+
 static int count_semzcnt (struct sem_array * sma, ushort semnum)
 {
 	int semzcnt;
@@ -441,7 +436,7 @@
 	}
 }
 
-int semctl_nolock(int semid, int semnum, int cmd, int version, union semun arg)
+static int semctl_nolock(int semid, int semnum, int cmd, int version, union semun arg)
 {
 	int err = -EINVAL;
 
@@ -1115,7 +1110,7 @@
 			spin_lock_init(&undo_list->lock);
 		atomic_inc(&undo_list->refcnt);
 		tsk->sysvsem.undo_list = undo_list;
-	} else 
+	} else
 		tsk->sysvsem.undo_list = NULL;
 
 	return 0;
@@ -1226,52 +1221,3 @@
 
 	unlock_kernel();
 }
-
-#ifdef CONFIG_PROC_FS
-static int sysvipc_sem_read_proc(char *buffer, char **start, off_t offset, int length, int *eof, void *data)
-{
-	off_t pos = 0;
-	off_t begin = 0;
-	int i, len = 0;
-
-	len += sprintf(buffer, "       key      semid perms      nsems   uid   gid  cuid  cgid      otime      ctime\n");
-	down(&sem_ids.sem);
-
-	for(i = 0; i <= sem_ids.max_id; i++) {
-		struct sem_array *sma;
-		sma = sem_lock(i);
-		if(sma) {
-			len += sprintf(buffer + len, "%10d %10d  %4o %10lu %5u %5u %5u %5u %10lu %10lu\n",
-				sma->sem_perm.key,
-				sem_buildid(i,sma->sem_perm.seq),
-				sma->sem_perm.mode,
-				sma->sem_nsems,
-				sma->sem_perm.uid,
-				sma->sem_perm.gid,
-				sma->sem_perm.cuid,
-				sma->sem_perm.cgid,
-				sma->sem_otime,
-				sma->sem_ctime);
-			sem_unlock(i);
-
-			pos += len;
-			if(pos < offset) {
-				len = 0;
-	    			begin = pos;
-			}
-			if(pos > offset + length)
-				goto done;
-		}
-	}
-	*eof = 1;
-done:
-	up(&sem_ids.sem);
-	*start = buffer + (offset - begin);
-	len -= (offset - begin);
-	if(len > length)
-		len = length;
-	if(len < 0)
-		len = 0;
-	return len;
-}
-#endif
diff -urN linux-2.5.17/ipc/shm.c linux/ipc/shm.c
--- linux-2.5.17/ipc/shm.c	2002-05-21 07:07:39.000000000 +0200
+++ linux/ipc/shm.c	2002-05-22 22:44:16.000000000 +0200
@@ -22,7 +22,6 @@
 #include <linux/init.h>
 #include <linux/file.h>
 #include <linux/mman.h>
-#include <linux/proc_fs.h>
 #include <linux/shmem_fs.h>
 #include <asm/uaccess.h>
 
@@ -60,9 +59,6 @@
 static int newseg (key_t key, int shmflg, size_t size);
 static void shm_open (struct vm_area_struct *shmd);
 static void shm_close (struct vm_area_struct *shmd);
-#ifdef CONFIG_PROC_FS
-static int sysvipc_shm_read_proc(char *buffer, char **start, off_t offset, int length, int *eof, void *data);
-#endif
 
 size_t	shm_ctlmax = SHMMAX;
 size_t 	shm_ctlall = SHMALL;
@@ -73,9 +69,6 @@
 void __init shm_init (void)
 {
 	ipc_init_ids(&shm_ids, 1);
-#ifdef CONFIG_PROC_FS
-	create_proc_read_entry("sysvipc/shm", 0, 0, sysvipc_shm_read_proc, NULL);
-#endif
 }
 
 static inline int shm_checkid(struct shmid_kernel *s, int id)
@@ -688,65 +681,3 @@
 	up_write(&mm->mmap_sem);
 	return retval;
 }
-
-#ifdef CONFIG_PROC_FS
-static int sysvipc_shm_read_proc(char *buffer, char **start, off_t offset, int length, int *eof, void *data)
-{
-	off_t pos = 0;
-	off_t begin = 0;
-	int i, len = 0;
-
-	down(&shm_ids.sem);
-	len += sprintf(buffer, "       key      shmid perms       size  cpid  lpid nattch   uid   gid  cuid  cgid      atime      dtime      ctime\n");
-
-	for(i = 0; i <= shm_ids.max_id; i++) {
-		struct shmid_kernel* shp;
-
-		shp = shm_lock(i);
-		if(shp!=NULL) {
-#define SMALL_STRING "%10d %10d  %4o %10u %5u %5u  %5d %5u %5u %5u %5u %10lu %10lu %10lu\n"
-#define BIG_STRING   "%10d %10d  %4o %21u %5u %5u  %5d %5u %5u %5u %5u %10lu %10lu %10lu\n"
-			char *format;
-
-			if (sizeof(size_t) <= sizeof(int))
-				format = SMALL_STRING;
-			else
-				format = BIG_STRING;
-			len += sprintf(buffer + len, format,
-				shp->shm_perm.key,
-				shm_buildid(i, shp->shm_perm.seq),
-				shp->shm_flags,
-				shp->shm_segsz,
-				shp->shm_cprid,
-				shp->shm_lprid,
-				shp->shm_nattch,
-				shp->shm_perm.uid,
-				shp->shm_perm.gid,
-				shp->shm_perm.cuid,
-				shp->shm_perm.cgid,
-				shp->shm_atim,
-				shp->shm_dtim,
-				shp->shm_ctim);
-			shm_unlock(i);
-
-			pos += len;
-			if(pos < offset) {
-				len = 0;
-				begin = pos;
-			}
-			if(pos > offset + length)
-				goto done;
-		}
-	}
-	*eof = 1;
-done:
-	up(&shm_ids.sem);
-	*start = buffer + (offset - begin);
-	len -= (offset - begin);
-	if(len > length)
-		len = length;
-	if(len < 0)
-		len = 0;
-	return len;
-}
-#endif
diff -urN linux-2.5.17/ipc/util.h linux/ipc/util.h
--- linux-2.5.17/ipc/util.h	2002-05-21 07:07:40.000000000 +0200
+++ linux/ipc/util.h	2002-05-22 22:55:08.000000000 +0200
@@ -42,15 +42,15 @@
 /* for rare, potentially huge allocations.
  * both function can sleep
  */
-void* ipc_alloc(int size);
-void ipc_free(void* ptr, int size);
+extern void* ipc_alloc(int size);
+extern void ipc_free(void* ptr, int size);
 
-extern inline void ipc_lockall(struct ipc_ids* ids)
+static inline void ipc_lockall(struct ipc_ids* ids)
 {
 	spin_lock(&ids->ary);
 }
 
-extern inline struct kern_ipc_perm* ipc_get(struct ipc_ids* ids, int id)
+static inline struct kern_ipc_perm* ipc_get(struct ipc_ids* ids, int id)
 {
 	struct kern_ipc_perm* out;
 	int lid = id % SEQ_MULTIPLIER;
@@ -61,11 +61,12 @@
 	return out;
 }
 
-extern inline void ipc_unlockall(struct ipc_ids* ids)
+static inline void ipc_unlockall(struct ipc_ids* ids)
 {
 	spin_unlock(&ids->ary);
 }
-extern inline struct kern_ipc_perm* ipc_lock(struct ipc_ids* ids, int id)
+
+static inline struct kern_ipc_perm* ipc_lock(struct ipc_ids* ids, int id)
 {
 	struct kern_ipc_perm* out;
 	int lid = id % SEQ_MULTIPLIER;
@@ -79,28 +80,28 @@
 	return out;
 }
 
-extern inline void ipc_unlock(struct ipc_ids* ids, int id)
+static inline void ipc_unlock(struct ipc_ids* ids, int id)
 {
 	spin_unlock(&ids->ary);
 }
 
-extern inline int ipc_buildid(struct ipc_ids* ids, int id, int seq)
+static inline int ipc_buildid(struct ipc_ids* ids, int id, int seq)
 {
 	return SEQ_MULTIPLIER*seq + id;
 }
 
-extern inline int ipc_checkid(struct ipc_ids* ids, struct kern_ipc_perm* ipcp, int uid)
+static inline int ipc_checkid(struct ipc_ids* ids, struct kern_ipc_perm* ipcp, int uid)
 {
 	if(uid/SEQ_MULTIPLIER != ipcp->seq)
 		return 1;
 	return 0;
 }
 
-void kernel_to_ipc64_perm(struct kern_ipc_perm *in, struct ipc64_perm *out);
-void ipc64_perm_to_ipc_perm(struct ipc64_perm *in, struct ipc_perm *out);
+extern void kernel_to_ipc64_perm(struct kern_ipc_perm *in, struct ipc64_perm *out);
+extern void ipc64_perm_to_ipc_perm(struct ipc64_perm *in, struct ipc_perm *out);
 
 #ifdef __ia64__
-  /* On IA-64, we always use the "64-bit version" of the IPC structures.  */ 
+  /* On IA-64, we always use the "64-bit version" of the IPC structures.  */
 # define ipc_parse_version(cmd)	IPC_64
 #else
 int ipc_parse_version (int *cmd);

Re: Linux-2.5.17

[Martin Dalecki]

[-- Attachment #1: Type: text/plain, Size: 1378 bytes --]

Thu May 23 14:37:50 CEST 2002 ide-clean-70

- Apply host chip driver cleanups by Bartomiej Zonierkiewicz.

- Take the draft device type driver implementation from Adam Richter and make
   it actually work with some of the drivers we have at hand. Quite a lot
   of it was fixed by me as well to have the desired effects.

   We have added a attach method for the sub device type drivers to make it
   possible dor sub device type drivers to attach devices to the overall
   infrastructure. UNIX has something like this SCSI code is implementing
   something like this, just for some unknown reasons Linux block device
   operations don't have it...

- ide_drive_t is finally gone. Please use struct ata_device instead.

   Hint the ide.h specific byte type should go over time as well, sine there
   is no need to invent something already handled by the kernel. Please use
   the unambigious u8 type instead where possible.

- Add a bit of documentation about cabling issues. ide.txt needs a lot of
   improvement at some time still.

Well the fact that this is collecting many bits from different people
again makes this patch unfortunately rather big.  I compress it
therefore, since sending the patches to lkml turned out the be sucsessfull
in attracting more people to the overall effort.

Once again I would like to express my many thanks to all of them who are
involved!.

[-- Attachment #2: ide-clean-70.diff.gz --]
[-- Type: application/x-gzip, Size: 22327 bytes --]

[PATCH] 2.5.17 IDE 70

[Martin Dalecki]

[-- Attachment #1: Type: text/plain, Size: 1477 bytes --]

Sh*t I did forget to adjust the topic line. So here it is again just
for reassurance.

Thu May 23 14:37:50 CEST 2002 ide-clean-70

- Apply host chip driver cleanups by Bartomiej Zonierkiewicz.

- Take the draft device type driver implementation from Adam Richter and make
    it actually work with some of the drivers we have at hand. Quite a lot
    of it was fixed by me as well to have the desired effects.

    We have added a attach method for the sub device type drivers to make it
    possible dor sub device type drivers to attach devices to the overall
    infrastructure. UNIX has something like this SCSI code is implementing
    something like this, just for some unknown reasons Linux block device
    operations don't have it...

- ide_drive_t is finally gone. Please use struct ata_device instead.

    Hint the ide.h specific byte type should go over time as well, sine there
    is no need to invent something already handled by the kernel. Please use
    the unambigious u8 type instead where possible.

- Add a bit of documentation about cabling issues. ide.txt needs a lot of
    improvement at some time still.

Well the fact that this is collecting many bits from different people
again makes this patch unfortunately rather big.  I compress it
therefore, since sending the patches to lkml turned out the be sucsessfull
in attracting more people to the overall effort.

Once again I would like to express my many thanks to all of them who are
involved!.


[-- Attachment #2: ide-clean-70.diff.gz --]
[-- Type: application/x-gzip, Size: 22327 bytes --]

Re: Linux-2.5.17

[Wayne.Brown]

Under 2.5.17 there is a problem with gtop 1.0.9.  It opens a window but never
fills in any details; there's just a blank background.  The process becomes
unkillable, even with -9, and although I can do a normal shutdown, the root
partition can't be unmounted because the gtop process is still running and so
fsck is forced on reboot.  There are no oops messages that I can find in any of
the logs.

Actually, this happens with all the most recent 2.5.x kernels.  I'm not sure how
far back it goes, but I believe it was working OK prior to 2.5.8.  (If necessary
I can try to narrow it down further.)  It still works great with 2.4.19-pre8 and
2.4.19-pre8-ac5.

Re: Linux-2.5.17

[David S. Miller]

   From: Wayne.Brown@altec.com
   Date: Tue, 21 May 2002 13:52:08 -0500

   Under 2.5.17 there is a problem with gtop 1.0.9.

The /proc/meminfo output changed, and this makes a lot of programs
reading that file explode.

Re: Linux-2.5.17

[Helge Hafting]

"David S. Miller" wrote:
[...]
>    Under 2.5.17 there is a problem with gtop 1.0.9.
> 
> The /proc/meminfo output changed, and this makes a lot of programs
> reading that file explode.

No problem with file format breakage, but the programs 
"exploding" shouldn't get stuck unkillable by kill -9.
looks like a kernel problem to me.

Helge Hafting

Re: Linux-2.5.17

[Wayne.Brown]

Thanks for the information.

So, I'm just getting used to the idea of using new tools to build kernels, and
now I learn that 2.5 breaks an ordinary program that I use all day, every day.
It just keeps getting better and better...





"David S. Miller" <davem@redhat.com> on 05/21/2002 04:30:11 PM

To:   Wayne Brown/Corporate/Altec@Altec
cc:   linux-kernel@vger.kernel.org

Subject:  Re: Linux-2.5.17



   From: Wayne.Brown@altec.com
   Date: Tue, 21 May 2002 13:52:08 -0500

   Under 2.5.17 there is a problem with gtop 1.0.9.

The /proc/meminfo output changed, and this makes a lot of programs
reading that file explode.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Linux-2.5.17

[Russell King]

On Tue, May 21, 2002 at 06:20:56PM -0500, Wayne.Brown@altec.com wrote:
> So, I'm just getting used to the idea of using new tools to build kernels,
> and now I learn that 2.5 breaks an ordinary program that I use all day,
> every day. It just keeps getting better and better...

The 2.<odd> series, like 2.5 is a strictly development kernel series; new
features go into these all the time.  You can expect it to:

1. not build.
2. crash.
3. silently eat your filesystems.
4. break userspace programs.

or any combination of the above.  If you're looking for stability, stick
with the 2.<even> series.
-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: Linux-2.5.17

[Joel Jaeggli]

On Tue, 21 May 2002 Wayne.Brown@altec.com wrote:

> 
> 
> Thanks for the information.
> 
> So, I'm just getting used to the idea of using new tools to build kernels, and
> now I learn that 2.5 breaks an ordinary program that I use all day, every day.
> It just keeps getting better and better...
> 

I don't recall how many time stuff that touched proc broke in the 
.99-1.3-2.0 era but it was a few...

the proc filesystem is not an snmp mib.
 
> 
> 
> 
> "David S. Miller" <davem@redhat.com> on 05/21/2002 04:30:11 PM
> 
> To:   Wayne Brown/Corporate/Altec@Altec
> cc:   linux-kernel@vger.kernel.org
> 
> Subject:  Re: Linux-2.5.17
> 
> 
> 
>    From: Wayne.Brown@altec.com
>    Date: Tue, 21 May 2002 13:52:08 -0500
> 
>    Under 2.5.17 there is a problem with gtop 1.0.9.
> 
> The /proc/meminfo output changed, and this makes a lot of programs
> reading that file explode.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
-------------------------------------------------------------------------- 
Joel Jaeggli	      Academic User Services   joelja@darkwing.uoregon.edu    
--    PGP Key Fingerprint: 1DE9 8FCA 51FB 4195 B42A 9C32 A30D 121E      --
  In Dr. Johnson's famous dictionary patriotism is defined as the last
  resort of the scoundrel.  With all due respect to an enlightened but
  inferior lexicographer I beg to submit that it is the first.
	   	            -- Ambrose Bierce, "The Devil's Dictionary"

Re: Linux-2.5.17

[Wayne.Brown]

I can live with not building, crashing, or even eating filesystems.  Those
things will be fixed sooner or later.  But breaking userspace programs -- that
may well be permanent.  If there was a good chance it would be working again by
the time 2.6 comes out, it wouldn't bother me.  But I really don't expect this
to change, so it looks like I won't be able to use 2.5 (or anything later) until
I get another version of gtop, or fix this one myself.  And so there will be yet
another nonstandard change to my patchwork system that I'll have to deal with
(if I even remember it) the next time I try to upgrade (i.e., reinstall)
Slackware.





Russell King <rmk@arm.linux.org.uk> on 05/21/2002 06:29:23 PM

To:   Wayne Brown/Corporate/Altec@Altec
cc:   linux-kernel@vger.kernel.org

Subject:  Re: Linux-2.5.17



On Tue, May 21, 2002 at 06:20:56PM -0500, Wayne.Brown@altec.com wrote:
> So, I'm just getting used to the idea of using new tools to build kernels,
> and now I learn that 2.5 breaks an ordinary program that I use all day,
> every day. It just keeps getting better and better...

The 2.<odd> series, like 2.5 is a strictly development kernel series; new
features go into these all the time.  You can expect it to:

1. not build.
2. crash.
3. silently eat your filesystems.
4. break userspace programs.

or any combination of the above.  If you're looking for stability, stick
with the 2.<even> series.
--
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Linux-2.5.17

[Zwane Mwaikambo]

On Tue, 21 May 2002 Wayne.Brown@altec.com wrote:

> I get another version of gtop, or fix this one myself.  And so there will be yet
> another nonstandard change to my patchwork system that I'll have to deal with
> (if I even remember it) the next time I try to upgrade (i.e., reinstall)
> Slackware.

Change isn't such a bad thing, and if you want consistency, 2.5 is hardly 
what you should be using.

Cheers,
	Zwane Mwaikambo

-- 
http://function.linuxpower.ca
		

Re: Linux-2.5.17

[Nick Holloway]

In <86256BC1.001146A6.00@smtpnotes.altec.com> Wayne.Brown@altec.com writes:
> I can live with not building, crashing, or even eating filesystems.  Those
> things will be fixed sooner or later.  But breaking userspace programs -- that
> may well be permanent.

Looking at the source code to libgtop-1.0.6 (the version I have
easy access to), the parser used to extract the swap information from
/proc/meminfo is extremely fragile (read: broken).  Rather than looking
at the tag at the start of each line for the one it requires, it assumes
that the "Swap:" details are on the 3rd line (and doesn't even verify
the label).

You can't expect the kernel to keep compatability for such poor user-space
code (especially during a development cycle).

The change to /proc/meminfo came about in 2.5.1, and this removed
the first two lines from the old, inflexible layout (that has been
deprecated for a while, and should probably been removed during the
2.1.x development cycle).

-- 
 `O O'  | Nick.Holloway@pyrites.org.uk
// ^ \\ | http://www.pyrites.org.uk/

Re: Linux-2.5.17

[Wayne.Brown]

Thanks for pointing me in the right direction; I found the same code in my copy
of libgtop (1.0.9) and see the problem.  Maybe now I can hack something together
to make it work.

In comparing /proc/meminfo in 2.4.19-pre8 and 2.5.17 I see that there is very
little difference except that the information gtop relies upon is missing.  The
lines it needs aren't changed or rearranged, just gone altogether.  Was there
any particular purpose for that, other than breaking programs like gtop?  I'm a
firm believer that adding something new to a system should never break existing
functionality unless absolutely necessary.  Was it necessary in this case, or
was it done because someone was offended that it wasn't "clean" enough?





Nick.Holloway@pyrites.org.uk (Nick Holloway) on 05/22/2002 06:49:59 AM

To:   linux-kernel@vger.kernel.org
cc:    (bcc: Wayne Brown/Corporate/Altec)

Subject:  Re: Linux-2.5.17



In <86256BC1.001146A6.00@smtpnotes.altec.com> Wayne.Brown@altec.com writes:
> I can live with not building, crashing, or even eating filesystems.  Those
> things will be fixed sooner or later.  But breaking userspace programs -- that
> may well be permanent.

Looking at the source code to libgtop-1.0.6 (the version I have
easy access to), the parser used to extract the swap information from
/proc/meminfo is extremely fragile (read: broken).  Rather than looking
at the tag at the start of each line for the one it requires, it assumes
that the "Swap:" details are on the 3rd line (and doesn't even verify
the label).

You can't expect the kernel to keep compatability for such poor user-space
code (especially during a development cycle).

The change to /proc/meminfo came about in 2.5.1, and this removed
the first two lines from the old, inflexible layout (that has been
deprecated for a while, and should probably been removed during the
2.1.x development cycle).

--
 `O O'  | Nick.Holloway@pyrites.org.uk
// ^ \\ | http://www.pyrites.org.uk/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Linux-2.5.17

[Nick Holloway]

In <86256BC1.0076F247.00@smtpnotes.altec.com> Wayne.Brown@altec.com writes:
> In comparing /proc/meminfo in 2.4.19-pre8 and 2.5.17 I see that there is very
> little difference except that the information gtop relies upon is missing.  The
> lines it needs aren't changed or rearranged, just gone altogether.  Was there
> any particular purpose for that, other than breaking programs like gtop?

I confess to submitting the patch to Linus to remove the compatability
lines when 2.5.0 was created.

The change that made the first two lines redundant was originally made in
1.3.68[1], and the intention was that they would removed -- the comment
in get_meminfo had:

    Tagged format, for easy grepping and expansion. The above will go away
    eventually, once the tools have been updated.

As I had an application that parsed /proc/meminfo around the time of
the change, I wrote the parser to handle the new format in preparation
for the old format going away.  Others would have done the same.

Someone mentioned that this compatability was still present during
2.4 development, so I made a mental note to submit a patch when 2.5
development started.

> I'm a firm believer that adding something new to a system should
> never break existing functionality unless absolutely necessary.  Was it
> necessary in this case, or was it done because someone was offended that
> it wasn't "clean" enough?

Where possible, interfaces should not be changed for the sake of change,
but you shouldn't keep old interfaces because that way lies bloat.

One of the beauties of Linux (IMO) is that broken interfaces don't have
to be maintained ad-infinitum.  It should be expected that interfaces
may be broken during the development series of kernels, and the tools
have a chance to catch up during this cycle.

The interface that gtop (implicitly) required is that "Swap:" should be
the 3rd line in /proc/meminfo.  You'll never get anyone to admit that
this is the interface to /proc/meminfo that should be maintained.

-
[1] http://groups.google.com/groups?selm=4glagm%24pno%40treflan.shout.net

-- 
 `O O'  | Nick.Holloway@pyrites.org.uk
// ^ \\ | http://www.pyrites.org.uk/