Re: svn commit: r314420 - head/sys/kern

[Scott Long via svn-src-head]

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-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r300219 - in head/sys: kern sys

[Scott Long via svn-src-head]

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(>tq_queue);
>>   TAILQ_INIT(>tq_active);
>> @@ -153,7 +154,7 @@ _taskqueue_create(const char *name, int
>>   queue->tq_flags |= TQ_FLAGS_UNLOCKED_ENQUEUE;
>>   mtx_init(>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-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r300201 - head/sys/sys

[Scott Long via svn-src-head]

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-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r300154 - head/sys/net

[Scott Long via svn-src-head]

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-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r288122 - in head/sys: kern vm

[Scott Long via svn-src-head]

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)
> - 

Re: svn commit: r287934 - head/sys/boot/efi/loader

[Scott Long via svn-src-head]

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-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r284297 - in head: cddl/contrib/opensolaris/cmd/lockstat sys/kern sys/sys

[Scott Long via svn-src-head]

 On Jun 12, 2015, at 4:01 AM, Andriy Gapon a...@freebsd.org 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-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

[Scott Long via svn-src-head]

 On Jul 25, 2015, at 8:36 AM, Alexey Dokuchaev da...@freebsd.org 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-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

[Scott Long via svn-src-head]

 On Jul 25, 2015, at 2:10 AM, Mark R V Murray ma...@freebsd.org wrote:
 
 
 On 25 Jul 2015, at 06:06, Scott Long scott4l...@yahoo.com 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-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

[Scott Long via svn-src-head]

 On Jul 24, 2015, at 12:59 AM, Mark R V Murray ma...@freebsd.org wrote:
 
 
 On 24 Jul 2015, at 02:25, John-Mark Gurney j...@funkthat.com 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-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

[Scott Long via svn-src-head]

 On Jul 23, 2015, at 1:03 AM, Mark R V Murray ma...@freebsd.org wrote:
 
 
 On 23 Jul 2015, at 00:53, Warner Losh i...@bsdimp.com 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to 

Re: svn commit: r282227 - head/sys/cam/scsi

[Scott Long via svn-src-head]

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 p...@freebsd.org 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r281451 - head/sys/vm

[Scott Long via svn-src-head]

 On Apr 26, 2015, at 11:13 PM, Julian Elischer jul...@freebsd.org 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r282064 - head/sys/boot/common

[Scott Long via svn-src-head]

 On Apr 27, 2015, at 1:40 AM, Garrett Cooper yaneurab...@gmail.com wrote:
 
 On Apr 27, 2015, at 0:38, Scott Long sco...@freebsd.org 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: r281942 - head/sys/vm

[Scott Long via svn-src-head]

 On Apr 25, 2015, at 2:30 AM, Chagin Dmitry dcha...@freebsd.org 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r281451 - head/sys/vm

[Scott Long via svn-src-head]

 On Apr 25, 2015, at 1:06 AM, Chagin Dmitry dcha...@freebsd.org 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r281451 - head/sys/vm

[Scott Long via svn-src-head]

 On Apr 25, 2015, at 1:59 PM, Dmitry Morozovsky ma...@rinet.ru 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r281451 - head/sys/vm

[Scott Long via svn-src-head]

 On Apr 23, 2015, at 6:19 AM, Scott Long scott4l...@yahoo.com wrote:
 
 
 On Apr 12, 2015, at 12:21 AM, Dmitry Chagin dcha...@freebsd.org 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r281451 - head/sys/vm

[Scott Long via svn-src-head]

 On Apr 12, 2015, at 12:21 AM, Dmitry Chagin dcha...@freebsd.org 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r281451 - head/sys/vm

[Scott Long via svn-src-head]

 On Apr 23, 2015, at 1:28 PM, Chagin Dmitry dcha...@freebsd.org wrote:
 
 On Thu, Apr 23, 2015 at 12:49:51PM -0600, Scott Long wrote:
 
 On Apr 23, 2015, at 6:19 AM, Scott Long scott4l...@yahoo.com wrote:
 
 
 On Apr 12, 2015, at 12:21 AM, Dmitry Chagin dcha...@freebsd.org 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r279828 - head/sys/dev/cadence

[Scott Long via svn-src-head]

 On Mar 9, 2015, at 4:39 PM, Ian Lepore i...@freebsd.org 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r277693 - head/sys/boot/forth

[Scott Long via svn-src-head]

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 d...@freebsd.org 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r274489 - in head/sys/amd64: amd64 include

[Scott Long via svn-src-head]

 On Nov 20, 2014, at 11:33 PM, Rui Paulo rpa...@me.com wrote:
 
 On Nov 13, 2014, at 14:11, Scott Long sco...@freebsd.org 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-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r266481 - head/sys/x86/x86

[Scott Long via svn-src-head]

On May 21, 2014, at 10:03 AM, Konstantin Belousov kostik...@gmail.com 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