From owner-freebsd-arch@FreeBSD.ORG Sun Feb 17 02:42:08 2013 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id B668437F; Sun, 17 Feb 2013 02:42:08 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id C157A662; Sun, 17 Feb 2013 02:42:07 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id A262B7300A; Sun, 17 Feb 2013 03:42:20 +0100 (CET) Date: Sun, 17 Feb 2013 03:42:20 +0100 From: Luigi Rizzo To: Alfred Perlstein Subject: Re: [RFC/RFT] calloutng Message-ID: <20130217024220.GA81224@onelab2.iet.unipi.it> References: <50D03173.9080904@FreeBSD.org> <20121225232126.GA47692@alchemy.franken.de> <50DB4EFE.2020600@FreeBSD.org> <20130106152313.GD26039@alchemy.franken.de> <50EBF921.2000304@FreeBSD.org> <20130113180940.GM26039@alchemy.franken.de> <50F30CAB.3000001@FreeBSD.org> <20130121095457.GL85306@alchemy.franken.de> <511DEA46.5010509@mu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <511DEA46.5010509@mu.org> User-Agent: Mutt/1.5.20 (2009-06-14) Cc: Davide Italiano , Alexander Motin , Marius Strobl , Poul-Henning Kamp , FreeBSD Current , freebsd-arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 17 Feb 2013 02:42:08 -0000 On Thu, Feb 14, 2013 at 11:56:54PM -0800, Alfred Perlstein wrote: > [ added Luigi Rizzo to thread ] > > > On 2/11/13 12:26 PM, Davide Italiano wrote: > > [trimmed old mails] > > > > Here's a new version of the patch: > > http://people.freebsd.org/~davide/patches/calloutng-11022012.diff > > > > Significant bits changed (after wider discussion and suggestion by phk@): > > - Introduction of the new sbintime_t type (32.32 fixed point) with the > > respective conversion (sbintime2bintime, sbintime2timeval etc...) > > - switch from 64.64 (struct bintime) format to measure time to sbintime_t > > - Use sbintime_t to represent expected sleep time instead of measuring > > it in microseconds (cpu_idle_acpi(), cpu_idle_spin() ...). > > > Luigi and I discussed this at BAFUG tonight and he expressed an interest > in shepherding the code in at this point. > > Luigi, can you reiterate any points that still remain before we > integrate this code? I am mostly ok with the patch in the current state. I have few comments/suggestions below but they are mostly on documenting/style/trivial bugfixes. So now I would really like to see this code go in HEAD, to give people a chance to see how it works and possibly figure out whether the new API needs modifications. To recall, my major concerns were the following: - API explosion, with multiple versions of the callout routines. This seems to be solved now, with only one version (the *_sbt() functions) and macros remapping the old functions to the new ones. - the use of struct bintime for timeouts and precision. This is also solved thanks to the introduction of sbintime_t values (fixed-point 32.32 times) - Some measurements also indicated that the default "precision" could give large (absolute) errors on the timeouts, especially with large timeouts. I am not sure what is the situation with this new version, but i believe this a relatively minor issue because it surely simple to customize, e.g. having a couple of sysctl setting the default precision (percentage) and maximum error (absolute time). So users could e.g. set a 5% precision and a maximum error of 100us on a desktop, and 10% and 10ms on a laptop. - Another thing that we should do (but after the patch is in) is to see if any existing code is converting times to ticks just to call the timeout routines -- right now the macros convert back to times, clearly it would be best to avoid the double conversion. comments inline below, search for XXX-lr thanks again for your work on this, and for following up with changes after the previous discussion cheers luigi Index: sys/dev/acpica/acpi_cpu.c =================================================================== --- sys/dev/acpica/acpi_cpu.c (.../head) (revision 246685) +++ sys/dev/acpica/acpi_cpu.c (.../projects/calloutng) (revision 246685) @@ -980,13 +980,16 @@ } /* Find the lowest state that has small enough latency. */ + us = sc->cpu_prev_sleep; + if (sbt >= 0 && us > sbt / SBT_1US) XXX-lr can we have us2sbt() , ms2sbt() and the like ? + us = sbt / SBT_1US; cx_next_idx = 0; if (cpu_disable_deep_sleep) i = min(sc->cpu_cx_lowest, sc->cpu_non_c3); else i = sc->cpu_cx_lowest; for (; i >= 0; i--) { - if (sc->cpu_cx_states[i].trans_lat * 3 <= sc->cpu_prev_sleep) { + if (sc->cpu_cx_states[i].trans_lat * 3 <= us) { cx_next_idx = i; break; } Index: sys/kern/kern_synch.c =================================================================== --- sys/kern/kern_synch.c (.../head) (revision 246685) +++ sys/kern/kern_synch.c (.../projects/calloutng) (revision 246685) @@ -146,12 +146,12 @@ */ int _sleep(void *ident, struct lock_object *lock, int priority, - const char *wmesg, int timo) + const char *wmesg, sbintime_t sbt, sbintime_t pr, int flags) { struct thread *td; struct proc *p; struct lock_class *class; - int catch, flags, lock_state, pri, rval; + int catch, sleepq_flags, lock_state, pri, rval; XXX-lr keep flags, use _sleep_flags as function argument ? WITNESS_SAVE_DECL(lock_witness); td = curthread; @@ -348,28 +349,30 @@ * to a "timo" value of one. */ int -pause(const char *wmesg, int timo) +pause_sbt(const char *wmesg, sbintime_t sbt, sbintime_t pr, int flags) { - KASSERT(timo >= 0, ("pause: timo must be >= 0")); + int sbt_sec; XXX-lr localize to the cold block + sbt_sec = sbintime_getsec(sbt); + KASSERT(sbt_sec >= 0, ("pause: timo must be >= 0")); + /* silently convert invalid timeouts */ - if (timo < 1) - timo = 1; + if (sbt == 0) + sbt = tick_sbt; if (cold) { /* - * We delay one HZ at a time to avoid overflowing the + * We delay one second at a time to avoid overflowing the * system specific DELAY() function(s): */ - while (timo >= hz) { + while (sbt_sec > 0) { DELAY(1000000); - timo -= hz; + sbt_sec--; } - if (timo > 0) - DELAY(timo * tick); + DELAY((sbt & 0xffffffff) / SBT_1US); XXX-lr sbt2us() ? return (0); } - return (tsleep(&pause_wchan, 0, wmesg, timo)); + return (_sleep(&pause_wchan, NULL, 0, wmesg, sbt, pr, flags)); } /* @@ -560,8 +563,9 @@ * random variation to avoid synchronisation with processes that * run at regular intervals. */ - callout_reset(&loadav_callout, hz * 4 + (int)(random() % (hz * 2 + 1)), - loadav, NULL); + callout_reset_sbt(&loadav_callout, XXX-lr we have better resolution than HZ here ? + tick_sbt * (hz * 4 + (int)(random() % (hz * 2 + 1))), 0, + loadav, NULL, C_DIRECT_EXEC | C_HARDCLOCK); } /* ARGSUSED */ Index: sys/kern/sys_generic.c =================================================================== --- sys/kern/sys_generic.c (.../head) (revision 246685) +++ sys/kern/sys_generic.c (.../projects/calloutng) (revision 246685) @@ -995,35 +996,29 @@ if (nbufbytes != 0) bzero(selbits, nbufbytes / 2); + precision = 0; if (tvp != NULL) { - atv = *tvp; - if (itimerfix(&atv)) { + rtv = *tvp; + if (rtv.tv_sec < 0 || rtv.tv_usec < 0 || XXX-lr itimerfix or some check routine ? several places + rtv.tv_usec >= 1000000) { error = EINVAL; goto done; } - getmicrouptime(&rtv); - timevaladd(&atv, &rtv); - } else { - atv.tv_sec = 0; - atv.tv_usec = 0; - } - timo = 0; + rsbt = timeval2sbintime(rtv); + precision = rsbt; + precision >>= tc_precexp; + if (TIMESEL(&asbt, rsbt)) + asbt += tc_tick_sbt; + asbt += rsbt; + } else + asbt = -1; seltdinit(td); /* Iterate until the timeout expires or descriptors become ready. */ for (;;) { error = selscan(td, ibits, obits, nd); if (error || td->td_retval[0] != 0) break; - if (atv.tv_sec || atv.tv_usec) { - getmicrouptime(&rtv); - if (timevalcmp(&rtv, &atv, >=)) - break; - ttv = atv; - timevalsub(&ttv, &rtv); - timo = ttv.tv_sec > 24 * 60 * 60 ? - 24 * 60 * 60 * hz : tvtohz(&ttv); - } - error = seltdwait(td, timo); + error = seltdwait(td, asbt, precision); if (error) break; error = selrescan(td, ibits, obits); Index: sys/kern/kern_tc.c =================================================================== --- sys/kern/kern_tc.c (.../head) (revision 246685) +++ sys/kern/kern_tc.c (.../projects/calloutng) (revision 246685) @@ -119,6 +120,21 @@ SYSCTL_INT(_kern_timecounter, OID_AUTO, stepwarnings, CTLFLAG_RW, ×tepwarnings, 0, "Log time steps"); +struct bintime bt_timethreshold; XXX-lr document these variables +struct bintime bt_tickthreshold; +sbintime_t sbt_timethreshold; +sbintime_t sbt_tickthreshold; +struct bintime tc_tick_bt; +sbintime_t tc_tick_sbt; +int tc_precexp; +int tc_timepercentage = TC_DEFAULTPERC; +TUNABLE_INT("kern.timecounter.alloweddeviation", &tc_timepercentage); +static int sysctl_kern_timecounter_adjprecision(SYSCTL_HANDLER_ARGS); +SYSCTL_PROC(_kern_timecounter, OID_AUTO, alloweddeviation, + CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_MPSAFE, 0, 0, + sysctl_kern_timecounter_adjprecision, "I", + "Allowed time interval deviation in percents"); + static void tc_windup(void); static void cpu_tick_calibrate(int); @@ -883,6 +919,16 @@ } void +sbinuptime(sbintime_t sbt) XXX-lr wrong argument ? not compile-tested ? Index: sys/kern/kern_timeout.c =================================================================== --- sys/kern/kern_timeout.c (.../head) (revision 246685) +++ sys/kern/kern_timeout.c (.../projects/calloutng) (revision 246685) @@ -87,58 +105,62 @@ int callwheelsize, callwheelmask; /* - * The callout cpu migration entity represents informations necessary for - * describing the migrating callout to the new callout cpu. + * The callout cpu exec entities represent informations necessary for + * describing the state of callouts currently running on the CPU and the ones + * necessary for migrating callouts to the new callout cpu. In particular, + * the first entry of the array cc_exec_entity holds informations for callout + * running in SWI thread context, while the second one holds informations + * for callout running directly from hardware interrupt context. * The cached informations are very important for deferring migration when * the migrating callout is already running. */ -struct cc_mig_ent { +struct cc_exec { + struct callout *cc_next; + struct callout *cc_curr; #ifdef SMP - void (*ce_migration_func)(void *); - void *ce_migration_arg; - int ce_migration_cpu; - int ce_migration_ticks; + void (*ce_migration_func)(void *); + void *ce_migration_arg; + int ce_migration_cpu; + sbintime_t ce_migration_time; #endif + int cc_cancel; + int cc_waiting; }; - + /* - * There is one struct callout_cpu per cpu, holding all relevant + * There is one struct callou_cpu per cpu, holding all relevant * state for the callout processing thread on the individual CPU. - * In particular: - * cc_ticks is incremented once per tick in callout_cpu(). - * It tracks the global 'ticks' but in a way that the individual - * threads should not worry about races in the order in which - * hardclock() and hardclock_cpu() run on the various CPUs. - * cc_softclock is advanced in callout_cpu() to point to the - * first entry in cc_callwheel that may need handling. In turn, - * a softclock() is scheduled so it can serve the various entries i - * such that cc_softclock <= i <= cc_ticks . - * XXX maybe cc_softclock and cc_ticks should be volatile ? - * - * cc_ticks is also used in callout_reset_cpu() to determine - * when the callout should be served. */ struct callout_cpu { struct mtx_padalign cc_lock; - struct cc_mig_ent cc_migrating_entity; + struct cc_exec cc_exec_entity[2]; XXX-lr repeat (short) explanation on why 2 entities struct callout *cc_callout; struct callout_tailq *cc_callwheel; + struct callout_tailq cc_expireq; struct callout_list cc_callfree; - struct callout *cc_next; - struct callout *cc_curr; + sbintime_t cc_firstevent; + sbintime_t cc_lastscan; void *cc_cookie; - int cc_ticks; - int cc_softticks; - int cc_cancel; - int cc_waiting; - int cc_firsttick; }; +#define cc_exec_curr cc_exec_entity[0].cc_curr +#define cc_exec_next cc_exec_entity[0].cc_next +#define cc_exec_cancel cc_exec_entity[0].cc_cancel +#define cc_exec_waiting cc_exec_entity[0].cc_waiting +#define cc_exec_curr_dir cc_exec_entity[1].cc_curr +#define cc_exec_next_dir cc_exec_entity[1].cc_next +#define cc_exec_cancel_dir cc_exec_entity[1].cc_cancel +#define cc_exec_waiting_dir cc_exec_entity[1].cc_waiting + #ifdef SMP -#define cc_migration_func cc_migrating_entity.ce_migration_func -#define cc_migration_arg cc_migrating_entity.ce_migration_arg -#define cc_migration_cpu cc_migrating_entity.ce_migration_cpu -#define cc_migration_ticks cc_migrating_entity.ce_migration_ticks +#define cc_migration_func cc_exec_entity[0].ce_migration_func +#define cc_migration_arg cc_exec_entity[0].ce_migration_arg +#define cc_migration_cpu cc_exec_entity[0].ce_migration_cpu +#define cc_migration_time cc_exec_entity[0].ce_migration_time +#define cc_migration_func_dir cc_exec_entity[1].ce_migration_func +#define cc_migration_arg_dir cc_exec_entity[1].ce_migration_arg +#define cc_migration_cpu_dir cc_exec_entity[1].ce_migration_cpu +#define cc_migration_time_dir cc_exec_entity[1].ce_migration_time struct callout_cpu cc_cpu[MAXCPU]; #define CPUBLOCK MAXCPU Index: sys/ia64/ia64/machdep.c =================================================================== --- sys/ia64/ia64/machdep.c (.../head) (revision 246685) +++ sys/ia64/ia64/machdep.c (.../projects/calloutng) (revision 246685) @@ -404,7 +405,7 @@ if (sched_runnable()) ia64_enable_intr(); else if (cpu_idle_hook != NULL) { - (*cpu_idle_hook)(); XXX-lr style ? just use cpu_idle_hook(sbt); + (*cpu_idle_hook)(sbt); /* The hook must enable interrupts! */ } else { ia64_call_pal_static(PAL_HALT_LIGHT, 0, 0, 0); Index: sys/ofed/include/linux/timer.h =================================================================== --- sys/ofed/include/linux/timer.h (.../head) (revision 246685) +++ sys/ofed/include/linux/timer.h (.../projects/calloutng) (revision 246685) @@ -65,13 +64,16 @@ callout_init(&(timer)->timer_callout, CALLOUT_MPSAFE); \ } while (0) -#define mod_timer(timer, expire) \ XXX-lr gratuitous rename - callout_reset(&(timer)->timer_callout, (expire) - jiffies, \ - _timer_fn, (timer)) +#define mod_timer(timer, exp) \ +do { \ + (timer)->expires = exp; \ + callout_reset(&(timer)->timer_callout, (exp) - jiffies, \ + _timer_fn, (timer)); \ +} while (0) #define add_timer(timer) \ callout_reset(&(timer)->timer_callout, \ - (timer)->timer_callout.c_time - jiffies, _timer_fn, (timer)) + (timer)->expires - jiffies, _timer_fn, (timer)) #define del_timer(timer) callout_stop(&(timer)->timer_callout) #define del_timer_sync(timer) callout_drain(&(timer)->timer_callout) Index: sys/sys/callout.h =================================================================== --- sys/sys/callout.h (.../head) (revision 246685) +++ sys/sys/callout.h (.../projects/calloutng) (revision 246685) @@ -47,7 +47,17 @@ #define CALLOUT_RETURNUNLOCKED 0x0010 /* handler returns with mtx unlocked */ #define CALLOUT_SHAREDLOCK 0x0020 /* callout lock held in shared mode */ #define CALLOUT_DFRMIGRATION 0x0040 /* callout in deferred migration mode */ +#define CALLOUT_PROCESSED 0x0080 /* callout in wheel or processing list? */ +#define CALLOUT_DIRECT 0x0100 /* allow exec from hw int context */ +#define C_DIRECT_EXEC 0x0001 /* direct execution of callout */ +#define C_PRELBITS 7 XXX-lr document all +#define C_PRELRANGE ((1 << C_PRELBITS) - 1) +#define C_PREL(x) (((x) + 1) << 1) +#define C_PRELGET(x) (int)((((x) >> 1) & C_PRELRANGE) - 1) +#define C_HARDCLOCK 0x0100 /* align to hardclock() calls */ +#define C_ABSOLUTE 0x0200 /* event time is absolute. */ + struct callout_handle { struct callout *callout; }; Index: sys/sys/time.h =================================================================== --- sys/sys/time.h (.../head) (revision 246685) +++ sys/sys/time.h (.../projects/calloutng) (revision 246685) @@ -109,6 +124,45 @@ ((a)->frac cmp (b)->frac) : \ ((a)->sec cmp (b)->sec)) +typedef int64_t sbintime_t; XXX-lr add function ns2sbt(), us2sbt(), ms2sbt() +#define SBT_1S ((sbintime_t)1 << 32) +#define SBT_1M (SBT_1S * 60) +#define SBT_1MS (SBT_1S / 1000) +#define SBT_1US (SBT_1S / 1000000) +#define SBT_1NS (SBT_1S / 1000000000) + +static __inline int +sbintime_getsec(sbintime_t sbt) +{ + + return (int)(sbt >> 32); +} + +static __inline sbintime_t +bintime2sbintime(const struct bintime bt) +{ + + return (((sbintime_t)bt.sec << 32) + (bt.frac >> 32)); +} + +static __inline struct bintime +sbintime2bintime(sbintime_t sbt) +{ + struct bintime bt; + + bt.sec = sbt >> 32; + bt.frac = sbt << 32; + return (bt); + +} + +#ifdef _KERNEL + +extern struct bintime tick_bt; +extern sbintime_t tick_sbt; + +#endif /* KERNEL */ + /*- * Background information: * @@ -290,7 +381,15 @@ extern volatile time_t time_second; extern volatile time_t time_uptime; extern struct bintime boottimebin; +extern struct bintime tc_tick_bt; +extern sbintime_t tc_tick_sbt; XXX-lr comment the new vars extern struct timeval boottime; +extern int tc_precexp; XXX-lr comment +extern int tc_timepercentage; +extern struct bintime bt_timethreshold; +extern struct bintime bt_tickthreshold; +extern sbintime_t sbt_timethreshold; +extern sbintime_t sbt_tickthreshold; /* * Functions for looking at our clock: [get]{bin,nano,micro}[up]time() @@ -337,6 +438,23 @@ void timevaladd(struct timeval *t1, const struct timeval *t2); void timevalsub(struct timeval *t1, const struct timeval *t2); int tvtohz(struct timeval *tv); + +#define TC_DEFAULTPERC 5 XXX-lr comment + +#define BT2FREQ(bt) \ + (((uint64_t)0x8000000000000000 + ((bt)->frac >> 2)) / \ + ((bt)->frac >> 1)) + +#define FREQ2BT(freq, bt) \ +{ \ + (bt)->sec = 0; \ + (bt)->frac = ((uint64_t)0x8000000000000000 / (freq)) << 1; \ +} + +#define TIMESEL(sbt, sbt2) \ + (((sbt2) >= sbt_timethreshold) ? \ + (getsbinuptime(sbt), 1) : (sbinuptime(sbt), 0)) + #else /* !_KERNEL */ #include Index: sys/netinet/tcp_timer.c =================================================================== --- sys/netinet/tcp_timer.c (.../head) (revision 246685) +++ sys/netinet/tcp_timer.c (.../projects/calloutng) (revision 246685) @@ -719,20 +719,24 @@ #define ticks_to_msecs(t) (1000*(t) / hz) void -tcp_timer_to_xtimer(struct tcpcb *tp, struct tcp_timer *timer, struct xtcp_timer *xtimer) +tcp_timer_to_xtimer(struct tcpcb *tp, struct tcp_timer *timer, + struct xtcp_timer *xtimer) { - bzero(xtimer, sizeof(struct xtcp_timer)); + sbintime_t now; + + bzero(xtimer, sizeof(*xtimer)); XXX-lr use sbd2ms() if (timer == NULL) return; + getsbinuptime(&now); if (callout_active(&timer->tt_delack)) - xtimer->tt_delack = ticks_to_msecs(timer->tt_delack.c_time - ticks); + xtimer->tt_delack = (timer->tt_delack.c_time - now) / SBT_1MS; if (callout_active(&timer->tt_rexmt)) - xtimer->tt_rexmt = ticks_to_msecs(timer->tt_rexmt.c_time - ticks); + xtimer->tt_rexmt = (timer->tt_rexmt.c_time - now) / SBT_1MS; if (callout_active(&timer->tt_persist)) - xtimer->tt_persist = ticks_to_msecs(timer->tt_persist.c_time - ticks); + xtimer->tt_persist = (timer->tt_persist.c_time - now) / SBT_1MS; if (callout_active(&timer->tt_keep)) - xtimer->tt_keep = ticks_to_msecs(timer->tt_keep.c_time - ticks); + xtimer->tt_keep = (timer->tt_keep.c_time - now) / SBT_1MS; if (callout_active(&timer->tt_2msl)) - xtimer->tt_2msl = ticks_to_msecs(timer->tt_2msl.c_time - ticks); + xtimer->tt_2msl = (timer->tt_2msl.c_time - now) / SBT_1MS; xtimer->t_rcvtime = ticks_to_msecs(ticks - tp->t_rcvtime); } From owner-freebsd-arch@FreeBSD.ORG Sun Feb 17 03:21:03 2013 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 9074389D; Sun, 17 Feb 2013 03:21:03 +0000 (UTC) (envelope-from des@des.no) Received: from smtp.des.no (smtp.des.no [194.63.250.102]) by mx1.freebsd.org (Postfix) with ESMTP id 464207C1; Sun, 17 Feb 2013 03:21:02 +0000 (UTC) Received: from ds4.des.no (smtp.des.no [194.63.250.102]) by smtp-int.des.no (Postfix) with ESMTP id AE6EC6992; Sun, 17 Feb 2013 03:21:01 +0000 (UTC) Received: by ds4.des.no (Postfix, from userid 1001) id 61EB8924E; Sun, 17 Feb 2013 04:21:01 +0100 (CET) From: =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= To: Pawel Jakub Dawidek Subject: Re: bindat(2) and connectat(2) syscalls for review. References: <20130213230354.GC1375@garage.freebsd.pl> <20130213232004.GA2522@kib.kiev.ua> <20130213234030.GD1375@garage.freebsd.pl> <20130214185549.GA36288@stack.nl> <86ip5saqiu.fsf@ds4.des.no> <20130216232039.GD2023@garage.freebsd.pl> Date: Sun, 17 Feb 2013 04:21:00 +0100 In-Reply-To: <20130216232039.GD2023@garage.freebsd.pl> (Pawel Jakub Dawidek's message of "Sun, 17 Feb 2013 00:20:39 +0100") Message-ID: <86y5enaan7.fsf@ds4.des.no> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2 (berkeley-unix) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: Konstantin Belousov , Jilles Tjoelker , freebsd-arch@FreeBSD.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 17 Feb 2013 03:21:03 -0000 Pawel Jakub Dawidek writes: > Dag-Erling Sm=C3=B8rgrav writes: > > Jilles Tjoelker writes: > > > A flag parameter is a good thing; you may not know yet what you will > > > need it for. > > int bind(int s, const struct sockaddr *addr, socklen_t addrlen); > > int connect(int s, const struct sockaddr *name, socklen_t namelen); > > Where's the flag argument? > The is no flag argument in my patch, but it was proposed by Kostik in > the e-mail Jilles replied to. That was a rhetorical question... unless Jilles wants to go through our entire code base and add a flag argument to every syscall or library function that doesn't already have one, "just in case". DES --=20 Dag-Erling Sm=C3=B8rgrav - des@des.no From owner-freebsd-arch@FreeBSD.ORG Sun Feb 17 09:58:12 2013 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id A551E6EA for ; Sun, 17 Feb 2013 09:58:12 +0000 (UTC) (envelope-from pawel@dawidek.net) Received: from mail.dawidek.net (garage.dawidek.net [91.121.88.72]) by mx1.freebsd.org (Postfix) with ESMTP id 5FF11F86 for ; Sun, 17 Feb 2013 09:58:11 +0000 (UTC) Received: from localhost (89-73-195-149.dynamic.chello.pl [89.73.195.149]) by mail.dawidek.net (Postfix) with ESMTPSA id 3FDF1F1E; Sun, 17 Feb 2013 10:55:18 +0100 (CET) Date: Sun, 17 Feb 2013 10:59:12 +0100 From: Pawel Jakub Dawidek To: Dag-Erling =?iso-8859-1?Q?Sm=F8rgrav?= Subject: Re: bindat(2) and connectat(2) syscalls for review. Message-ID: <20130217095912.GF2023@garage.freebsd.pl> References: <20130213230354.GC1375@garage.freebsd.pl> <20130213232004.GA2522@kib.kiev.ua> <20130213234030.GD1375@garage.freebsd.pl> <20130214185549.GA36288@stack.nl> <86ip5saqiu.fsf@ds4.des.no> <20130216232039.GD2023@garage.freebsd.pl> <86y5enaan7.fsf@ds4.des.no> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ffoCPvUAPMgSXi6H" Content-Disposition: inline In-Reply-To: <86y5enaan7.fsf@ds4.des.no> X-OS: FreeBSD 10.0-CURRENT amd64 User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Konstantin Belousov , Jilles Tjoelker , freebsd-arch@FreeBSD.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 17 Feb 2013 09:58:12 -0000 --ffoCPvUAPMgSXi6H Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Feb 17, 2013 at 04:21:00AM +0100, Dag-Erling Sm=F8rgrav wrote: > Pawel Jakub Dawidek writes: > > Dag-Erling Sm=F8rgrav writes: > > > Jilles Tjoelker writes: > > > > A flag parameter is a good thing; you may not know yet what you will > > > > need it for. > > > int bind(int s, const struct sockaddr *addr, socklen_t addrlen); > > > int connect(int s, const struct sockaddr *name, socklen_t namelen); > > > Where's the flag argument? > > The is no flag argument in my patch, but it was proposed by Kostik in > > the e-mail Jilles replied to. >=20 > That was a rhetorical question... unless Jilles wants to go through our > entire code base and add a flag argument to every syscall or library > function that doesn't already have one, "just in case". Sorry, I thought you pasted bindat() and connectat() prototypes... --=20 Pawel Jakub Dawidek http://www.wheelsystems.com FreeBSD committer http://www.FreeBSD.org Am I Evil? Yes, I Am! http://tupytaj.pl --ffoCPvUAPMgSXi6H Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iEYEARECAAYFAlEgqfAACgkQForvXbEpPzTcgwCgs1rxjY6/bsSV/wtqaZ8mAbfO 5XYAoNxnERBQjsQo7q5aCechYUVWN+i7 =n1B5 -----END PGP SIGNATURE----- --ffoCPvUAPMgSXi6H-- From owner-freebsd-arch@FreeBSD.ORG Sun Feb 17 14:20:54 2013 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 6125AFA9; Sun, 17 Feb 2013 14:20:54 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (unknown [IPv6:2001:610:1108:5012::107]) by mx1.freebsd.org (Postfix) with ESMTP id 298B29C7; Sun, 17 Feb 2013 14:20:54 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id DEDAA1203C5; Sun, 17 Feb 2013 15:20:38 +0100 (CET) Received: by snail.stack.nl (Postfix, from userid 1677) id C96CF2848C; Sun, 17 Feb 2013 15:20:38 +0100 (CET) Date: Sun, 17 Feb 2013 15:20:38 +0100 From: Jilles Tjoelker To: Dag-Erling =?iso-8859-1?Q?Sm=F8rgrav?= Subject: Re: bindat(2) and connectat(2) syscalls for review. Message-ID: <20130217142038.GA55034@stack.nl> References: <20130213230354.GC1375@garage.freebsd.pl> <20130213232004.GA2522@kib.kiev.ua> <20130213234030.GD1375@garage.freebsd.pl> <20130214185549.GA36288@stack.nl> <86ip5saqiu.fsf@ds4.des.no> <20130216232039.GD2023@garage.freebsd.pl> <86y5enaan7.fsf@ds4.des.no> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <86y5enaan7.fsf@ds4.des.no> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Konstantin Belousov , freebsd-arch@FreeBSD.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 17 Feb 2013 14:20:54 -0000 On Sun, Feb 17, 2013 at 04:21:00AM +0100, Dag-Erling Smørgrav wrote: > Pawel Jakub Dawidek writes: > > Dag-Erling Smørgrav writes: > > > Jilles Tjoelker writes: > > > > A flag parameter is a good thing; you may not know yet what you will > > > > need it for. > > > int bind(int s, const struct sockaddr *addr, socklen_t addrlen); > > > int connect(int s, const struct sockaddr *name, socklen_t namelen); > > > Where's the flag argument? > > The is no flag argument in my patch, but it was proposed by Kostik in > > the e-mail Jilles replied to. > That was a rhetorical question... unless Jilles wants to go through our > entire code base and add a flag argument to every syscall or library > function that doesn't already have one, "just in case". The point is not to add a flags argument everywhere for the sake of it; rather, to add a flags argument if a function is extended or created anyway. This leaves some space for future extensibility. For example, POSIX considered eaccess() not useful enough, but when access() was extended to faccessat(), a flag argument was added which allowed the eaccess() functionality. -- Jilles Tjoelker From owner-freebsd-arch@FreeBSD.ORG Sun Feb 17 14:42:22 2013 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 40ED4588 for ; Sun, 17 Feb 2013 14:42:22 +0000 (UTC) (envelope-from pawel@dawidek.net) Received: from mail.dawidek.net (garage.dawidek.net [91.121.88.72]) by mx1.freebsd.org (Postfix) with ESMTP id 0B317A3D for ; Sun, 17 Feb 2013 14:42:21 +0000 (UTC) Received: from localhost (89-73-195-149.dynamic.chello.pl [89.73.195.149]) by mail.dawidek.net (Postfix) with ESMTPSA id DF6F0F96; Sun, 17 Feb 2013 15:39:27 +0100 (CET) Date: Sun, 17 Feb 2013 15:43:22 +0100 From: Pawel Jakub Dawidek To: Jilles Tjoelker Subject: Re: bindat(2) and connectat(2) syscalls for review. Message-ID: <20130217144321.GJ2023@garage.freebsd.pl> References: <20130213230354.GC1375@garage.freebsd.pl> <20130213232004.GA2522@kib.kiev.ua> <20130213234030.GD1375@garage.freebsd.pl> <20130214185549.GA36288@stack.nl> <86ip5saqiu.fsf@ds4.des.no> <20130216232039.GD2023@garage.freebsd.pl> <86y5enaan7.fsf@ds4.des.no> <20130217142038.GA55034@stack.nl> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="k+G3HLlWI7eRTl+h" Content-Disposition: inline In-Reply-To: <20130217142038.GA55034@stack.nl> X-OS: FreeBSD 10.0-CURRENT amd64 User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Konstantin Belousov , Dag-Erling =?iso-8859-1?Q?Sm=F8rgrav?= , freebsd-arch@FreeBSD.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 17 Feb 2013 14:42:22 -0000 --k+G3HLlWI7eRTl+h Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Feb 17, 2013 at 03:20:38PM +0100, Jilles Tjoelker wrote: > On Sun, Feb 17, 2013 at 04:21:00AM +0100, Dag-Erling Sm=F8rgrav wrote: > > Pawel Jakub Dawidek writes: > > > Dag-Erling Sm=F8rgrav writes: > > > > Jilles Tjoelker writes: > > > > > A flag parameter is a good thing; you may not know yet what you w= ill > > > > > need it for. > > > > int bind(int s, const struct sockaddr *addr, socklen_t addrlen); > > > > int connect(int s, const struct sockaddr *name, socklen_t namelen); > > > > Where's the flag argument? > > > The is no flag argument in my patch, but it was proposed by Kostik in > > > the e-mail Jilles replied to. >=20 > > That was a rhetorical question... unless Jilles wants to go through our > > entire code base and add a flag argument to every syscall or library > > function that doesn't already have one, "just in case". >=20 > The point is not to add a flags argument everywhere for the sake of it; > rather, to add a flags argument if a function is extended or created > anyway. This leaves some space for future extensibility. >=20 > For example, POSIX considered eaccess() not useful enough, but when > access() was extended to faccessat(), a flag argument was added which > allowed the eaccess() functionality. But if we are going to do that, it would be nice to have at least one useful flag to use in there:) --=20 Pawel Jakub Dawidek http://www.wheelsystems.com FreeBSD committer http://www.FreeBSD.org Am I Evil? Yes, I Am! http://tupytaj.pl --k+G3HLlWI7eRTl+h Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iEYEARECAAYFAlEg7IkACgkQForvXbEpPzRkMQCgl22uax6ucfmuYmxp0DrQ9Ufe 19sAoNophK9ls8gYVjnbuTOd9yAOru8j =8o6B -----END PGP SIGNATURE----- --k+G3HLlWI7eRTl+h-- From owner-freebsd-arch@FreeBSD.ORG Sun Feb 17 22:26:02 2013 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id EC439638; Sun, 17 Feb 2013 22:26:02 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (unknown [IPv6:2001:610:1108:5012::107]) by mx1.freebsd.org (Postfix) with ESMTP id AE4DAB0E; Sun, 17 Feb 2013 22:26:02 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id 8973C1203C5; Sun, 17 Feb 2013 23:25:46 +0100 (CET) Received: by snail.stack.nl (Postfix, from userid 1677) id 09CB92848C; Sun, 17 Feb 2013 23:25:46 +0100 (CET) Date: Sun, 17 Feb 2013 23:25:45 +0100 From: Jilles Tjoelker To: Pawel Jakub Dawidek Subject: Re: bindat(2) and connectat(2) syscalls for review. Message-ID: <20130217222545.GA58436@stack.nl> References: <20130213230354.GC1375@garage.freebsd.pl> <20130213232004.GA2522@kib.kiev.ua> <20130213234030.GD1375@garage.freebsd.pl> <20130214185549.GA36288@stack.nl> <20130214220853.GB1407@garage.freebsd.pl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130214220853.GB1407@garage.freebsd.pl> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Konstantin Belousov , freebsd-arch@FreeBSD.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 17 Feb 2013 22:26:03 -0000 On Thu, Feb 14, 2013 at 11:08:53PM +0100, Pawel Jakub Dawidek wrote: > bind(2) and connect(2) are used just fine currently without any flags. > I'd like to see good example before I decide to add such argument. The > AT_SYMLINK_NOFOLLOW flag is of no use here, it is used for syscalls that > can operate on symlinks (you can chmod, chown or stat a symlink, so it > does make sense there). By that reasoning, the O_NOFOLLOW open flag should not exist. However, it seems that it is uncommon to bind/connect to sockets located in untrusted directories. Also, any flag could be implemented instead as a setsockopt() on the socket. -- Jilles Tjoelker From owner-freebsd-arch@FreeBSD.ORG Tue Feb 19 16:33:34 2013 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 21783BD5; Tue, 19 Feb 2013 16:33:34 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id ED26824C; Tue, 19 Feb 2013 16:33:33 +0000 (UTC) Received: from pakbsde14.localnet (unknown [38.105.238.108]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 5D22EB943; Tue, 19 Feb 2013 11:33:33 -0500 (EST) From: John Baldwin To: freebsd-arch@freebsd.org Subject: Re: Proposal: Unify printing the function name in panic messages() Date: Tue, 19 Feb 2013 11:33:08 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p25; KDE/4.5.5; amd64; ; ) References: <51141E33.4080103@gmx.de> <51194D73.2010804@FreeBSD.org> In-Reply-To: <51194D73.2010804@FreeBSD.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201302191133.08938.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Tue, 19 Feb 2013 11:33:33 -0500 (EST) Cc: Kirk McKusick , Christoph Mallon , Garance A Drosehn X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Feb 2013 16:33:34 -0000 On Monday, February 11, 2013 2:58:43 pm Garance A Drosehn wrote: > On 2/7/13 4:35 PM, Christoph Mallon wrote: > > > > currently the style for printing panic messages varies greatly. > > Here are some examples of the most common styles: > > panic("just a message"); > > panic("function_name: message"); > > panic("wrong_function_name: message"); > > panic("%s: message", __func__); > > Especially the third -- a wrong function name being shown -- is annoying > > and quite common. I propose a simple macro, which wraps panic() and > > ensures that the right name is shown always by using __func__. > > > Do you have suggestions to improve this proposal? > > > > Chrsopth > > While I'm replying to the first message in the thread, I've read > most of the messages up to this point. I do very little development > in the kernel, but as it happens I've spent the last three weeks > untangling a somewhat large C-based project which has similar > messages, and due to the nature of the project it's nearly impossible > to run debuggers on it. So I can't just pop into the debugger and > get some kind of stack trace. > > That project uses __func__ on some printf()s, and __FILE__ on others. > I found it quite useful there. When tracking down a few specific > bugs there were multiple places which could generate the error > message I was looking for, so I added __LINE__ to those printf()s. > This was helpful, particularly in one case where the entire message > was specified as a constant string in one place, but the same error > *could* be printed out by a different printf which built the message > via format specifiers. So scanning for the message would pick up > the first case as an obvious hit, and never notice the second case. > And, of course, it turned out that the message in my log file was > coming from that second case. > > So maybe it'd be good to have two panic options. One with the > __LINE__ number, and one without. Here's a (possibly) useful suggestion. Rather than having a macro that alters the format string, perhaps change panic() to be this: static void dopanic(const char *func, const char *file, int line, const char *fmt, ...); #define panic(...) do_panic(__func__, __FILE__, __LINE__, __VA_ARGS__) You could then use a runtime sysctl to decide if you want to log the extra metadata along with a panic. Keeping the "raw" format string works better for my testing stuff since I have a hook at the start of panic() and it can use the string without the extra metadata (and even possibly supply the metadata in the 'catch' phase so I can ignore panics based on that metadata. However, one consideration with this is how much bloat comes from having the various __func__ and __FILE__ strings (esp. the latter). You may want to have an option to strip them out (see LOCK_FILE/LOCK_LINE where we use NULL instead in production kernels when calling locking primitive routines to avoid the bloat of filenames). Also, if you want to use __FILE__ at all you will likely need something akin to 'fixup_filename()' from subr_witness.c. I think that one is tailored more to old style kernel builds and probably doesn't DTRT for the 'make buildkernel' case. It maybe that fixup_filename() should just be basename() at this point. -- John Baldwin From owner-freebsd-arch@FreeBSD.ORG Tue Feb 19 19:26:26 2013 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id C606C91F; Tue, 19 Feb 2013 19:26:26 +0000 (UTC) (envelope-from gad@FreeBSD.org) Received: from smtp5.server.rpi.edu (smtp5.server.rpi.edu [128.113.2.225]) by mx1.freebsd.org (Postfix) with ESMTP id 7E70E14D; Tue, 19 Feb 2013 19:26:26 +0000 (UTC) Received: from gilead.netel.rpi.edu (gilead.netel.rpi.edu [128.113.124.121]) by smtp5.server.rpi.edu (8.13.1/8.13.1) with ESMTP id r1JIKt2c028525; Tue, 19 Feb 2013 13:20:55 -0500 Message-ID: <5123C287.4040201@FreeBSD.org> Date: Tue, 19 Feb 2013 13:20:55 -0500 From: Garance A Drosehn User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.9) Gecko/20100722 Eudora/3.0.4 MIME-Version: 1.0 To: John Baldwin Subject: Re: Proposal: Unify printing the function name in panic messages() References: <51141E33.4080103@gmx.de> <51194D73.2010804@FreeBSD.org> <201302191133.08938.jhb@freebsd.org> In-Reply-To: <201302191133.08938.jhb@freebsd.org> Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Bayes-Prob: 0.0001 (Score 0) X-RPI-SA-Score: 0.70 () [Hold at 11.00] COMBINED_FROM,J_CHICKENPOX_75 X-CanItPRO-Stream: outgoing X-Canit-Stats-ID: 57188035 - 16fe276b51eb X-Scanned-By: CanIt (www . roaringpenguin . com) on 128.113.2.225 Cc: Kirk McKusick , Christoph Mallon , freebsd-arch@FreeBSD.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Feb 2013 19:26:26 -0000 On 2/19/13 11:33 AM, John Baldwin wrote: > On Monday, February 11, 2013 Garance A Drosehn wrote: >> >> That project uses __func__ on some printf()s, and __FILE__ on others. >> I found it quite useful there. When tracking down a few specific >> bugs there were multiple places which could generate the error >> message I was looking for, so I added __LINE__ to those printf()s. >> This was helpful, particularly in one case where the entire message >> was specified as a constant string in one place, but the same error >> *could* be printed out by a different printf which built the message >> via format specifiers. So scanning for the message would pick up >> the first case as an obvious hit, and never notice the second case. >> And, of course, it turned out that the message in my log file was >> coming from that second case. >> >> So maybe it'd be good to have two panic options. One with the >> __LINE__ number, and one without. > > Here's a (possibly) useful suggestion. Rather than having a > macro that alters the format string, perhaps change panic() > to be this: > > static void > dopanic(const char *func, const char *file, int line, > const char *fmt, ...); > > #define panic(...) do_panic(__func__, __FILE__, \ > __LINE__, __VA_ARGS__) > > You could then use a runtime sysctl to decide if you want to > log the extra metadata along with a panic. Keeping the "raw" > format string works better for my testing stuff since I have > a hook at the start of panic() and it can use the string > without the extra metadata (and even possibly supply the > metadata in the 'catch' phase so I can ignore panics based > on that metadata. I've tried this, and there's one minor annoyance with it. Let's say the programmer does: panic(badaddr: invalid size (%s)", int_size); The compiler will warn that parameter #5 does not match the format-code, not parameter #2. In this example it's easy to see the problem, but it gets more confusing if the call does have a lot of parameters. But in glancing through the panic() messages in the base source, almost all of them are so short that this wouldn't be much of an issue. Yet another alternate tactic would be to have: static void prepanic1(const char *func); static void prepanic2(const char *file, int line); static void prepanic3(const char *file, const char *func, int line); And then on a per-source-file basis, one could select one of: #define panic(...) { prepanic1(__func__) ; panic(__VA_ARGS__) } #define panic(...) { prepanic2(__FILE__, __LINE__) ; \ panic(__VA_ARGS__) } #define panic(...) { prepanic3(__FILE__, __func__, __LINE__) ; \ panic(__VA_ARGS__) } One could even just #define panic1(), panic2(), and panic3(), and then let the programmer decide on a per-call basis how much detail any given panic message will need. Either way, someone who wanted to recompile could trim unwanted bloat with very little effort. Apologies if I seem obsessed about these alternatives. It's just that I have happened to be working on something similar to this over the last two weeks. (for a project unreleated to FreeBSD). > However, one consideration with this is how much bloat comes from > having the various __func__ and __FILE__ strings (esp. the latter). > You may want to have an option to strip them out (see > LOCK_FILE/LOCK_LINE where we use NULL instead in production kernels > when calling locking primitive routines to avoid the bloat of > filenames). Also, if you want to use __FILE__ at all you will > likely need something akin to 'fixup_filename()' from subr_witness.c. > I think that one is tailored more to old style kernel builds and > probably doesn't DTRT for the 'make buildkernel' case. It maybe > that fixup_filename() should just be basename() at this point. Hmm. In the places where I've used this, __FILE__ has produced just the basename of the file. Maybe I'm always compiling things where 'cc' was given just the basename, but I'm pretty sure some of my things are compiled as "subdir/somefile.c". Quite possible that I've just been lucky on this! -- Garance Alistair Drosehn = gad@gilead.netel.rpi.edu Senior Systems Programmer or gad@freebsd.org Rensselaer Polytechnic Institute or drosih@rpi.edu From owner-freebsd-arch@FreeBSD.ORG Tue Feb 19 22:26:34 2013 Return-Path: Delivered-To: arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 65CBE4BD for ; Tue, 19 Feb 2013 22:26:34 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id B707AB75 for ; Tue, 19 Feb 2013 22:26:33 +0000 (UTC) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id AAA19003; Wed, 20 Feb 2013 00:26:21 +0200 (EET) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1U7vdl-0006Gd-8o; Wed, 20 Feb 2013 00:26:21 +0200 Message-ID: <5123FC0B.3020503@FreeBSD.org> Date: Wed, 20 Feb 2013 00:26:19 +0200 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/20130121 Thunderbird/17.0.2 MIME-Version: 1.0 To: Alfred Perlstein Subject: Re: request for preliminary review, enhanced watchdog. References: <511AE9C4.4030301@mu.org> In-Reply-To: <511AE9C4.4030301@mu.org> X-Enigmail-Version: 1.4.6 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: "arch@freebsd.org" , Poul-Henning Kamp X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Feb 2013 22:26:34 -0000 on 13/02/2013 03:17 Alfred Perlstein said the following: > At work we've had some issues with superfluous watchdog timeouts firing. > > Since we use an ipmi/external watchdog the system is completely reset and we are > unable to gather metrics. > > I investigated the issue and then compared to what is offered by Linux and > decided to crib from their API such that we can benefit from an enhanced watchdog. > > I have a WIP at this time in a branch that I would hope people could weigh in on > and review as well as make technical suggestions. Alfred, I think that this is very useful work. Some comments below. > The branch is located here: > svn+ssh://svn.freebsd.org/base/user/alfred/ewatchdog > > The easy way to get changes: > svn log --stop-on-copy svn+ssh://svn.freebsd.org/base/user/alfred/ewatchdog > > 1) Support for pre-watchdog timeout. This means that so long as the kernel is > somewhat functional (callouts are working) we can trigger a configurable action > (panic,ddb,log) if the watchdog program is otherwise hung. I see where this can be useful. The unfortunate drawback which you mentioned is that the solution is "semi-reliable" - it won't help much if a hang is such that the callouts no longer fire. But it could be still desirable to obtain something for postmortem analysis even in that condition. > 2) Support for built-in software watchdog that has the same options > (panic,ddb,log) if the watchdog times out. This is useful for prototyping and > was done instead of using the SW_WATCHDOG in kern_clock.c because of the ease of > working the code into watchdog.c versus communication via the EVENTHANDLER api. I see why you chose (or had to choose) this option, but this is kind of unfortunate - more below. > 3) Support for Linux-like API. (WDIOC_GETTIMELEFT, > WDIOC_SETTIMEOUT,WDIOC_GETTIMEOUT, etc) I haven't looked at the complete Linux API, but from you quote above - what are the Linux and potential FreeBSD use-cases for the ioctls like GETTIMELEFT and GETTIMEOUT? > 4) Modifications to watchdogd(8): > - Warn if the watchdog program takes too long. > - Disable activation of the system watchdog so that one can test the > watchdogd script > without potentially rebooting the system. > - Ability to log to syslog when scripts begin to timeout. > - When told to measure time, do not unconditionally nap for 'sleep' seconds, > instead adjust > the naptime by the elapsed time so as not to trigger the watchdog. I don't have anything to say about the userland part. In general these new things sound useful. > I've not yet hooked in the optional pre-timeout code into watchdogd(8) but plan > on doing so later in the week. > > It would be really helpful if we could decide on a way of selecting which > watchdogs to arm/fire and how to query them. I may adopt the Linux API unless > someone has alternative suggestions that make a strong enough case to forge our > own API. Again, I haven't examined Linux API, so I can't say much about it. The following is how I imagine our watchdog infrastructure. I think that we should have some quality and feature flags associated with various watchdog drivers (somewhat similarly to e.g. eventtimers), which would describe things like: - I am implemented in software or hardware - I am able to generate system reset - I am able to generate a "hard" debug event (NMI) - (for software wd) I work via NMIs or regular interrupts Then ,I think that watchdogd should support at least two timeouts: for debug watchdog and reset watchdog. The ioctl interface should of course support setting timeouts per watchdog type. This way a user should be able to specify a timeout (e.g. 10 seconds) for a debug watchdog with an intent of dumping a core (or other debugging action) and a different timeout (say 60 seconds) for a reset watchdog, which should make sure in a fail-safe manner that a system doesn't get stuck in the debug/dump/etc code. Then, the kernel should auto-select the best watchdog driver for each of the watchdog classes. But sysctl interface should allow a user to override the selection in case that there are multiple drivers with sufficient capabilities. Also, and only partially related to your WIP, I think that it is long overdue that we got a software watchdog driven by (periodic) NMIs as opposed to SW_WATCHDOG (or your "callout" "watchdog" [in quotes only because it is not implemented as a real watchdog(9) driver, but is blended into the infrastructure]) that is driven by regular timer interrupts. My opinion is that such infrastructure could be more powerful and flexible (and reliable) than what you currently have in the branch. We could let a multitude of watchdog drivers co-exist and "cooperate" by ensuring that each of them does its special part of the overall job. Of course, it requires more work too. -- Andriy Gapon From owner-freebsd-arch@FreeBSD.ORG Tue Feb 19 23:14:52 2013 Return-Path: Delivered-To: arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id AC0A333E; Tue, 19 Feb 2013 23:14:52 +0000 (UTC) (envelope-from bright@mu.org) Received: from elvis.mu.org (elvis.mu.org [192.203.228.196]) by mx1.freebsd.org (Postfix) with ESMTP id 81BF6FF4; Tue, 19 Feb 2013 23:14:52 +0000 (UTC) Received: from Alfreds-MacBook-Pro-9.local (c-67-180-208-218.hsd1.ca.comcast.net [67.180.208.218]) by elvis.mu.org (Postfix) with ESMTPSA id 80D021A3C6A; Tue, 19 Feb 2013 15:14:33 -0800 (PST) Message-ID: <51240759.9030900@mu.org> Date: Tue, 19 Feb 2013 15:14:33 -0800 From: Alfred Perlstein User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:17.0) Gecko/20130107 Thunderbird/17.0.2 MIME-Version: 1.0 To: Andriy Gapon Subject: Re: request for preliminary review, enhanced watchdog. References: <511AE9C4.4030301@mu.org> <5123FC0B.3020503@FreeBSD.org> In-Reply-To: <5123FC0B.3020503@FreeBSD.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: "arch@freebsd.org" , Poul-Henning Kamp X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Feb 2013 23:14:52 -0000 On 2/19/13 2:26 PM, Andriy Gapon wrote: > on 13/02/2013 03:17 Alfred Perlstein said the following: >> At work we've had some issues with superfluous watchdog timeouts firing. >> >> Since we use an ipmi/external watchdog the system is completely reset and we are >> unable to gather metrics. >> >> I investigated the issue and then compared to what is offered by Linux and >> decided to crib from their API such that we can benefit from an enhanced watchdog. >> >> I have a WIP at this time in a branch that I would hope people could weigh in on >> and review as well as make technical suggestions. > Alfred, > > I think that this is very useful work. > Some comments below. > >> The branch is located here: >> svn+ssh://svn.freebsd.org/base/user/alfred/ewatchdog >> >> The easy way to get changes: >> svn log --stop-on-copy svn+ssh://svn.freebsd.org/base/user/alfred/ewatchdog >> >> 1) Support for pre-watchdog timeout. This means that so long as the kernel is >> somewhat functional (callouts are working) we can trigger a configurable action >> (panic,ddb,log) if the watchdog program is otherwise hung. > I see where this can be useful. > The unfortunate drawback which you mentioned is that the solution is > "semi-reliable" - it won't help much if a hang is such that the callouts no > longer fire. > But it could be still desirable to obtain something for postmortem analysis even > in that condition. Yes. Exactly why I have done it so. > >> 2) Support for built-in software watchdog that has the same options >> (panic,ddb,log) if the watchdog times out. This is useful for prototyping and >> was done instead of using the SW_WATCHDOG in kern_clock.c because of the ease of >> working the code into watchdog.c versus communication via the EVENTHANDLER api. > I see why you chose (or had to choose) this option, but this is kind of > unfortunate - more below. Agreed. > >> 3) Support for Linux-like API. (WDIOC_GETTIMELEFT, >> WDIOC_SETTIMEOUT,WDIOC_GETTIMEOUT, etc) > I haven't looked at the complete Linux API, but from you quote above - what are > the Linux and potential FreeBSD use-cases for the ioctls like GETTIMELEFT and > GETTIMEOUT? This would be for reporting purposes. > >> 4) Modifications to watchdogd(8): >> - Warn if the watchdog program takes too long. >> - Disable activation of the system watchdog so that one can test the >> watchdogd script >> without potentially rebooting the system. >> - Ability to log to syslog when scripts begin to timeout. >> - When told to measure time, do not unconditionally nap for 'sleep' seconds, >> instead adjust >> the naptime by the elapsed time so as not to trigger the watchdog. > I don't have anything to say about the userland part. In general these new > things sound useful. > >> I've not yet hooked in the optional pre-timeout code into watchdogd(8) but plan >> on doing so later in the week. >> >> It would be really helpful if we could decide on a way of selecting which >> watchdogs to arm/fire and how to query them. I may adopt the Linux API unless >> someone has alternative suggestions that make a strong enough case to forge our >> own API. > Again, I haven't examined Linux API, so I can't say much about it. > The following is how I imagine our watchdog infrastructure. > > I think that we should have some quality and feature flags associated with > various watchdog drivers (somewhat similarly to e.g. eventtimers), which would > describe things like: > - I am implemented in software or hardware > - I am able to generate system reset > - I am able to generate a "hard" debug event (NMI) > - (for software wd) I work via NMIs or regular interrupts > > Then ,I think that watchdogd should support at least two timeouts: for debug > watchdog and reset watchdog. The ioctl interface should of course support > setting timeouts per watchdog type. > This way a user should be able to specify a timeout (e.g. 10 seconds) for a > debug watchdog with an intent of dumping a core (or other debugging action) and > a different timeout (say 60 seconds) for a reset watchdog, which should make > sure in a fail-safe manner that a system doesn't get stuck in the debug/dump/etc > code. > > Then, the kernel should auto-select the best watchdog driver for each of the > watchdog classes. But sysctl interface should allow a user to override the > selection in case that there are multiple drivers with sufficient capabilities. > > Also, and only partially related to your WIP, I think that it is long overdue > that we got a software watchdog driven by (periodic) NMIs as opposed to > SW_WATCHDOG (or your "callout" "watchdog" [in quotes only because it is not > implemented as a real watchdog(9) driver, but is blended into the > infrastructure]) that is driven by regular timer interrupts. > > My opinion is that such infrastructure could be more powerful and flexible (and > reliable) than what you currently have in the branch. We could let a multitude > of watchdog drivers co-exist and "cooperate" by ensuring that each of them does > its special part of the overall job. Of course, it requires more work too. > As far as this API, this is close enough to the Linux API that it makes sense for us to do this. Do you think that the current work (plus some documentation that is due) is good enough for a step forward and inclusion in the system? -Alfred From owner-freebsd-arch@FreeBSD.ORG Wed Feb 20 12:51:41 2013 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id DBD1B129; Wed, 20 Feb 2013 12:51:41 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 88BD8FAE; Wed, 20 Feb 2013 12:51:41 +0000 (UTC) Received: from pakbsde14.localnet (unknown [38.105.238.108]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id DC67CB95E; Wed, 20 Feb 2013 07:51:40 -0500 (EST) From: John Baldwin To: Garance A Drosehn Subject: Re: Proposal: Unify printing the function name in panic messages() Date: Tue, 19 Feb 2013 15:41:22 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p25; KDE/4.5.5; amd64; ; ) References: <51141E33.4080103@gmx.de> <201302191133.08938.jhb@freebsd.org> <5123C287.4040201@FreeBSD.org> In-Reply-To: <5123C287.4040201@FreeBSD.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201302191541.22311.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Wed, 20 Feb 2013 07:51:41 -0500 (EST) Cc: Kirk McKusick , Christoph Mallon , freebsd-arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Feb 2013 12:51:41 -0000 On Tuesday, February 19, 2013 1:20:55 pm Garance A Drosehn wrote: > On 2/19/13 11:33 AM, John Baldwin wrote: > > On Monday, February 11, 2013 Garance A Drosehn wrote: > >> > >> That project uses __func__ on some printf()s, and __FILE__ on others. > >> I found it quite useful there. When tracking down a few specific > >> bugs there were multiple places which could generate the error > >> message I was looking for, so I added __LINE__ to those printf()s. > >> This was helpful, particularly in one case where the entire message > >> was specified as a constant string in one place, but the same error > >> *could* be printed out by a different printf which built the message > >> via format specifiers. So scanning for the message would pick up > >> the first case as an obvious hit, and never notice the second case. > >> And, of course, it turned out that the message in my log file was > >> coming from that second case. > >> > >> So maybe it'd be good to have two panic options. One with the > >> __LINE__ number, and one without. > > > > Here's a (possibly) useful suggestion. Rather than having a > > macro that alters the format string, perhaps change panic() > > to be this: > > > > static void > > dopanic(const char *func, const char *file, int line, > > const char *fmt, ...); > > > > #define panic(...) do_panic(__func__, __FILE__, \ > > __LINE__, __VA_ARGS__) > > > > You could then use a runtime sysctl to decide if you want to > > log the extra metadata along with a panic. Keeping the "raw" > > format string works better for my testing stuff since I have > > a hook at the start of panic() and it can use the string > > without the extra metadata (and even possibly supply the > > metadata in the 'catch' phase so I can ignore panics based > > on that metadata. > > I've tried this, and there's one minor annoyance with it. Let's say > the programmer does: > panic(badaddr: invalid size (%s)", int_size); > The compiler will warn that parameter #5 does not match the format-code, > not parameter #2. In this example it's easy to see the problem, but it > gets more confusing if the call does have a lot of parameters. But in > glancing through the panic() messages in the base source, almost all > of them are so short that this wouldn't be much of an issue. Eww, that is annoying. > Yet another alternate tactic would be to have: > > static void prepanic1(const char *func); > static void prepanic2(const char *file, int line); > static void prepanic3(const char *file, const char *func, int line); > > And then on a per-source-file basis, one could select one of: > > #define panic(...) { prepanic1(__func__) ; panic(__VA_ARGS__) } > #define panic(...) { prepanic2(__FILE__, __LINE__) ; \ > panic(__VA_ARGS__) } > #define panic(...) { prepanic3(__FILE__, __func__, __LINE__) ; \ > panic(__VA_ARGS__) } > > One could even just #define panic1(), panic2(), and panic3(), and > then let the programmer decide on a per-call basis how much detail > any given panic message will need. Either way, someone who wanted > to recompile could trim unwanted bloat with very little effort. Interesting. You'd have to stuff that data somewhere, perhaps hang it off of 'struct thread' until the real panic() was called. > > However, one consideration with this is how much bloat comes from > > having the various __func__ and __FILE__ strings (esp. the latter). > > You may want to have an option to strip them out (see > > LOCK_FILE/LOCK_LINE where we use NULL instead in production kernels > > when calling locking primitive routines to avoid the bloat of > > filenames). Also, if you want to use __FILE__ at all you will > > likely need something akin to 'fixup_filename()' from subr_witness.c. > > I think that one is tailored more to old style kernel builds and > > probably doesn't DTRT for the 'make buildkernel' case. It maybe > > that fixup_filename() should just be basename() at this point. > > Hmm. In the places where I've used this, __FILE__ has produced just > the basename of the file. Maybe I'm always compiling things where > 'cc' was given just the basename, but I'm pretty sure some of my > things are compiled as "subdir/somefile.c". Quite possible that > I've just been lucky on this! I think you've just been lucky. :) % cat /usr/tmp/foo.c const char *somefunc(void); const char * somefunc(void) { return (__FILE__); } % cc -S -o /dev/stdout /usr/tmp/foo.c .file "foo.c" .section .rodata .LC0: .string "/usr/tmp/foo.c" .text .p2align 4,,15 .globl somefunc .type somefunc, @function somefunc: .LFB2: pushq %rbp .LCFI0: movq %rsp, %rbp .LCFI1: movl $.LC0, %eax leave ret .LFE2: Even better: % cc -S -o /dev/stdout /usr/tmp/../../usr/tmp/foo.c .file "foo.c" .section .rodata .LC0: .string "/usr/tmp/../../usr/tmp/foo.c" .text -- John Baldwin From owner-freebsd-arch@FreeBSD.ORG Wed Feb 20 15:59:45 2013 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 7303B863 for ; Wed, 20 Feb 2013 15:59:45 +0000 (UTC) (envelope-from damjan.jov@gmail.com) Received: from mail-la0-x22d.google.com (mail-la0-x22d.google.com [IPv6:2a00:1450:4010:c03::22d]) by mx1.freebsd.org (Postfix) with ESMTP id D234AE1F for ; Wed, 20 Feb 2013 15:59:44 +0000 (UTC) Received: by mail-la0-f45.google.com with SMTP id er20so7757847lab.4 for ; Wed, 20 Feb 2013 07:59:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:mime-version:in-reply-to:references:from:date:message-id :subject:to:content-type; bh=OFTq7Afd6aHqX7pbtMXoDnLJbjkm1aHqdmplHTqYZPg=; b=eYeKSR6coU7hXolrTrlOaP4grw59purtk8qoCEHYiNaAeNF1AWeQu8AS/Q9GTD2NFU NuUVbklyI+dvWfGEf4T9CRGHAk6HiaNNuIBQsCdEk7k+dovSXvZghS1kySKD6zYDujys 3Z56Jd/JebLWcCFsSAYTboCZvQO8X4Q8pyj9w1qWEg1XQMSrO46h9NabuRdGkIGHuEFr b3yWsXpwlt/e/+hs943+VMk160UHEkpExqjalnsXGMv2kmN1V4gyqG5mu9EI6+Di7l5h KleRU8ieZ65ZbbpshzFF0WNSZkKbepkwer7qq06c1fqpmGiAjlp+pI4f5Tbv3v0RsWzr yOgg== X-Received: by 10.152.108.203 with SMTP id hm11mr18098037lab.4.1361375983816; Wed, 20 Feb 2013 07:59:43 -0800 (PST) MIME-Version: 1.0 Received: by 10.152.20.138 with HTTP; Wed, 20 Feb 2013 07:59:23 -0800 (PST) In-Reply-To: <20120223171210.GB79013@zim.MIT.EDU> References: <201202061916.45998.tijl@coosemans.org> <20120223171210.GB79013@zim.MIT.EDU> From: Damjan Jovanovic Date: Wed, 20 Feb 2013 17:59:23 +0200 Message-ID: Subject: Re: amd64 cc -m32 support and headers project branch To: Tijl Coosemans , freebsd-arch@freebsd.org Content-Type: text/plain; charset=ISO-8859-1 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Feb 2013 15:59:45 -0000 On Thu, Feb 23, 2012 at 7:12 PM, David Schultz wrote: > On Mon, Feb 06, 2012, Tijl Coosemans wrote: >> After a hiatus of almost a year I'd like to continue with merging i386 >> and amd64 headers to get "cc -m32" working on amd64. Previously I tried >> to fix any problems with the headers first before merging, but this turned >> out to be a long tedious process. So, because lack of -m32 support is >> blocking other development I'd like to just merge a minimal set of >> headers without any macro/type definition changes or other >> reorganisations. The result will be similar to existing powerpc and mips >> headers which already support both 32 and 64 bit. >> >> When that's done I can create a development branch to work on the >> problems that have come up. Possible goals for such a branch are: >> >> - Fix inconsistencies such as types defined in sys/ and their limits >> in machine/. Also, sometimes the limits don't have the correct type. >> - For types/limits defined under machine/ there is a lot of duplication >> between architectures. Try to move some to sys/. >> - Try to make headers compiler agnostic; limit use of __attribute__, >> __builtin_* to a single file. >> - Maybe remove support for gcc 2.*. The oldest version in ports is 3.4 >> and I suspect some headers don't compile anymore with gcc 2.*. >> - Add support for new compiler attributes/built-ins. >> - Add missing POSIX prototypes, maybe commented out with /* UNIMPL ... */ >> or similar so they can be grepped. >> - The gcc ports currently patch some system headers where they think >> something is non-standard. These patched headers override the system >> headers which means you have to rebuild these ports whenever you do >> installworld to make sure they contain the latest changes. Some headers >> are trivial to fix, others less so. >> >> >> If there are no objections, I'd like to start with the -m32 work in >> a week or so. > > This sounds like it could degenerate into an error-prone ifdef > hell. Wouldn't it be much easier to just have a /usr/include32 > directory? But we don't have include32 for any other architecture. And how would it be implemented, compiler magic or #ifdef __LP64__ ..... #else #include <../include32/myself.h> #endif? On a possibly unrelated note, please let's try avoid the colossal fail that Debian's/Ubuntu's "multiarch" ideas has become, where they succeeded in separating 32 and 64 bit libraries into /usr/lib/i386-linux-gnu and /usr/lib/x86_64-linux-gnu, only to fail at doing the same for /usr/include, causing you to be able to parallel install the 32 and 64 bit binary versions of a package, but not the 32 and 64 bit development packages... From owner-freebsd-arch@FreeBSD.ORG Wed Feb 20 17:25:51 2013 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id AD633610 for ; Wed, 20 Feb 2013 17:25:51 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from smtpauth4.wiscmail.wisc.edu (wmauth4.doit.wisc.edu [144.92.197.145]) by mx1.freebsd.org (Postfix) with ESMTP id 8904B362 for ; Wed, 20 Feb 2013 17:25:51 +0000 (UTC) MIME-version: 1.0 Content-transfer-encoding: 7BIT Content-type: text/plain; CHARSET=US-ASCII; format=flowed Received: from avs-daemon.smtpauth4.wiscmail.wisc.edu by smtpauth4.wiscmail.wisc.edu (Oracle Communications Messaging Server 7u4-27.01(7.0.4.27.0) 64bit (built Aug 30 2012)) id <0MII00600Z6AI400@smtpauth4.wiscmail.wisc.edu> for freebsd-arch@freebsd.org; Wed, 20 Feb 2013 10:25:45 -0600 (CST) X-Spam-PmxInfo: Server=avs-4, Version=5.6.1.2065439, Antispam-Engine: 2.7.2.376379, Antispam-Data: 2013.2.20.161219, SenderIP=0.0.0.0 X-Spam-Report: AuthenticatedSender=yes, SenderIP=0.0.0.0 Received: from terminus.icecube.wisc.edu (i3-user-nat.icecube.wisc.edu [128.104.255.12]) by smtpauth4.wiscmail.wisc.edu (Oracle Communications Messaging Server 7u4-27.01(7.0.4.27.0) 64bit (built Aug 30 2012)) with ESMTPSA id <0MIJ008661MWES20@smtpauth4.wiscmail.wisc.edu>; Wed, 20 Feb 2013 10:25:44 -0600 (CST) Message-id: <5124F908.3010901@freebsd.org> Date: Wed, 20 Feb 2013 10:25:44 -0600 From: Nathan Whitehorn User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/20130218 Thunderbird/17.0.2 To: Damjan Jovanovic Subject: Re: amd64 cc -m32 support and headers project branch References: <201202061916.45998.tijl@coosemans.org> <20120223171210.GB79013@zim.MIT.EDU> In-reply-to: Cc: Tijl Coosemans , freebsd-arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Feb 2013 17:25:51 -0000 On 02/20/13 09:59, Damjan Jovanovic wrote: > On Thu, Feb 23, 2012 at 7:12 PM, David Schultz wrote: >> On Mon, Feb 06, 2012, Tijl Coosemans wrote: >>> After a hiatus of almost a year I'd like to continue with merging i386 >>> and amd64 headers to get "cc -m32" working on amd64. Previously I tried >>> to fix any problems with the headers first before merging, but this turned >>> out to be a long tedious process. So, because lack of -m32 support is >>> blocking other development I'd like to just merge a minimal set of >>> headers without any macro/type definition changes or other >>> reorganisations. The result will be similar to existing powerpc and mips >>> headers which already support both 32 and 64 bit. >>> >>> When that's done I can create a development branch to work on the >>> problems that have come up. Possible goals for such a branch are: >>> >>> - Fix inconsistencies such as types defined in sys/ and their limits >>> in machine/. Also, sometimes the limits don't have the correct type. >>> - For types/limits defined under machine/ there is a lot of duplication >>> between architectures. Try to move some to sys/. >>> - Try to make headers compiler agnostic; limit use of __attribute__, >>> __builtin_* to a single file. >>> - Maybe remove support for gcc 2.*. The oldest version in ports is 3.4 >>> and I suspect some headers don't compile anymore with gcc 2.*. >>> - Add support for new compiler attributes/built-ins. >>> - Add missing POSIX prototypes, maybe commented out with /* UNIMPL ... */ >>> or similar so they can be grepped. >>> - The gcc ports currently patch some system headers where they think >>> something is non-standard. These patched headers override the system >>> headers which means you have to rebuild these ports whenever you do >>> installworld to make sure they contain the latest changes. Some headers >>> are trivial to fix, others less so. >>> >>> >>> If there are no objections, I'd like to start with the -m32 work in >>> a week or so. >> This sounds like it could degenerate into an error-prone ifdef >> hell. Wouldn't it be much easier to just have a /usr/include32 >> directory? > But we don't have include32 for any other architecture. And how would > it be implemented, compiler magic or #ifdef __LP64__ ..... #else > #include <../include32/myself.h> #endif? > > On a possibly unrelated note, please let's try avoid the colossal fail > that Debian's/Ubuntu's "multiarch" ideas has become, where they > succeeded in separating 32 and 64 bit libraries into > /usr/lib/i386-linux-gnu and /usr/lib/x86_64-linux-gnu, only to fail at > doing the same for /usr/include, causing you to be able to parallel > install the 32 and 64 bit binary versions of a package, but not the 32 > and 64 bit development packages... > _______________________________________________ > freebsd-arch@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" I'll point out again that the #ifdef route is what we are currently doing, and have done for a couple years, for -m32 on PowerPC and MIPS. All the header files are actually shared for these architectures between 32 and 64-bit versions (similar to sys/x86, but more complete) and it all works very well with a minimum of ugliness. I'd strongly encourage the same route for x86. -Nathan From owner-freebsd-arch@FreeBSD.ORG Wed Feb 20 17:34:47 2013 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id EEC31DA0; Wed, 20 Feb 2013 17:34:47 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) by mx1.freebsd.org (Postfix) with ESMTP id 5CCA7677; Wed, 20 Feb 2013 17:34:47 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.6/8.14.6) with ESMTP id r1KHYeUm085980; Wed, 20 Feb 2013 19:34:40 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.7.4 kib.kiev.ua r1KHYeUm085980 Received: (from kostik@localhost) by tom.home (8.14.6/8.14.6/Submit) id r1KHYe10085979; Wed, 20 Feb 2013 19:34:40 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Wed, 20 Feb 2013 19:34:40 +0200 From: Konstantin Belousov To: Nathan Whitehorn Subject: Re: amd64 cc -m32 support and headers project branch Message-ID: <20130220173440.GH2598@kib.kiev.ua> References: <201202061916.45998.tijl@coosemans.org> <20120223171210.GB79013@zim.MIT.EDU> <5124F908.3010901@freebsd.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="OdQvBiqfLsaeimeB" Content-Disposition: inline In-Reply-To: <5124F908.3010901@freebsd.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on tom.home Cc: Damjan Jovanovic , Tijl Coosemans , freebsd-arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Feb 2013 17:34:48 -0000 --OdQvBiqfLsaeimeB Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Feb 20, 2013 at 10:25:44AM -0600, Nathan Whitehorn wrote: > On 02/20/13 09:59, Damjan Jovanovic wrote: > > On Thu, Feb 23, 2012 at 7:12 PM, David Schultz wrote: > >> On Mon, Feb 06, 2012, Tijl Coosemans wrote: > >>> After a hiatus of almost a year I'd like to continue with merging i386 > >>> and amd64 headers to get "cc -m32" working on amd64. Previously I tri= ed > >>> to fix any problems with the headers first before merging, but this t= urned > >>> out to be a long tedious process. So, because lack of -m32 support is > >>> blocking other development I'd like to just merge a minimal set of > >>> headers without any macro/type definition changes or other > >>> reorganisations. The result will be similar to existing powerpc and m= ips > >>> headers which already support both 32 and 64 bit. > >>> > >>> When that's done I can create a development branch to work on the > >>> problems that have come up. Possible goals for such a branch are: > >>> > >>> - Fix inconsistencies such as types defined in sys/ and their limits > >>> in machine/. Also, sometimes the limits don't have the correct typ= e. > >>> - For types/limits defined under machine/ there is a lot of duplicati= on > >>> between architectures. Try to move some to sys/. > >>> - Try to make headers compiler agnostic; limit use of __attribute__, > >>> __builtin_* to a single file. > >>> - Maybe remove support for gcc 2.*. The oldest version in ports is 3.4 > >>> and I suspect some headers don't compile anymore with gcc 2.*. > >>> - Add support for new compiler attributes/built-ins. > >>> - Add missing POSIX prototypes, maybe commented out with /* UNIMPL ..= =2E */ > >>> or similar so they can be grepped. > >>> - The gcc ports currently patch some system headers where they think > >>> something is non-standard. These patched headers override the syst= em > >>> headers which means you have to rebuild these ports whenever you do > >>> installworld to make sure they contain the latest changes. Some he= aders > >>> are trivial to fix, others less so. > >>> > >>> > >>> If there are no objections, I'd like to start with the -m32 work in > >>> a week or so. > >> This sounds like it could degenerate into an error-prone ifdef > >> hell. Wouldn't it be much easier to just have a /usr/include32 > >> directory? > > But we don't have include32 for any other architecture. And how would > > it be implemented, compiler magic or #ifdef __LP64__ ..... #else > > #include <../include32/myself.h> #endif? > > > > On a possibly unrelated note, please let's try avoid the colossal fail > > that Debian's/Ubuntu's "multiarch" ideas has become, where they > > succeeded in separating 32 and 64 bit libraries into > > /usr/lib/i386-linux-gnu and /usr/lib/x86_64-linux-gnu, only to fail at > > doing the same for /usr/include, causing you to be able to parallel > > install the 32 and 64 bit binary versions of a package, but not the 32 > > and 64 bit development packages... > > _______________________________________________ > > freebsd-arch@freebsd.org mailing list > > http://lists.freebsd.org/mailman/listinfo/freebsd-arch > > To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" >=20 > I'll point out again that the #ifdef route is what we are currently=20 > doing, and have done for a couple years, for -m32 on PowerPC and MIPS.=20 > All the header files are actually shared for these architectures between= =20 > 32 and 64-bit versions (similar to sys/x86, but more complete) and it=20 > all works very well with a minimum of ugliness. I'd strongly encourage=20 > the same route for x86. Fully agreed. I will commit the last missing bits for the sys/ headers merge for x86 in a minute. The only significant piece left is the libm headers. --OdQvBiqfLsaeimeB Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iQIcBAEBAgAGBQJRJQkvAAoJEJDCuSvBvK1BpP0P/Ammqay2uipnvyPLM8r8BqqK oVk3WQorUcCpaJAcprai6xZQ5B83/tomxHRIosoPG9GX9A7WUFybWxJxWyEllir5 di8cJck8Iw4CGNUb3XWBpavO8ap6DcpFWPkPe5qM8NkcKAO4ldG1wvEfK+eCo9jQ dUmGkcAQMChSAFgZtbG8oHVBmmi5YAcUyOpUkmikkfRy+sYQlJRWZ3qpC0nC6OqK Cy+weImHZOwRKyRAUS0lUrVuuwqr6BKZbFSK/BY0S5yrAuFxCKkG2Ouqg/GBHb1M Uv6poW3wuUHyYDhYyE2jUBYO+eUG0Z9oM6Ju6Gjav4K9iInjgqIPqerXH5PTyY1t 7mDQ9DNL3Wam+dSGUZjX/BcFGgWWYf41mY1vhDMEP7hF5cYu/t4KKKSFOn0xScNR geMsIx9QTgKNyunCd2ZOG6uKjVInrpiVHrH3UcdFKr+oLAzdpGRifI1xdcRoApbG Rz0jZywM3M+g+kBne0d9FF7XVSSVBJGhC5KeYSpgNpUk+cpgWVFu/JO2O3s3QA2t sjjLogqAt5FYA5D0FMT6S2MKYrEFIB+d4vF4wnCJdQKhwngmYWccsCdy8F/Q0m/4 rSKtqSixcBlpUY/Xw4djzV/d5OMFVOlCLtLAzshN7oNOXvJ0xZhG3XOze6wWTbOJ 5JAp7s5JwDuFHMIXQfJ5 =NH+E -----END PGP SIGNATURE----- --OdQvBiqfLsaeimeB-- From owner-freebsd-arch@FreeBSD.ORG Wed Feb 20 19:51:21 2013 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 24E3CFF8; Wed, 20 Feb 2013 19:51:21 +0000 (UTC) (envelope-from gibbs@scsiguy.com) Received: from aslan.scsiguy.com (aslan.scsiguy.com [70.89.174.89]) by mx1.freebsd.org (Postfix) with ESMTP id D664AFCA; Wed, 20 Feb 2013 19:51:20 +0000 (UTC) Received: from [192.168.6.85] (207-225-98-3.dia.static.qwest.net [207.225.98.3]) (authenticated bits=0) by aslan.scsiguy.com (8.14.5/8.14.5) with ESMTP id r1KJpJDH049458 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Wed, 20 Feb 2013 12:51:19 -0700 (MST) (envelope-from gibbs@scsiguy.com) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 6.2 \(1499\)) Subject: Re: bsd.own.mk - just let WITHOUT_* take precedence From: "Justin T. Gibbs" In-Reply-To: <20121025192715.GA31501@dragon.NUXI.org> Date: Wed, 20 Feb 2013 12:51:14 -0700 Content-Transfer-Encoding: quoted-printable Message-Id: <033D35FE-A51C-46B0-9A20-A8E0DAC76B24@scsiguy.com> References: <20121007001423.9878F58094@chaos.jnpr.net> <20121008154853.GC23400@lor.one-eyed-alien.net> <20121022193903.GA88336@dragon.NUXI.org> <20121024154508.GA93546@lor.one-eyed-alien.net> <20121025192715.GA31501@dragon.NUXI.org> To: freebsd-arch@freebsd.org X-Mailer: Apple Mail (2.1499) X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (aslan.scsiguy.com [70.89.174.89]); Wed, 20 Feb 2013 12:51:20 -0700 (MST) Cc: Brooks Davis , James Rogers , Simon Gerraty X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Feb 2013 19:51:21 -0000 On Oct 25, 2012, at 1:27 PM, David O'Brien wrote: > On Wed, Oct 24, 2012 at 10:45:08AM -0500, Brooks Davis wrote: >>> Note that our Handbook () still has: >>>