2000-10-30 19:33:32

by Linus Torvalds

[permalink] [raw]
Subject: test10-pre7


Ok, this one contains at least a preliminary fix for the problem with
truncate together with a concurrent page access - the bug that causes
oopses in block_read_full_page() and filemap_nopage().

This is a fairly minimal fix, and I'll still have to verify that I caught
all the relevant places, but I wanted people who have seen this problem to
please test this out asap - I'll make a real test10 later once I've
integrated some further patches from Alan and Jeff, but this should fix
the major show-stopper bug.

Thanks,

Linus

----
- pre7:
- Niels Jensen: remove no-longer-needed workarounds for old gcc versions
- Ingo Molnar & Rik v Riel: VM inactive list maintenance correction
- Randy Dunlap, USB: printer.c, usb-storage, usb identification and
memory leak fixes
- David Miller: networking updates
- David Mosberger: add AT_CLKTCK to elf information. And make AT_PAGESZ work
for static binaries too.
- oops. pcmcia broke by mistake
- Me: truncate vs page access race fix.

- pre6:
- Jeremy Fitzhardinge: autofs4 expiry fix
- David Miller: sparc driver updates, networking updates
- Mathieu Chouquet-Stringer: buffer overflow in sg_proc_dressz_write
- Ingo Molnar: wakeup race fix (admittedly the window was basically
non-existent, but still..)
- Rasmus Andersen: notice that "this_slice" is no longer used for
scheduling - delete the code that calculates it.
- ALI pirq routing update. It's even uglier than we initially thought..
- Dimitrios Michailidis: fix ipip locking bugs
- Various: face it - gcc-2.7.2.3 miscompiles structure initializers.
- Paul Cassella: locking comments on dev_base
- Trond Myklebust: NFS locking atomicity. refresh inode properly.
- Andre Hedrick: Serverworks Chipset driver, IDE-tape fix
- Paul Gortmaker: kill unused code from 8390 support.
- Andrea Arcangeli: fix nfsv3d wrong truncates over 4G
- Maciej W. Rozycki: PIIX4 needs the same USB quirk handling as PIIX3.
- me: if we cannot figure out the PCI bridge windows, just "inherit"
the window from the parent. Better than not booting.
- Ching-Ling Lee: ALI 5451 Audio core support update

- pre5:
- Mikael Pettersson: more Pentium IV cleanup.
- David Miller: non-x86 platforms missed "pte_same()".
- Russell King: NFS invalidate_inode_pages() can do bad things!
- Randy Dunlap: usb-core.c is gone - module fix
- Ben LaHaise: swapcache fixups for the new atomic pte update code
- Oleg Drokin: fix nm256_audio memory region confusion
- Randy Dunlap: USB printer fixes
- David Miller: sparc updates
- David Miller: off-by-one error in /proc socket dumper
- David Miller: restore non-local bind() behaviour.
- David Miller: wakeups on socket shutdown()
- Jeff Garzik: DEPCA net drvr fixes and CodingStyle
- Jeff Garzik: netsemi net drvr fix
- Jeff Garzik & Andrea Arkangeli: keyboard cleanup
- Jeff Garzik: VIA audio update
- Andrea Arkangeli: mxcsr initialization cleanup and fix
- Gabriel Paubert: better twd_i387_to_fxsr() emulation
- Andries Brouwer: proper error return in ext2 mkdir()

- pre4:
- disable writing to /proc/xxx/mem. Sure, it works now, but it's still
a security risk.
- IDE driver update (Victroy66 SouthBridge support)
- i810 rng driver cleanup
- fix sbus Makefile
- named initializers in module..
- ppoe: remove explicit initializer - it's done with initcalls.
- x86 WP bit detection: do it cleanly with exception handling
- Arnaldo Carvalho de Melo: memory leaks in drivers/media/video
- Bartlomiej Zolnierkiewicz: video init functions get __init
- David Miller: get rid of net/protocols.c - they get to initialize themselves
- David Miller: get rid of dev_mc_lock - we hold dev->xmit_lock anyway.
- Geert Uytterhoeven: Zorro (Amiga) bus support update
- David Miller: work around gcc-2.7.2 bug
- Geert Uytterhoeven: mark struct consw's "const".
- Jeff Garzik: network driver cleanups, ns558 joystick driver oops fix
- Tigran Aivazian: clean up __alloc_pages(), kill_super() and
notify_change()
- Tigran Aivazian: move stuff from .data to .bss
- Jeff Garzik: divert.h typename cleanups
- James Simmons: mdacon using spinlocks
- Tigran Aivazian: fix BFS free block calculation
- David Miller: sparc32 works again
- Bernd Schmidt: fix undefined C code (set/use without a sequence point)
- Mikael Pettersson: nicer Pentium IV setup handling.
- Georg Acher: usb-uhci cpia oops fix
- Kanoj Sarcar: more node_data cleanups for [non]NUMA.
- Richard Henderson: alpha update to new vmalloc setup
- Ben LaHaise: atomic pte updates (don't lose dirty bit)
- David Brownell: ohci memory debugging (== use separate slabs for allocation)

- pre3:
- update email address of Joerg Reuter
- Andries Brouwer: spelling fixes, missing atari brelse(), breada() fix
- Geert Uytterhoeven: used named initializers for "struct console".
- Carsten Paeth: ISDN capifs - iput() only once.
- Petr Vandrovec: VFAT short name generation fix
- Jeff Garzik: i810_rng cleanup, and i815 chipset added.
- Bartlomiej Zolnierkiewicz: clean up some remaining old-style Makefiles
- Dave Jones: x86 setup fixes (recognize Pentium IV etc).
- x86: do the "fast A20" setup too in setup.S
- NIIBE Yutaka: update SuperH for the global page table (vmalloc) change.
- David Miller: sparc updates (vmalloc stuff still pending)
- David Miller: CodaFS warnings and 64-bit warnings in pci_size()
- David Miller: pcnet32 - correct NULL test
- David Miller: vmlist lock -> page_table_lock clarification
- Trond Myklebust: Ouch. rpcauth_lookup_credcache() memory corruption bug
- Matthew Wilcox: file locking cleanups
- David Woodhouse: USB audio spinlock fixes
- Torben Mathiasen: tlan driver cleanups
- Randy Dunlap: Yenta: CACHE_LINE_SIZE is in dwords, not bytes.
- Randy Dunlap: more USB updates
- Kanoj Sarcar: clean up the NUMA interfaces (pg_data instead of nodes)
- "save_fpu()" was broken. Need to clear pending errors: save_init_fpu().

- pre2:
- remember to change the kernel version ;)
- isapnp.txt bugfix
- ia64 update
- sparc update
- networking update (pppoe init, frame diverter, fix tcp_sendmsg,
fix udp_recvmsg).
- Compile for WinChip must _not_ use "-march=i686". It's a i586.
- Randy Dunlap: more USB updates
- clarify the Firewire AIC-5800 situation. It's not supported yet.
- PCI-space decode size fix. This is needed for some (broken?) hardware
- /proc/self/maps off-by-one error
- 3c501, 3c507, cs89x0 network drivers drop unnecessary check_region
- Asahi Kasei AK4540: new codec ID. Yamaha: new PCI ID's.
- ne2k-pci net driver documentation update
- Paul Gortmaker: delete paranoia check in rtc_exit
- scsi_merge: memset the right amount of memory.
- sun3fb: old __initfunc() not supported any more.
- synclink: remove unnecessary task state games
- xd.c: proper casting for 64-bit architectures
- vmalloc: page table update race condition.

- pre1:
- Roger Larsson: ">=" instead of ">" to make the VM not get stuck.
- Gideon Glass: brw_kiovec() failure case oops fix
- Rik van Riel: better memory balancing and OOM killer
- Ivan Kokshaysky: alpha compile fixes
- Vojtech Pavlik: forgotten ENOUGH macro in via82cxxx ide driver
- Arnaldo Carvalho de Melo: acpi resource leak fix
- Brian Gerst: use mov's instead of xchg in kernel trap entry
- Torben Mathiasen: tlan timer being added twice bug
- Andrzej Krzysztofowicz: config file fixes
- Jean Tourrilhes: Wavelan lockup on SMP fix
- Roman Zippel: initdata must be initialized (even if it is to zero:
gcc is strange)
- Jean Tourrilhes: hp100 driver lockup at startup on SMP
- Russell King: fix silly minixfs uninitialized error bug
- (various): fix uid hashing to use "uid_t" instead of "unsigned short"
- Jaroslav Kysela: isapnp timeout fix. NULL ptr dereference fix.
- Alain Knaff: fdformat should work again.
- Randy Dunlap: USB - fix bluetooth, acm, printer, serial to work
with urb->dev changes.
- Randy Dunlap: USB whiteheat serial driver firmware update.
- Randy Dunlap: USB hub memory corruption and pegasus driver update
- Andre Hedrick: IDE Makefile cleanup


2000-10-30 20:35:04

by Alexander Viro

[permalink] [raw]
Subject: [PATCH] Re: test10-pre7



On Mon, 30 Oct 2000, Linus Torvalds wrote:

>
> Ok, this one contains at least a preliminary fix for the problem with
> truncate together with a concurrent page access - the bug that causes
> oopses in block_read_full_page() and filemap_nopage().
>
> This is a fairly minimal fix, and I'll still have to verify that I caught
> all the relevant places, but I wanted people who have seen this problem to
> please test this out asap - I'll make a real test10 later once I've
> integrated some further patches from Alan and Jeff, but this should fix
> the major show-stopper bug.

Unfortunately, it doesn't fix the thing. ->sync_page() is called when we
do not own the page lock and nfs_sync_page() uses page->mapping. Yes, we
check it before calling the bloody thing, but we don't own the lock.
Problem only for NFS, but I'm not sure what to do about it - the whole
point of ->sync_page() seems to be (if I understood Trond's intentions
right) in forcing the ->readpage() in progress.

Another place you've missed is in read_cache_page(). That one is easy - we've
just locked the page and we should just repeat the whole thing if it's out
of cache.

One more is in filemap_swapout() - dunno, I just shifted the check to
filemap_write_page().

One more: check in do_generic_file_read() for ->mapping->i_shared_mmap.
Fix: trivial.

The last one is in deactivate_page_nolock() - there we check the ->mapping
without pagecache_lock and without page lock. Hell knows whether it's a
bug or not. Rik?

Minimal patch (against -pre7) follows. It still leaves sync_page() problem
open - any suggestions on that one are very welcome. Other than that and
deactivate_page_nolock() we should be safe wrt. ->mapping. Please, apply -
after that we will be in sync. nfs_sync_page() is still a problem and if
somebody (Trond?) might tell WTF it is supposed to be...
Cheers,
Al

--- filemap.c Mon Oct 30 18:46:17 2000
+++ filemap.c.new Mon Oct 30 18:54:05 2000
@@ -981,7 +981,7 @@
* virtual addresses, take care about potential aliasing
* before reading the page on the kernel side.
*/
- if (page->mapping->i_mmap_shared != NULL)
+ if (mapping->i_mmap_shared != NULL)
flush_dcache_page(page);

/*
@@ -1473,7 +1473,8 @@
* vma/file is guaranteed to exist in the unmap/sync cases because
* mmap_sem is held.
*/
- return page->mapping->a_ops->writepage(file, page);
+ /* Nothing to do if somebody truncated the page from under us.. */
+ return page->mapping?page->mapping->a_ops->writepage(file, page):0;
}


@@ -1544,9 +1545,7 @@
lock_page(page);

error = 0;
- /* Nothing to do if somebody truncated the page from under us.. */
- if (page->mapping)
- error = filemap_write_page(vma->vm_file, page, 1);
+ error = filemap_write_page(vma->vm_file, page, 1);

UnlockPage(page);
page_cache_free(page);
@@ -2313,13 +2312,20 @@
int (*filler)(void *,struct page*),
void *data)
{
- struct page *page = __read_cache_page(mapping, index, filler, data);
+ struct page *page;
+retry:
+ page = __read_cache_page(mapping, index, filler, data);
int err;

if (IS_ERR(page) || Page_Uptodate(page))
goto out;

lock_page(page);
+ if (!page->mapping) {
+ UnlockPage(page);
+ page_cache_release(page);
+ goto retry;
+ }
if (Page_Uptodate(page)) {
UnlockPage(page);
goto out;

2000-10-30 21:03:12

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] Re: test10-pre7



On Mon, 30 Oct 2000, Alexander Viro wrote:
>
> Unfortunately, it doesn't fix the thing. ->sync_page() is called when we
> do not own the page lock and nfs_sync_page() uses page->mapping. Yes, we
> check it before calling the bloody thing, but we don't own the lock.

Good catch.

> Problem only for NFS, but I'm not sure what to do about it - the whole
> point of ->sync_page() seems to be (if I understood Trond's intentions
> right) in forcing the ->readpage() in progress.

How about just changing ->sync_page() semantics to own the page lock? That
sound slike the right thing anyway, no?

> Another place you've missed is in read_cache_page(). That one is easy - we've
> just locked the page and we should just repeat the whole thing if it's out
> of cache.

I didn't actually miss it, I just looked at the users and decided that it
looks like they should never have this issue. But I might have missed
something. As far as I can tell, "read_cache_page()" is only used for
meta-data like things that cannot be truncated.

But you're right, we should do it for consistency.

> One more is in filemap_swapout() - dunno, I just shifted the check to
> filemap_write_page().

I'd really like to do these in the thing that locks the page, and make the
rule be that the page locker needs to do the work. That's why I'd prefer
to let the test be in the _caller_ of filemap_write_page(), as that's the
point where we got the lock.

Linus

2000-10-30 21:23:48

by Alexander Viro

[permalink] [raw]
Subject: Re: [PATCH] Re: test10-pre7



On Mon, 30 Oct 2000, Linus Torvalds wrote:

> How about just changing ->sync_page() semantics to own the page lock? That
> sound slike the right thing anyway, no?

It would kill the ->sync_page(), but yes, _that_ might be the right thing ;-)

> I didn't actually miss it, I just looked at the users and decided that it
> looks like they should never have this issue. But I might have missed
> something. As far as I can tell, "read_cache_page()" is only used for
> meta-data like things that cannot be truncated.

invalidate_inode_pages().

> I'd really like to do these in the thing that locks the page, and make the
> rule be that the page locker needs to do the work. That's why I'd prefer
> to let the test be in the _caller_ of filemap_write_page(), as that's the
> point where we got the lock.

Fine with me, but then we would have to do it in try_to_swap_out() and
that would be Wrong Thing(tm) (e.g. because ->swapout() makes sense for
anonymous pages).

We could do it in filemap_swapout(), but the lock is taken by its caller,
so...
Cheers,
Al

2000-10-30 21:38:01

by Keith Owens

[permalink] [raw]
Subject: Re: test10-pre7

On Mon, 30 Oct 2000 11:32:33 -0800 (PST),
Linus Torvalds <[email protected]> wrote:
> - pre7:
> - Randy Dunlap, USB: printer.c, usb-storage, usb identification and
> memory leak fixes

USB still gets unresolved symbols when part is in kernel, part is in
modules and modversions are set. Patch against 2.4.0-test10-pre7, only
affects drivers/usb/Makefile.

Index: 0-test10-pre7.1/drivers/usb/Makefile
--- 0-test10-pre7.1/drivers/usb/Makefile Tue, 24 Oct 2000 14:20:12 +1100 kaos (linux-2.4/n/b/19_Makefile 1.1.1.11 644)
+++ 0-test10-pre7.1(w)/drivers/usb/Makefile Tue, 31 Oct 2000 08:33:46 +1100 kaos (linux-2.4/n/b/19_Makefile 1.1.1.11 644)
@@ -18,6 +18,18 @@ O_OBJS :=

export-objs := usb.o

+# usb.o contains usb_init which is marked as __initcall (actually
+# module_init). usb_init must be executed before all other usb __initcall
+# routines, otherwise the individual drivers will be initialized before the
+# hub driver is, causing the hub driver initialization sequence to
+# needlessly probe every USB driver with the root hub device. This causes
+# a lot of unnecessary system log messages, a lot of user confusion, and
+# has been known to cause a incorrectly programmed USB device driver to
+# grab the root hub device improperly.
+# Greg Kroah-Hartman, 27 Oct 2000
+
+LINK_FIRST := usb.o
+
# Multipart objects.

list-multi := usbcore.o
@@ -98,6 +110,10 @@ int-m := $(sort $(foreach m, $(multi-m)

obj-m := $(filter-out $(obj-y), $(obj-m))
int-m := $(filter-out $(int-y), $(int-m))
+
+# Take multi-part drivers out of obj-y and put components in.
+
+obj-y := $(filter-out $(list-multi), $(obj-y)) $(int-y)

# Translate to Rules.make lists.

Index: 0-test10-pre7.1/Rules.make
--- 0-test10-pre7.1/Rules.make Tue, 19 Sep 2000 10:36:07 +1100 kaos (linux-2.4/B/c/24_Rules.make 1.2.1.4 644)
+++ 0-test10-pre7.1(w)/Rules.make Tue, 31 Oct 2000 08:33:46 +1100 kaos (linux-2.4/B/c/24_Rules.make 1.2.1.4 644)
@@ -31,6 +31,9 @@ unexport LX_OBJS
unexport MX_OBJS
unexport MIX_OBJS
unexport SYMTAB_OBJS
+# Control link order, added 29 Oct 2000 Keith Owens <[email protected]>
+unexport LINK_FIRST
+unexport LINK_LAST

#
# Get things started.
@@ -84,8 +87,19 @@ all_targets: $(O_TARGET) $(L_TARGET)
#
# Rule to compile a set of .o files into one .o file
#
+# Note: if LINK_FIRST or LINK_LAST are specified, the rest of the
+# object files are sorted to remove duplicates. Thus, if you use
+# LINK_FIRST/LAST, make sure they specify all ordering requirements.
+#
ifdef O_TARGET
-ALL_O = $(OX_OBJS) $(O_OBJS)
+ ALL_O = $(OX_OBJS) $(O_OBJS)
+ ifneq ($(strip $(LINK_FIRST)$(LINK_LAST)),)
+ ALL_O := $(sort $(ALL_O))
+ ALL_O := \
+ $(filter $(ALL_O), $(LINK_FIRST)) \
+ $(filter-out $(LINK_FIRST) $(LINK_LAST), $(ALL_O)) \
+ $(filter $(ALL_O), $(LINK_LAST))
+ endif
$(O_TARGET): $(ALL_O)
rm -f $@
ifneq "$(strip $(ALL_O))" ""
Index: 0-test10-pre7.1/Documentation/kbuild/makefiles.txt
--- 0-test10-pre7.1/Documentation/kbuild/makefiles.txt Tue, 31 Oct 2000 08:28:16 +1100 kaos (linux-2.4/b/d/12_makefiles. 1.4 644)
+++ 0-test10-pre7.1(w)/Documentation/kbuild/makefiles.txt Tue, 31 Oct 2000 08:33:46 +1100 kaos (linux-2.4/b/d/12_makefiles. 1.4 644)
@@ -1,6 +1,9 @@
Linux Kernel Makefiles
2000-September-14
Michael Elizabeth Chastain, <[email protected]>
+2000-October-29
+LINK_FIRST/LAST Keith Owens <[email protected]>,
+ Peter Samuelson <[email protected]>



@@ -319,7 +322,7 @@ architecture-specific values.
# arch/alpha/Makefile

SUBDIRS := $(SUBDIRS) arch/alpha/kernel arch/alpha/mm \
- arch/alpha/lib arch/alpha/math-emu
+ arch/alpha/lib arch/alpha/math-emu

This list may depend on the configuration:

@@ -645,12 +648,17 @@ The public interface of Rules.make consi
with the name $(O_TARGET). This $(O_TARGET) name also appears
in the top Makefile.

- The order of files in $(O_OBJS) and $(OX_OBJS) is significant.
- All $(OX_OBJS) files come first, in the order listed, followed by
- all $(O_OBJS) files, in the order listed. Duplicates in the lists
- are allowed: the first instance will be linked into $(O_TARGET)
- and succeeding instances will be ignored. (Note: Rules.make may
- emit warning messages for duplicates, but this is harmless).
+ Even if a subdirectory Makefile has an $(O_TARGET), the .config
+ options still control whether or not its $(O_TARGET) goes into
+ vmlinux. See the $(M_OBJS) example below.
+
+ If neither $(LINK_FIRST) nor $(LINK_LAST) are defined, the order of
+ files in $(O_OBJS) and $(OX_OBJS) is significant. All $(OX_OBJS)
+ files come first, in the order listed, followed by all $(O_OBJS)
+ files, in the order listed. Duplicates in the lists are allowed:
+ the first instance will be linked into $(O_TARGET) and succeeding
+ instances will be ignored. (Note: Rules.make may emit warning
+ messages for duplicates, but this is harmless).

Example:

@@ -669,9 +677,61 @@ The public interface of Rules.make consi
O_OBJS += pci.o pci_iommu.o
endif

- Even if a subdirectory Makefile has an $(O_TARGET), the .config
- options still control whether or not its $(O_TARGET) goes into
- vmlinux. See the $(M_OBJS) example below.
+ If either $(LINK_FIRST) or $(LINK_LAST) are defined, the order of
+ files in $(O_OBJS) and $(OX_OBJS) is ignored. Instead the files are
+ linked in the order $(LINK_FIRST), the rest, $(LINK_LAST). The
+ order of entries in $(LINK_FIRST) and $(LINK_LAST) is preserved
+ exactly as specified. The order of the rest of the files is
+ undefined; currently it is alphabetical, but you must not rely on
+ this. When either $(LINK_FIRST) or $(LINK_LAST) are defined, they
+ must satisfy all possible ordering requirements for the
+ corresponding $(O_TARGET).
+
+ The only justification for $(LINK_FIRST) and $(LINK_LAST) is to
+ control the order of initialization routines. Routines which are
+ defined as __initcall or module_init and are linked into the kernel
+ will be executed during kernel startup in the order they were
+ linked.
+
+ Use $(LINK_FIRST) to ensure that certain routines, if present, are
+ executed before all others in the current directory. For example,
+ usb_init() in usb.c must be executed before all other usb
+ initialization routines:
+
+ # drivers/usb/Makefile
+ LINK_FIRST := usb.o
+
+ Use $(LINK_LAST) to ensure that initialization routines, if present,
+ are executed after all other such routines in the current directory.
+ Typically this is needed where you have multiple drivers that can
+ recognise a piece of hardware and you want the older drivers to be
+ tried last. For example, SCSI card `foo' can be controlled by
+ drivers bar.o and baz.o but baz.o is preferred, if present.
+ ``LINK_LAST := bar.o'' will ensure that the initialization routines
+ in bar.o are tried last.
+
+ [Note that the only way to control the kernel link order *between*
+ directories is by manipulating variables such as $(DRIVERS-y) in the
+ toplevel Makefile. This has directory-level granularity; if
+ finer-grained control is needed, you must use a workaround. Such
+ cases should be rare, if they exist at all.]
+
+ $(LINK_FIRST) and $(LINK_LAST) must not contain any duplicate object
+ names. For this reason, you should define them unconditionally,
+ i.e. they should not depend on the kernel configuration. They do
+ not need to, because they only affect the link order, not the actual
+ list of objects linked to $(O_TARGET). In other words, if an object
+ appears in $(LINK_FIRST) or $(LINK_LAST) but does not appear in
+ $(O_OBJS) or $(OX_OBJS), it is ignored.
+
+ All uses of $(LINK_FIRST) and $(LINK_LAST) must be justified and
+ fully documented in the Makefile. Historically, entries in
+ Makefiles were manually ordered with no documentation. This is
+ unfortunate because now, in some cases, we cannot be sure whether a
+ particular ordering is by chance or by necessity -- or, if by
+ necessity, what the reason was. This lack of critical information
+ is unacceptable. See drivers/usb/Makefile for an example of the
+ level of detail required.




2000-10-30 22:01:53

by Alexander Viro

[permalink] [raw]
Subject: Re: [PATCH] Re: test10-pre7



On Mon, 30 Oct 2000, Alexander Viro wrote:

>
>
> On Mon, 30 Oct 2000, Linus Torvalds wrote:
>
> > How about just changing ->sync_page() semantics to own the page lock? That
> > sound slike the right thing anyway, no?
>
> It would kill the ->sync_page(), but yes, _that_ might be the right thing ;-)

To elaborate: the thing is called if we get a contention on the page lock.
Essentially, its use in NFS is renice -20 for the requests on our page
wrt RPC scheduler. By the time when page gets unlocked it becomes a NOP.
On local filesystems it just runs the tq_disk - nothing in common with
the NFS case and IMO Trond was wrong lumping them together. In effect,
we are getting run_task_queue(&tq_disk) executed _very_ often and I'm less
than sure that it's a good idea. I think that ->sync_page() is not a
well-defined operation and NFS scheduler should use the locking of its own,
both for inavlidate_... and here.
Cheers,
Al

2000-10-30 22:02:04

by Jeff Garzik

[permalink] [raw]
Subject: Re: test10-pre7

Keith Owens wrote:
>
> On Mon, 30 Oct 2000 11:32:33 -0800 (PST),
> Linus Torvalds <[email protected]> wrote:
> > - pre7:
> > - Randy Dunlap, USB: printer.c, usb-storage, usb identification and
> > memory leak fixes
>
> USB still gets unresolved symbols when part is in kernel, part is in
> modules and modversions are set. Patch against 2.4.0-test10-pre7, only
> affects drivers/usb/Makefile.

Or instead of all that, you could simply call the core init function
from init/main.c...

Ya know, sorting those lists causes this problem, too... usb.o is
listed first in the various lists, as is usbcore.o. Is it possible to
avoid sorting? Doing so will fix this, and also any other link order
breakage like this that exists, too.

Jeff


--
Jeff Garzik | "Mind if I drive?" -Sam
Building 1024 | "Not if you don't mind me clawing at the
MandrakeSoft | dash and shrieking like a cheerleader."
| -Max

2000-10-30 22:06:43

by Keith Owens

[permalink] [raw]
Subject: Re: test10-pre7

On Mon, 30 Oct 2000 17:01:20 -0500,
Jeff Garzik <[email protected]> wrote:
>Keith Owens wrote:
>> USB still gets unresolved symbols when part is in kernel, part is in
>> modules and modversions are set. Patch against 2.4.0-test10-pre7, only
>> affects drivers/usb/Makefile.
>
>Or instead of all that, you could simply call the core init function
>from init/main.c...

Does that work when all of usb is a module? The point of __initcall is
to avoid all the conditional code that used to be in main.c.

>Ya know, sorting those lists causes this problem, too... usb.o is
>listed first in the various lists, as is usbcore.o. Is it possible to
>avoid sorting? Doing so will fix this, and also any other link order
>breakage like this that exists, too.

You have it backwards. Rules.make does *not* sort, the link order is
implicit in the declaration order of objects in the Makefiles. For
most makefiles, this kludge works, it does not work for USB. See
http://www.uwsg.indiana.edu/hypermail/linux/kernel/0010.3/0661.html

2000-10-30 22:07:33

by Rik van Riel

[permalink] [raw]
Subject: Re: [PATCH] Re: test10-pre7

On Mon, 30 Oct 2000, Alexander Viro wrote:

> The last one is in deactivate_page_nolock() - there we check the
> ->mapping without pagecache_lock and without page lock. Hell
> knows whether it's a bug or not. Rik?

Shouldn't be a problem, since we'll have the lock at a time
we actually /do/ something with those pointers.

In deactivate_page_nolock(), all we can modify is the list
in which the page resides, the flags indicating on which
list the page is and the referenced bit + page age. No other
stuff is touched.

Furthermore, the locking order (first pagecache lock, then
the page_list_lock) would make it difficult to do this right...

regards,

Rik
--
"What you're running that piece of shit Gnome?!?!"
-- Miguel de Icaza, UKUUG 2000

http://www.conectiva.com/ http://www.surriel.com/

2000-10-30 22:15:04

by Jeff Garzik

[permalink] [raw]
Subject: Re: test10-pre7

Keith Owens wrote:
>
> On Mon, 30 Oct 2000 17:01:20 -0500,
> Jeff Garzik <[email protected]> wrote:
> >Keith Owens wrote:
> >> USB still gets unresolved symbols when part is in kernel, part is in
> >> modules and modversions are set. Patch against 2.4.0-test10-pre7, only
> >> affects drivers/usb/Makefile.
> >
> >Or instead of all that, you could simply call the core init function
> >from init/main.c...
>
> Does that work when all of usb is a module? The point of __initcall is
> to avoid all the conditional code that used to be in main.c.

When all of usb is a module, there are no initcalls.

If you need static initialization for in-kernel init, here is the
shortest solution I can come up with:

/********************* usb.c **********************/

int usbcore_init() {...}

#ifdef MODULE
module_init(usbcore_init);
#endif
module_exit(usbcore_exit);

/******************** main.c ******************/

extern int usbcore_init (void);
/* ... */
#ifdef CONFIG_USB
usbcore_init();
#endif

--
Jeff Garzik | "Mind if I drive?" -Sam
Building 1024 | "Not if you don't mind me clawing at the
MandrakeSoft | dash and shrieking like a cheerleader."
| -Max

2000-10-30 22:21:55

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] Re: test10-pre7



On Mon, 30 Oct 2000, Alexander Viro wrote:
>
> > I didn't actually miss it, I just looked at the users and decided that it
> > looks like they should never have this issue. But I might have missed
> > something. As far as I can tell, "read_cache_page()" is only used for
> > meta-data like things that cannot be truncated.
>
> invalidate_inode_pages().

Nope. It checks the page count these days, so it would never kill such a
page from under us (we increment the page count while holding the
pagecache lock).

But yes, I'm starting to agree with you more and more..

Linus

2000-10-30 22:24:56

by Linus Torvalds

[permalink] [raw]
Subject: Re: test10-pre7



On Mon, 30 Oct 2000, Jeff Garzik wrote:
>
> Ya know, sorting those lists causes this problem, too... usb.o is
> listed first in the various lists, as is usbcore.o. Is it possible to
> avoid sorting? Doing so will fix this, and also any other link order
> breakage like this that exists, too.

This is the right fix. We MUST NOT sort those things.

The only reason for sorting is apparently to remove the "multi-objs"
things, and replace them with the object files they are composed of.

To which I say "Why?"

It makes more sense to just leave the multi's there.

Linus

2000-10-30 22:42:20

by Keith Owens

[permalink] [raw]
Subject: Re: test10-pre7

On Mon, 30 Oct 2000 14:24:13 -0800 (PST),
Linus Torvalds <[email protected]> wrote:
>This is the right fix. We MUST NOT sort those things.

Correction. We can sort them if we know what the correct link order
should be. In far too many Makefiles, we have no idea if the existing
order is required or is just historical so we fail safe and do not sort
them. For USB we know what the link order must be, usb.o must be
first, the rest do not matter. This patch only affects usb because it
is the only one that uses LINK_FIRST.

>The only reason for sorting is apparently to remove the "multi-objs"
>things, and replace them with the object files they are composed of.
>
>To which I say "Why?"
>
>It makes more sense to just leave the multi's there.

obj-y is used together with export-objs to split objects into O_OBJS
(no export symbol) and OX_OBJS (export symbol). If usbcore.o (multi)
is not replaced by its components then usb.o (in export-objs) is not
added to OX_OBJS so usb.c gets compiled with the wrong flags which
causes incorrect module symbols. Multi's in obj-y have to replaced by
their components before being split into O_OBS and OX_OBJS.

2000-10-30 22:52:30

by Linus Torvalds

[permalink] [raw]
Subject: Re: test10-pre7



On Tue, 31 Oct 2000, Keith Owens wrote:
>
> obj-y is used together with export-objs to split objects into O_OBJS
> (no export symbol) and OX_OBJS (export symbol). If usbcore.o (multi)
> is not replaced by its components then usb.o (in export-objs) is not
> added to OX_OBJS so usb.c gets compiled with the wrong flags which
> causes incorrect module symbols. Multi's in obj-y have to replaced by
> their components before being split into O_OBS and OX_OBJS.

Your honour, I object.

What would be wrong with just splitting it the other way, ie make OX_OBJS
be the expanded (but not ordered) list?

That should take care of it, no?

Linus

2000-10-30 23:03:12

by Jeff Garzik

[permalink] [raw]
Subject: Re: test10-pre7

Linus Torvalds wrote:
>
> On Tue, 31 Oct 2000, Keith Owens wrote:
> >
> > obj-y is used together with export-objs to split objects into O_OBJS
> > (no export symbol) and OX_OBJS (export symbol). If usbcore.o (multi)
> > is not replaced by its components then usb.o (in export-objs) is not
> > added to OX_OBJS so usb.c gets compiled with the wrong flags which
> > causes incorrect module symbols. Multi's in obj-y have to replaced by
> > their components before being split into O_OBS and OX_OBJS.
>
> Your honour, I object.
>
> What would be wrong with just splitting it the other way, ie make OX_OBJS
> be the expanded (but not ordered) list?
>
> That should take care of it, no?

As an aside: remember you mentioned we should try to go 100% OX_OBJS
anyway, eliminating O_OBJS completely...

--
Jeff Garzik | "Mind if I drive?" -Sam
Building 1024 | "Not if you don't mind me clawing at the
MandrakeSoft | dash and shrieking like a cheerleader."
| -Max

2000-10-30 23:04:02

by Keith Owens

[permalink] [raw]
Subject: Re: test10-pre7

On Mon, 30 Oct 2000 14:51:25 -0800 (PST),
Linus Torvalds <[email protected]> wrote:
>On Tue, 31 Oct 2000, Keith Owens wrote:
>>
>> obj-y is used together with export-objs to split objects into O_OBJS
>> (no export symbol) and OX_OBJS (export symbol). If usbcore.o (multi)
>> is not replaced by its components then usb.o (in export-objs) is not
>> added to OX_OBJS so usb.c gets compiled with the wrong flags which
>> causes incorrect module symbols. Multi's in obj-y have to replaced by
>> their components before being split into O_OBS and OX_OBJS.
>
>Your honour, I object.
>
>What would be wrong with just splitting it the other way, ie make OX_OBJS
>be the expanded (but not ordered) list?
>
>That should take care of it, no?

usbcore.o is both multi part *and* order critical. This is a
combination that the existing "link order relies on declaration order"
kludge cannot cope with. It requires an explicit declaration of link
order, which is exactly what LINK_FIRST implements.

FWIW, 2.5 kbuild will use LINK_FIRST and LINK_LAST exclusively, instead
of relying on the declaration order. This is primarily so we get
documentation of link order and why it matters. But it will also mean
that we can neatly sort declarations by CONFIG_ name if we want to.
That global change is only for 2.5, but there is nothing to stop us
using the preferred technique now, if nothing else works.

For usb, no other Makefile techniques will work, it needs LINK_FIRST.
I don't want to change the USB source code to overcome kbuild problems,
especially when those problems will disappear in 2.5. And I repeat,
this change only affects usb in 2.4.

2000-10-30 23:05:32

by Keith Owens

[permalink] [raw]
Subject: Re: test10-pre7

On Mon, 30 Oct 2000 18:02:34 -0500,
Jeff Garzik <[email protected]> wrote:
>As an aside: remember you mentioned we should try to go 100% OX_OBJS
>anyway, eliminating O_OBJS completely...

That is a global change for 2.5, it would massively break 2.4 kbuild.

2000-10-30 23:06:22

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] Re: test10-pre7



On Mon, 30 Oct 2000, Alexander Viro wrote:
>
> [ sync_page brokenness ]
>
> To elaborate: the thing is called if we get a contention on the page lock.

Ok, sync_page() looks like a broken design, but I suspect that for
expediency the simplest fix is to just make the NFS sync_page() (re-)check
for "mapping == NULL", and let it be at that. Avoid the NULL pointer
dereference (very small window already).

We should probably in the long run make "page->buffers" be a more generic
thing, and let NFS use it as a wb-info thing, and be done with it. That's
obviously not 2.4.x material, though.

Linus

2000-10-30 23:09:12

by Linus Torvalds

[permalink] [raw]
Subject: Re: test10-pre7



On Mon, 30 Oct 2000, Jeff Garzik wrote:
> >
> > What would be wrong with just splitting it the other way, ie make OX_OBJS
> > be the expanded (but not ordered) list?
> >
> > That should take care of it, no?
>
> As an aside: remember you mentioned we should try to go 100% OX_OBJS
> anyway, eliminating O_OBJS completely...

The only problem is that those unfortunate people without tons of
CPU-power would get really fed up with the extra "made depend" overhead.

So as a less drastic step we should just make it more of a hint, and less
of a design that impacts the link-order..

Linus

2000-10-30 23:15:24

by Alexander Viro

[permalink] [raw]
Subject: Re: [PATCH] Re: test10-pre7



On Mon, 30 Oct 2000, Linus Torvalds wrote:

> Ok, sync_page() looks like a broken design, but I suspect that for
> expediency the simplest fix is to just make the NFS sync_page() (re-)check
> for "mapping == NULL", and let it be at that. Avoid the NULL pointer
> dereference (very small window already).

Fine with me. Just let's remember that it should be revisited in 2.5.
What about filemap_swapout()? If you agree with checking ->mapping
there... looks like we are done with that crap for the time being.
If it's OK with you I'll send such patch against vanilla -pre7.
Cheers,
Al

2000-10-30 23:16:43

by Linus Torvalds

[permalink] [raw]
Subject: Re: test10-pre7



On Tue, 31 Oct 2000, Keith Owens wrote:
> >
> >What would be wrong with just splitting it the other way, ie make OX_OBJS
> >be the expanded (but not ordered) list?
> >
> >That should take care of it, no?
>
> usbcore.o is both multi part *and* order critical. This is a
> combination that the existing "link order relies on declaration order"
> kludge cannot cope with. It requires an explicit declaration of link
> order, which is exactly what LINK_FIRST implements.

I don't see your point.

I'm saying that EVERYTHING should be order-critical.

It is NEVER acceptable to change the order of object files.

If the Makefile said that the ordering should be

obj-y = usb.o usbcore.o third.o last.o

then the fact that usbcore.o is a multi-part object file SHOULD NOT
MATTER.

We should just link it in the order specified:

ld -r usbdrv.o $(obj-y)

No re-ordering. No expansion of multi-objs. No games. Do what the Makefile
author expected.

In short, we should _remove_ all traces of stuff like

O_OBJS = $(filter-out $(export-objs), $(obj-y))

It's wrong.

We should just have

O_OBJS = $(obj-y)

which is always right.

Then we change the meaning of OX_OBJS, and instead of saying

ALL_O = $(OX_OBJS) $(O_OBJS)

we just say

ALL_O = $(O_OBJS)

and the meaning of $OX_OBJS is the _subset_ of object file that have
SYMTAB objects.

This should all work pretty much as-is, with som every simple
modifications to existing old-style Makefiles, and with some even simpler
modifications to the new-style ones. In fact, it should remove pretty much
all the ugly games that new-style files do.

And it should make all this FIRST/LAST object file mockery a total
non-issue, because the whole concept turns out to be completely
unnecessary.

Is there anything that makes this more complex than what I've outlined
above?

Linus

2000-10-30 23:17:43

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] Re: test10-pre7



On Mon, 30 Oct 2000, Alexander Viro wrote:
>
> Fine with me. Just let's remember that it should be revisited in 2.5.
> What about filemap_swapout()? If you agree with checking ->mapping
> there... looks like we are done with that crap for the time being.

Yup, I agree. I already applied your patch, and did the additional
"mapping" check in nfs_sync_page. We should be ok for now, the only wart
being the fact that sync_page() is ugly.

But better ugly than broken.

Linus

2000-10-30 23:34:43

by Christoph Hellwig

[permalink] [raw]
Subject: Re: test10-pre7

In article <[email protected]> you wrote:


> We should just link it in the order specified:

> ld -r usbdrv.o $(obj-y)
>
> [...]
>
> Then we change the meaning of OX_OBJS, and instead of saying
>
> ALL_O = $(OX_OBJS) $(O_OBJS)
>
> we just say
>
> ALL_O = $(O_OBJS)
>
> and the meaning of $OX_OBJS is the _subset_ of object file that have
> SYMTAB objects.
>
> This should all work pretty much as-is, with som every simple
> modifications to existing old-style Makefiles,

It is simple - but a change in _every_ makefile is required.
And it is not really needed for old-style makefiles.

Would you accept a patch that makes the new-styles include
a separated Makefile library (e.g. $(TOPDIR)/Makefile.inc)
and leaves the old-style one as is (in hope of eleminating
them fast)?

Christoph

--
Always remember that you are unique. Just like everyone else.

2000-10-30 23:39:24

by Keith Owens

[permalink] [raw]
Subject: Re: test10-pre7

On Mon, 30 Oct 2000 15:15:57 -0800 (PST),
Linus Torvalds <[email protected]> wrote:
>I'm saying that EVERYTHING should be order-critical.

We (almost) agree about that, we are arguing about implementation
details. The existing implementation relies on the order that objects
are declared. In almost all cases there are no documented reasons for
the existing order, people who know about the link order problems are
scared to change declaration orders. OTOH, relying on declaration
order is error prone, people who do not know about the side effects of
declaration order try to change it and sometimes it works, sometimes it
breaks.

kbuild 2.5 splits link order into three categories. Those that must
come first, in the order they are specified - LINK_FIRST. Those that
must come last, in the order they are specified - LINK_LAST.
Everything else, in no defined order. This solves the documentation
problem, use of LINK_FIRST and LINK_LAST is explicit and the reasons
for the order will be documented, or else! Declaration order is then
irrelevant, it can be any order that makes sense to the developers.
The end effect if the same, LINK_FIRST/LAST is a better implementation.

>It is NEVER acceptable to change the order of object files.

It is NEVER acceptable to change the order of object files, but only
for those files where the developer has explicitly said what the order
must be. In the case of USB, the developers say usb.o must be first,
the rest can be in any order.

>Then we change the meaning of OX_OBJS, and instead of saying
>
> ALL_O = $(OX_OBJS) $(O_OBJS)
>
>we just say
>
> ALL_O = $(O_OBJS)
>
>and the meaning of $OX_OBJS is the _subset_ of object file that have
>SYMTAB objects.

We do not have an automatic way of detecting SYMTAB objects, OX_OBJS is
the only way that 2.4 kbuild can tell if an source has SYMTAB or not.
I could change Rules.make to grep the sources and work out what the
flags should be but that is messy and affects all of 2.4 kbuild.

>This should all work pretty much as-is, with som every simple
>modifications to existing old-style Makefiles, and with some even simpler
>modifications to the new-style ones. In fact, it should remove pretty much
>all the ugly games that new-style files do.

Let me get this straight. I provide a minimal patch that helps
document link order, is compatible with kbuild 2.5 and only affects
usb. But you want me to change the meaning of OX_OBJS, add grep to
Rules.make, edit all the old style Makefiles, change all the
bolierplate code in new style makefiles, in short to hit all of 2.4
kbuild. Why?

>And it should make all this FIRST/LAST object file mockery a total
>non-issue, because the whole concept turns out to be completely
>unnecessary.

Only if you think that documentation is unncessary.

2000-10-30 23:41:15

by Linus Torvalds

[permalink] [raw]
Subject: Re: test10-pre7



On Tue, 31 Oct 2000, Christoph Hellwig wrote:
>
> It is simple - but a change in _every_ makefile is required.
> And it is not really needed for old-style makefiles.

Actually, you don't have to change every makefile, because you CAN do this
all with a simple backwards-compatibility layer, something like:

OXONLY = $(filter-out $(O_OBJS), $(OX_OBJS))
ALL_O = $(OXONLY) $(O_OBJS)

which is a no-op for a "proper" makefile that follows the new rules
(OXONLY will be empty, because all OX_OBJS files will be part of O_OBJS),
but it will make old-style stuff act the same..

I'd actually prefer to just change every Makefile, but hey, I think
something like the above (untested) would make them work unmodified too.

Linus

2000-10-30 23:46:04

by Christoph Hellwig

[permalink] [raw]
Subject: Re: test10-pre7

On Mon, Oct 30, 2000 at 03:40:24PM -0800, Linus Torvalds wrote:
>
>
> On Tue, 31 Oct 2000, Christoph Hellwig wrote:
> >
> > It is simple - but a change in _every_ makefile is required.
> > And it is not really needed for old-style makefiles.
>
> Actually, you don't have to change every makefile, because you CAN do this
> all with a simple backwards-compatibility layer, something like:
>
> OXONLY = $(filter-out $(O_OBJS), $(OX_OBJS))
> ALL_O = $(OXONLY) $(O_OBJS)
>
> which is a no-op for a "proper" makefile that follows the new rules
> (OXONLY will be empty, because all OX_OBJS files will be part of O_OBJS),
> but it will make old-style stuff act the same..

Ok, that should do the job - but it is horribly ugly ...

> I'd actually prefer to just change every Makefile, but hey, I think
> something like the above (untested) would make them work unmodified too.

But when we are changing makefiles everywhere - why not do the proper think
and let the new-style makefiles share their code?

(I have a patch ready - it just needs some forward-porting and testing)

Christoph

--
Always remember that you are unique. Just like everyone else.

2000-10-30 23:48:46

by Linus Torvalds

[permalink] [raw]
Subject: Re: test10-pre7



On Tue, 31 Oct 2000, Keith Owens wrote:
>
> >It is NEVER acceptable to change the order of object files.
>
> It is NEVER acceptable to change the order of object files, but only
> for those files where the developer has explicitly said what the order
> must be. In the case of USB, the developers say usb.o must be first,
> the rest can be in any order.

How much do you want to bet that this can and will change if people were
made aware of how easy ordering can be?

I think we have too many "subtle" rules already.

We should have some REALLY simple and to-the-point rules. Namely:

- object files get linked in the order specified

No ifs, buts, "except when the user doesn't care", or anything like that.
No extra new logic with fancy new names for FIRST and LAST objects. No,
that's the wrong thing.

> > ALL_O = $(O_OBJS)
> >
> >and the meaning of $OX_OBJS is the _subset_ of object file that have
> >SYMTAB objects.
>
> We do not have an automatic way of detecting SYMTAB objects, OX_OBJS is
> the only way that 2.4 kbuild can tell if an source has SYMTAB or not.

I _know_.

I'm saying that we should not care. OX_OBJS still exists, but it has
nothing to do with _linking_. It has everything to do with the build
rules.

OX_OBJS is just a list of files that have exports.

It won't affect linking. It will only affect the list of SYMTAB_OBJS,
_nothing_ more.

For example, the old-style kernel/Makefile, you'd have O_OBJS containing
signal.o and sys.o. As would OX_OBJS. They'd be in both places, because
O_OBJS would tell that yes, we want to link it into the kernel, and
OX_OBJS would tell that yes, we need to generate symtab informaiton for
the files in question.

The two things are entirely orthogonal, as far as I can see. Except
historically we've mixed them up (OX_OBJS + O_OBJS is the link-list,
O_OBJS is the symtab information). And this mixup is what the problems
come from.

Linus

2000-10-30 23:52:58

by Linus Torvalds

[permalink] [raw]
Subject: Re: test10-pre7



On Tue, 31 Oct 2000, Christoph Hellwig wrote:
>
> But when we are changing makefiles everywhere - why not do the proper think
> and let the new-style makefiles share their code?
>
> (I have a patch ready - it just needs some forward-porting and testing)

I hate your patch.

I'd rather see "Rules.make" just base itself entirely off the new-style
Makefiles, and have it use "$(obj-y)" instead of O_OBJS etc.

Then, _old_style Makefiles could be fixed up by doing a

include Compat.make

or preferably by just fixing them. I don't want to have another
Rules.make. I want to fix the old users.

(Compat.make would then look like

obj-y = $(OX_OBJS) $(O_OBJS)
export-objs = $(OX_OBJS)
...

and make _old_ Makefiles look like new ones as far as Rules.make is
concerned.

See?

This is the same as with source code. I do NOT want to have backwards
compatibility in source code - if compatibility is needed, I'd much rather
have it be _forwards_ compatibility, where the old setup is made to look
like the new with wrapper functions etc.

Linus

2000-10-30 23:58:38

by Christoph Hellwig

[permalink] [raw]
Subject: Re: test10-pre7

On Mon, Oct 30, 2000 at 03:51:53PM -0800, Linus Torvalds wrote:
> I hate your patch.
>
> I'd rather see "Rules.make" just base itself entirely off the new-style
> Makefiles, and have it use "$(obj-y)" instead of O_OBJS etc.
>
> Then, _old_style Makefiles could be fixed up by doing a
>
> include Compat.make

That can't be done.
Old-style Makefiles are playing dirty tricks with defining
L_TARGET and then using O_TARGET for linking some onjects into
an intermediate object.

But the patch I have proposed is _not_ a resend of that old patch.
Instead this is a separate Makefile.inc that does not include the
old Rules.make - because it needs to do the different handling of
symtab objects - and btw it gets simpler because much of the Rule.make
logic is similar to the list-style makefiles.

So Rule.make would only be for the old-style Makefiles that should be
killed as fast as possible.

Christoph

--
Always remember that you are unique. Just like everyone else.

2000-10-31 00:03:58

by Keith Owens

[permalink] [raw]
Subject: Re: test10-pre7

On Mon, 30 Oct 2000 15:47:59 -0800 (PST),
Linus Torvalds <[email protected]> wrote:
>On Tue, 31 Oct 2000, Keith Owens wrote:
>We should have some REALLY simple and to-the-point rules. Namely:
>
> - object files get linked in the order specified
>
>No ifs, buts, "except when the user doesn't care", or anything like that.
>No extra new logic with fancy new names for FIRST and LAST objects. No,
>that's the wrong thing.

It is the right thing because it self documents which objects really
need a link order and why. The existing mechanism has demonstrably
failed to do this, resulting in fragile and error prone makefiles.

>The two things are entirely orthogonal, as far as I can see. Except
>historically we've mixed them up (OX_OBJS + O_OBJS is the link-list,
>O_OBJS is the symtab information). And this mixup is what the problems
>come from.

True, which is one of the reasons that kbuild 2.5 will remove OX_OBJS,
MX_OBJS and MIX_OBJS. But that change affects all Makefiles, we are
supposed to be in a code freeze. My patch fixes usb and only affects
usb, not the entire kernel.

2000-10-31 00:47:56

by Linus Torvalds

[permalink] [raw]
Subject: Re: test10-pre7



On Tue, 31 Oct 2000, Christoph Hellwig wrote:
>
> Old-style Makefiles are playing dirty tricks with defining
> L_TARGET and then using O_TARGET for linking some onjects into
> an intermediate object.

Actually, I think I have an even simpler solution, which is to change the
newstyle rule to something very simple:

# Translate to Rules.make lists.

O_OBJS := $(obj-y)
M_OBJS := $(obj-m)
MIX_OBJS := $(export-objs)

# The global Rules.make.

include $(TOPDIR)/Rules.make

And you're done..

Does anybody see anything wrong with this approach?

It's kin dof cheesy, but I think it should work. The magic is that by
avoiding OX_OBJS and MX_OBJS, we avoid all the sorting issues. We
basically lie, and say that we don't have anything like that.

Then, MIX_OBJS picks up the stragglers, and makes sure that we consider
the proper files to be SYMTAB_OBJS.

This works for me for USB (ie just remove all the stuff with "int-y" and
multi's etc). Does it work for anybody else?

Linus

Subject: Re: test10-pre7

Let me see if I have all this straight:

(1) Change Rules.make to use "new style" variables as its native form.
(1A) Add a "Compat.make" for old style Makefiles, and
(1B) Continue to convert all the remaining old style Makefiles.

(2) Go with the "export-objs" style of declaring source files that need
to be run through genksyms. Files never get built just because they
are in $(export-objs); $(export-objs) just determines who gets
processed by genksyms at compile time.

(3) No LINK_FIRST / LINK_LAST. Whatever is in the Makefile gets linked
in that order. We won't use $(sort ...) to eliminate duplicates
(we will continue to handle them another way).

(4) When a multi is built into the resident kernel, the whole multi goes in,
with no splitting into component parts.

Is that your plan, Linus?

I disagree with (3) because I think that initialization order requirements
should be spelled out and documented. But I accept it.

Historical note on (4): as Keith said, I had to split up the multi's in
order to get the components into the OX_OBJS list. But with a more
thorough implementation of (2), this becomes unnecessary.

Michael

2000-10-31 01:02:53

by Christoph Hellwig

[permalink] [raw]
Subject: Re: test10-pre7

On Mon, Oct 30, 2000 at 04:47:15PM -0800, Linus Torvalds wrote:
>
>
> On Tue, 31 Oct 2000, Christoph Hellwig wrote:
> >
> > Old-style Makefiles are playing dirty tricks with defining
> > L_TARGET and then using O_TARGET for linking some onjects into
> > an intermediate object.
>
> Actually, I think I have an even simpler solution, which is to change the
> newstyle rule to something very simple:
>
> # Translate to Rules.make lists.
>
> O_OBJS := $(obj-y)
> M_OBJS := $(obj-m)

This will destroy one nice feature of list-style makefiles:
when you have and object both in obj-y and obj-m it will be removed
from obj-m with the old boiler-plates, not with your proposal.

> MIX_OBJS := $(export-objs)

The MIX_OBJS change is wrong. It may not hurt the resulting
kernel image but you will build all export-objs, not only the
ones you actually have selected. But we might get around this
with some $(filter ...) magic.


> # The global Rules.make.
>
> include $(TOPDIR)/Rules.make
>
> And you're done..
>
> Does anybody see anything wrong with this approach?
>
> It's kin dof cheesy, but I think it should work. The magic is that by
> avoiding OX_OBJS and MX_OBJS, we avoid all the sorting issues. We
> basically lie, and say that we don't have anything like that.
>
> Then, MIX_OBJS picks up the stragglers, and makes sure that we consider
> the proper files to be SYMTAB_OBJS.
>
> This works for me for USB (ie just remove all the stuff with "int-y" and
> multi's etc). Does it work for anybody else?

The idea looks great, but it looks like the implementation needs a little
bit work.

Keith do you want to hack on this now - or should I prepare a patch tomorrow?

Christoph

--
Always remember that you are unique. Just like everyone else.

2000-10-31 01:07:53

by Christoph Hellwig

[permalink] [raw]
Subject: Re: test10-pre7

On Mon, Oct 30, 2000 at 06:52:08PM -0600, Michael Elizabeth Chastain wrote:
> Let me see if I have all this straight:
>
> (1) Change Rules.make to use "new style" variables as its native form.
> (1A) Add a "Compat.make" for old style Makefiles, and
> (1B) Continue to convert all the remaining old style Makefiles.

This is difficult because old-style makefiles can do much more magic
then list-style ones. But after a bit more thinking it looks like is
is possible ... (yeah I said otherwise some time ago).

Christoph

--
Always remember that you are unique. Just like everyone else.

2000-10-31 01:49:41

by Keith Owens

[permalink] [raw]
Subject: Re: test10-pre7

On Mon, 30 Oct 2000 16:47:15 -0800 (PST),
Linus Torvalds <[email protected]> wrote:
>Actually, I think I have an even simpler solution, which is to change the
>newstyle rule to something very simple:
>
> # Translate to Rules.make lists.
>
> O_OBJS := $(obj-y)
> M_OBJS := $(obj-m)
> MIX_OBJS := $(export-objs)

It makes kbuild variables in USB mean something different from the rest
of the kernel. Unless you plan to change all Makefiles (code freeze,
what code freeze?).

make modules depends on MIX_OBJS, with the above change make modules
now depends on kernel objects. Can be fixed in Rules.make, but only if
every Makefile is changed (code freeze, what code freeze?).

You will compile all export objects, whether they are configured or
not. The "obvious" fix does not work.

MIX_OBJS := $(filter $(export-objs),$(obj-y) $(obj-m))

export_objs contains usb.o, obj-y contains usb_core.o, it does not
contain usb.o. Multi lists in obj-y and obj-m need to be expanded
while preserving the required link order (which is where we came in).

It still does not document the only real link order constraint in USB.
The almost complete lack of documentation on which link orders are
required and which are historical is extremely annoying and _must_ be
fixed, instead we just propagate the problem.

If you cannot do sort then you cannot (easily) remove duplicate objects
from the lists, resulting in make warning messages. Doing an explicit
link first, list last then sort the rest also fixes the problem of
duplicate objects.

2000-10-31 02:07:59

by Keith Owens

[permalink] [raw]
Subject: Re: test10-pre7

On Tue, 31 Oct 2000 12:49:12 +1100,
Keith Owens <[email protected]> wrote:
>You will compile all export objects, whether they are configured or
>not. The "obvious" fix does not work.
>
> MIX_OBJS := $(filter $(export-objs),$(obj-y) $(obj-m))
>
>export_objs contains usb.o, obj-y contains usb_core.o, it does not
>contain usb.o. Multi lists in obj-y and obj-m need to be expanded
>while preserving the required link order.

Correction to my own mail. Multi lists in obj-y and obj-m just need to
be expanded, the order does not matter in MIX_OBJS.

2000-10-31 02:55:39

by Linus Torvalds

[permalink] [raw]
Subject: Re: test10-pre7



On Tue, 31 Oct 2000, Christoph Hellwig wrote:
> > newstyle rule to something very simple:
> >
> > # Translate to Rules.make lists.
> >
> > O_OBJS := $(obj-y)
> > M_OBJS := $(obj-m)
>
> This will destroy one nice feature of list-style makefiles:
> when you have and object both in obj-y and obj-m it will be removed
> from obj-m with the old boiler-plates, not with your proposal.

Ok. That's fine, the "obj-m" thing doesn't have any ordering constraints,
so we can do whatever we want to it. Including the $(filter-out ..) thing.

> > MIX_OBJS := $(export-objs)
>
> The MIX_OBJS change is wrong. It may not hurt the resulting
> kernel image but you will build all export-objs, not only the
> ones you actually have selected. But we might get around this
> with some $(filter ...) magic.

Yes. That's fine, again MIX_OBJS does not care about ordering, so
filtering etc is fine here.

The only thing I really care about is O_OBJS = $(obj-y), and with this
setup it seems to be a valid thing to do, with some slight hackery on the
other ones.

Linus

2000-10-31 02:58:58

by Linus Torvalds

[permalink] [raw]
Subject: Re: test10-pre7



On Tue, 31 Oct 2000, Keith Owens wrote:
>
> You will compile all export objects, whether they are configured or
> not. The "obvious" fix does not work.
>
> MIX_OBJS := $(filter $(export-objs),$(obj-y) $(obj-m))
>
> export_objs contains usb.o, obj-y contains usb_core.o, it does not
> contain usb.o. Multi lists in obj-y and obj-m need to be expanded
> while preserving the required link order (which is where we came in).

No. We can expand multi-lists at ANY OTHER POINT than O_OBJS. That's ok.
It's only O_OBJS that has any ordering issues.

And we just shouldn't use OX_OBJS at all, as that breaks ordering _and_
can be done equally well with MIX_OBJS instead.

> It still does not document the only real link order constraint in USB.
> The almost complete lack of documentation on which link orders are
> required and which are historical is extremely annoying and _must_ be
> fixed, instead we just propagate the problem.

We can add a comment to the Makefile. That's trivial.

What's not trivial, and what I WANT DONE is to make sure that _when_
somebody wants to maintain link ordering, he can do so in an easy and
obvious way. Not with Yet Another Hack.

Linus

2000-10-31 04:57:41

by Rusty Russell

[permalink] [raw]
Subject: Re: test10-pre7

In message <[email protected]> you write:
> On Mon, 30 Oct 2000 16:47:15 -0800 (PST),
> Linus Torvalds <[email protected]> wrote:
> >Actually, I think I have an even simpler solution, which is to change the
> >newstyle rule to something very simple:
> >
> > # Translate to Rules.make lists.
> >
> > O_OBJS := $(obj-y)
> > M_OBJS := $(obj-m)
> > MIX_OBJS := $(export-objs)
>
> make modules depends on MIX_OBJS, with the above change make modules
> now depends on kernel objects. Can be fixed in Rules.make, but only if
> every Makefile is changed (code freeze, what code freeze?).

Quiet suggestion:

Maybe better is to get rid of the X version variables? Append -EXPORTS
to everything that exports, and generate the genksyms food from:

$(patsubst %.o-EXPORTS,%.c, $(filter %-EXPORTS, $(OBJS))

And the link line from:

$(patsubst %-EXPORTS, %, $(OBJS))

This allows complete control over the link order.
Rusty.
--
Hacking time.

2000-10-31 06:10:38

by Linus Torvalds

[permalink] [raw]
Subject: Re: test10-pre7



On Tue, 31 Oct 2000, Rusty Russell wrote:
>
> Quiet suggestion:

If I understood the GNU make syntax correctly (which is possibly not the
case - GNU make is possibly the only example of "overkill" to rival GNU
emacs), this looks like a reasonable idea.

However, it also looks like much more of a change than to change the
fairly boiler-plate OX_OBJS etc stuff in new-style makefiles. And quite
frankly, I don't see how it would get the multi-part object file case
right, but that's probably because you left off some of the black magic
required to do that (and it's not as if the current Makefile magic doesn't
do black magic for that already).

Why do I really care? We actually have the same issue in the SCSI driver
directory, where the ordering restraints are much stricter than for USB.
Now that case has fewer export-objs, and that case isn't a part of a
multi-list, but I really want to have something that works for both these
cases with minimal (and reasonably straightforward) surgery.

In fact, I suspect the SCSI rules would work almost as-is. They break
ordering for the export-objs entry, but that looks fixable. This is how it
looks now:

# Extract lists of the multi-part drivers.
# The 'int-*' lists are the intermediate files used to build the multi's.
multi-y := $(filter $(list-multi), $(obj-y))
multi-m := $(filter $(list-multi), $(obj-m))
int-y := $(sort $(foreach m, $(multi-y), $($(basename $(m))-objs)))
int-m := $(sort $(foreach m, $(multi-m), $($(basename $(m))-objs)))

# Files that are both resident and modular: remove from modular.
obj-m := $(filter-out $(obj-y), $(obj-m))
int-m := $(filter-out $(int-y), $(int-m))

O_OBJS := $(filter-out $(export-objs), $(obj-y))
OX_OBJS := $(filter $(export-objs), $(obj-y))
M_OBJS := $(sort $(filter-out $(export-objs), $(obj-m)))
MX_OBJS := $(sort $(filter $(export-objs), $(obj-m)))
MI_OBJS := $(sort $(filter-out $(export-objs), $(int-m)))
MIX_OBJS := $(sort $(filter $(export-objs), $(int-m)))

In the above, the only problem is OX_OBJS and the breaking of ordering of
"export-objs" (which SCSI doesn't care about, unlike USB, partly because
SCSI uses the old-fashioned "every export in a special file" approach).
And it looks like even THAT could be fixed by changing it to

O_OBJS := $(obj-y)
OX_OBJS :=
MIX_OBJS := $(sort $(filter $(export-objs), $(int-m) $(obj-y)))

(and the others are unchanged) which looks like it would handle it all
correctly. Basically, the changes would mean that the export-objs subset
of $(obj-y) would stay in O_OBJS instead of moving to OX_OBJS, but
additionally those objs would also be added to MIX_OBJS.

Would this satisfy everybody? It _is_ complex enough that I guess it
easily rates having it's own rule-file and be included by new-style
Makefiles instead of being copied over and over again..

Rusty's suggestion would mean having to actually change all the lists
themselves, which at this point sounds a bit dangerous.

Linus

2000-10-31 08:19:18

by Rogier Wolff

[permalink] [raw]
Subject: Re: test10-pre7

Linus Torvalds wrote:
>
>
> On Mon, 30 Oct 2000, Jeff Garzik wrote:
> >
> > Ya know, sorting those lists causes this problem, too... usb.o is
> > listed first in the various lists, as is usbcore.o. Is it possible to
> > avoid sorting? Doing so will fix this, and also any other link order
> > breakage like this that exists, too.
>
> This is the right fix. We MUST NOT sort those things.
>
> The only reason for sorting is apparently to remove the "multi-objs"
> things, and replace them with the object files they are composed of.

No. It is NOT the only reason.

Some driver have a "lowerlevel" driver that needs to be included or
can be loaded as a module whenever the driver is enabled.

This happens to be the case with sx, rio and generic_serial. So both
SX and RIO, when enabled pull in generic_serial, which gets sorted out
by the sort-and-uniqify.

I used to have horrendously complicated Makefile rules to get this
right, but this was simplified enormously by the "eliminate duplicate
objects" that is now in the Makefiles.

Roger.

--
** [email protected] ** http://www.BitWizard.nl/ ** +31-15-2137555 **
*-- BitWizard writes Linux device drivers for any device you may have! --*
* Common sense is the collection of *
****** prejudices acquired by age eighteen. -- Albert Einstein ********

2000-10-31 10:06:17

by John Kennedy

[permalink] [raw]
Subject: Re: [PATCH] Re: test10-pre7

On Mon, Oct 30, 2000 at 03:34:44PM -0500, Alexander Viro wrote:
> Unfortunately, it doesn't fix the thing. ->sync_page() is called ...
> Minimal patch (against -pre7) follows. It still leaves sync_page() problem
> open - any suggestions on that one are very welcome. ...

I needed to patch your patch to get it to compile.


Attachments:
PATCH (301.00 B)

2000-10-31 10:06:48

by Russell King

[permalink] [raw]
Subject: Re: test10-pre7

Keith Owens writes:
> kbuild 2.5 splits link order into three categories. Those that must
> come first, in the order they are specified - LINK_FIRST. Those that
> must come last, in the order they are specified - LINK_LAST.

Keith, this sounds like a K-ludge.

Take the instance where we need to link a.o first, z.o second, f.o third
and p.o fourth. How does LINK_FIRST / LINK_LAST guarantee this?

LINK_FIRST = a.o z.o
LINK_LAST = f.o p.o

But then what guarantees that 'a.o' will be linked before 'z.o'?

A first/last implementation can *not* specify precisely a link order without
guaranteeing that the order of the LINK_FIRST *and* the LINK_LAST objects
is preserved, which incidentally is the same requirement for the obj-y
implementation.

I don't see what this LINK_FIRST / LINK_LAST gains us other than more
complexity for zero gain.
_____
|_____| ------------------------------------------------- ---+---+-
| | Russell King [email protected] --- ---
| | | | http://www.arm.linux.org.uk/~rmk/aboutme.html / / |
| +-+-+ --- -+-
/ | THE developer of ARM Linux |+| /|\
/ | | | --- |
+-+-+ ------------------------------------------------- /\\\ |

2000-10-31 12:00:42

by Peter Samuelson

[permalink] [raw]
Subject: Re: test10-pre7


[Linus]
> In short, we should _remove_ all traces of stuff like
>
> O_OBJS = $(filter-out $(export-objs), $(obj-y))
>
> It's wrong.
>
> We should just have
>
> O_OBJS = $(obj-y)
>
> which is always right.

This part I agree with..

> And it should make all this FIRST/LAST object file mockery a total
> non-issue, because the whole concept turns out to be completely
> unnecessary.
>
> Is there anything that makes this more complex than what I've
> outlined above?

One thing. The main benefit of $(sort), which I haven't heard you
address yet, is to remove duplicate files. Think about 8390.o, and how
many net drivers require it. There are two ways to handle this:

obj-$(CONFIG_WD80x3) += wd.o 8390.o
obj-$(CONFIG_EL2) += 3c503.o 8390.o
obj-$(CONFIG_NE2000) += ne.o 8390.o
obj-$(CONFIG_NE2_MCA) += ne2.o 8390.o
obj-$(CONFIG_HPLAN) += hp.o 8390.o

and then remove duplicates from $(obj-y) using $(sort)...

...Or do horrible games with 'if' statements and temporary variables
with names like $(NEED_8390) to ensure that it gets included once if
needed and not if not -- thereby pretty much defeating the readability
of the new-style makefiles.

Oh. There's a third way: ignore the issue and hope users don't feel
the need for both ne.o and wd.o linked into the same kernel. I do hope
you aren't advocating *that* solution, which unfortunately appears to
be the status quo in drivers/net/Makefile. I guess solution #2 was
seen as too much trouble there.

The horrible games with 'if' statements have been played in any number
of kernel makefiles and I'd really like to see them go away.

That is the real reason I like LINK_FIRST.

Peter

2000-10-31 13:55:52

by Peter Samuelson

[permalink] [raw]
Subject: Re: test10-pre7


[kaos]
> > It still does not document the only real link order constraint in
> > USB. The almost complete lack of documentation on which link
> > orders are required and which are historical is extremely annoying
> > and _must_ be fixed, instead we just propagate the problem.

[Linus]
> We can add a comment to the Makefile. That's trivial.

True.

The thing that Keith's patch does is flush these things out into the
open. By using LINK_FIRST/LINK_LAST, we declare that "these are the
known issues" -- and then the rest of the objects are reordered, and if
something breaks, we track it down and add it to LINK_FIRST.

That way these things *will* get documented eventually. I have very
little hope that they otherwise will, since some ordering requirements
may have already been lost to the mists of time.

Obviously, "flushing issues out into the open" is not 2.4 material,
which is why Keith's patch does *not* reorder unless you explicitly
declare a LINK_FIRST line -- i.e. only drivers/usb/Makefile is affected
at the moment. The plan has been to change that behavior in 2.5.

> What's not trivial, and what I WANT DONE is to make sure that _when_
> somebody wants to maintain link ordering, he can do so in an easy and
> obvious way. Not with Yet Another Hack.

One man's hack is another man's clean design ... but I concede the
point that LINK_FIRST is a hack, because I respect your judgment.
However, I still maintain that it is a less ugly hack than any
alternatives I've seen for preventing/removing duplicate object files
in link lines (see my last mail).

A few months ago I actually tried to write a make function (yes, GNU
make has these!) to remove duplicates while not sorting, but GNU make
doesn't seem to support the necessary iteration/(tail-)recursion
features. (Surprising, considering GNU's overall LISP-ish worldview.)

Peter

2000-10-31 14:02:52

by Keith Owens

[permalink] [raw]
Subject: Re: test10-pre7

On Tue, 31 Oct 2000 09:37:09 +0000 (GMT),
Russell King <[email protected]> wrote:
>Keith Owens writes:
>> kbuild 2.5 splits link order into three categories. Those that must
>> come first, in the order they are specified - LINK_FIRST. Those that
>> must come last, in the order they are specified - LINK_LAST.
>Take the instance where we need to link a.o first, z.o second, f.o third
>and p.o fourth. How does LINK_FIRST / LINK_LAST guarantee this?
>
>LINK_FIRST = a.o z.o
>LINK_LAST = f.o p.o
>
>But then what guarantees that 'a.o' will be linked before 'z.o'?

LINK_FIRST is processed in the order it is specified, so a.o will be
linked before z.o when both are present. See the patch.

>A first/last implementation can *not* specify precisely a link order without
>guaranteeing that the order of the LINK_FIRST *and* the LINK_LAST objects
>is preserved, which incidentally is the same requirement for the obj-y
>implementation.

It is preserved, see the patch.

>I don't see what this LINK_FIRST / LINK_LAST gains us other than more
>complexity for zero gain.

The vast majority of objects have no link order dependencies. You only
specify LINK_FIRST or LINK_LAST where it is needed and only for the
small subset of objects that have critical ordering.

There are 82 obj-$(CONFIG_xxx) entries in drivers/scsi. Which ones
must be executed first? Which ones must be executed last? Why do
these requirements (if any) exist? Can I safely change the order of
any of the driver/scsi entries? If not, why not?

LINK_FIRST and LINK_LAST serve two purposes. They self document the
link order where that order matters. And they solve the problem of
multi part objects being linked into the kernel, although that problem
_may_ have other solutions. Having documentation makes life easier for
everybody. Please do not say that the link order could be documented
in the existing Makefiles, that method has completely failed to work.

2000-10-31 14:16:42

by Peter Samuelson

[permalink] [raw]
Subject: Re: test10-pre7


[rmk]
> > Take the instance where we need to link a.o first, z.o second, f.o
> > third and p.o fourth. How does LINK_FIRST / LINK_LAST guarantee
> > this?

It does. Read the patch. LINK_FIRST *itself* is not sorted.

> > LINK_FIRST = a.o z.o
> > LINK_LAST = f.o p.o
> >
> > But then what guarantees that 'a.o' will be linked before 'z.o'?

[kaos]
> LINK_FIRST is processed in the order it is specified, so a.o will be
> linked before z.o when both are present. See the patch.

Indeed, the right solution is

LINK_FIRST := a.o z.o f.o p.o

which is self-documenting, as Keith has said: by looking at that line,
the intended behavior is obvious. You should still accompany this with
a comment explaining *why* the ordering is needed, but even if you
don't, you are giving us much more information than the status quo
(which is "this link order works, any other order is quite possibly
sane but who knows for sure").

Peter

2000-10-31 16:20:49

by Vladislav Malyshkin

[permalink] [raw]
Subject: Re: test10-pre7

Hi, Peter,
You can easily remove duplicates in object files without sorting. You
can just use a shell written function.
This is an example of such function (bash written).
It removes the duplicates from the argument and prints the result to
stdout.
No sorting used.

# This function removes duplicates from a string
remove_duplicates() {
if [ $# -ne 1 ]; then
echo "usage: remove_duplicates string" 1>&2
exit 1
fi
str=""
for tmpvar1 in $1 ; do
flag_found=0
for tmpvar2 in $str ; do
if [ "${tmpvar1}" = "${tmpvar2}" ]; then
flag_found=1
break
fi
done
if [ "${flag_found}" -eq 0 ]; then
str="${str} ${tmpvar1}"
fi
done
echo "${str}"
return 0
}

# This is a usage example.
x="`remove_duplicates \"a b c d e a b c\"`"
echo "$x"

Vladislav

---Peter Samuelson wrote:
>> And it should make all this FIRST/LAST object file mockery a total
>> non-issue, because the whole concept turns out to be completely
>> unnecessary.
>>
>> Is there anything that makes this more complex than what I've
>> outlined above?
>
>One thing. The main benefit of $(sort), which I haven't heard you
>address yet, is to remove duplicate files. Think about 8390.o, and how
>many net drivers require it. There are two ways to handle this:
>
> obj-$(CONFIG_WD80x3) += wd.o 8390.o
> obj-$(CONFIG_EL2) += 3c503.o 8390.o
> obj-$(CONFIG_NE2000) += ne.o 8390.o
> obj-$(CONFIG_NE2_MCA) += ne2.o 8390.o
> obj-$(CONFIG_HPLAN) += hp.o 8390.o

2000-10-31 16:46:36

by Peter Samuelson

[permalink] [raw]
Subject: Re: test10-pre7


[Vladislav Malyshkin <[email protected]>]
> You can easily remove duplicates in object files without sorting.
> You can just use a shell written function.

This is true. That was something I forgot to mention. I have looked
at that as well, and it strikes me as even more of a hack than the
solutions I mentioned: it is yet another external shell process for
each invocation of Rules.make (ie each directory). As I said before,
though, one man's hack is another man's clean design, so whatever.

Your function is rather long; try this one instead (untested):

remove_duplicates () {
str='';
for i; do
case "$str " in *" $i "*) ;; *) str="$str $i" ;; esac
done
echo "$str"
}

I still think anything outside the makefiles that's needed to organize
the build process is a hack. That includes scripts/pathdown.sh (yes, I
do have a scheme to get rid of it) and 2.2.18 scripts/kwhich (yes, I
did propose a working alternative). It doesn't include scripts/mkdep.c
(which must do a lot of work as efficiently as possible),
scripts/Configure et al (which are really standalone programs), or
scripts/split-include.c (which is really a continuation of Configure).

Peter

2000-10-31 17:30:47

by Linus Torvalds

[permalink] [raw]
Subject: Re: test10-pre7



On Tue, 31 Oct 2000, Peter Samuelson wrote:
>
> The thing that Keith's patch does is flush these things out into the
> open. By using LINK_FIRST/LINK_LAST, we declare that "these are the
> known issues" -- and then the rest of the objects are reordered, and if
> something breaks, we track it down and add it to LINK_FIRST.

But it doesn't even WORK.

You need to have

LINK_FIRST1
LINK_FIRST2
LINK_FIRST3
...

etc to get the proper ordering.

USB is the _easy_ case. There happen sto be only one file that cares about
ordering.

In many other cases, like SCSI, we need almost _total_ ordering. For such
a case, theer is no "first" or "last" - there is a well-specific ORDER.

Do you see the problem now? LINK_FIRST/LINK_LAST is a complete and utter
hack, and it WON'T EVER WORK. The only way it would work is to make
LINK_FIRST maintain the order, but once you do that LINK_FIRST is
completely superfluous, as it ends up being exactly the same as $(obj-y).

See the fallacy? LINK_FIRST doesn't solve anything, because in the end it
has to do everything O_OBJS will have to do anyway: maintain the full
order.

So trust me, LINK_FIRST/LINK_LAST is not going to happen.

> A few months ago I actually tried to write a make function (yes, GNU
> make has these!) to remove duplicates while not sorting, but GNU make
> doesn't seem to support the necessary iteration/(tail-)recursion
> features. (Surprising, considering GNU's overall LISP-ish worldview.)

Ehh..

There are multiple solutions to this. One is to simply not do that then.
We've done that before, it's not all that painful at all. The classic
example is the current drivers/net/ Makefile, which is badly written
anyway (a mixture of old-style and new-style stuff), and has that 8390.o
and slhc.o multiple thing. It's not that hard to re-write by just adding a
special

obj-8390-$(CONFIG_xxx) = y

for every config that wants 8390.o (and same thing for slhc) and at the
very end do a final

obj-$(obj-8390-y) += 8390.o

Not as simple as what we have now, but not a disaster like the current
lack of ordering is.

And if you really want to remove duplicates, at worst we can even use an
external program for it - which would solve all these things once and for
all. The difference between

$(filter .. black magic lies here ..)

and

$(shell .. black magic lies here ..)

is not that big.

Linus

2000-10-31 17:32:07

by Linus Torvalds

[permalink] [raw]
Subject: Re: test10-pre7



On Wed, 1 Nov 2000, Keith Owens wrote:
>
> LINK_FIRST is processed in the order it is specified, so a.o will be
> linked before z.o when both are present. See the patch.

So why don't you do the same thing for obj-y, then?

Why can't you do

LINK_FIRST=$(obj-y)

and be done with it?

Linus

2000-10-31 17:39:08

by H. Peter Anvin

[permalink] [raw]
Subject: Re: test10-pre7

Followup to: <[email protected]>
By author: Linus Torvalds <[email protected]>
In newsgroup: linux.dev.kernel
>
> On Tue, 31 Oct 2000, Peter Samuelson wrote:
> >
> > The thing that Keith's patch does is flush these things out into the
> > open. By using LINK_FIRST/LINK_LAST, we declare that "these are the
> > known issues" -- and then the rest of the objects are reordered, and if
> > something breaks, we track it down and add it to LINK_FIRST.
>
> But it doesn't even WORK.
>
> You need to have
>
> LINK_FIRST1
> LINK_FIRST2
> LINK_FIRST3
> ...
>
> etc to get the proper ordering.
>
> USB is the _easy_ case. There happen sto be only one file that cares about
> ordering.
>
> In many other cases, like SCSI, we need almost _total_ ordering. For such
> a case, theer is no "first" or "last" - there is a well-specific ORDER.
>

Sounds like what you actually need is LINK_BEFORE() LINK_AFTER() and a
topological sort.

-hpa
--
<[email protected]> at work, <[email protected]> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt

2000-10-31 18:12:53

by Vladislav Malyshkin

[permalink] [raw]
Subject: Re: test10-pre7

Also, the function remove_duplicates can be written using make rules and
functions.
Using functions "foreach" "if" from make and comparison you can easily
build
a function remove_duplicates in make, no shell involved.
so instead of $(sort) your will have $(remove_duplicates)
written entirely in make.
Vladislav

Peter Samuelson wrote:

> [Vladislav Malyshkin <[email protected]>]
> > You can easily remove duplicates in object files without sorting.
> > You can just use a shell written function.
>
> This is true. That was something I forgot to mention. I have looked
> at that as well, and it strikes me as even more of a hack than the
> solutions I mentioned: it is yet another external shell process for
> each invocation of Rules.make (ie each directory). As I said before,
> though, one man's hack is another man's clean design, so whatever.
>
> Your function is rather long; try this one instead (untested):
>
> remove_duplicates () {
> str='';
> for i; do
> case "$str " in *" $i "*) ;; *) str="$str $i" ;; esac
> done
> echo "$str"
> }
>
> I still think anything outside the makefiles that's needed to organize
> the build process is a hack. That includes scripts/pathdown.sh (yes, I
> do have a scheme to get rid of it) and 2.2.18 scripts/kwhich (yes, I
> did propose a working alternative). It doesn't include scripts/mkdep.c
> (which must do a lot of work as efficiently as possible),
> scripts/Configure et al (which are really standalone programs), or
> scripts/split-include.c (which is really a continuation of Configure).
>
> Peter

2000-10-31 18:39:33

by Linus Torvalds

[permalink] [raw]
Subject: Re: test10-pre7



Ok, how about this approach? It only works for the case where we do not
have the kind of multiple stuff that drivers/net has, but hey, we don't
actually need to handle all the cases right now.

We can leave that for the future, as the configuration process is likely
to change anyway during 2.5.x, and the multiple object case may go away
entirely (ie the case of slhc and 8390 will become just a normal
configuration dependency: you'd have a "CONFIG_SLHC" entry that is
computed by the dependency graph at configuration time, rather than by the
Makefile at build time).

This is the simplest rule base that I could come up with that should work
for both SCSI and USB:

# Translate to Rules.make lists.
multi-used := $(filter $(list-multi), $(obj-y) $(obj-m))
multi-objs := $(foreach m, $(multi-used), $($(basename $(m))-objs))
active-objs := $(sort $(multi-objs) $(obj-y) $(obj-m))

O_OBJS := $(obj-y)
M_OBJS := $(obj-m)
MIX_OBJS := $(filter $(export-objs), $(active-objs))

Does anybody see any problems with it? Basically, we're sidestepping the
sorting, because neither SCSI nor USB need it. Making the problem simpler
is always good.

Now, the above won't work for drivers/net, but I think it will work for
just about anything else. So let's just leave drivers/net alone for now.
Simplicity is good.

Linus

2000-10-31 19:17:03

by H. Peter Anvin

[permalink] [raw]
Subject: Re: test10-pre7

Followup to: <[email protected]>
By author: Linus Torvalds <[email protected]>
In newsgroup: linux.dev.kernel
>
> Does anybody see any problems with it? Basically, we're sidestepping the
> sorting, because neither SCSI nor USB need it. Making the problem simpler
> is always good.
>
> Now, the above won't work for drivers/net, but I think it will work for
> just about anything else. So let's just leave drivers/net alone for now.
> Simplicity is good.
>

I was going to ask to what extent we genuinely need sorting, and if we
might be better off trying to eliminate that need as much as possible.

-hpa
--
<[email protected]> at work, <[email protected]> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt

2000-10-31 19:53:58

by Horst H. von Brand

[permalink] [raw]
Subject: Re: test10-pre7

"H. Peter Anvin" <[email protected]> said:

[...]

> Sounds like what you actually need is LINK_BEFORE() LINK_AFTER() and a
> topological sort.

Was suggested before, and shot down by Linus himself...

tsort(1) et al come handy.
--
Dr. Horst H. von Brand mailto:[email protected]
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria +56 32 654239
Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513

2000-10-31 20:56:19

by Russell King

[permalink] [raw]
Subject: Re: test10-pre7

Linus Torvalds writes:
> On Wed, 1 Nov 2000, Keith Owens wrote:
> > LINK_FIRST is processed in the order it is specified, so a.o will be
> > linked before z.o when both are present. See the patch.
>
> So why don't you do the same thing for obj-y, then?
>
> Why can't you do
>
> LINK_FIRST=$(obj-y)
>
> and be done with it?

Hmm, so why don't we just call it obj-y and be done with it? ;)

Since someone kindly enlightened me that LINK_FIRST was unsorted, I'm finding
it very hard to grasp what the difference is between an unsorted LINK_FIRST
and unsorted LINK_LAST list, and an unsorted obj-y list. From what I
understand, obj-y = $(LINK_FIRST) $(LINK_LAST) ?

Therefore, there is little difference between:

LINK_FIRST = a.o z.o
LINK_LAST = y.o p.o

and

obj-y = a.o z.o y.o p.o

I still don't see what LINK_FIRST and LINK_LAST gains us, other than
potentially more confusion. Instead of having one order-dependent
variable, we now have 2. Please enlighten me further!
_____
|_____| ------------------------------------------------- ---+---+-
| | Russell King [email protected] --- ---
| | | | http://www.arm.linux.org.uk/personal/aboutme.html / / |
| +-+-+ --- -+-
/ | THE developer of ARM Linux |+| /|\
/ | | | --- |
+-+-+ ------------------------------------------------- /\\\ |

2000-10-31 21:00:39

by Linus Torvalds

[permalink] [raw]
Subject: Re: test10-pre7



On Tue, 31 Oct 2000, Russell King wrote:

> Linus Torvalds writes:
> > On Wed, 1 Nov 2000, Keith Owens wrote:
> > > LINK_FIRST is processed in the order it is specified, so a.o will be
> > > linked before z.o when both are present. See the patch.
> >
> > So why don't you do the same thing for obj-y, then?
> >
> > Why can't you do
> >
> > LINK_FIRST=$(obj-y)
> >
> > and be done with it?
>
> Hmm, so why don't we just call it obj-y and be done with it? ;)

That was going to be my next question if somebody actually said "sure".

The question was rhetorical, since the way LINK_FIRST is implemented means
that it has all the same problems that $(obj-y) has, and is hard to get
right in the generic case (but you can get it trivially right for the
subset case, like for USB).

Linus

2000-10-31 21:02:59

by John Alvord

[permalink] [raw]
Subject: Re: test10-pre7

On Tue, 31 Oct 2000 05:59:59 -0600, Peter Samuelson
<[email protected]> wrote:

>
>[Linus]
>> In short, we should _remove_ all traces of stuff like
>>
>> O_OBJS = $(filter-out $(export-objs), $(obj-y))
>>
>> It's wrong.
>>
>> We should just have
>>
>> O_OBJS = $(obj-y)
>>
>> which is always right.
>
>This part I agree with..
>
>> And it should make all this FIRST/LAST object file mockery a total
>> non-issue, because the whole concept turns out to be completely
>> unnecessary.
>>
>> Is there anything that makes this more complex than what I've
>> outlined above?
>
>One thing. The main benefit of $(sort), which I haven't heard you
>address yet, is to remove duplicate files. Think about 8390.o, and how
>many net drivers require it. There are two ways to handle this:
>
> obj-$(CONFIG_WD80x3) += wd.o 8390.o
> obj-$(CONFIG_EL2) += 3c503.o 8390.o
> obj-$(CONFIG_NE2000) += ne.o 8390.o
> obj-$(CONFIG_NE2_MCA) += ne2.o 8390.o
> obj-$(CONFIG_HPLAN) += hp.o 8390.o
>
You can avoid duplicates with
obj-$(CONFIG_WD80x3) += wd.o
ifneq (,$(findstring 8390.o,obj-$(CONFIG_WD80x3))
obj-$(CONFIG_WD80x3) += 8390.o
endif

Which is wordy but accomplishes the objective of avoiding duplicates.

john alvord

2000-11-01 00:05:59

by Dunlap, Randy

[permalink] [raw]
Subject: Re: test10-pre7 (LINK ordering)

Linus Torvalds wrote:
>
[snip]
>
> That was going to be my next question if somebody actually said "sure".
>
> The question was rhetorical, since the way LINK_FIRST is implemented
> means
> that it has all the same problems that $(obj-y) has, and is hard to get
> right in the generic case (but you can get it trivially right for the
> subset case, like for USB).


So now we have something in 2.4.0-test10, but there's
still a problem. Help is appreciated^W wanted. !!!

With CONFIG_USB=y and all other USB modules built as
modules (=m), linking usbdrv.o into the kernel image
gives this:

ld -m elf_i386 -T /work/linsrc/240-test10/arch/i386/vmlinux.lds -e stext
arch/i386/kernel/head.o arch/i386/kernel/init_task.o init/main.o
init/version.o \
--start-group \
arch/i386/kernel/kernel.o arch/i386/mm/mm.o kernel/kernel.o
mm/mm.o fs/fs.o ipc/ipc.o \
drivers/block/block.o drivers/char/char.o drivers/misc/misc.o
drivers/net/net.o drivers/media/media.o drivers/parport/parport.a
drivers/ide/idedriver.o drivers/scsi/scsidrv.o drivers/cdrom/cdrom.a
drivers/sound/sounddrivers.o drivers/pci/pci.a drivers/video/video.o
drivers/usb/usbdrv.o drivers/input/inputdrv.o drivers/i2c/i2c.o \
net/network.o \
/work/linsrc/240-test10/arch/i386/lib/lib.a
/work/linsrc/240-test10/lib/lib.a
/work/linsrc/240-test10/arch/i386/lib/lib.a \
--end-group \
-o vmlinux
drivers/usb/usbdrv.o(.data+0x2f4): undefined reference to
`__this_module'
make: *** [vmlinux] Error 1
[rdunlap@dragon linux]$


I believe that this is caused by drivers/usb/inode.c:

static DECLARE_FSTYPE(usbdevice_fs_type, "usbdevfs",
usbdevfs_read_super, 0);

in which this macro uses "THIS_MODULE". inode.c already #includes
module.h. What else does it need to do?
(inode.c is part of the usbcore in this case, so it shouldn't be
compiled with -DMODULE.)

Help ?!?

~Randy

2000-11-01 00:53:23

by Jeff Garzik

[permalink] [raw]
Subject: Re: test10-pre7 (LINK ordering)

Randy Dunlap wrote:
> With CONFIG_USB=y and all other USB modules built as
> modules (=m), linking usbdrv.o into the kernel image
> gives this:

> drivers/usb/usbdrv.o(.data+0x2f4): undefined reference to

Works for me here, .config attached. Local changes, merge error, or
similar? I don't have any local USB patches...

--
Jeff Garzik | "Mind if I drive?" -Sam
Building 1024 | "Not if you don't mind me clawing at the
MandrakeSoft | dash and shrieking like a cheerleader."
| -Max


Attachments:
config.bz2 (3.75 kB)

2000-11-01 02:33:51

by Peter Samuelson

[permalink] [raw]
Subject: Re: test10-pre7


[Linus]
> But it doesn't even WORK.
>
> You need to have
>
> LINK_FIRST1
> LINK_FIRST2
> LINK_FIRST3
> ...
>
> etc to get the proper ordering.

??? No you don't. Perhaps you mean something else. Here's how
LINK_FIRST works:

Say you have foo.o, bar.o, baz.o and lots of other objects. foo.o must
come before bar.o which must come before baz.o which must come before
some other object. But of course all of the above are conditional:
they can be configured as modules, or not at all.

LINK_FIRST := foo.o bar.o baz.o

obj-$(CONFIG_BAR) += bar.o
obj-$(CONFIG_BAZ) += baz.o
obj-$(CONFIG_BLURFL) += blurfl.o
obj-$(CONFIG_FOO) += foo.o
obj-$(CONFIG_...).....

Problem solved. If CONFIG_FOO=y CONFIG_BAR=n CONFIG_BAZ=y etc, link
order is

foo.o baz.o {everything else}

In short, LINK_FIRST/LINK_LAST take care of any case I can think of in
the kernel. Including things like "buslogic and aha174x must come
before aha1520, but the two parallel zip drivers must come last in
drivers/scsi because you don't want to renumber scsi drives more than
you have to" or "certain ISA cards must come after ne.c because of
autoprobe lockups on cheap ne clones, but ne2kpci should come *before*
ne so ne won't get the pci cards".

> In many other cases, like SCSI, we need almost _total_ ordering. For such
> a case, theer is no "first" or "last" - there is a well-specific ORDER.

I don't understand why we need *total* ordering -- I am only aware of a
few specific requirements.

Anyway, we still need to remove duplicates. NCR53c9x.o appears a lot.
If we can make all those cases go away by use of CONFIG_SCSI_53C9X or
something, I will be a lot happier. Your proposed method,

obj-53C9X-$(CONFIG_SCSI_MCA_53C9X) := y
obj-53C9X-$(CONFIG_SCSI_OKTAGON) := y
obj-$(obj-53C9X-y) := y
obj-$(obj-53C9X-m) := m

is definitely less ugly than a lot of what we do now ... but I still
don't like it. Mostly because each shared object file creates
special-case code in the makefile.

> The only way it would work is to make LINK_FIRST maintain the order,
> but once you do that LINK_FIRST is completely superfluous, as it ends
> up being exactly the same as $(obj-y).

The theory behind LINK_FIRST is that most drivers do not care about
their order: the ones that do are the exception, not the rule. If in
fact you *do* care about the order of every last driver in the kernel,
then I agree that LINK_FIRST is a bad idea.

> So trust me, LINK_FIRST/LINK_LAST is not going to happen.

> And if you really want to remove duplicates, at worst we can even use
> an external program for it - which would solve all these things once
> and for all.

This is true. Mainly what we disagree on is which method is the bigger
kludge. (:

Peter

2000-11-01 02:36:21

by Keith Owens

[permalink] [raw]
Subject: Re: test10-pre7

On Tue, 31 Oct 2000 09:31:09 -0800 (PST),
Linus Torvalds <[email protected]> wrote:
>On Wed, 1 Nov 2000, Keith Owens wrote:
>>
>> LINK_FIRST is processed in the order it is specified, so a.o will be
>> linked before z.o when both are present. See the patch.
>
>So why don't you do the same thing for obj-y, then?
>
>Why can't you do
>
> LINK_FIRST=$(obj-y)
>
>and be done with it?

You are assuming that every object in obj-y has a link order
requirement. This is *not* true. To use your own example

O_OBJS is all objects.
OX_OBJS is the subset of O_OBJS that have SYMTABS.
LINK_FIRST is the subset of O_OBJS that have link order dependencies
and must be linked first if present.
LINK_LAST is the subset of O_OBJS that have link order dependencies
and must be linked last if present.

You see - OX_OBJS, LINK_FIRST, LINK_LAST are subset indicators which
modify the set of O_OBJS.

>You need to have
> LINK_FIRST1
> LINK_FIRST2
> LINK_FIRST3
> ...
>etc to get the proper ordering.

No. LINK_FIRST := $(LINK_FIRST1) $(LINK_FIRST2) $(LINK_FIRST3)
The existing declaration order is a linear list so LINK_FIRST can
always be a linear list, no need for multiple lists. If you really did
need multiple LINK_FIRSTn entries than the existing single order would
not be good enough either.

In almost all cases, LINK_FIRST will be one or two objects, LINK_LAST
will be zero, one or two objects. The rest of the objects will have no
link order dependencies. Some Makefiles already sort their obj-y list
because they have _zero_ link order requirements, they have no problems.

Look at the possible cases :-

* No link order requirements. Do not specify LINK_FIRST/LAST.

Object A must precede B, C must precede D, no other dependencies, in
particular A and C can be in any order, B and D can be in any order.
LINK_FIRST := A.o C.o
or
LINK_FIRST := C.o A.o
You do not specify _all_ the ordering, just the ones that must come
first. The rest of the order drops out automatically.

* Card foo is supported by drivers baz, bar, foop. Try baz last.
LINK_LAST := baz.o.
You do not specify _all_ the ordering, just the ones that must come
last. The rest of the order drops out automatically.

* SCSI. This is poorly documented (one of the problems that LINK_xxx
will solve) but AFAIK the requirements are
buslogic must be before aha1542
NCR53c406a must be before qlogic
st, sd_mod, sr_mod, sg must be after all drivers.
LINK_FIRST := BusLogic.o NCR53c406a.o
LINK_LAST := st.o sd_mod.o sr_mod.o sg.o

>In many other cases, like SCSI, we need almost _total_ ordering. For such
>a case, theer is no "first" or "last" - there is a well-specific ORDER.

I refuse to believe that SCSI needs a total order. There are only a
few inter driver problems that require the probe to run in a specific
order. The rest of the drivers can run in any order.

If all of the 82 config options in SCSI really need to be in that exact
order, where is it documented and why do they need to be in that order?
Having a single fixed probe order to handle machines with mutiple types
of SCSI cards is not a good enough reason. People with multiple SCSI
cards already change the order of scsi entries to get the probe order
that suits them, LINK_FIRST will make that even easier.

2000-11-01 03:07:42

by Peter Samuelson

[permalink] [raw]
Subject: Re: test10-pre7


[Russell King]
> Since someone kindly enlightened me that LINK_FIRST was unsorted, I'm
> finding it very hard to grasp what the difference is between an
> unsorted LINK_FIRST and unsorted LINK_LAST list, and an unsorted
> obj-y list. From what I understand, obj-y = $(LINK_FIRST)
> $(LINK_LAST) ?

Not quite. If that's how you understand it, I see why you think it's a
bad idea. Here's what is *really* happening:

obj-y = {subset of LINK_FIRST that is in obj-y} \
{subset of obj-y that is not in LINK_FIRST or LINK_LAST} \
{subset of LINK_LAST that is in obj-y}

GNU make has extensions that make this easy to implement -- no more
verbose than the pseudocode, in fact.

The biggest difference between LINK_FIRST and obj-y is that LINK_FIRST
is meant to be a static list, not dependent on CONFIG_*, and specifies
*only* those objects which must be linked before (or after, for
LINK_LAST) other objects. In the common case, most object files do
*not* appear in LINK_FIRST or LINK_LAST, but just in O_OBJS.

In the pathological case of strict requirements for the whole
directory, LINK_FIRST would contain all of obj-y. Keith and I think
this is a rare case -- a more common case is the opposite:
LINK_FIRST/LAST are empty because there are *no* ordering requirements.


Again, anything that appears in O_OBJS but not in LINK_FIRST is linked
in arbitrary order. Anything that appears in LINK_FIRST but not in
O_OBJS is ignored. That is why it can be a static list.

Since LINK_FIRST is a (usually short) static list, it is easy for the
author to guarantee that it has no duplicate files in it. By contrast,
O_OBJS (or obj-y) frequently has duplicates, because of things like

obj-$(CONFIG_FOO) := foo.o xxx.o
obj-$(CONFIG_BAR) := bar.o xxx.o

where xxx.o is something like 8390 support for network cards.

Removing duplicates is a side effect of the GNU make 'sort' function,
which is THE ONLY REASON we want to sort $(O_OBJS). The reordering is
the "other" side effect, the less desirable one. GNU make does not
provide a 'uniq-without-sort' function, and while one is trivial to
write in e.g. shell, some of us consider a shell hack to be, well, more
hackish than LINK_FIRST.

** BTW, the only reason I'm still posting to this thread, which seems
pretty moot because "Linus Has Spoken", is that I believe there is
still a lot of misunderstanding about what LINK_FIRST actually does.
When I'm satisfied that the opponents truly *understand* LINK_FIRST
and still oppose it, I'll shut up.

Peter

2000-11-01 03:15:32

by Peter Samuelson

[permalink] [raw]
Subject: Re: test10-pre7


[hpa]
> I was going to ask to what extent we genuinely need sorting, and if
> we might be better off trying to eliminate that need as much as
> possible.

We don't need sorting. We need removing of duplicates. The GNU make
sort function removes duplicates as a side effect, which is why we want
to use it.

As has been discussed, there are lots of ways to remove duplicates.
You can spawn a shell script, you can keep track of each "common" file
with a tristate make variable, you can play with deeply nested if
statements, and rumor has it you may be able to write a custom GNU make
function (though I have doubts, as I have tried this before and
couldn't get anything to work).

To Keith, Michael and me, the cleanest way to remove duplicates is
$(sort). Since some object files must *not* be sorted, we came up with
a simple, readable way to declare that certain things had to come in a
certain order -- the idea being that most of the time it would not be
needed. Linus disagrees that our solution is simple, readable or
otherwise desirable. That's basically the whole issue in a nutshell.

Peter

2000-11-01 03:31:12

by Peter Samuelson

[permalink] [raw]
Subject: Re: test10-pre7


[Peter Samuelson]
> > There are two ways to handle this:
> >
> > obj-$(CONFIG_WD80x3) += wd.o 8390.o
> > obj-$(CONFIG_EL2) += 3c503.o 8390.o
> > obj-$(CONFIG_NE2000) += ne.o 8390.o
> > obj-$(CONFIG_NE2_MCA) += ne2.o 8390.o
> > obj-$(CONFIG_HPLAN) += hp.o 8390.o

[John Alvord <[email protected]>]
> You can avoid duplicates with
> obj-$(CONFIG_WD80x3) += wd.o
> ifneq (,$(findstring 8390.o,obj-$(CONFIG_WD80x3))
> obj-$(CONFIG_WD80x3) += 8390.o
> endif
>
> Which is wordy but accomplishes the objective of avoiding duplicates.

I said "there are two ways to handle this". You snipped the second,
which was:

> > ...Or do horrible games with 'if' statements and temporary
> > variables with names like $(NEED_8390) to ensure that it gets
> > included once if needed and not if not -- thereby pretty much
> > defeating the readability of the new-style makefiles.

I would consider your approach a variant of the "horrible games with if
statements and temporary variables". (:

Here's an exercise to the reader: reformat drivers/net/Makefile using
John Alford's approach, diff the two, and take a look. Then come back
and tell me LINK_FIRST -- 0-2 lines in the Makefile depending on your
ordering requirements, plus about five lines in Rules.make (*yes*, it
really is that simple) -- is really uglier.

Peter

2000-11-01 06:13:50

by H. Peter Anvin

[permalink] [raw]
Subject: Re: test10-pre7

Peter Samuelson wrote:
>
> To Keith, Michael and me, the cleanest way to remove duplicates is
> $(sort). Since some object files must *not* be sorted, we came up with
> a simple, readable way to declare that certain things had to come in a
> certain order -- the idea being that most of the time it would not be
> needed. Linus disagrees that our solution is simple, readable or
> otherwise desirable. That's basically the whole issue in a nutshell.
>

I would tend to agree with Linus on that. If that's truly what you're
doing, it would be rather nonobvious.

But the question, perhaps, is when does ordering matter. I'm a little
concerned about things highly dependent on link ordering.

-hpa

--
<[email protected]> at work, <[email protected]> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt

2000-11-01 06:19:53

by Lyle Coder

[permalink] [raw]
Subject: Linus's poll variation

Hello,
Is someone working on Linus's poll variation discussed in this list a week
ago?

Thanks
Lyle

2000-11-01 06:31:47

by Peter Samuelson

[permalink] [raw]
Subject: Re: test10-pre7


[hpa]
> I would tend to agree with Linus on that. If that's truly what
> you're doing, it would be rather nonobvious.

Well, ok, opinion vs. opinion. The thing is, userspace code almost
*never* needs to care about link order -- and, not counting boot loader
magic, kernel code didn't care about it either until 2.3.16 or so
(debut of __initcall). The reason link order suddenly became an issue
is that people started cutting out explicit lists of init code like
net/Space.c and relying on ld. All in all that was a Good Thing -- it
made code more modular, etc.

My point here is that in most of the world, people are not accustomed
to caring about link order. So I don't see why it's seen as such a
horrible thing to now say "ok link order is once again something you
ideally should not care about, *but* if there's a really good reason
otherwise, add your entry to LINK_FIRST." Because the link-order-
*does*-matter paradigm is less than a year old, here.

> But the question, perhaps, is when does ordering matter. I'm a
> little concerned about things highly dependent on link ordering.

Agreed. And that is another point: we (Keith, Michael and I wrote all
this expecting link order to usually *not* matter -- LINK_FIRST was
just to cover corner cases. Then we get Linus back saying the whole
drivers/scsi/ must be ordered *just so* (presumably so people don't
have to use boot flags for their multiple scsi host adapters) which is
either an unproven claim or, since it's Linus, a fiat. (Which is OK,
don't get me wrong, better him than me making these decisions!)

And as for "rather nonobvious" -- it's not as if our stuff isn't
documented, at least. Keith's patch includes several paragraphs added
to Documentation/kbuild/makefiles.txt, a file every kernel developer
should read anyway so that they know how to write correct makefiles.

Oh well. Life goes on. We can do fine without LINK_FIRST, but
possibly at the cost of increased cruft in the rest of the makefile
system. (That depends on taste, I suppose.)

Peter

2000-11-01 07:42:43

by Peter Samuelson

[permalink] [raw]
Subject: Re: test10-pre7


[Vladislav Malyshkin <[email protected]>]
> Also, the function remove_duplicates can be written using make rules
> and functions. Using functions "foreach" "if" from make and
> comparison you can easily build a function remove_duplicates in make,
> no shell involved.

Could you please write me this function? I am curious to see how you
do it.

I am also a bit skeptical. About 3 months ago, I thought it would be
possible to do this, so I spent a few hours fiddling around and reading
documentation. I failed; nothing I tried worked.

> so instead of $(sort) your will have $(remove_duplicates) written
> entirely in make.

That would make me happy.

Peter

2000-11-01 12:47:20

by Alan

[permalink] [raw]
Subject: Re: test10-pre7

> of SCSI cards is not a good enough reason. People with multiple SCSI
> cards already change the order of scsi entries to get the probe order
> that suits them, LINK_FIRST will make that even easier.

Why not solve the generic problem. If you give a list of requirements to
tsort one per line it will tell you the order. You just need to turn a set
of ordering needs into a tsort input

Alan

2000-11-01 16:27:49

by Dan Kegel

[permalink] [raw]
Subject: Re: Linus's poll variation

Lyle asked:
> Is someone working on Linus's poll variation discussed in this list a week ago?

I think the interface still needs some discussion.
The interface Linus proposed is IMHO oriented towards ease of
kernel implementation, and doesn't appear to be easy to use
for all applications.
cf. recent posts by John Gardiner Myers <[email protected]>
e.g.
http://boudicca.tux.org/hypermail/linux-kernel/2000week44/0966.html
http://boudicca.tux.org/hypermail/linux-kernel/2000week45/0212.html
IMHO kqueue()/kevent() is closer to what the apps want.

Mike Jagdis, however, has done some work on speeding up the
existing poll() system call, and has an eye on implementing
the /dev/poll interface. See
http://boudicca.tux.org/hypermail/linux-kernel/2000week45/0266.html
- Dan

2000-11-03 16:31:35

by Vladislav Malyshkin

[permalink] [raw]
Subject: Re: test10-pre7

Hi, Peter

> [Vladislav Malyshkin <[email protected]>]
> > Also, the function remove_duplicates can be written using make rules
> > and functions. Using functions "foreach" "if" from make and
> > comparison you can easily build a function remove_duplicates in make,
> > no shell involved.
>
> Could you please write me this function? I am curious to see how you
> do it.
>
> I am also a bit skeptical. About 3 months ago, I thought it would be
> possible to do this, so I spent a few hours fiddling around and reading
> documentation. I failed; nothing I tried worked.
>
> > so instead of $(sort) your will have $(remove_duplicates) written
> > entirely in make.
>
> That would make me happy.

Absense of recursive macros in make makes the problem not that clear.
An alternative may be if to put \n into variable value,
so the make commands will be automatically generated
---- like this
remove_duplicates = $(2) := ; $(foreach tmpvar1,$(1),$(2) += $$(if
$$(findstring $(tmpvar1),$$($(2))),,$(tmpvar1)); )
$(call remove_duplicates, x y a a c c a b c,ABCD)
--- in this example the variable ABCD is set to the string without
duplicates
but make seems does not allow \n as a value and does not understand
several assignments in one line, so
A = B ; C = D
does not work as expected.

So the best what we can get without using shell is below (the code and
usage example)
The function $(call remove_duplicates,string with duplicates)
removes the duplicates from the string.

# joins four strings
join4 = $(join $(join $(1),$(2)),$(join $(3),$(4)))
# generates indexes
numbers = $(foreach x4,0 1 2 3 4 5 6 7 8 9,\
$(strip $(foreach x3,0 1 2 3 4 5 6 7 8 9,\
$(strip $(foreach x2,0 1 2 3 4 5 6 7 8 9,\
$(strip $(foreach x1,0 1 2 3 4 5 6 7 8 9,\
$(strip $(if $(findstring $(call join4,$(x4),$(x3),$(x2),$(x1)),0000),,\
$(if $(word $(call join4,$(x4),$(x3),$(x2),$(x1)),$(1)),\
$(call join4,$(x4),$(x3),$(x2),$(x1)))))))))))))
# generates indexes
numbers1 = $(wordlist 1,$(words $(wordlist 2,$(words $(1)),$(1))),\
$(call numbers,$(1)))
# Remove duplicates from a line of up to 10000 words
remove_duplicates = $(strip $(firstword $(1)) \
$(foreach tmpvar,$(call numbers1,$(1)),\
$(if $(findstring \
$(word $(tmpvar),$(wordlist 2,$(words $(1)),$(1))),\
$(wordlist 1,$(tmpvar),$(1))),,\
$(word $(tmpvar),$(wordlist 2,$(words $(1)),$(1))))))

f := x x y a b c d x e y jj jj j2 j2 j2 j7
all:
echo '$(call remove_duplicates,$(f))'