em(4) vs long mbuf chains
em(4) appears to be susceptible to the same problem that if_bge.c
r1.355 addressed.
long mbuf chains can cause em_encap to fail, which will cause OACTIVE
to be set on the interface, and the problem packet to remain at the
start of the send queue. this will cause no packets to end up on
the tx ring, so em_txeof wont run which is where OACTIVE is cleared.
this diff makes em(4) defrag long mbuf chains.
i havent tested this, could someone give it a spin? or an ok?
Index: if_em.c
===
RCS file: /cvs/src/sys/dev/pci/if_em.c,v
retrieving revision 1.280
diff -u -p -r1.280 if_em.c
--- if_em.c 11 Jun 2014 04:28:43 - 1.280
+++ if_em.c 4 Jul 2014 04:03:23 -
@@ -1133,10 +1133,21 @@ em_encap(struct em_softc *sc, struct mbu
map = tx_buffer->map;
error = bus_dmamap_load_mbuf(sc->txtag, map, m_head, BUS_DMA_NOWAIT);
- if (error != 0) {
+ switch (error) {
+ case 0:
+ break;
+ case EFBIG:
+ if ((error = m_defrag(m_head, M_DONTWAIT)) == 0 &&
+ (error = bus_dmamap_load_mbuf(sc->txtag, map, m_head,
+BUS_DMA_NOWAIT)) == 0)
+ break;
+
+ /* FALLTHROUGH */
+ default:
sc->no_tx_dma_setup++;
goto loaderr;
}
+
EM_KASSERT(map->dm_nsegs!= 0, ("em_encap: empty packet"));
if (map->dm_nsegs > sc->num_tx_desc_avail - 2)
Re: Patch for caesar(6)
Thank you so much for pointing out the problems with my patch - I didn't know about __dead until I saw your counter-patch, so I'm glad I learned something new. I somehow totally missed those other two printit()s (super derpy of me). For the lurkers on the list (I got an email from one already), I did a little post-mortem on my patch, first running basic tests on the three different paths that uses printit: $ echo abcxyz | /usr/games/caesar ghidef $ echo abcxyz | ./caesar ghidef $ echo abcxyz | /usr/games/caesar 4 efgbcd $ echo abcxyz | ./caesar 4 efgbcd $ echo abcxyz | /usr/games/rot13 nopklm $ echo abcxyz | ./rot13 nopklm Odd, because you'd think the second printit would have fired a duplicate set of output. Except it isn't. That's because the first pass of printit reaches the end of the stdin stream with getchar. With the second printit, there's nothing from the stream (a possible race condition, still bad news). I'm really enjoying OpenBSD so far, probably the most fun in computing I've had in a long time :-) Thanks! On Thu, Jul 03, 2014 at 10:32:45PM +0200, Ingo Schwarze wrote: > Hi, > > David Crosby wrote on Thu, Jul 03, 2014 at 07:44:12AM -0600: > > > This little patch fixes a compiler warning > > i just committed my version of the fix. > Thanks to David for reporting the issue > and to Miod for checking my patch. > > Yours, > Ingo >
diff: class-static-route of dhcp-option
ok?
Fix classless-{ms-,}static-routes to comply RFC 3442. Number of
octets should be changed by corresponding to the prefix length. And 0
should be allowed for the prefix length. Also fix white spaces.
Based on diff from Yuuichi Someya.
Index: usr.sbin/dhcpd/confpars.c
===
RCS file: /cvs/src/usr.sbin/dhcpd/confpars.c,v
retrieving revision 1.22
diff -u -p -r1.22 confpars.c
--- usr.sbin/dhcpd/confpars.c 21 Jan 2014 03:07:51 - 1.22
+++ usr.sbin/dhcpd/confpars.c 4 Jul 2014 01:21:18 -
@@ -823,7 +823,7 @@ void parse_group_declaration(cfile, grou
* bit-count :== 0..32
*/
int
-parse_cidr(FILE *cfile, unsigned char *addr, unsigned char *prefix)
+parse_cidr(FILE *cfile, unsigned char *addr, unsigned char *prefix)
{
char *val;
int token;
@@ -835,7 +835,7 @@ parse_cidr(FILE *cfile, unsigned char *a
parse_warn("Expecting CIDR subnet");
goto nocidr;
}
-
+
token = next_token(&val, cfile);
if (token != '/') {
parse_warn("Expecting '/'");
@@ -847,7 +847,7 @@ parse_cidr(FILE *cfile, unsigned char *a
if (token == TOK_NUMBER)
convert_num(prefix, val, 10, 8);
- if (token != TOK_NUMBER || *prefix < 1 || *prefix > 32) {
+ if (token != TOK_NUMBER || *prefix > 32) {
parse_warn("Expecting CIDR prefix length, got '%s'", val);
goto nocidr;
}
@@ -1156,8 +1156,9 @@ void parse_option_param(cfile, group)
return;
tree = tree_concat(tree, tree_const(&cprefix,
sizeof(cprefix)));
- tree = tree_concat(tree, tree_const(buf,
- sizeof(buf)));
+ if (cprefix > 0)
+ tree = tree_concat(tree, tree_const(buf,
+ cprefix / NBBY));
break;
default:
warning("Bad format %c in parse_option_param.",
Reclaim some wasted pages on amd64
I spent yesterday trying to really grok early pmap initialization on
amd64, and I noticed what I believe to be wasted physical pages of
memory:
1. In locore.S, we setup both an "identity" and an "actual" mapping
for the kernel, to help us bootstrap to executing in high memory.
Also, we reserve pages as though these mappings are independent; but
actually the mappings cleverly (accidentally??) reuse the same L2 and
L3 tables, so we can safely reserve just one set of pages. (Just the
same, I've added checks to make sure this is safe, and also to sanity
check that we don't overflow the page tables.)
2. In pmap_bootstrap(), we setup the rest of the direct physical
memory mapping. locore.S has already mapped the first 4GB of memory,
and we don't remove or replace those mappings; so we can save another
couple pages by only allocating page directories for the rest of
physical memory.
In total, this saves a whopping 5 pages (20kB) of physical memory on
my amd64 laptop, as measured by dmesg's "avail mem".
So not terribly important, but I have a patch to enable >512GB of
physical address space that somewhat builds on top of this, and I
think it will be easier to review if these changes are already in
place.
This could probably use some testing under memory-intensive workloads,
especially on systems with >4GB of RAM.
Index: amd64/locore.S
===
RCS file: /home/matthew/anoncvs/cvs/src/sys/arch/amd64/amd64/locore.S,v
retrieving revision 1.54
diff -u -p -r1.54 locore.S
--- amd64/locore.S 10 Nov 2012 09:45:05 - 1.54
+++ amd64/locore.S 3 Jul 2014 23:33:25 -
@@ -358,29 +358,48 @@ bi_size_ok:
* 0 1 2 3
*/
-#if L2_SLOT_KERNBASE > 0
-#define TABLE_L2_ENTRIES (2 * (NKL2_KIMG_ENTRIES + 1))
-#else
-#define TABLE_L2_ENTRIES (NKL2_KIMG_ENTRIES + 1)
-#endif
+/*
+ * We now want to enable paging, move the kernel to high virtual memory, and
+ * jump there; but unfortunately, we can't do that atomically. Instead, we
+ * setup a page table that maps the kernel pages to their eventual virtual
+ * address ranges, but also includes an "identity map" that keeps those pages
+ * available via their physical addresses even once paging is enabled. Later,
+ * once we're executing out of the the eventual VA range, we remove the
+ * identity map.
+ *
+ * As an additional trick to conserve a few pages, the actual map and the
+ * identity map share L2 and L3 tables. This works as long as they don't
+ * require inexactly overlapping table entries, which we check for below.
+ * (It also means the kernel may end up mapped multiple times in virtual
+ * memory, but only until we tear down the identity map.)
+ */
+/* XXX(matthew): Why do we always add 1 to NKL2_KIMG_ENTRIES? */
-#if L3_SLOT_KERNBASE > 0
-#define TABLE_L3_ENTRIES (2 * NKL3_KIMG_ENTRIES)
-#else
-#define TABLE_L3_ENTRIES NKL3_KIMG_ENTRIES
+#if L2_SLOT_KERNBASE > 0 && L2_SLOT_KERNBASE < NKL2_KIMG_ENTRIES + 1
+#error L2 table collision between identity and actual map entries
+#endif
+#if L3_SLOT_KERNBASE > 0 && L3_SLOT_KERNBASE < NKL3_KIMG_ENTRIES
+#error L3 table collision between identity and actual map entries
#endif
+/* Sanity check. */
+#if L2_SLOT_KERNBASE + NKL2_KIMG_ENTRIES + 1 > NPDPG
+#error L2 table overflow
+#endif
+#if L3_SLOT_KERNBASE + NKL3_KIMG_ENTRIES > NPDPG
+#error L3 table overflow
+#endif
#define PROC0_PML4_OFF 0
#define PROC0_STK_OFF (PROC0_PML4_OFF + NBPG)
#define PROC0_PTP3_OFF (PROC0_STK_OFF + UPAGES * NBPG)
#define PROC0_PTP2_OFF (PROC0_PTP3_OFF + NKL4_KIMG_ENTRIES * NBPG)
-#define PROC0_PTP1_OFF (PROC0_PTP2_OFF + TABLE_L3_ENTRIES * NBPG)
-#definePROC0_DMP3_OFF (PROC0_PTP1_OFF + TABLE_L2_ENTRIES * NBPG)
+#define PROC0_PTP1_OFF (PROC0_PTP2_OFF + NKL3_KIMG_ENTRIES * NBPG)
+#definePROC0_DMP3_OFF (PROC0_PTP1_OFF + (NKL2_KIMG_ENTRIES + 1) *
NBPG)
#define PROC0_DMP2_OFF (PROC0_DMP3_OFF + NDML3_ENTRIES * NBPG)
#define TABLESIZE \
-((NKL4_KIMG_ENTRIES + TABLE_L3_ENTRIES + TABLE_L2_ENTRIES + 1 + UPAGES + \
- NDML3_ENTRIES + NDML2_ENTRIES) * NBPG)
+((NKL4_KIMG_ENTRIES + NKL3_KIMG_ENTRIES + (NKL2_KIMG_ENTRIES + 1) + 1 + \
+ UPAGES + NDML3_ENTRIES + NDML2_ENTRIES) * NBPG)
#define fillkpt\
1: movl%eax,(%ebx) ; /* store phys addr */ \
Index: amd64/pmap.c
===
RCS file: /home/matthew/anoncvs/cvs/src/sys/arch/amd64/amd64/pmap.c,v
retrieving revision 1.70
diff -u -p -r1.70 pmap.c
--- amd64/pmap.c15 Jun 2014 11:43:24 - 1.70
+++ amd64/pmap.c3 Jul 2014 23:41:34 -
@@ -510,10 +510,9 @@ pmap_bootstrap(paddr_t first_avail, padd
{
vaddr_t kva, kva_end, kva_start = VM_MIN_KERNEL_ADDRESS;
struct pmap *kpm;
- int i;
+ size_t i, ndmpdp;
unsigned long p1i;
pt_entry_t pg_nx = (cpu_feature & CPUID_NXE? PG_NX : 0);
- long ndmpdp;
Re: daily(8) scratch and junk files removal
Hi Rafael, Rafael Zalamena wrote on Wed, Jul 02, 2014 at 02:21:25PM -0300: > I've made a quick test and there is no problem in 'tmux-*' existing in > /var/tmp after a reboot. Running 'tmux attach' in a socket with no tmux > process controlling just spits out: 'no sessions' and if you run 'tmux' > a new session starts with no problem. I don't know about 'ssh-*' though. > > With the diff I mailed we should be compatible with people who expect > /tmp to be a symbolic link for /var/tmp, but if we want to fix it for > good we should remove the other /var/tmp exceptions and warn users about > it. While i don't consider linking /tmp and /var/tmp to each other a particularly good idea and expect people will encounter occasional trouble doing so, i'd rather not add even more stuff to the list of things that might break, unless for a good reason. Right now, people having separate /tmp and /var/tmp are not hurt by the /var/tmp exceptions: They simply won't have such files in /var/tmp. So i'd rather leave all the exceptions in place. It is also easier to maintain one consistent list (even two copies of it) that two subtly different lists. So, no further code change required unless i'm missing something. As for warning people: Both /tmp and /var/tmp are directories contained in the default install. It goes without saying that you shouldn't fiddle with them unless you know what you are doing and want the pain. The list of bad ideas you could come up with is infinite. We don't warn people either that moving all files from /bin to /usr/bin and deleting /bin is a bad idea, even though that is likely to hurt even more. Assuming that we don't *recommend* linking the two together somewhere, i don't see a documentation change that might be helpful. Yours, Ingo
Re: Patch for caesar(6)
Hi, David Crosby wrote on Thu, Jul 03, 2014 at 07:44:12AM -0600: > This little patch fixes a compiler warning i just committed my version of the fix. Thanks to David for reporting the issue and to Miod for checking my patch. Yours, Ingo
Re: Patch for caesar(6)
Hi David,
David Crosby wrote on Thu, Jul 03, 2014 at 07:44:12AM -0600:
> This little patch fixes a compiler warning
> (found with WARNINGS turned on),
A correct fix for that is below.
OK to commit it?
> unambiguates the printit function, and removes an unnecessary
> comment.
Even though we don't have lint(1) any longer, /* NOT REACHED */
is one of the comments that are occasionally considered useful
even for human eyes, to help reading and auditing code.
I think this is such a case, so I'd just leave the comment.
> Program behaviour the same.
No, it isn't, your patch is wrong.
There are three calls to printit(), and you only exit after one.
> This is my first patch to you folks, sorry if I screwed up somewhere.
No problem, when you start sending patches, expect that some get
rejected. Just try to be diligent and don't give up.
Yours,
Ingo
Index: caesar.c
===
RCS file: /cvs/src/games/caesar/caesar.c,v
retrieving revision 1.15
diff -u -p -r1.15 caesar.c
--- caesar.c22 Feb 2010 18:57:42 - 1.15
+++ caesar.c3 Jul 2014 19:49:42 -
@@ -61,8 +61,8 @@ double stdf[26] = {
2.62, 0.81, 1.88, 0.23, 2.07, 0.06
};
-void printit(int);
-void usage(void);
+__dead void printit(int);
+__dead void usage(void);
int
Patch for caesar(6)
This little patch fixes a compiler warning (found with WARNINGS turned on), unambiguates the printit function, and removes an unnecessary comment. Program behaviour the same. This is my first patch to you folks, sorry if I screwed up somewhere. Index: games/caesar/caesar.c === RCS file: /cvs/src/games/caesar/caesar.c,v retrieving revision 1.15 diff -u -p -w -r1.15 caesar.c --- games/caesar/caesar.c 22 Feb 2010 18:57:42 - 1.15 +++ games/caesar/caesar.c 3 Jul 2014 13:36:59 - @@ -129,7 +129,7 @@ main(int argc, char *argv[]) putchar(ROTATE(ch, winner)); } printit(winner); - /* NOT REACHED */ + exit(0); } void @@ -142,7 +142,6 @@ printit(int rot) while ((ch = getchar()) != EOF) putchar(ROTATE(ch, rot)); - exit(0); } void
