Re: svn commit: r266481 - head/sys/x86/x86
On May 21, 2014, at 10:03 AM, Konstantin Belousov wrote: >> > You changed the handling of the alignment, which is probably not correct. > The problematic parameter, if any, is boundary. Thanks, this crept in when I re-formatted the code for check-in. I’ll fix it. Scott signature.asc Description: Message signed with OpenPGP using GPGMail
Re: svn commit: r274489 - in head/sys/amd64: amd64 include
> On Nov 20, 2014, at 11:33 PM, Rui Paulo wrote: > > On Nov 13, 2014, at 14:11, Scott Long wrote: >> >> Author: scottl >> Date: Thu Nov 13 22:11:44 2014 >> New Revision: 274489 >> URL: https://svnweb.freebsd.org/changeset/base/274489 >> >> Log: >> Extend earlier addition of stack frames to most of support.S. This makes >> stack traces in KDB, HWPMC, and DTrace much more reliable and useful. > > No performance differences? The kernel enables/disables the compiler option > to omit the frame pointer based on the kernel config file. If DDB, DTrace, > or HWPMC is enabled, the frame pointer is always saved in C functions. > > Some of these functions are in the hot path, so if you didn't see any > performance problem, I wonder if we should disable -fomit-frame-pointer > always. That’s a good question to look further into. I didn’t see any measurable differences with this change. I think that the cost of the function call itself masks the cost of a few extra instructions, but I didn’t test with switching it on/off for the entire kernel. That said, I purposely implemented this as macros so it could be easily changed in the future. If someone finds that this measurably impacts a certain workload, I wouldn’t object to making it conditional, though it does complicate any hand-written ASM code that tries to access the stack via %esp offsets. We don’t have anything like that now, but Kip was concerned enough about it in the future that I left it enabled unconditionally. Scott ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r277693 - head/sys/boot/forth
This is completely arbitrary. Please revert it, or I will revert it in 48 hours. Scott > On Jan 25, 2015, at 4:57 AM, Dag-Erling Sm?rgrav wrote: > > Author: des > Date: Sun Jan 25 11:57:18 2015 > New Revision: 277693 > URL: https://svnweb.freebsd.org/changeset/base/277693 > > Log: > Fix the font in the text version. This has bothered me for a long time... > > MFC after: 1 week > > Modified: > head/sys/boot/forth/beastie.4th > head/sys/boot/forth/brand.4th > > Modified: head/sys/boot/forth/beastie.4th > == > --- head/sys/boot/forth/beastie.4th Sun Jan 25 08:16:51 2015 > (r277692) > +++ head/sys/boot/forth/beastie.4th Sun Jan 25 11:57:18 2015 > (r277693) > @@ -89,7 +89,7 @@ variable logoY > 0 25 at-xy > ; > > -: fbsdbw-logo ( x y -- ) \ "FreeBSD" logo in B/W (13 rows x 21 columns) > +: fbsdbw-logo ( x y -- ) \ "FreeBSD" logo in B/W (12 rows x 21 columns) > > \ We used to use the beastie himself as our default... until the > \ eventual complaint derided his reign of the advanced boot-menu. > @@ -106,17 +106,16 @@ variable logoY > 5 + swap 6 + swap > > 2dup at-xy ." __" 1+ > - 2dup at-xy ." | | __ ___ ___ " 1+ > - 2dup at-xy ." | |__ | '__/ _ \/ _ \" 1+ > - 2dup at-xy ." | __|| | | __/ __/" 1+ > - 2dup at-xy ." | | | | |||" 1+ > + 2dup at-xy ." | |" 1+ > + 2dup at-xy ." | |__ _ __ ___ ___ " 1+ > + 2dup at-xy ." | __|| '__/ _ \/ _ \" 1+ > + 2dup at-xy ." | | | | | __/ __/" 1+ > 2dup at-xy ." |_| |_| \___|\___|" 1+ > 2dup at-xy ." _ _" 1+ > 2dup at-xy ." | _ \ / | __ \" 1+ > 2dup at-xy ." | |_) | (___ | | | |" 1+ > 2dup at-xy ." | _ < \___ \| | | |" 1+ > 2dup at-xy ." | |_) |) | |__| |" 1+ > - 2dup at-xy ." | | | |" 1+ >at-xy ." |/|_/|_/" > > \ Put the cursor back at the bottom > > Modified: head/sys/boot/forth/brand.4th > == > --- head/sys/boot/forth/brand.4th Sun Jan 25 08:16:51 2015 > (r277692) > +++ head/sys/boot/forth/brand.4th Sun Jan 25 11:57:18 2015 > (r277693) > @@ -33,14 +33,13 @@ variable brandY > 2 brandX ! > 1 brandY ! > > -: fbsd-logo ( x y -- ) \ "FreeBSD" [wide] logo in B/W (7 rows x 42 columns) > +: fbsd-logo ( x y -- ) \ "FreeBSD" [wide] logo in B/W (6 rows x 42 columns) > > 2dup at-xy ." __ _ _ " 1+ > 2dup at-xy ." | | | _ \ / | __ \ " 1+ > 2dup at-xy ." | |___ _ __ ___ ___ | |_) | (___ | | | |" 1+ > 2dup at-xy ." | ___| '__/ _ \/ _ \| _ < \___ \| | | |" 1+ > 2dup at-xy ." | | | | | __/ __/| |_) |) | |__| |" 1+ > - 2dup at-xy ." | | | | |||| | | |" 1+ >at-xy ." |_| |_| \___|\___||/|_/|_/ " > > \ Put the cursor back at the bottom > ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r281451 - head/sys/vm
> On Apr 12, 2015, at 12:21 AM, Dmitry Chagin wrote: > > Author: dchagin > Date: Sun Apr 12 06:21:58 2015 > New Revision: 281451 > URL: https://svnweb.freebsd.org/changeset/base/281451 > > Log: > Rework r281162. Indeed, the flexible array member is preferable here. > > Suggested by: Justin T. Gibbs > > MFC after: 3 days > > Modified: > head/sys/vm/uma_core.c > head/sys/vm/uma_int.h There’s still something wrong with this. I have a machine with 28 cores (56 with hyperthreading) and 256GB of RAM, and ever since you committed r281162, it panics early in boot with a failed assertion. It looks like the first few members of a uma_slab_t are getting overwritten accidentally, and somehow the padding of the extra member in the uma_zone_t was previously protecting it. I don’t know the exact cause yet, but I must ask that you revert to r281161 in HEAD and stable/10 until the problem is resolved. Thanks, Scott ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r281451 - head/sys/vm
> On Apr 23, 2015, at 6:19 AM, Scott Long wrote: > >> >> On Apr 12, 2015, at 12:21 AM, Dmitry Chagin wrote: >> >> Author: dchagin >> Date: Sun Apr 12 06:21:58 2015 >> New Revision: 281451 >> URL: https://svnweb.freebsd.org/changeset/base/281451 >> >> Log: >> Rework r281162. Indeed, the flexible array member is preferable here. >> >> Suggested by: Justin T. Gibbs >> >> MFC after: 3 days >> >> Modified: >> head/sys/vm/uma_core.c >> head/sys/vm/uma_int.h > > There’s still something wrong with this. I have a machine with 28 cores (56 > with hyperthreading) and 256GB of RAM, and ever since you committed r281162, > it panics early in boot with a failed assertion. It looks like the first few > members of a uma_slab_t are getting overwritten accidentally, and somehow the > padding of the extra member in the uma_zone_t was previously protecting it. > I don’t know the exact cause yet, but I must ask that you revert to r281161 > in HEAD and stable/10 until the problem is resolved. > I think the problem is that the masterzone_k and masterzone_z objects that are statically allocated in uma_core.c no longer have space for the uz_cpu field, but uma_zalloc_arg() always assumes that it’s there. Early in boot when the ‘kegs' and ‘zones’ zones are being initialized, there’s only 1 CPU so pre-allocating 1 uz_cpu element in the uma_zone is enough. I can’t see any way around this without significantly changing how uma_zalloc_arg() treats per-cpu caches. I think it’s best to revert this change. Scott ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r281451 - head/sys/vm
> On Apr 23, 2015, at 1:28 PM, Chagin Dmitry wrote: > > On Thu, Apr 23, 2015 at 12:49:51PM -0600, Scott Long wrote: >> >>> On Apr 23, 2015, at 6:19 AM, Scott Long wrote: >>> On Apr 12, 2015, at 12:21 AM, Dmitry Chagin wrote: Author: dchagin Date: Sun Apr 12 06:21:58 2015 New Revision: 281451 URL: https://svnweb.freebsd.org/changeset/base/281451 Log: Rework r281162. Indeed, the flexible array member is preferable here. Suggested by: Justin T. Gibbs MFC after: 3 days Modified: head/sys/vm/uma_core.c head/sys/vm/uma_int.h >>> >>> There???s still something wrong with this. I have a machine with 28 cores >>> (56 with hyperthreading) and 256GB of RAM, and ever since you committed >>> r281162, it panics early in boot with a failed assertion. It looks like >>> the first few members of a uma_slab_t are getting overwritten accidentally, >>> and somehow the padding of the extra member in the uma_zone_t was >>> previously protecting it. I don???t know the exact cause yet, but I must >>> ask that you revert to r281161 in HEAD and stable/10 until the problem is >>> resolved. >>> >> >> I think the problem is that the masterzone_k and masterzone_z objects that >> are statically allocated in uma_core.c no longer have space for the uz_cpu >> field, but uma_zalloc_arg() always assumes that it???s there. Early in boot >> when the ???kegs' and ???zones??? zones are being initialized, there???s >> only 1 CPU so pre-allocating 1 uz_cpu element in the uma_zone is enough. I >> can???t see any way around this without significantly changing how >> uma_zalloc_arg() treats per-cpu caches. I think it???s best to revert this >> change. >> > Hi, > they initialized in uma_startup() and not used before. > I have a private converstion with a man which stable/10 hangs in > vm_mem_init().\ > with my commit. weird. > > I do not object to revert, but give me a chance to figure out what's going on. With INVARIANTS enabled, the system will panic. Without it, it will spin in vm_mem_init(), as you noted. Even if it’s not happening to everyone, it’s a serious problem for such a minor anticipated benefit. I must insist that it be reverted. Scott ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r281451 - head/sys/vm
> On Apr 25, 2015, at 1:06 AM, Chagin Dmitry wrote: > > On Fri, Apr 24, 2015 at 05:37:21AM -0700, Chris Torek wrote: >> True, it's not actually odd, it's just surprising the first time >> one comes across it. >> >> Also, I goofed in the text: >> With the flexible array, (sizeof(struct uma_cache)) is going to be 32 bytes smaller than without it. >> >> It's `struct uma_zone` that shrinks by (potentially) more than one >> would expect. (The uma_cache struct is itself 32 bytes and is not >> changed, so the args.size value should be the same -- implying that >> some OTHER "sizeof(struct uma_zone)" is where things are going wrong.) >> > vm_mem_init() called before uma init. unfortunately, we have not seen > a proper info about panic from scottl > -- > Have fun! > chd Too early in boot to get a crashdump. ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r281451 - head/sys/vm
> On Apr 25, 2015, at 1:59 PM, Dmitry Morozovsky wrote: > > On Sat, 25 Apr 2015, Scott Long via svn-src-all wrote: > >>>> True, it's not actually odd, it's just surprising the first time >>>> one comes across it. >>>> >>>> Also, I goofed in the text: >>>> >>>>>> With the flexible array, (sizeof(struct uma_cache)) is going to be >>>>>> 32 bytes smaller than without it. >>>> >>>> It's `struct uma_zone` that shrinks by (potentially) more than one >>>> would expect. (The uma_cache struct is itself 32 bytes and is not >>>> changed, so the args.size value should be the same -- implying that >>>> some OTHER "sizeof(struct uma_zone)" is where things are going wrong.) >>>> >>> vm_mem_init() called before uma init. unfortunately, we have not seen >>> a proper info about panic from scottl > >> Too early in boot to get a crashdump. > > ... and even console not set up enough to have a backtrace/some debugging > printfs displayed? Maybe you didn’t see the backtrace that was printed and included? What debugging printfs should I have predicted that you would want? Scott ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r281942 - head/sys/vm
> On Apr 25, 2015, at 2:30 AM, Chagin Dmitry wrote: > > On Fri, Apr 24, 2015 at 05:03:53PM +, Scott Long wrote: >> Author: scottl >> Date: Fri Apr 24 17:03:53 2015 >> New Revision: 281942 >> URL: https://svnweb.freebsd.org/changeset/base/281942 >> >> Log: >> Revert r281451. It causes a panic/hang early in boot for a number of >> users, myself included. The original code is likely papering over a >> larger bug that needs to be explored, but for now get things back to >> a working state. >> >> Obtained from: Netflix, Inc. >> MFC after: immediately >> > in my POV, at vm_mem_init stage vm_map_init() call > uma_zcreate() that uses uinitialized zones (which initialized > in uma_startup()). I bet zones contains garbage. > I don’t follow. vm_mem_init() is called at SI_SUB_VM sysinit, and vm_map_init() is called much later at SI_SUB_INTRINSIC. vm_mem_init() calls uma_startup() almost immediately, which will then call zone_ctor() on the “kegs” and “zones” that were allocated from bss. I don’t think that they’re being used prior to that. The problem that I see is that both of these zones are allocated statically, and contain no storage for the uz_cpu member when that member is declared as a zero-length array. All other zones are created dynamically and include space for these members. uma_startup() is initializing these zones at the right time, before their first use, but isn’t giving them enough room. According to the stack trace I posted, the problem triggers in the second call to uma_zcreate() from uma_startup(). I think what happens is that the first call to uma_zcreate() winds up writing to the zero-length uz_cpu member of masterzone_z from inside of uma_zalloc_args(). This overwrites the adjacent “kegs” and “zones” pointers in the bss. The next call to uma_zcreate() then follows a path of trying to look in the kegs, and eventually blows up. I’m not entirely certain on this chain of events though as it’s a bit twisty inside of uma_zcreate() and I’m not sure I’ve found a link to where it calls uma_zalloc_args(). Scott ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r282064 - head/sys/boot/common
> On Apr 27, 2015, at 1:40 AM, Garrett Cooper wrote: > > On Apr 27, 2015, at 0:38, Scott Long wrote: > >> Author: scottl >> Date: Mon Apr 27 07:38:46 2015 >> New Revision: 282064 >> URL: https://svnweb.freebsd.org/changeset/base/282064 >> >> Log: >> Small change in header order to allow this to compile. >> >> Obtained from: Netflix, Inc. >> MFC after: 3 days >> >> Modified: >> head/sys/boot/common/md.c > > Hi Scott, > How was this broken before? > Thanks! > -NGie It didn’t compile. Note that you have to set MD_IMAGE_SIZE in order to even attempt to compile it. Scott signature.asc Description: Message signed with OpenPGP using GPGMail
Re: svn commit: r281451 - head/sys/vm
> On Apr 26, 2015, at 11:13 PM, Julian Elischer wrote: > > On 4/26/15 3:28 AM, Scott Long wrote: >> >> Too early in boot to get a crashdump. > but not too early for gdb live. > > > Guys, seriously, the amount of unproductive comments from the sidelines is stupid. The commit was trivial optimization that proved to cause problems. Nothing was lost in the revert. If anyone want to run this to ground, I can provide hardware with a remote console. Scott ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r282227 - head/sys/cam/scsi
This commit is broken, please revert:
Fatal trap 12: page fault while in kernel mode
cpuid = 0; apic id = 00
fault virtual address = 0x2102d8
fault code = supervisor read data, page not present
instruction pointer = 0x20:0x802fd074
stack pointer = 0x28:0xfe100678f960
frame pointer = 0x28:0xfe100678f9e0
code segment= base 0x0, limit 0xf, type 0x1b
= DPL 0, pres 1, long 1, def32 0, gran 1
processor eflags= interrupt enabled, resume, IOPL = 0
current process = 2 (doneq0)
[ thread pid 2 tid 100025 ]
Stopped at scsi_scan_bus+0x54: movq0x58(%rax),%rdi
db> bt
Tracing pid 2 tid 100025 td 0xf8000d3ac940
scsi_scan_bus() at scsi_scan_bus+0x54/frame 0xfe100678f9e0
xpt_done_process() at xpt_done_process+0x521/frame 0xfe100678fa20
xpt_done_td() at xpt_done_td+0xf6/frame 0xfe100678fa70
fork_exit() at fork_exit+0x71/frame 0xfe100678fab0
fork_trampoline() at fork_trampoline+0xe/frame 0xfe100678fab0
--- trap 0, rip = 0, rsp = 0xfe100678fb70, rbp = 0 ---
db>
> On Apr 29, 2015, at 9:46 AM, Pedro F. Giffuni wrote:
>
> Author: pfg
> Date: Wed Apr 29 15:46:57 2015
> New Revision: 282227
> URL: https://svnweb.freebsd.org/changeset/base/282227
>
> Log:
> Fix memory leak in scsi_scan_bus()
>
> CID: 1007770
> PR: 199671
>
> Modified:
> head/sys/cam/scsi/scsi_xpt.c
>
> Modified: head/sys/cam/scsi/scsi_xpt.c
> ==
> --- head/sys/cam/scsi/scsi_xpt.c Wed Apr 29 15:41:19 2015
> (r282226)
> +++ head/sys/cam/scsi/scsi_xpt.c Wed Apr 29 15:46:57 2015
> (r282227)
> @@ -2008,6 +2008,7 @@ scsi_scan_bus(struct cam_periph *periph,
> " with status %#x, bus scan halted\n",
> status);
> free(scan_info, M_CAMXPT);
> + scan_info = NULL;
> request_ccb->ccb_h.status = status;
> xpt_free_ccb(work_ccb);
> xpt_done(request_ccb);
> @@ -2017,6 +2018,7 @@ scsi_scan_bus(struct cam_periph *periph,
> if (work_ccb == NULL) {
> xpt_free_ccb((union ccb *)scan_info->cpi);
> free(scan_info, M_CAMXPT);
> + scan_info = NULL;
> xpt_free_path(path);
> request_ccb->ccb_h.status = CAM_RESRC_UNAVAIL;
> xpt_done(request_ccb);
> @@ -2032,6 +2034,7 @@ scsi_scan_bus(struct cam_periph *periph,
> xpt_action(work_ccb);
> }
>
> + free(scan_info, M_CAMXPT);
> mtx_lock(mtx);
> break;
> }
>
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r279828 - head/sys/dev/cadence
> On Mar 9, 2015, at 4:39 PM, Ian Lepore wrote: > > Author: ian > Date: Mon Mar 9 22:39:58 2015 > New Revision: 279828 > URL: https://svnweb.freebsd.org/changeset/base/279828 > > Log: > Use the new ifnet API. Also, allocate bus_dma_maps as needed instead of > pre-allocating them all at start-up. Also fix a bug in cgem_stop(); before, > it wasn't properly unloading dma maps due to a cut-and-paste error. > It’s generally a very bad idea to create and destroy maps for every transaction. If the map results in being non-NULL, then you’re allocating and freeing memory on every transaction, and possibly allocating and freeing large chunks of memory for bounce buffers. Is there a reason why you’ve made this change? Thanks, Scott ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r284959 - in head: . share/man/man4 share/man/man9 sys/conf sys/dev/glxsb sys/dev/hifn sys/dev/random sys/dev/rndtest sys/dev/safe sys/dev/syscons sys/dev/ubsec sys/dev/virtio/random s
> On Jul 23, 2015, at 1:03 AM, Mark R V Murray wrote: > > >> On 23 Jul 2015, at 00:53, Warner Losh wrote: >> > Neither filesystem operations nor allocations are random events. They > are trivially influenced by user code. A malicious attacker could create > repeated patterns of allocations or filesystem activity through the > syscall path to degrade your random sample source. I?m not sure I accept that - Fortuna is very careful about using non-reversible hashing in it?s accumulation, and countering such degradation is one of the algorithm?s strong points. There is perhaps risk of *no* entropy, but even the per-event timing jitter will be providing this, if nothing else. >> >> I’m not sure I’m happy about this answer. Do you have some research backing >> up such cavalier claims? > > It was not my intention to sound cavalier. Apologies. > > Fortuna was developed to account for many sources of entropy, good and bad > alike, and Jeff’s observation is an attack on that design. I accept that the > randomness of these events is poor, but they are high-rate, and this product > of high-rate*low entropy is what I seek. I pulled out numbers with dtrace, > and basic statistics showed that the harvesting was not useless. I completely > understand that under the right circumstances these numbers might be lousy - > please read the Fortuna design document to understand why this doesn’t > matter. *ALL* entropy inputs to Fortuna are considered attackable, including > the dedicated hardware sources. > > I have also read cryptanalyses of Fortuna, not all of them to be sure, and so > far the design appears strong. The best attack that I have seen (very > academic) suggests an improvement which I may incorporate. > > Perhaps more importantly to me, this is an unacceptable performance > burden for the allocator. At a minimum it should compile out by default. > Great care has been taken to reduce the fast path of the allocator to the > minimum number of cycles and even cache misses. As currently set up in etc/rc.d/* by default, there is a simple check at each UMA harvesting opportunity, and no further action. I asked Robert Watson if this was burdensome, and he said it was not. >>> >>> I find this burdensome. You can easily add a macro around the calls or >>> hide them in an inline with a default to off. Even a function call that >>> checks a global and does nothing else is a handful of new cache misses. A >>> microbenchmark will not realize the full cost of this. You will instead >>> get the dozen or so instructions of overhead which I still find >>> objectionable. >>> >>> Kip's observations about packet cycle budgets in high-performance >>> applications are accurate and this is something we have put great care into >>> over time. >> >> A certain video streaming company will be pushing the envelope to get to >> 100Gbps very soon. Even a few extra instructions on every packet / >> allocation will be a killer. Especially if one is an almost guaranteed cache >> miss. This most certainly will be burdensome. There absolutely must be a way >> to turn this off at compile time. We don’t care that much about entropy to >> leave performance on the table. > > OK - I’m sold! I’ll make a kernel option defaulting to off. :-) > > Hi Mark, Thanks for making this concession. I wanted to add a bit of historical perspective. When Yarrow was introduced in the previous decade, it was initially wired into nearly all interrupt sources. It turned out to be so expensive to those sources, especially for high-speed sources at the time like network and caching RAID drivers, that we then spent months disabling it from those sources. In the end, a lot of code thrash happened and the effectiveness of Yarrow was questionable. Fast forward to now with your recent work. If UMA becomes expensive for high-speed use, everyone will go back to developing private per-driver and per-subsystem allocators to avoid it. This will happen whether or not the UMA collector is controllable at run-time; if it’s enabled by default, benchmarks will be impacted and people will react. That’ll be a huge step backwards for FreeBSD. I also strongly agree with Jeff’s point on the questionable nature of this kind of fast-and-monotonic entropy collection, and Warner and Kip’s point on the finite number of clock cycles available for doing 100Gb networking. If really high quality entropy is desired, won’t most serious people use a hardware source instead of a software source? Not that I think that software entropy is useless, but it’s a question of how much effort and tradeoffs are put into it for what result. An academically beautiful entropy system that hamstrings the OS from doing other essential things isn’t all that interesting, IMO. Scott ___ svn-src-all@freebsd.org
Re: svn commit: r284959 - in head: . share/man/man4 share/man/man9 sys/conf sys/dev/glxsb sys/dev/hifn sys/dev/random sys/dev/rndtest sys/dev/safe sys/dev/syscons sys/dev/ubsec sys/dev/virtio/random s
> On Jul 24, 2015, at 12:59 AM, Mark R V Murray wrote: > > >> On 24 Jul 2015, at 02:25, John-Mark Gurney wrote: >> >> I would like to point out that the goal of collecting large amounts >> is starting to fall out of favor, and I happen to agree with the likes >> of djb[1] that we don't need an infinite amount of entropy collected by >> the system. If the attacker can read out our RNG state, then we are >> already screwed due to many other vulns. > > I’m working on a premise of “tools, not policy”. I’d like there to be > enough harvesting points for the box owner to get the warm fuzzies. > If they choose to use less, fine by me. > Sure, and that’s not an unreasonable goal, but the devil is in the details. It’s an unfortunate fact of modern CPU architecture that even something as simple and innocent as a run-time control that checks a variable can cause significant performance problems, thanks to the penalty of cache misses and bus contention between lots of CPU cores. Maybe these “extended” collection points should be controlled with a compile-time option? >> Many of the issues that FreeBSD sees with lack of entropy at start up >> is more of a problem on how systems are installed and provisioned. I >> don't believe that we currently store any entropy from the install >> process, yet this is one of the best places to get it, the user is >> banging on keyboard selecting options, etc. If an image is designed >> to be cloned (vm images or appliance images) we need to have a >> mechanism to ensure that before we start, we get the entropy from >> other sources, be it a hardware RNG or the console. > > Getting an initial entropy bundle for first boot is high up on my > TODO list. :-) Patches welcome! We need the usual /entropy (or > /var/db/entropy/… or whatever) and crucially we need /boot/entropy > and the correct invocation in /boot/loader.conf. > I agree that bootstrap entropy is a big deal, especially with the increasing prevalence of using virtual machine containers, cloned images, and datacenter automation. Addressing that is an interesting problem, and goes well beyond the scope of in-kernel entropy collection. I wish I had a simple answer or a patch for you ;-) >> I would like to see us scale back the entropy collection, and replace >> it with something like scan the zone once an hour or something >> similar. Or do something dtrace style, where we nop/jmp the >> collection after we feel that the system has collected enough. > > Most of the current entropy gathering is just about invisible > anyway. I think the above goes too far, but may be a useful way > of enabling/disabling (say) UMA gathering on the fly. > >> Heck, piping in mic data to /dev/random is a good way to seed the >> rng on many machines. > > Well, sure, but what if you don’t have microphone? I want lots > of choices, in anticipation of only a subset being usable. > I still think that for most use cases where you have a high likelyhood of draining /dev/random of useful bits, you’re likely already on a tight budget for CPU cycles and you’ll probably just look to a hardware accelerator rather than sacrifice even more CPU cycles. At least, that’s what the nice sale people at Cavium and Intel tell me ;-) Scott ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r284959 - in head: . share/man/man4 share/man/man9 sys/conf sys/dev/glxsb sys/dev/hifn sys/dev/random sys/dev/rndtest sys/dev/safe sys/dev/syscons sys/dev/ubsec sys/dev/virtio/random s
> On Jul 25, 2015, at 8:36 AM, Alexey Dokuchaev wrote: > > On Fri, Jul 24, 2015 at 07:59:35AM +0100, Mark R V Murray wrote: >> [...] >>> Heck, piping in mic data to /dev/random is a good way to seed the >>> rng on many machines. >> >> Well, sure, but what if you don't have microphone? I want lots >> of choices, in anticipation of only a subset being usable. > > I like the microphone idea. Not just it adds another hard-to-mess-with > (?) entropy source, it can also be a nice "reference" example for people > wanting to write their own sources and plug them into the RNG framework. > Microphones don’t typically exist on virtual machines, servers, appliances/embedded gadgets, and trusted computers. Nice idea for the desktop, but far from universal. Scott ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r284959 - in head: . share/man/man4 share/man/man9 sys/conf sys/dev/glxsb sys/dev/hifn sys/dev/random sys/dev/rndtest sys/dev/safe sys/dev/syscons sys/dev/ubsec sys/dev/virtio/random s
> On Jul 25, 2015, at 2:10 AM, Mark R V Murray wrote: > > >> On 25 Jul 2015, at 06:06, Scott Long wrote: >> >>> I’m working on a premise of “tools, not policy”. I’d like there to be >>> enough harvesting points for the box owner to get the warm fuzzies. >>> If they choose to use less, fine by me. >>> >> >> Sure, and that’s not an unreasonable goal, but the devil is in the details. > > Yes, indeed! > >> It’s an unfortunate fact of modern CPU architecture that even something >> as simple and innocent as a run-time control that checks a variable can >> cause significant performance problems, thanks to the penalty of cache >> misses and bus contention between lots of CPU cores. Maybe these >> “extended” collection points should be controlled with a compile-time >> option? > > They can. I’ve coded it already, but not tested it properly, and will > commit in a week or two. :-) What you posted this morning for review is a great start. Thanks for the productive conversation on this. Scott ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r284297 - in head: cddl/contrib/opensolaris/cmd/lockstat sys/kern sys/sys
> On Jun 12, 2015, at 4:01 AM, Andriy Gapon wrote: > > Author: avg > Date: Fri Jun 12 10:01:24 2015 > New Revision: 284297 > URL: https://svnweb.freebsd.org/changeset/base/284297 > > Log: > several lockstat improvements > > 0. For spin events report time spent spinning, not a loop count. > While loop count is much easier and cheaper to obtain it is hard > to reason about the reported numbers, espcially for adaptive locks > where both spinning and sleeping can happen. > So, it's better to compare apples and apples. > This causes spinning to be exceptionally more expensive just by having KDTRACE_HOOKS enabled, whether or not Dtrace is actually in use. It makes it undesirable to deploy with Dtrace by default since it impacts performance. Is there a way to make the expensive collection optional, or only enable when dtrace is using the lockstat module? Also have you seen the other recent performance complaint related to this commit? Thanks, Scott ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r314420 - head/sys/kern
Sure, but it doesn’t match the comments in that if-else construct.
Scott
> On Feb 28, 2017, at 2:02 PM, Conrad Meyer wrote:
>
> On Tue, Feb 28, 2017 at 1:27 PM, Scott Long wrote:
>> Author: scottl
>> Date: Tue Feb 28 21:27:51 2017
>> New Revision: 314420
>> URL: https://svnweb.freebsd.org/changeset/base/314420
>>
>> Log:
>> Provide a comment on why stdio.h needs to be included.
>>
>> Modified:
>> head/sys/kern/subr_prf.c
>>
>> Modified: head/sys/kern/subr_prf.c
>> ==
>> --- head/sys/kern/subr_prf.cTue Feb 28 21:18:45 2017(r314419)
>> +++ head/sys/kern/subr_prf.cTue Feb 28 21:27:51 2017(r314420)
>> @@ -76,6 +76,13 @@ __FBSDID("$FreeBSD$");
>> #include
>> #else
>> #include
>> +#endif
>> +
>> +/*
>> + * This is needed for sbuf_putbuf() when compiled into userland. Due to the
>> + * shared nature of this file, it's the only place to put it.
>
> Couldn't it go in the #else clause above?
>
> Best,
> Conrad
>
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r287934 - head/sys/boot/efi/loader
As a side note, I’m still waiting for feedback on the patch I sent you for
scsi_sg. I’d like to get this fixed.
Scott
> On Sep 21, 2015, at 3:53 PM, Rui Paulo wrote:
>
> No, that doesn't work very well. Not only the modules don't auto-load, the
> way the modules are compiled is different. See, for example, cam ctl which
> doesn't compile the sg code when it's built into the kernel, but compiles it
> when it's built as a module. The sg code is currently buggy and causes
> insta-panics with GNOME 3 (perhaps the auto-mounter in hald (?)).
> --
> Rui Paulo
>
>
> On Sep 21, 2015, at 11:24 AM, Adrian Chadd wrote:
>
>> Hi,
>>
>> Warner has been working on the modular kernel thing. But honestly, I
>> think we should just start biting that bullet and ship a modules-only
>> GENERIC by default..
>>
>>
>> -a
>>
>>
>> On 21 September 2015 at 11:02, Rui Paulo wrote:
>>> So, we're going to keep ignoring the problem and keep patching things up?
>>> It's a bit sad that a single driver (pmspcv) is able to cause so much
>>> problems.
>>>
>>> --
>>> Rui Paulo
>>>
>>>
>>> On Sep 17, 2015, at 01:36 PM, John Baldwin wrote:
>>>
>>> Author: jhb
>>> Date: Thu Sep 17 20:36:46 2015
>>> New Revision: 287934
>>> URL: https://svnweb.freebsd.org/changeset/base/287934
>>>
>>>
>>> Log:
>>> The EFI boot loader allocates a single chunk of contiguous memory to
>>> hold the kernel, modules, and any other loaded data. This memory block
>>> is relocated to the kernel's expected location during the transfer of
>>> control from the loader to the kernel.
>>>
>>> The GENERIC kernel on amd64 has recently grown such that a kernel + zfs.ko
>>> no longer fits in the default staging size. Bump the default size from
>>> 32MB to 48MB to provide more breathing room.
>>>
>>> PR: 201679
>>> Reviewed by: imp
>>> MFC after: 1 week
>>> Differential Revision: https://reviews.freebsd.org/D3666
>>>
>>>
>>> Modified:
>>> head/sys/boot/efi/loader/copy.c
>>>
>>> Modified: head/sys/boot/efi/loader/copy.c
>>> ==
>>> --- head/sys/boot/efi/loader/copy.c Thu Sep 17 20:36:34 2015
>>> (r287933)
>>> +++ head/sys/boot/efi/loader/copy.c Thu Sep 17 20:36:46 2015
>>> (r287934)
>>> @@ -38,7 +38,7 @@ __FBSDID("$FreeBSD$");
>>> #include
>>>
>>> #ifndef EFI_STAGING_SIZE
>>> -#define EFI_STAGING_SIZE 32
>>> +#define EFI_STAGING_SIZE 48
>>> #endif
>>>
>>> #define STAGE_PAGES ((EFI_STAGING_SIZE) * 1024 * 1024 / 4096)
>>>
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r288122 - in head/sys: kern vm
It should be noted that Netflix has been running with an earlier version of
this patch for nearly 10 months.
Scott
> On Sep 22, 2015, at 11:16 AM, Alan Cox wrote:
>
> Author: alc
> Date: Tue Sep 22 18:16:52 2015
> New Revision: 288122
> URL: https://svnweb.freebsd.org/changeset/base/288122
>
> Log:
> Change vm_page_unwire() such that it (1) accepts PQ_NONE as the specified
> queue and (2) returns a Boolean indicating whether the page's wire count
> transitioned to zero.
>
> Exploit this change in vfs_vmio_release() to avoid pointlessly enqueueing
> a page that is about to be freed.
>
> (An earlier version of this change was developed by attilio@ and kmacy@.
> Any errors in this version are my own.)
>
> Reviewed by: kib
> Sponsored by:EMC / Isilon Storage Division
>
> Modified:
> head/sys/kern/vfs_bio.c
> head/sys/vm/vm_page.c
> head/sys/vm/vm_page.h
>
> Modified: head/sys/kern/vfs_bio.c
> ==
> --- head/sys/kern/vfs_bio.c Tue Sep 22 17:34:51 2015(r288121)
> +++ head/sys/kern/vfs_bio.c Tue Sep 22 18:16:52 2015(r288122)
> @@ -2076,6 +2076,7 @@ vfs_vmio_release(struct buf *bp)
> vm_object_t obj;
> vm_page_t m;
> int i;
> + bool freed;
>
> if (buf_mapped(bp)) {
> BUF_CHECK_MAPPED(bp);
> @@ -2088,23 +2089,28 @@ vfs_vmio_release(struct buf *bp)
> for (i = 0; i < bp->b_npages; i++) {
> m = bp->b_pages[i];
> bp->b_pages[i] = NULL;
> - /*
> - * In order to keep page LRU ordering consistent, put
> - * everything on the inactive queue.
> - */
> vm_page_lock(m);
> - vm_page_unwire(m, PQ_INACTIVE);
> -
> - /*
> - * Might as well free the page if we can and it has
> - * no valid data. We also free the page if the
> - * buffer was used for direct I/O
> - */
> - if ((bp->b_flags & B_ASYNC) == 0 && !m->valid) {
> - if (m->wire_count == 0 && !vm_page_busied(m))
> - vm_page_free(m);
> - } else if (bp->b_flags & B_DIRECT)
> - vm_page_try_to_free(m);
> + if (vm_page_unwire(m, PQ_NONE)) {
> + /*
> + * Determine if the page should be freed before adding
> + * it to the inactive queue.
> + */
> + if ((bp->b_flags & B_ASYNC) == 0 && m->valid == 0) {
> + freed = !vm_page_busied(m);
> + if (freed)
> + vm_page_free(m);
> + } else if ((bp->b_flags & B_DIRECT) != 0)
> + freed = vm_page_try_to_free(m);
> + else
> + freed = false;
> + if (!freed) {
> + /*
> + * In order to maintain LRU page ordering, put
> + * the page at the tail of the inactive queue.
> + */
> + vm_page_deactivate(m);
> + }
> + }
> vm_page_unlock(m);
> }
> if (obj != NULL)
>
> Modified: head/sys/vm/vm_page.c
> ==
> --- head/sys/vm/vm_page.c Tue Sep 22 17:34:51 2015(r288121)
> +++ head/sys/vm/vm_page.c Tue Sep 22 18:16:52 2015(r288122)
> @@ -2476,42 +2476,46 @@ vm_page_wire(vm_page_t m)
> /*
> * vm_page_unwire:
> *
> - * Release one wiring of the specified page, potentially enabling it to be
> - * paged again. If paging is enabled, then the value of the parameter
> - * "queue" determines the queue to which the page is added.
> - *
> - * However, unless the page belongs to an object, it is not enqueued because
> - * it cannot be paged out.
> + * Release one wiring of the specified page, potentially allowing it to be
> + * paged out. Returns TRUE if the number of wirings transitions to zero and
> + * FALSE otherwise.
> + *
> + * Only managed pages belonging to an object can be paged out. If the number
> + * of wirings transitions to zero and the page is eligible for page out, then
> + * the page is added to the specified paging queue (unless PQ_NONE is
> + * specified).
> *
> * If a page is fictitious, then its wire count must always be one.
> *
> * A managed page must be locked.
> */
> -void
> +boolean_t
> vm_page_unwire(vm_page_t m, uint8_t queue)
> {
>
> - KASSERT(queue < PQ_COUNT,
> + KASSERT(queue < PQ_COUNT || queue == PQ_NONE,
> ("vm_page_unwire: invalid queue %u request for page %p",
> queue, m));
> if ((m->oflags & VPO_UNMANAGED) == 0)
> - vm_page_lock_assert(m, MA_
Re: svn commit: r300154 - head/sys/net
What should I use instead?
Scott
> On May 18, 2016, at 10:03 AM, Warner Losh wrote:
>
> Ditto with mips.
>
> On Wed, May 18, 2016 at 9:50 AM, Justin Hibbits wrote:
> On Wed, 18 May 2016 15:45:12 + (UTC)
> Scott Long wrote:
>
> > Author: scottl
> > Date: Wed May 18 15:45:12 2016
> > New Revision: 300154
> > URL: https://svnweb.freebsd.org/changeset/base/300154
> >
> > Log:
> > Activate the NO_64BIT_ATOMICS code for mips and powerpc
> >
> > Modified:
> > head/sys/net/mp_ring.c
> >
> > Modified: head/sys/net/mp_ring.c
> > ==
> > --- head/sys/net/mp_ring.cWed May 18 15:44:45 2016
> > (r300153) +++ head/sys/net/mp_ring.c Wed May 18 15:45:12
> > 2016 (r300154) @@ -37,15 +37,17 @@ __FBSDID("$FreeBSD$");
> > #include
> > #include
> >
> > -
> > -
> > -#include
> > +#if defined(__powerpc__) || defined(__mips__)
> > +#define NO_64BIT_ATOMICS
> > +#endif
> >
> > #if defined(__i386__)
> > #define atomic_cmpset_acq_64 atomic_cmpset_64
> > #define atomic_cmpset_rel_64 atomic_cmpset_64
> > #endif
> >
> > +#include
> > +
> > union ring_state {
> > struct {
> > uint16_t pidx_head;
> >
>
> powerpc64 defines both __powerpc__ and __powerpc64__, so you're killing
> atomics on powerpc64 with this.
>
> - Justin
>
>
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r300201 - head/sys/sys
Thanks!
Scott
> On May 19, 2016, at 5:19 AM, Alexander Motin wrote:
>
> Author: mav
> Date: Thu May 19 11:19:37 2016
> New Revision: 300201
> URL: https://svnweb.freebsd.org/changeset/base/300201
>
> Log:
> Add ta_flags initialization in old macros missed in 300113.
>
> Depending on uninitialized memory content it could cause loss of wakeup()
> call in taskqueue_run_locked().
>
> Modified:
> head/sys/sys/taskqueue.h
>
> Modified: head/sys/sys/taskqueue.h
> ==
> --- head/sys/sys/taskqueue.h Thu May 19 11:02:39 2016(r300200)
> +++ head/sys/sys/taskqueue.h Thu May 19 11:19:37 2016(r300201)
> @@ -97,6 +97,7 @@ voidtaskqueue_set_callback(struct taskq
>
> #define TASK_INITIALIZER(priority, func, context) \
> { .ta_pending = 0, \
> + .ta_flags = 0,\
> .ta_priority = (priority),\
> .ta_func = (func),\
> .ta_context = (context) }
> @@ -112,6 +113,7 @@ void taskqueue_thread_enqueue(void *cont
> */
> #define TASK_INIT(task, priority, func, context) do { \
> (task)->ta_pending = 0; \
> + (task)->ta_flags = 0; \
> (task)->ta_priority = (priority); \
> (task)->ta_func = (func); \
> (task)->ta_context = (context); \
>
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r300219 - in head/sys: kern sys
Yeah, I was following existing style. Feel free to fix up if you like.
Scott
> On May 19, 2016, at 12:51 PM, Steven Hartland wrote:
>
> I thought it was considered better to use if (var == NULL) instead of
> if (!var) for pointers as they aren't bools?
>
>> On 19 May 2016, at 18:14, Scott Long wrote:
>>
>> Author: scottl
>> Date: Thu May 19 17:14:24 2016
>> New Revision: 300219
>> URL: https://svnweb.freebsd.org/changeset/base/300219
>>
>> Log:
>> Adjust the creation of tq_name so it can be freed correctly
>>
>> Reviewed by:jhb, allanjude
>> Differential Revision:D6454
>>
>> Modified:
>> head/sys/kern/subr_taskqueue.c
>> head/sys/sys/taskqueue.h
>>
>> Modified: head/sys/kern/subr_taskqueue.c
>> ==
>> --- head/sys/kern/subr_taskqueue.cThu May 19 17:02:33 2016(r300218)
>> +++ head/sys/kern/subr_taskqueue.cThu May 19 17:14:24 2016(r300219)
>> @@ -128,16 +128,17 @@ _taskqueue_create(const char *name, int
>>int mtxflags, const char *mtxname __unused)
>> {
>> struct taskqueue *queue;
>> -char *tq_name = NULL;
>> +char *tq_name;
>>
>> -if (name != NULL)
>> -tq_name = strndup(name, 32, M_TASKQUEUE);
>> -if (tq_name == NULL)
>> -tq_name = "taskqueue";
>> +tq_name = malloc(TASKQUEUE_NAMELEN, M_TASKQUEUE, mflags | M_ZERO);
>> +if (!tq_name)
>> +return (NULL);
>> +
>> +snprintf(tq_name, TASKQUEUE_NAMELEN, "%s", (name) ? name : "taskqueue");
>>
>> queue = malloc(sizeof(struct taskqueue), M_TASKQUEUE, mflags | M_ZERO);
>> if (!queue)
>> -return NULL;
>> +return (NULL);
>>
>> STAILQ_INIT(&queue->tq_queue);
>> TAILQ_INIT(&queue->tq_active);
>> @@ -153,7 +154,7 @@ _taskqueue_create(const char *name, int
>> queue->tq_flags |= TQ_FLAGS_UNLOCKED_ENQUEUE;
>> mtx_init(&queue->tq_mutex, tq_name, NULL, mtxflags);
>>
>> -return queue;
>> +return (queue);
>> }
>>
>> struct taskqueue *
>>
>> Modified: head/sys/sys/taskqueue.h
>> ==
>> --- head/sys/sys/taskqueue.hThu May 19 17:02:33 2016(r300218)
>> +++ head/sys/sys/taskqueue.hThu May 19 17:14:24 2016(r300219)
>> @@ -56,6 +56,7 @@ enum taskqueue_callback_type {
>> #defineTASKQUEUE_CALLBACK_TYPE_MINTASKQUEUE_CALLBACK_TYPE_INIT
>> #defineTASKQUEUE_CALLBACK_TYPE_MAXTASKQUEUE_CALLBACK_TYPE_SHUTDOWN
>> #defineTASKQUEUE_NUM_CALLBACKSTASKQUEUE_CALLBACK_TYPE_MAX + 1
>> +#defineTASKQUEUE_NAMELEN32
>>
>> typedef void (*taskqueue_callback_fn)(void *context);
>>
>>
>
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
