Re: Possible bug in m_split() when splitting M_EXT mbufs
Jacques, On Fri, Jan 11, 2013 at 09:34:32AM +0200, Jacques Fourie wrote: J Could someone please verify if m_split as in svn rev 245286 is doing the J right thing in the scenario where a mbuf chain is split with len0 falling J on a mbuf boundary and the mbuf in question being a M_EXT mbuf? Consider J the following example where m0 is a mbuf chain consisting of 2 M_EXT mbufs, J both 1448 bytes in length. Let len0 be 1448. The 'len0 m-m_len' check J will be false so the for loop will not be entered in this case. We now have J len = 1448 and remain = 0 and m still points to the first mbuf in the J chain. Also assume that m0 is a pkthdr mbuf. A new pkthdr mbuf n will be J allocated and initialized before the following piece of code is executed : J J extpacket: J if (m-m_flags M_EXT) { J n-m_data = m-m_data + len; J mb_dupcl(n, m); J } else { J bcopy(mtod(m, caddr_t) + len, mtod(n, caddr_t), remain); J } J n-m_len = remain; J m-m_len = len; J n-m_next = m-m_next; J m-m_next = NULL; J return (n); J J As m is a M_EXT mbuf the code in the if() clause will be executed. The J problem is that m still points to the first mbuf so effectively the data J pointer for n is assigned to the end of m's data pointer. It should J actually point to the start of the data pointer of the next mbuf in the J original m0 chain, right? Thanks for analysis, Jacques. IMHO, the following patch should suffice and won't break anything: Index: uipc_mbuf.c === --- uipc_mbuf.c (revision 245223) +++ uipc_mbuf.c (working copy) @@ -1126,7 +1126,7 @@ u_int len = len0, remain; MBUF_CHECKSLEEP(wait); - for (m = m0; m len m-m_len; m = m-m_next) + for (m = m0; m len = m-m_len; m = m-m_next) len -= m-m_len; if (m == NULL) return (NULL); Can you please test it? -- Totus tuus, Glebius. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: Inconsistent TCP state in tcp_do_segment()
Jacques, On Fri, Jan 11, 2013 at 09:18:07AM +0200, Jacques Fourie wrote: J I've been using the kernel socket API and noticed that every once in a J while the data received at the remote end of a TCP connection doesn't match J the data sent locally using sosend(). I tracked it down to the piece of J code in tcp_do_segment() starting at line 2665: (svn rev 245286) J Jif (acked so-so_snd.sb_cc) { J tp-snd_wnd -= so-so_snd.sb_cc; J sbdrop_locked(so-so_snd, (int)so-so_snd.sb_cc); J ourfinisacked = 1; J } else { J sbdrop_locked(so-so_snd, acked); J tp-snd_wnd -= acked; J ourfinisacked = 0; J } J sowwakeup_locked(so); J /* Detect una wraparound. */ J if (!IN_RECOVERY(tp-t_flags) J SEQ_GT(tp-snd_una, tp-snd_recover) J SEQ_LEQ(th-th_ack, tp-snd_recover)) J tp-snd_recover = th-th_ack - 1; J /* XXXLAS: Can this be moved up into cc_post_recovery? */ J if (IN_RECOVERY(tp-t_flags) J SEQ_GEQ(th-th_ack, tp-snd_recover)) { J EXIT_RECOVERY(tp-t_flags); J } J tp-snd_una = th-th_ack; J if (tp-t_flags TF_SACK_PERMIT) { J if (SEQ_GT(tp-snd_una, tp-snd_recover)) J tp-snd_recover = tp-snd_una; J } J if (SEQ_LT(tp-snd_nxt, tp-snd_una)) J tp-snd_nxt = tp-snd_una; J J The issue is that sowwakeup_locked() is called before all the tcpcb fields J are updated to account for the current ACK processing as can be seen from J the tp-snd_una = th-th_ack in line 2686. My code that uses the kernel J socket API calls sosend() in the upcall path resulting from J sowwakeup_locked() which in turn can lead to tcp_output() being called with J inconsistent TCP state. If I move the call to sowwakeup_locked() to after J the 'if (SEQ_LT(tp-snd_nxt, tp-snd_una))' line in the code snippet above J the data corruption issue seems to be fixed. Again I think that your analysis is correct, thanks! I have added Robert Watson to Cc of this email, so that he can review your suggestion. However, it seems to me that it isn't safe to call sosend() from the upcall directly. I suppose that if you run your code with WITNESS it will report lock order reversals. The correct way for your module is to run sosend() in separate context, for example using taskqueue(9) API, or running separate thread for that. -- Totus tuus, Glebius. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: Possible bug in m_split() when splitting M_EXT mbufs
On Fri, Jan 11, 2013 at 10:12 AM, Gleb Smirnoff gleb...@freebsd.org wrote: Jacques, On Fri, Jan 11, 2013 at 09:34:32AM +0200, Jacques Fourie wrote: J Could someone please verify if m_split as in svn rev 245286 is doing the J right thing in the scenario where a mbuf chain is split with len0 falling J on a mbuf boundary and the mbuf in question being a M_EXT mbuf? Consider J the following example where m0 is a mbuf chain consisting of 2 M_EXT mbufs, J both 1448 bytes in length. Let len0 be 1448. The 'len0 m-m_len' check J will be false so the for loop will not be entered in this case. We now have J len = 1448 and remain = 0 and m still points to the first mbuf in the J chain. Also assume that m0 is a pkthdr mbuf. A new pkthdr mbuf n will be J allocated and initialized before the following piece of code is executed : J J extpacket: J if (m-m_flags M_EXT) { J n-m_data = m-m_data + len; J mb_dupcl(n, m); J } else { J bcopy(mtod(m, caddr_t) + len, mtod(n, caddr_t), remain); J } J n-m_len = remain; J m-m_len = len; J n-m_next = m-m_next; J m-m_next = NULL; J return (n); J J As m is a M_EXT mbuf the code in the if() clause will be executed. The J problem is that m still points to the first mbuf so effectively the data J pointer for n is assigned to the end of m's data pointer. It should J actually point to the start of the data pointer of the next mbuf in the J original m0 chain, right? Thanks for analysis, Jacques. IMHO, the following patch should suffice and won't break anything: Index: uipc_mbuf.c === --- uipc_mbuf.c (revision 245223) +++ uipc_mbuf.c (working copy) @@ -1126,7 +1126,7 @@ u_int len = len0, remain; MBUF_CHECKSLEEP(wait); - for (m = m0; m len m-m_len; m = m-m_next) + for (m = m0; m len = m-m_len; m = m-m_next) len -= m-m_len; if (m == NULL) return (NULL); Can you please test it? I think that your patch may cause other issues - m now points to the first mbuf in the tail portion. The final piece of code under the extpacket: label will then set m-m_len to 0 for example. -- Totus tuus, Glebius. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: Possible bug in m_split() when splitting M_EXT mbufs
On Fri, Jan 11, 2013 at 10:53 AM, Jacques Fourie jacques.fou...@gmail.comwrote: On Fri, Jan 11, 2013 at 10:12 AM, Gleb Smirnoff gleb...@freebsd.orgwrote: Jacques, On Fri, Jan 11, 2013 at 09:34:32AM +0200, Jacques Fourie wrote: J Could someone please verify if m_split as in svn rev 245286 is doing the J right thing in the scenario where a mbuf chain is split with len0 falling J on a mbuf boundary and the mbuf in question being a M_EXT mbuf? Consider J the following example where m0 is a mbuf chain consisting of 2 M_EXT mbufs, J both 1448 bytes in length. Let len0 be 1448. The 'len0 m-m_len' check J will be false so the for loop will not be entered in this case. We now have J len = 1448 and remain = 0 and m still points to the first mbuf in the J chain. Also assume that m0 is a pkthdr mbuf. A new pkthdr mbuf n will be J allocated and initialized before the following piece of code is executed : J J extpacket: J if (m-m_flags M_EXT) { J n-m_data = m-m_data + len; J mb_dupcl(n, m); J } else { J bcopy(mtod(m, caddr_t) + len, mtod(n, caddr_t), remain); J } J n-m_len = remain; J m-m_len = len; J n-m_next = m-m_next; J m-m_next = NULL; J return (n); J J As m is a M_EXT mbuf the code in the if() clause will be executed. The J problem is that m still points to the first mbuf so effectively the data J pointer for n is assigned to the end of m's data pointer. It should J actually point to the start of the data pointer of the next mbuf in the J original m0 chain, right? Thanks for analysis, Jacques. IMHO, the following patch should suffice and won't break anything: Index: uipc_mbuf.c === --- uipc_mbuf.c (revision 245223) +++ uipc_mbuf.c (working copy) @@ -1126,7 +1126,7 @@ u_int len = len0, remain; MBUF_CHECKSLEEP(wait); - for (m = m0; m len m-m_len; m = m-m_next) + for (m = m0; m len = m-m_len; m = m-m_next) len -= m-m_len; if (m == NULL) return (NULL); Can you please test it? I think that your patch may cause other issues - m now points to the first mbuf in the tail portion. The final piece of code under the extpacket: label will then set m-m_len to 0 for example. -- Totus tuus, Glebius. I have been using the following patch that seems to fix the issue for me : diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c index ab6163d..5c397fa 100644 --- a/sys/kern/uipc_mbuf.c +++ b/sys/kern/uipc_mbuf.c @@ -1132,6 +1132,23 @@ m_split(struct mbuf *m0, int len0, int wait) return (NULL); remain = m-m_len - len; if (m0-m_flags M_PKTHDR) { + if (remain == 0) { + if (m-m_next == NULL) + return (NULL); + if (!(m-m_next-m_flags M_PKTHDR)) { + MGETHDR(n, wait, m0-m_type); + if (n == NULL) + return (NULL); + MH_ALIGN(n, 0); + n-m_next = m-m_next; + } else + n = m-m_next; + m-m_next = NULL; + n-m_pkthdr.rcvif = m0-m_pkthdr.rcvif; + n-m_pkthdr.len = m0-m_pkthdr.len - len0; + m0-m_pkthdr.len = len0; + return (n); + } ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
dirty hack asmc for Macbook 3,1
Dear All, My dirty hack just works but not perfect. --- /usr/src/sys/dev/asmc/asmcvar.h.original2013-01-11 09:36:53.0 + +++ /usr/src/sys/dev/asmc/asmcvar.h 2013-01-11 10:21:02.0 + @@ -141,10 +141,22 @@ #define ASMC_MB_TEMPDESCS { Enclosure Bottomside, \ Northbridge Point 1, \ Northbridge Point 2, Heatsink 1, \ Heatsink 2, Memory Bank A, } +#define ASMC_MB31_TEMPS { TB0T, TN0P, Th0H, Th1H, \ + TM0P, NULL } + +#define ASMC_MB31_TEMPNAMES { enclosure, northbridge1, \ + heatsink1, heatsink2,\ + memory, } + +#define ASMC_MB31_TEMPDESCS { Enclosure Bottomside, \ + Northbridge Point 1, \ + Heatsink 1, Heatsink2, \ + Memory Bank A, } + #define ASMC_MBP_TEMPS { TB0T, Th0H, Th1H, Tm0P, \ TG0H, TG0P, TG0T, NULL } #define ASMC_MBP_TEMPNAMES { enclosure, heatsink1, \ heatsink2, memory, graphics, \ --- /usr/src/sys/dev/asmc/asmc.c.original 2013-01-11 05:26:22.0 + +++ /usr/src/sys/dev/asmc/asmc.c2013-01-11 09:51:01.0 + @@ -153,10 +153,16 @@ MacBook2,1, Apple SMC MacBook Core 2 Duo, ASMC_SMS_FUNCS, ASMC_FAN_FUNCS, NULL, NULL, NULL, ASMC_MB_TEMPS, ASMC_MB_TEMPNAMES, ASMC_MB_TEMPDESCS }, +{ + MacBook3,1, Apple SMC MacBook Core 2 Duo, + ASMC_SMS_FUNCS, ASMC_FAN_FUNCS, NULL, NULL, NULL, + ASMC_MB31_TEMPS, ASMC_MB31_TEMPNAMES, ASMC_MB31_TEMPDESCS +}, + { MacBookPro1,1, Apple SMC MacBook Pro Core Duo (15-inch), ASMC_SMS_FUNCS, ASMC_FAN_FUNCS, ASMC_LIGHT_FUNCS, ASMC_MBP_TEMPS, ASMC_MBP_TEMPNAMES, ASMC_MBP_TEMPDESCS }, The output of this new asmc on Macbook 3,1 : dev.asmc.0.%desc: Apple SMC MacBook Core 2 Duo dev.asmc.0.%driver: asmc dev.asmc.0.%location: handle=\_SB_.PCI0.LPCB.SMC_ dev.asmc.0.%pnpinfo: _HID=APP0001 _UID=0 dev.asmc.0.%parent: acpi0 dev.asmc.0.fan.0.speed: 4535 dev.asmc.0.fan.0.safespeed: 0 dev.asmc.0.fan.0.minspeed: 1800 dev.asmc.0.fan.0.maxspeed: 6200 dev.asmc.0.fan.0.targetspeed: 4525 dev.asmc.0.temp.enclosure: 28 dev.asmc.0.temp.northbridge1: 52 dev.asmc.0.temp.heatsink1: 57 dev.asmc.0.temp.heatsink2: 55 dev.asmc.0.temp.memory: 51 dev.asmc.0.sms.x: -20 dev.asmc.0.sms.y: 45 dev.asmc.0.sms.z: 264 Regards, zeissoctopus -- View this message in context: http://freebsd.1045724.n5.nabble.com/dirty-hack-asmc-for-Macbook-3-1-tp5776597.html Sent from the freebsd-hackers mailing list archive at Nabble.com. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Tracking FreeBSD development with SVNews
This is a testimonial for a tool for tracking FreeBSD that doesn't get enough publicity. The tool is SVNnews. Too see all FreeBSD modifications for the last week try: http://www.secnetix.de/olli/FreeBSD/svnews/ The real power is when you interested in looking at subsysystems. How about all of the modifications to FreeBSD 9 for the last week? http://www.secnetix.de/olli/FreeBSD/svnews/?p=stable/9 How about all the FreeBSD 9 kernel modifications for the last week? http://www.secnetix.de/olli/FreeBSD/svnews/?p=stable/9/sys Hopefully you get the idea. My thanks to Oliver for putting together this useful tool. -- Larry Baird Global Technology Associates, Inc. 1992-2012| http://www.gta.com Celebrating Twenty Years of Software Innovation | Orlando, FL Email: l...@gta.com | TEL 407-380-0220 ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: SATA disk disappears
GEOM_MIRROR: Component ada2 (device home1) broken, skipping. GEOM_MIRROR: Cannot add disk ada2 to home1 (error=22). started gmirror rebuild and it now works at full speed. GEOM_MIRROR: Device home1: rebuilding provider ada2. What kind of hardware failure may it be? smartctl -a /dev/ada2 shows disk is fine. I had a new WD drive recently that had a write error. Reallocated sector count did not go up, but it quickly failed the SMART self-test, short or long. See smartctl(8) about the -t parameters. this is WD green 3TB ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: SATA disk disappears
On Fri, 11 Jan 2013, Wojciech Puchar wrote: GEOM_MIRROR: Component ada2 (device home1) broken, skipping. GEOM_MIRROR: Cannot add disk ada2 to home1 (error=22). started gmirror rebuild and it now works at full speed. GEOM_MIRROR: Device home1: rebuilding provider ada2. What kind of hardware failure may it be? smartctl -a /dev/ada2 shows disk is fine. I had a new WD drive recently that had a write error. Reallocated sector count did not go up, but it quickly failed the SMART self-test, short or long. See smartctl(8) about the -t parameters. this is WD green 3TB 1T Red here. The firmware is likely very similar. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: dirty hack asmc for Macbook 3,1
Please file a PR? :) Thanks! ADrian On 11 January 2013 03:15, zeissoctopus cheungho...@gmail.com wrote: Dear All, My dirty hack just works but not perfect. --- /usr/src/sys/dev/asmc/asmcvar.h.original2013-01-11 09:36:53.0 + +++ /usr/src/sys/dev/asmc/asmcvar.h 2013-01-11 10:21:02.0 + @@ -141,10 +141,22 @@ #define ASMC_MB_TEMPDESCS { Enclosure Bottomside, \ Northbridge Point 1, \ Northbridge Point 2, Heatsink 1, \ Heatsink 2, Memory Bank A, } +#define ASMC_MB31_TEMPS { TB0T, TN0P, Th0H, Th1H, \ + TM0P, NULL } + +#define ASMC_MB31_TEMPNAMES { enclosure, northbridge1, \ + heatsink1, heatsink2,\ + memory, } + +#define ASMC_MB31_TEMPDESCS { Enclosure Bottomside, \ + Northbridge Point 1, \ + Heatsink 1, Heatsink2, \ + Memory Bank A, } + #define ASMC_MBP_TEMPS { TB0T, Th0H, Th1H, Tm0P, \ TG0H, TG0P, TG0T, NULL } #define ASMC_MBP_TEMPNAMES { enclosure, heatsink1, \ heatsink2, memory, graphics, \ --- /usr/src/sys/dev/asmc/asmc.c.original 2013-01-11 05:26:22.0 + +++ /usr/src/sys/dev/asmc/asmc.c2013-01-11 09:51:01.0 + @@ -153,10 +153,16 @@ MacBook2,1, Apple SMC MacBook Core 2 Duo, ASMC_SMS_FUNCS, ASMC_FAN_FUNCS, NULL, NULL, NULL, ASMC_MB_TEMPS, ASMC_MB_TEMPNAMES, ASMC_MB_TEMPDESCS }, +{ + MacBook3,1, Apple SMC MacBook Core 2 Duo, + ASMC_SMS_FUNCS, ASMC_FAN_FUNCS, NULL, NULL, NULL, + ASMC_MB31_TEMPS, ASMC_MB31_TEMPNAMES, ASMC_MB31_TEMPDESCS +}, + { MacBookPro1,1, Apple SMC MacBook Pro Core Duo (15-inch), ASMC_SMS_FUNCS, ASMC_FAN_FUNCS, ASMC_LIGHT_FUNCS, ASMC_MBP_TEMPS, ASMC_MBP_TEMPNAMES, ASMC_MBP_TEMPDESCS }, The output of this new asmc on Macbook 3,1 : dev.asmc.0.%desc: Apple SMC MacBook Core 2 Duo dev.asmc.0.%driver: asmc dev.asmc.0.%location: handle=\_SB_.PCI0.LPCB.SMC_ dev.asmc.0.%pnpinfo: _HID=APP0001 _UID=0 dev.asmc.0.%parent: acpi0 dev.asmc.0.fan.0.speed: 4535 dev.asmc.0.fan.0.safespeed: 0 dev.asmc.0.fan.0.minspeed: 1800 dev.asmc.0.fan.0.maxspeed: 6200 dev.asmc.0.fan.0.targetspeed: 4525 dev.asmc.0.temp.enclosure: 28 dev.asmc.0.temp.northbridge1: 52 dev.asmc.0.temp.heatsink1: 57 dev.asmc.0.temp.heatsink2: 55 dev.asmc.0.temp.memory: 51 dev.asmc.0.sms.x: -20 dev.asmc.0.sms.y: 45 dev.asmc.0.sms.z: 264 Regards, zeissoctopus -- View this message in context: http://freebsd.1045724.n5.nabble.com/dirty-hack-asmc-for-Macbook-3-1-tp5776597.html Sent from the freebsd-hackers mailing list archive at Nabble.com. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org