Re: svn commit: r335856 - in head/sys: netinet sys
Sorry guys I didn't spot it was just a revert as it was tagged on to the end of the description, I would have expected that to be in the subject. What do others think, is there an recommend style for revert commit messages? Regards Steve On 02/07/2018 17:30, Rodney W. Grimes wrote: [ Charset UTF-8 unsupported, converting... ] On Mon, Jul 2, 2018 at 10:44 AM Steven Hartland < steven.hartl...@multiplay.co.uk> wrote: You have M_WAITOK and a null check in this change And, that's the same as the way it was before his commits. So, he did exactly what he said he was doing and reverted his commits. I don't think it is good practice to mix reverts with other changes. It is a very bad practive to mix a revert with anything. Since you've noticed this, I think you should feel free to make the change. Jonathan ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r335856 - in head/sys: netinet sys
[ Charset UTF-8 unsupported, converting... ] > On Mon, Jul 2, 2018 at 10:44 AM Steven Hartland < > steven.hartl...@multiplay.co.uk> wrote: > > > > You have M_WAITOK and a null check in this change > > And, that's the same as the way it was before his commits. So, he did > exactly what he said he was doing and reverted his commits. I don't think > it is good practice to mix reverts with other changes. It is a very bad practive to mix a revert with anything. > Since you've noticed this, I think you should feel free to make the change. > Jonathan -- Rod Grimes rgri...@freebsd.org ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r335856 - in head/sys: netinet sys
In message , "Jonathan T. Looney" writes: > On Mon, Jul 2, 2018 at 10:44 AM Steven Hartland < > steven.hartl...@multiplay.co.uk> wrote: > > > > You have M_WAITOK and a null check in this change > > And, that's the same as the way it was before his commits. So, he did > exactly what he said he was doing and reverted his commits. I don't think > it is good practice to mix reverts with other changes. Yes, mixing reverts with other changes or batching changes together in one commit confuses history. This is my main criticism of Linux commit logs, IMO the worst example of commit log content. There they itemize a shopping list of changes or simply say, pull fixes for X from so-and-so, the patch passes our tests. Five years from now or even a year from now, will anyone remember? > > Since you've noticed this, I think you should feel free to make the change. > > Jonathan -- Cheers, Cy Schubert FreeBSD UNIX: Web: http://www.FreeBSD.org The need of the many outweighs the greed of the few. ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r335856 - in head/sys: netinet sys
On Mon, Jul 2, 2018 at 10:44 AM Steven Hartland < steven.hartl...@multiplay.co.uk> wrote: > > You have M_WAITOK and a null check in this change And, that's the same as the way it was before his commits. So, he did exactly what he said he was doing and reverted his commits. I don't think it is good practice to mix reverts with other changes. Since you've noticed this, I think you should feel free to make the change. Jonathan ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r335856 - in head/sys: netinet sys
You have M_WAITOK and a null check in this change On Mon, 2 Jul 2018 at 06:20, Matt Macy wrote: > Author: mmacy > Date: Mon Jul 2 05:19:44 2018 > New Revision: 335856 > URL: https://svnweb.freebsd.org/changeset/base/335856 > > Log: > inpcb: don't gratuitously defer frees > > Don't defer frees in sysctl handlers. It isn't necessary > and it just confuses things. > revert: r333911, r334104, and r334125 > > Requested by: jtl > > Modified: > head/sys/netinet/ip_divert.c > head/sys/netinet/raw_ip.c > head/sys/netinet/tcp_subr.c > head/sys/netinet/udp_usrreq.c > head/sys/sys/malloc.h > > Modified: head/sys/netinet/ip_divert.c > > == > --- head/sys/netinet/ip_divert.cMon Jul 2 01:30:33 2018 > (r335855) > +++ head/sys/netinet/ip_divert.cMon Jul 2 05:19:44 2018 > (r335856) > @@ -552,7 +552,6 @@ div_detach(struct socket *so) > KASSERT(inp != NULL, ("div_detach: inp == NULL")); > INP_INFO_WLOCK(&V_divcbinfo); > INP_WLOCK(inp); > - /* XXX defer destruction to epoch_call */ > in_pcbdetach(inp); > in_pcbfree(inp); > INP_INFO_WUNLOCK(&V_divcbinfo); > @@ -632,7 +631,6 @@ static int > div_pcblist(SYSCTL_HANDLER_ARGS) > { > int error, i, n; > - struct in_pcblist *il; > struct inpcb *inp, **inp_list; > inp_gen_t gencnt; > struct xinpgen xig; > @@ -672,8 +670,9 @@ div_pcblist(SYSCTL_HANDLER_ARGS) > if (error) > return error; > > - il = malloc(sizeof(struct in_pcblist) + n * sizeof(struct inpcb > *), M_TEMP, M_WAITOK|M_ZERO_INVARIANTS); > - inp_list = il->il_inp_list; > + inp_list = malloc(n * sizeof *inp_list, M_TEMP, M_WAITOK); > + if (inp_list == NULL) > + return ENOMEM; > > INP_INFO_RLOCK(&V_divcbinfo); > for (inp = CK_LIST_FIRST(V_divcbinfo.ipi_listhead), i = 0; inp && > i < n; > @@ -702,9 +701,14 @@ div_pcblist(SYSCTL_HANDLER_ARGS) > } else > INP_RUNLOCK(inp); > } > - il->il_count = n; > - il->il_pcbinfo = &V_divcbinfo; > - epoch_call(net_epoch_preempt, &il->il_epoch_ctx, > in_pcblist_rele_rlocked); > + INP_INFO_WLOCK(&V_divcbinfo); > + for (i = 0; i < n; i++) { > + inp = inp_list[i]; > + INP_RLOCK(inp); > + if (!in_pcbrele_rlocked(inp)) > + INP_RUNLOCK(inp); > + } > + INP_INFO_WUNLOCK(&V_divcbinfo); > > if (!error) { > /* > @@ -721,6 +725,7 @@ div_pcblist(SYSCTL_HANDLER_ARGS) > INP_INFO_RUNLOCK(&V_divcbinfo); > error = SYSCTL_OUT(req, &xig, sizeof xig); > } > + free(inp_list, M_TEMP); > return error; > } > > @@ -800,7 +805,6 @@ div_modevent(module_t mod, int type, void *unused) > break; > } > ip_divert_ptr = NULL; > - /* XXX defer to epoch_call ? */ > err = pf_proto_unregister(PF_INET, IPPROTO_DIVERT, > SOCK_RAW); > INP_INFO_WUNLOCK(&V_divcbinfo); > #ifndef VIMAGE > > Modified: head/sys/netinet/raw_ip.c > > == > --- head/sys/netinet/raw_ip.c Mon Jul 2 01:30:33 2018(r335855) > +++ head/sys/netinet/raw_ip.c Mon Jul 2 05:19:44 2018(r335856) > @@ -863,7 +863,6 @@ rip_detach(struct socket *so) > ip_rsvp_force_done(so); > if (so == V_ip_rsvpd) > ip_rsvp_done(); > - /* XXX defer to epoch_call */ > in_pcbdetach(inp); > in_pcbfree(inp); > INP_INFO_WUNLOCK(&V_ripcbinfo); > @@ -1033,7 +1032,6 @@ static int > rip_pcblist(SYSCTL_HANDLER_ARGS) > { > int error, i, n; > - struct in_pcblist *il; > struct inpcb *inp, **inp_list; > inp_gen_t gencnt; > struct xinpgen xig; > @@ -1068,8 +1066,9 @@ rip_pcblist(SYSCTL_HANDLER_ARGS) > if (error) > return (error); > > - il = malloc(sizeof(struct in_pcblist) + n * sizeof(struct inpcb > *), M_TEMP, M_WAITOK|M_ZERO_INVARIANTS); > - inp_list = il->il_inp_list; > + inp_list = malloc(n * sizeof *inp_list, M_TEMP, M_WAITOK); > + if (inp_list == NULL) > + return (ENOMEM); > > INP_INFO_RLOCK(&V_ripcbinfo); > for (inp = CK_LIST_FIRST(V_ripcbinfo.ipi_listhead), i = 0; inp && > i < n; > @@ -1098,9 +1097,14 @@ rip_pcblist(SYSCTL_HANDLER_ARGS) > } else > INP_RUNLOCK(inp); > } > - il->il_count = n; > - il->il_pcbinfo = &V_ripcbinfo; > - epoch_call(net_epoch_preempt, &il->il_epoch_ctx, > in_pcblist_rele_rlocked); > + INP_INFO_WLOCK(&V_ripcbinfo); > + for (i = 0; i < n; i++) { > + inp = inp_list[i]; > + INP_RLOCK(
svn commit: r335856 - in head/sys: netinet sys
Author: mmacy Date: Mon Jul 2 05:19:44 2018 New Revision: 335856 URL: https://svnweb.freebsd.org/changeset/base/335856 Log: inpcb: don't gratuitously defer frees Don't defer frees in sysctl handlers. It isn't necessary and it just confuses things. revert: r333911, r334104, and r334125 Requested by: jtl Modified: head/sys/netinet/ip_divert.c head/sys/netinet/raw_ip.c head/sys/netinet/tcp_subr.c head/sys/netinet/udp_usrreq.c head/sys/sys/malloc.h Modified: head/sys/netinet/ip_divert.c == --- head/sys/netinet/ip_divert.cMon Jul 2 01:30:33 2018 (r335855) +++ head/sys/netinet/ip_divert.cMon Jul 2 05:19:44 2018 (r335856) @@ -552,7 +552,6 @@ div_detach(struct socket *so) KASSERT(inp != NULL, ("div_detach: inp == NULL")); INP_INFO_WLOCK(&V_divcbinfo); INP_WLOCK(inp); - /* XXX defer destruction to epoch_call */ in_pcbdetach(inp); in_pcbfree(inp); INP_INFO_WUNLOCK(&V_divcbinfo); @@ -632,7 +631,6 @@ static int div_pcblist(SYSCTL_HANDLER_ARGS) { int error, i, n; - struct in_pcblist *il; struct inpcb *inp, **inp_list; inp_gen_t gencnt; struct xinpgen xig; @@ -672,8 +670,9 @@ div_pcblist(SYSCTL_HANDLER_ARGS) if (error) return error; - il = malloc(sizeof(struct in_pcblist) + n * sizeof(struct inpcb *), M_TEMP, M_WAITOK|M_ZERO_INVARIANTS); - inp_list = il->il_inp_list; + inp_list = malloc(n * sizeof *inp_list, M_TEMP, M_WAITOK); + if (inp_list == NULL) + return ENOMEM; INP_INFO_RLOCK(&V_divcbinfo); for (inp = CK_LIST_FIRST(V_divcbinfo.ipi_listhead), i = 0; inp && i < n; @@ -702,9 +701,14 @@ div_pcblist(SYSCTL_HANDLER_ARGS) } else INP_RUNLOCK(inp); } - il->il_count = n; - il->il_pcbinfo = &V_divcbinfo; - epoch_call(net_epoch_preempt, &il->il_epoch_ctx, in_pcblist_rele_rlocked); + INP_INFO_WLOCK(&V_divcbinfo); + for (i = 0; i < n; i++) { + inp = inp_list[i]; + INP_RLOCK(inp); + if (!in_pcbrele_rlocked(inp)) + INP_RUNLOCK(inp); + } + INP_INFO_WUNLOCK(&V_divcbinfo); if (!error) { /* @@ -721,6 +725,7 @@ div_pcblist(SYSCTL_HANDLER_ARGS) INP_INFO_RUNLOCK(&V_divcbinfo); error = SYSCTL_OUT(req, &xig, sizeof xig); } + free(inp_list, M_TEMP); return error; } @@ -800,7 +805,6 @@ div_modevent(module_t mod, int type, void *unused) break; } ip_divert_ptr = NULL; - /* XXX defer to epoch_call ? */ err = pf_proto_unregister(PF_INET, IPPROTO_DIVERT, SOCK_RAW); INP_INFO_WUNLOCK(&V_divcbinfo); #ifndef VIMAGE Modified: head/sys/netinet/raw_ip.c == --- head/sys/netinet/raw_ip.c Mon Jul 2 01:30:33 2018(r335855) +++ head/sys/netinet/raw_ip.c Mon Jul 2 05:19:44 2018(r335856) @@ -863,7 +863,6 @@ rip_detach(struct socket *so) ip_rsvp_force_done(so); if (so == V_ip_rsvpd) ip_rsvp_done(); - /* XXX defer to epoch_call */ in_pcbdetach(inp); in_pcbfree(inp); INP_INFO_WUNLOCK(&V_ripcbinfo); @@ -1033,7 +1032,6 @@ static int rip_pcblist(SYSCTL_HANDLER_ARGS) { int error, i, n; - struct in_pcblist *il; struct inpcb *inp, **inp_list; inp_gen_t gencnt; struct xinpgen xig; @@ -1068,8 +1066,9 @@ rip_pcblist(SYSCTL_HANDLER_ARGS) if (error) return (error); - il = malloc(sizeof(struct in_pcblist) + n * sizeof(struct inpcb *), M_TEMP, M_WAITOK|M_ZERO_INVARIANTS); - inp_list = il->il_inp_list; + inp_list = malloc(n * sizeof *inp_list, M_TEMP, M_WAITOK); + if (inp_list == NULL) + return (ENOMEM); INP_INFO_RLOCK(&V_ripcbinfo); for (inp = CK_LIST_FIRST(V_ripcbinfo.ipi_listhead), i = 0; inp && i < n; @@ -1098,9 +1097,14 @@ rip_pcblist(SYSCTL_HANDLER_ARGS) } else INP_RUNLOCK(inp); } - il->il_count = n; - il->il_pcbinfo = &V_ripcbinfo; - epoch_call(net_epoch_preempt, &il->il_epoch_ctx, in_pcblist_rele_rlocked); + INP_INFO_WLOCK(&V_ripcbinfo); + for (i = 0; i < n; i++) { + inp = inp_list[i]; + INP_RLOCK(inp); + if (!in_pcbrele_rlocked(inp)) + INP_RUNLOCK(inp); + } + INP_INFO_WUNLOCK(&V_ripcbinfo); if (!error) { /* @@ -1116,6 +1120,7 @@ rip_pcblist(SYSCTL_HANDLER_ARGS) INP_INFO_RUNLOCK(&V_ripcbinfo); error = SYSCTL_OUT(req, &xig