Re: CVS commit: src/sys/arch/x86/x86
On 08.07.2018 17:44, Kamil Rytarowski wrote: > I will try to scratch a new header unaligned.h with the set of macros > and submit it to evaluation. I've prepared a scratch of unaligned.h with get_unaligned(): http://netbsd.org/~kamil/kubsan/unaligned.h There are at least two problems to proceed: 1. GCC 8.x is required for no_sanitizer attributes https://gcc.gnu.org/gcc-8/changes.html This version will also ship with NetBSD code for sanitization. The basesystem GCC version in HEAD (v. 6.4.0) is too old. 2. get_unaligned() is a fundamental type oriented (char, int, long etc) A large part of the issues detected in the kernel are due to a misaligned pointer to a struct passed (like disklabel or in6_addr). I think that these cases shall to be addressed directly in the kernel code and treated as buggy. I'm deferring right now the work on unaligned.h and wait for a required minimum version of GCC in the base. I will keep KUBSan reports as non-fatals for now. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/sys/arch/x86/x86
On 09.07.2018 16:58, Thor Lancelot Simon wrote: > On Mon, Jul 09, 2018 at 12:24:15PM +0200, Kamil Rytarowski wrote: >> >> The C11 standard could indeed use consistent wording. In one place >> "correctly aligned" in other alignment "restrictions" and >> "requirements". None of these terms is marked as a keyword or term and I >> read them in the context of the document as the same phenomenon (I >> haven't found a different interpretation of this in the wild). > > Right, but, architecturally, x86 doesn't have these "restrictions" or > "requirements". Not for correctness, not with the overwhelming majority > of integer instructions. Only (sometimes) for performance. > I have an experience of boosting an application processing packets, that moving members of a struct boosted the performance of an application by 20%. In my case it was not misalignment, but introduction of gaps into structs and not fitting them into CPU cache. > Which makes relying on the standard's language about unaligned access > incorrect. There's no undefined behavior here because there can't be; > strict alignment is not a feature of the target architecture. > I think that is interpretation of undefined behavior in C to what will happen on x86. Kernel UBSan checks the former and treats it as a bug. With some assumptions we can predict what will happen in the latter case... without the assumptions we aren't safe on x86 either (like the EFLAGS bit state) > I understand we have some code that's on the borderline between being MI > and MD, or that's "MD but for several different architectures". ACPICA > is an obvious example. And that such code may legitimately be compiled for > targets where unaligned access is not architecturally valid, and compilers > are free to do odd things with code that tries to do it. But the right > thing to do, I think, is to acknowledge that such code is MI; not to, by > misguided policy, insist that _all_ "MD" code should be written as if it > were MI. > put_misaligned()/get_misaligned() will handle these special cases, without changing algorithms in the code (already used in DRMKMS). In my test-case there are 7 subsystems or kernel features that trigger KUBSan. Access will be marked with proper compiler decoration and the problem will be gone. If we will detect that in some scenario during fuzzing we can trigger misaligned read in e.g. msdos fs, we will know why reading the same the code breaks on RPi. > Because if you head down that road, we're going to be forced to > write a bunch more stuff in assembler that we'd figured out over the decades > how to write in a performant and readable way in C; and I don't think anyone > benefits from having more asm in our kernel. > There is no danger of any major rewrite of anything, mostly small patches here and there. Also we already down to misaligned access reports. > Thor > signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/sys/arch/x86/x86
In article <20180709145848.ga21...@panix.com>, Thor Lancelot Simon wrote: >On Mon, Jul 09, 2018 at 12:24:15PM +0200, Kamil Rytarowski wrote: >> >> The C11 standard could indeed use consistent wording. In one place >> "correctly aligned" in other alignment "restrictions" and >> "requirements". None of these terms is marked as a keyword or term and I >> read them in the context of the document as the same phenomenon (I >> haven't found a different interpretation of this in the wild). > >Right, but, architecturally, x86 doesn't have these "restrictions" or >"requirements". Not for correctness, not with the overwhelming majority >of integer instructions. Only (sometimes) for performance. Unless you change one bit in the PSL and then the above is wrong: #define PSL_AC 0x0004 /* alignment check flag */ christos
Re: CVS commit: src/sys/arch/x86/x86
On Mon, Jul 09, 2018 at 12:24:15PM +0200, Kamil Rytarowski wrote: > > The C11 standard could indeed use consistent wording. In one place > "correctly aligned" in other alignment "restrictions" and > "requirements". None of these terms is marked as a keyword or term and I > read them in the context of the document as the same phenomenon (I > haven't found a different interpretation of this in the wild). Right, but, architecturally, x86 doesn't have these "restrictions" or "requirements". Not for correctness, not with the overwhelming majority of integer instructions. Only (sometimes) for performance. Which makes relying on the standard's language about unaligned access incorrect. There's no undefined behavior here because there can't be; strict alignment is not a feature of the target architecture. That means that a compiler which emits broken code if it encounters such a pointer is the broken thing -- *not the source code* -- and whanging around x86 MD code in our tree to accomodate such a compiler would be broken too. I understand we have some code that's on the borderline between being MI and MD, or that's "MD but for several different architectures". ACPICA is an obvious example. And that such code may legitimately be compiled for targets where unaligned access is not architecturally valid, and compilers are free to do odd things with code that tries to do it. But the right thing to do, I think, is to acknowledge that such code is MI; not to, by misguided policy, insist that _all_ "MD" code should be written as if it were MI. Because if you head down that road, we're going to be forced to write a bunch more stuff in assembler that we'd figured out over the decades how to write in a performant and readable way in C; and I don't think anyone benefits from having more asm in our kernel. Thor
Re: CVS commit: src/sys/arch/x86/x86
On 09.07.2018 11:32, Martin Husemann wrote: > On Mon, Jul 09, 2018 at 11:00:15AM +0200, Kamil Rytarowski wrote: >> According to my understanding, alignment requirement for a type/object >> is implementation defined (6.2.8); however during the process of >> converting types, if the returned pointer is not correctly aligned the >> result is undefined behavior (6.3.2.3 p7). > > My point is: I see no connection in the standard between "correctly aligned" > (in 6.3.2.3 p7) and the implementation defined "Alignment of objects" (as > measured by _Alginof). There are various types of alignments defined in > 6.2.8 but no "correct" alignment. > > Martin > The C11 standard could indeed use consistent wording. In one place "correctly aligned" in other alignment "restrictions" and "requirements". None of these terms is marked as a keyword or term and I read them in the context of the document as the same phenomenon (I haven't found a different interpretation of this in the wild). C++11 specification document uses more consistently "alignment requirement" wording (3.11), however there is other wording in use "proper alignment", "suitable aligned", "aligned appropriately" to describe the same phenomenon in my interpretation. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/sys/arch/x86/x86
On Mon, Jul 09, 2018 at 11:00:15AM +0200, Kamil Rytarowski wrote: > According to my understanding, alignment requirement for a type/object > is implementation defined (6.2.8); however during the process of > converting types, if the returned pointer is not correctly aligned the > result is undefined behavior (6.3.2.3 p7). My point is: I see no connection in the standard between "correctly aligned" (in 6.3.2.3 p7) and the implementation defined "Alignment of objects" (as measured by _Alginof). There are various types of alignments defined in 6.2.8 but no "correct" alignment. Martin
Re: CVS commit: src/sys/arch/x86/x86
On 09.07.2018 10:09, Martin Husemann wrote:
> On Sun, Jul 08, 2018 at 03:30:36PM +0200, Kamil Rytarowski wrote:
>> Misaligned pointer is explicitly documented as undefined behavior in the
>> C standard (C11 6.3.2.3 p7). (In C++ it's basically the same.)
>
> Yes, but the standard dos not define what a misaligned pointer is
> (or "correctly aligned").
>
> It talks about the prefered alignment that implementations select and
> that you can query with _Alignof - but there is no connection of that
> value to the section you refer. Some implementations require minimal
> alignement like their _Alignof returns, for others "correctly aligned" is
> always true.
>
> I might be missing something though.
>
> Martin
>
According to my understanding, alignment requirement for a type/object
is implementation defined (6.2.8); however during the process of
converting types, if the returned pointer is not correctly aligned the
result is undefined behavior (6.3.2.3 p7).
int16_t (short for LP64) and int8_t (char) have different alignment
requirements on NetBSD/amd64. char has the weakest one (6.2.8.6).
$ cat test.c
#include
int
main(int argc, char **argv)
{
printf("alignof(char)=%zu\n", __alignof__(char));
printf("alignof(short)=%zu\n", __alignof__(short));
return 0;
}
$ ./a.out
alignof(char)=1
alignof(short)=2
If one alignment is larger than the other one, it's called stricter one
(6.2.8.7).
This means that mpbios_page (int8_t*) has weaker alignment than memtop
(int16_t*) and this cast is undefined behavior, because the returned
pointer is not correctly aligned for the destination type (as
&mpbios_page[0x413] violates it).
signature.asc
Description: OpenPGP digital signature
Re: CVS commit: src/sys/arch/x86/x86
On Sun, Jul 08, 2018 at 03:30:36PM +0200, Kamil Rytarowski wrote: > Misaligned pointer is explicitly documented as undefined behavior in the > C standard (C11 6.3.2.3 p7). (In C++ it's basically the same.) Yes, but the standard dos not define what a misaligned pointer is (or "correctly aligned"). It talks about the prefered alignment that implementations select and that you can query with _Alignof - but there is no connection of that value to the section you refer. Some implementations require minimal alignement like their _Alignof returns, for others "correctly aligned" is always true. I might be missing something though. Martin
Re: CVS commit: src/sys/arch/x86/x86
>>> Misaligned pointer is explicitly documented as undefined behavior >>> in the C standard (C11 6.3.2.3 p7). >> So what? Do you have reason to think that sys/arch/x86 will at some >> point be ported to a compiler [] that does something unexpected with >> that code? > Due to UB a compiler is free to perform optimization and treat x86 > like a RISC architecture... Indeed. But a compiler is also free to not do so. Writing machine-dependent code in something other than assembler is dependent on having a compiler that produces the desired result. A compiler that doesn't, whether because it's broken or because it takes advantage of latitude in the languageg spec, simply is not suitable for that use. /~\ The ASCII Mouse \ / Ribbon Campaign X Against HTMLmo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: CVS commit: src/sys/arch/x86/x86
On 08.07.2018 17:30, Mouse wrote: > Caveat: this is all opinion. I'm not the one doing the work here. > > src/sys/arch/x86/x86: mpbios.c > > Remove unaligned access to mpbios_page[] > > sys/arch/x86/x86/mpbios.c:308:11, load of misaligned address > 0x800031c7a413 for type 'const __uint16_t' which requires 2 byte > alignment > >>> Can we please do NOT do such stupid changes? > >> Kernel Undefined Behavior Sanitizer detects various portability >> issues including alignment. > > Portability issues are, in general, not issues when they are in code > that is inherently already nonportable - such as almost everything > under sys/arch/. > >> Misaligned pointer is explicitly documented as undefined behavior in >> the C standard (C11 6.3.2.3 p7). > > So what? Do you have reason to think that sys/arch/x86 will at some > point be ported to a compiler (I would say "and architecture", but it's > already tightly bound to a very small set of CPU architectures) that > does something unexpected with that code? Expecting the MD code in the > low levels of an OS to never produce formally implementation-defined or > undefined behaviour is a fool's dream. > > Programs such as undefined-behaviour detectors are tools to serve us, > not shackles to bind us. Intelligence should be applied when using > their results, including not expecting portability from inherently > nonportable code. > Due to UB a compiler is free to perform optimization and treat x86 like a RISC architecture... especially with more instruction sets like SSE (as noted by Christos). x86 sanitizers experienced alignment trouble too. MOVDQA used to break sanitizers in the interceptor of __tls_get_addr(). https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58066 I will try to propose a macro as noted in a reply to Jason Thorpe. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/sys/arch/x86/x86
On 08.07.2018 17:16, Jason Thorpe wrote: > > >> On Jul 8, 2018, at 6:30 AM, Kamil Rytarowski wrote: >> >> In future __NO_STRICT_ALIGNMENT could be defined for aarch64, at least >> for the use of acpica (which still contains a fallback for Itanium >> without misaligned access, but not actively maintained). >> >> Linux uses a different approach and ships get_unaligned() and >> set_unaligned() macros and implements it on per ABI and CPU-basis. > > In general, I get the utility of UBSan, but for these unaligned access > issues, an ad hoc approach like what was done in mpbios is the wrong way to > go. > > Here's my $0.02: > > -- Define a set of standard unaligned-accessors/mutators. I propose: > > uint16_t__unaligned_load16(const uint16_t *); > uint32_t__unaligned_load32(const uint32_t *); > uint64_t__unaligned_load64(const uint64_t *); > > void__unaligned_store16(uint16_t *, uint16_t); > void__unaligned_store32(uint32_t *, uint32_t); > void__unaligned_store64(uint64_t *, uint64_t); > > ...and maybe you need to have another set for the signed value flavor, dunno. > (I guess probably, because you want to preserve the type information as much > as possible ... no casting.) > > -- Implement them as static inlines in a suitable system header > (, maybe, although these types are types, yah? So, > adjust the types to __uint16_t or whatever as necessary). Decorate only > these static inlines with the "don't sanitize me" directives. Implementing > them as inline functions rather than macros has 2 benefits: avoids unintended > multiple-evaluation of arguments, allows the smallest possible "don't > sanitize" footprint. > > -- Implement 2 standard sets, one for __NO_STRICT_ALIGNMENT platforms, and > one for the strict-alignment case. > I did something similar once upon a time when I was working on libo(verflow). https://github.com/krytarowski/libo/blob/nbsd/overflow.h There is a compiler switch for all C types: char, signed char, short, int, long, long long, unsigned char, unsigned short, unsigned int, unsigned long, unsigned long long. However at that point of time, C11 (_Generic) wasn't adopted so widely and pcc was in more active development so I've deferred the libo library for future. Today a compiler without C11 is not an issue. I can reuse the same idea and template (no licensing issues) for unaligned accessor and I will propose a patch. BTW. we are using a subset of the get_unaligned() feature macro in DRMKMS -- sys/external/bsd/drm2/include/asm/unaligned.h this file is even (c) TNF. I will try to scratch a new header unaligned.h with the set of macros and submit it to evaluation. > -- For changes like what would be needed in acpica, GET INPUT FROM THE > UPSTREAM as to how to integrate use of these macros, and preferably push the > changes to the upstream FIRST so that we can simply import them in a regular > update. > The discussion about the ACPICA case is ongoing: https://github.com/acpica/acpica/issues/393 "NetBSD Kernel Undefined Behavior Sanitizer: acpica reports" It will probably end up as a new macro for decoration of a function. Harry (luserx0) is working on it as a GSoC (sub)task. I intend to import the fix for acpica together with a new version of this software. > -- thorpej > signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/sys/arch/x86/x86
Caveat: this is all opinion. I'm not the one doing the work here. src/sys/arch/x86/x86: mpbios.c Remove unaligned access to mpbios_page[] sys/arch/x86/x86/mpbios.c:308:11, load of misaligned address 0x800031c7a413 for type 'const __uint16_t' which requires 2 byte alignment >> Can we please do NOT do such stupid changes? > Kernel Undefined Behavior Sanitizer detects various portability > issues including alignment. Portability issues are, in general, not issues when they are in code that is inherently already nonportable - such as almost everything under sys/arch/. > Misaligned pointer is explicitly documented as undefined behavior in > the C standard (C11 6.3.2.3 p7). So what? Do you have reason to think that sys/arch/x86 will at some point be ported to a compiler (I would say "and architecture", but it's already tightly bound to a very small set of CPU architectures) that does something unexpected with that code? Expecting the MD code in the low levels of an OS to never produce formally implementation-defined or undefined behaviour is a fool's dream. Programs such as undefined-behaviour detectors are tools to serve us, not shackles to bind us. Intelligence should be applied when using their results, including not expecting portability from inherently nonportable code. /~\ The ASCII Mouse \ / Ribbon Campaign X Against HTMLmo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: CVS commit: src/sys/arch/x86/x86
> On Jul 8, 2018, at 6:30 AM, Kamil Rytarowski wrote: > > In future __NO_STRICT_ALIGNMENT could be defined for aarch64, at least > for the use of acpica (which still contains a fallback for Itanium > without misaligned access, but not actively maintained). > > Linux uses a different approach and ships get_unaligned() and > set_unaligned() macros and implements it on per ABI and CPU-basis. In general, I get the utility of UBSan, but for these unaligned access issues, an ad hoc approach like what was done in mpbios is the wrong way to go. Here's my $0.02: -- Define a set of standard unaligned-accessors/mutators. I propose: uint16_t__unaligned_load16(const uint16_t *); uint32_t__unaligned_load32(const uint32_t *); uint64_t__unaligned_load64(const uint64_t *); void__unaligned_store16(uint16_t *, uint16_t); void__unaligned_store32(uint32_t *, uint32_t); void__unaligned_store64(uint64_t *, uint64_t); ...and maybe you need to have another set for the signed value flavor, dunno. (I guess probably, because you want to preserve the type information as much as possible ... no casting.) -- Implement them as static inlines in a suitable system header (, maybe, although these types are types, yah? So, adjust the types to __uint16_t or whatever as necessary). Decorate only these static inlines with the "don't sanitize me" directives. Implementing them as inline functions rather than macros has 2 benefits: avoids unintended multiple-evaluation of arguments, allows the smallest possible "don't sanitize" footprint. -- Implement 2 standard sets, one for __NO_STRICT_ALIGNMENT platforms, and one for the strict-alignment case. -- For changes like what would be needed in acpica, GET INPUT FROM THE UPSTREAM as to how to integrate use of these macros, and preferably push the changes to the upstream FIRST so that we can simply import them in a regular update. -- thorpej
Re: CVS commit: src/sys/arch/x86/x86
On 08.07.2018 15:53, Jaromír Doleček wrote: > Le dim. 8 juil. 2018 à 15:29, Kamil Rytarowski a écrit : >> I've introduced the change to mpbios.c as it was small, selfcontained >> and without the need to decorate the whole function. > > Am I reading the code wrong or you actually introduced bug in mpbios.c? > > Shouldn't this: > > memtop |= (uint16_t)mpbios_page[0x414] << 8; > > be actually << 16 to keep the same semantics? > > Jaromir > We are moving a 16-bit unaligned integer into a variable. The first byte is 0 bits shifted, the second one 8. According to my checks old and new versions produce the same result. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/sys/arch/x86/x86
Le dim. 8 juil. 2018 à 15:29, Kamil Rytarowski a écrit : > I've introduced the change to mpbios.c as it was small, selfcontained > and without the need to decorate the whole function. Am I reading the code wrong or you actually introduced bug in mpbios.c? Shouldn't this: memtop |= (uint16_t)mpbios_page[0x414] << 8; be actually << 16 to keep the same semantics? Jaromir
Re: CVS commit: src/sys/arch/x86/x86
On 08.07.2018 11:24, Martin Husemann wrote: > On Sun, Jul 08, 2018 at 10:49:53AM +0200, Jaromír Dole?ek wrote: >>> Module Name:src >>> Committed By: kamil >>> Date: Sat Jul 7 23:05:50 UTC 2018 >>> >>> Modified Files: >>> src/sys/arch/x86/x86: mpbios.c >>> >>> Log Message: >>> Remove unaligned access to mpbios_page[] >>> >>> Replace unaligned pointer dereference with a more portable construct that >>> is free from Undefined Behavior semantics. >>> >>> sys/arch/x86/x86/mpbios.c:308:11, load of misaligned address >>> 0x800031c7a413 for type 'const __uint16_t' which requires 2 byte >>> alignment > > > Can we please do NOT do such stupid changes? > > This is a bogus error message, please restore the original code! > > Martin > On the request by Martin, I'm moving this discussion to tech-kern. Kernel Undefined Behavior Sanitizer detects various portability issues including alignment. Misaligned pointer is explicitly documented as undefined behavior in the C standard (C11 6.3.2.3 p7). (In C++ it's basically the same.) Depending on the CPU, misaligned access can be either supported (with or without performance hit) or unsupported (CPU exception, reinterpret instruction to perform a different operation etc). For some CPUs it's possible to define a dedicated exception handler to align the data access on the performance cost. There is a NetBSD header symbol that marks certain architectures as capable to handle miaslignment: __NO_STRICT_ALIGNMENT. It's defined right now for m68k, vax, amd64 and i386. In future __NO_STRICT_ALIGNMENT could be defined for aarch64, at least for the use of acpica (which still contains a fallback for Itanium without misaligned access, but not actively maintained). Linux uses a different approach and ships get_unaligned() and set_unaligned() macros and implements it on per ABI and CPU-basis. Almost all of the remaining issues detected by Kernel UBsan come from unaligned memory access: Report from a boot in qemu: http://netbsd.org/~kamil/kubsan/0006-boot.txt Report from a boot on a real hardware (with drmkms disabled): http://netbsd.org/~kamil/kubsan/0007-boot-real-hardware.txt The plan with Kernel Sanitizers is to mark the issues detected with them as fatal, with a warning/fatal switch option for UBSan. Marking UBSan issues as fatal makes this sanitizer useful for fuzzing and regular checks for regressions (ATF execution), because it will detect the issues immediately or just detect at all. However in order to make Kernel UBSan more useful there is need to address the reports, some of them might be cautious. Handling cautious KUBSan reports can be treated similarly to handling compiler warnings. We are already marking a lot of variables as initialized, signed/unsigned, long/short or even properly intended. Some of them are overcautious. UBSan detects a similar set of problems during the runtime execution. Regarding the unaligned access, there is an option to: - mark certain kernel functions with no-sanitize flags, - address the reported issues if possible. I've introduced the change to mpbios.c as it was small, selfcontained and without the need to decorate the whole function. Various device drivers (vge(4), vr(4), sip(4), wdc(4) etc) contain fixups for misalignment access under ifdef. Other ones like bwfm(4) [1] contain fixups inlined and were detecting when a driver was in the process of porting from x86 to a port with strict alignment. Such issues as bwfm(4) could be detected with the sanitizer aid more easily. [1] commit 44bb30584312c2fa6bc0661178986c4965b67c0c Author: jmcneill Date: Fri Oct 20 23:38:21 2017 + Fix an alignment problem with scan results within an escan event signature.asc Description: OpenPGP digital signature
Re: Module auto-unloading (was Re: CVS commit: src/sys/arch/x86/x86)
On Tue, Oct 18, 2011 at 12:50:58PM -0400, Jachym Holecek wrote: > # David Young 2011-10-18: > > On Tue, Oct 18, 2011 at 11:28:22AM -0400, Jared McNeill wrote: > > > I played around with driver module autoloading a while back, and it > > > worked pretty well but the implementation I came up with required > > > duplicating match data in .plist. > > > > This sounds like a step in the right direction (recording match data > > in a plist, not duplicating it). What would it take to get rid of the > > non-plist match data? > > Funny enough that's exactly what my GSoC project did some years ago. > There was a lot of manual work to abstract all accesses to match data > behind an API, major rework of autoconf(4) (not sure how much of that > was actually necessary), simple change to config(1) to emit match data > as plist array -- that sort of thing. No big science, just a lot of > typing... The match data array for builtin drivers is going to be > fairly huge in XML encoding, it was much better with bplist. Wonderful. Can you generate a patch for review? Dave -- David Young OJC Technologies is now Pixo dyo...@pixotech.com Urbana, IL (217) 344-0444 x24
Re: Module auto-unloading (was Re: CVS commit: src/sys/arch/x86/x86)
# David Young 2011-10-18: > On Tue, Oct 18, 2011 at 11:28:22AM -0400, Jared McNeill wrote: > > I played around with driver module autoloading a while back, and it > > worked pretty well but the implementation I came up with required > > duplicating match data in .plist. > > This sounds like a step in the right direction (recording match data > in a plist, not duplicating it). What would it take to get rid of the > non-plist match data? Funny enough that's exactly what my GSoC project did some years ago. There was a lot of manual work to abstract all accesses to match data behind an API, major rework of autoconf(4) (not sure how much of that was actually necessary), simple change to config(1) to emit match data as plist array -- that sort of thing. No big science, just a lot of typing... The match data array for builtin drivers is going to be fairly huge in XML encoding, it was much better with bplist. BR, -- Jachym
Re: Module auto-unloading (was Re: CVS commit: src/sys/arch/x86/x86)
I think you'd just need to find a way to supply that data for built-in modules too. On Tue, 18 Oct 2011, David Young wrote: On Tue, Oct 18, 2011 at 11:28:22AM -0400, Jared McNeill wrote: I played around with driver module autoloading a while back, and it worked pretty well but the implementation I came up with required duplicating match data in .plist. This sounds like a step in the right direction (recording match data in a plist, not duplicating it). What would it take to get rid of the non-plist match data? Dave -- David Young OJC Technologies is now Pixo dyo...@pixotech.com Urbana, IL (217) 344-0444 x24
Re: Module auto-unloading (was Re: CVS commit: src/sys/arch/x86/x86)
On Tue, Oct 18, 2011 at 11:28:22AM -0400, Jared McNeill wrote: > I played around with driver module autoloading a while back, and it > worked pretty well but the implementation I came up with required > duplicating match data in .plist. This sounds like a step in the right direction (recording match data in a plist, not duplicating it). What would it take to get rid of the non-plist match data? Dave -- David Young OJC Technologies is now Pixo dyo...@pixotech.com Urbana, IL (217) 344-0444 x24
Module auto-unloading (was Re: CVS commit: src/sys/arch/x86/x86)
We could use the reference counter in struct cfdriver to keep driver modules "busy", but I'm not sure if the system can figure out what's unneeded if a "needed" driver might be one for a hotpluggable device. Would you treat those drivers differently? What about drivers (if_ath_pci comes to mind) that can be used both for fixed or hotplug devices? I played around with driver module autoloading a while back, and it worked pretty well but the implementation I came up with required duplicating match data in .plist. On Tue, 18 Oct 2011, Warner Losh wrote: On Oct 18, 2011, at 4:39 AM, Jared McNeill wrote: I would argue that any manually loaded module shouldn't be autounloaded. What do you think about flagging modules as autoloaded and only autounloading the autoloaded ones? If I "manually" load a dozen drivers at boot because I have a dozen different boards with different devices. I'd kinda like the system to "automatically" figure out what isn't needed and unload those drivers. Warner On Tue, 18 Oct 2011, Jukka Ruohonen wrote: On Tue, Oct 18, 2011 at 08:43:46AM +0200, Marc Balmer wrote: Am 18.10.11 06:27, schrieb Jukka Ruohonen: On Tue, Oct 18, 2011 at 12:07:45AM +, Jared D. McNeill wrote: Module Name:src Committed By: jmcneill Date: Tue Oct 18 00:07:45 UTC 2011 Modified Files: src/sys/arch/x86/x86: vmt.c Log Message: don't allow module autounload I wonder should autounloading be prohibited for all driver-class modules? Why? When the parent goes away, why not autounload a driver? I am not sure. But have we thought about all the consequences and corner- cases? Unloading happens while modifying hardware state? Deferred calls in the drivers? And so on? To me it also seems that if I manually load a driver-module, I expect it to stay loaded until I unload it. - Jukka.
