Re: svn commit: r352619 - head/sys/mips/mips
On 2019-09-23 05:43, Kyle Evans wrote:
Author: kevans
Date: Mon Sep 23 12:43:08 2019
New Revision: 352619
URL: https://svnweb.freebsd.org/changeset/base/352619
Log:
mips: fix XLPN32 after r352434
SYSINIT usage was added, but the dependency was not added.
This worked by coincidence, as most of the mips configs have DDB enabled and
pmap.c gets via ddb.h pollution.
Reported by: dim
Thanks for fixing this.
Pointyhat to: jah
Modified:
head/sys/mips/mips/pmap.c
Modified: head/sys/mips/mips/pmap.c
==
--- head/sys/mips/mips/pmap.c Mon Sep 23 12:27:55 2019(r352618)
+++ head/sys/mips/mips/pmap.c Mon Sep 23 12:43:08 2019(r352619)
@@ -68,6 +68,7 @@ __FBSDID("$FreeBSD$");
#include
#include
+#include
#include
#include
#include
___
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: r344562 - head/sys/ufs/ffs
On 2/25/19 9:46 PM, Bruce Evans wrote: block_size <= PAGE_SIZE is very uncommon for ffs, even on systems with large pages. MINBSIZE is 4096 in ffs (except in my version, it is 512). The default is 32768 in newfs. I consider this excessive and only use it for file systems with many files larger than 1GB, but it is the most common size. It is larger than the large page size of 8192. I think this is a case of filesystem logical block size vs. media sector size, right? Here we're checking the devvp's block size, which I think should correspond to the sector size. I'd expect cases of that being greater than PAGE_SIZE to be uncommon, in fact geli warns when that is the case. I probably should've made that clearer in the commit message. ffs_getpages() already has an almost-never-used special case for small block sizes. It uses vnode_pager_generic_getpages() when !use_buf_pager and the block_size <= PAGE_SIZE, else vfs_bio_getpages(). But block_size <= PAGE_SIZE is unusual, and !use_buf_pager is also unusual, and use_buf_pager is mostly a debugging sysctl, so little would be lost but using vfs_bio_getpages() unconditionally. Bruce ___ 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: r343005 - head/sys/kern
No problem! It was fun to dig into a part of the kernel I hadn't worked on before. On Mon, Jan 14, 2019 at 3:24 PM Gleb Smirnoff wrote: > Jason, > > thanks a lot for fixing this. > > On Sun, Jan 13, 2019 at 08:33:55PM +, Jason A. Harmening wrote: > J> Author: jah > J> Date: Sun Jan 13 20:33:54 2019 > J> New Revision: 343005 > J> URL: https://svnweb.freebsd.org/changeset/base/343005 > J> > J> Log: > J> Handle SIGIO for listening sockets > J> > J> r319722 separated struct socket and parts of the socket I/O path into > J> listening-socket-specific and dataflow-socket-specific pieces. > Listening > J> socket connection notifications are now handled by solisten_wakeup() > instead > J> of sowakeup(), but solisten_wakeup() does not currently post SIGIO to > the > J> owning process. > J> > J> PR:234258 > J> Reported by: Kenneth Adelman > J> MFC after: 1 week > J> Differential Revision: https://reviews.freebsd.org/D18664 > J> > J> Modified: > J> head/sys/kern/uipc_socket.c > J> > J> Modified: head/sys/kern/uipc_socket.c > J> > == > J> --- head/sys/kern/uipc_socket.c Sun Jan 13 19:49:46 2019 > (r343004) > J> +++ head/sys/kern/uipc_socket.c Sun Jan 13 20:33:54 2019 > (r343005) > J> @@ -886,6 +886,8 @@ solisten_wakeup(struct socket *sol) > J> } > J> SOLISTEN_UNLOCK(sol); > J> wakeup_one(&sol->sol_comp); > J> +if ((sol->so_state & SS_ASYNC) && sol->so_sigio != NULL) > J> +pgsigio(&sol->so_sigio, SIGIO, 0); > J> } > J> > J> /* > J> > > -- > Gleb Smirnoff > ___ 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: r328489 - head/sys/conf
On Mon, Feb 5, 2018 at 1:25 PM, O. Hartmann wrote:
> Am Sun, 4 Feb 2018 10:22:31 -0800
> Jason Harmening schrieb:
>
> > On 02/04/18 04:33, O. Hartmann wrote:
> > > Am Mon, 29 Jan 2018 05:00:22 -0800
> > > David Wolfskill schrieb:
> > >
> > >> On Mon, Jan 29, 2018 at 02:10:04AM -0800, Jason Harmening wrote:
> > >>>>
> > >>>> This happens now if PORTS_MODULE=x11/nvidia-driver is defined in
> /etc/src.conf:
> > >>>>
> > >>>> [...]
> > >>>> --- kernel-install ---
> > >>>> mkdir -p /boot/kernel
> > >>>> install -p -m 555 -o root -g wheel kernel /boot/kernel/
> > >>>> --- ports-install ---
> > >>>> Variable OBJTOP is recursive.
> > >>>>
> > >>>> make[8]: stopped
> > >>> ...
> > >>>
> > >>> David sent me logs of the failing case; thanks David!
> > >>
> > >> Happy to help! :-)
> > >>
> > >>> The failure happens when buildkernel and installkernel are run
> > >>> separately instead of all-up, e.g. 'make kernel'. The installkernel
> > >>> step is leaving MK_AUTO_OBJ=no in the env passed to the port build.
> It
> > >>> looks like at least one of the install stages of nvidia-driver needs
> to
> > >>> generate temporary output, which leads to confusion when the port
> isn't
> > >>> built as though it's an in-tree component.
> > >>>
> > >>> Can you guys try out the attached patch?
> > >>
> > >> I tried it both on my build machine (which does not use kernel modules
> > >> from ports, and thus did not exhibit the problem -- but I thought that
> > >> verifying that the patch did not break that case worth checking) and
> on
> > >> my laptop (which did exhibit the problem).
> > >>
> > >> It worked in both cases with no issues for me.
> > >>
> > >> Thanks! :-)
> > >>
> > >> Peace,
> > >> david
> > >
> > > The problem still persists!
> > >
> > > I'm on CURRENT, FreeBSD 12.0-CURRENT #32: Sun Feb 4 09:41:39 CET 2018
> amd64, the
> > > source tree is at revision 328839.
> > >
> > >
> > > I use WITH_META_MODE=YES in /etc/src.conf. My /etc/make.conf consists
> of a .include
> > > statement which reels in /usr/local/etc/ports.conf in which I define
> everything
> > > outside the source tree for ports (in case of the nvidia driver, its
> DISTVERSION).
> > > This worked before and should work again. Today I checked out a
> completely
> > > fresh /usr/src and gleanced the /usr/obj folder and rebuilt the system
> - and get the
> > > error again:
> > >
> > > [...]
> > > ===> Ports module x11/nvidia-driver (install)
> > > cd ${PORTSDIR:-/usr/ports}/x11/nvidia-driver; env -u CC -u CXX -u
> CPP -u
> > > MAKESYSPATH MAKEFLAGS="-j 4 -J 15,16 .MAKE.LEVEL.ENV=MAKELEVEL
> KERNEL=kernel
> > > MK_AUTO_OBJ=no
> > ^
> > Looks like you haven't applied the patch? MK_AUTO_OBJ being left set in
> > MAKEFLAGS by installkernel was part of the problem.
> >
> > That said, the fix I have up for review is slightly different:
> > https://reviews.freebsd.org/D14143
> >
> > > TARGET=amd64 TARGET_ARCH=amd64" SYSDIR=/usr/src/sys
> > > PATH=/usr/obj/usr/src/amd64.amd64/tmp/legacy/usr/sbin:/
> usr/obj/usr/src/amd64.amd64/tmp/legacy/usr/bin:/usr/obj/
> usr/src/amd64.amd64/tmp/legacy/bin:/usr/obj/usr/src/
> amd64.amd64/tmp/usr/sbin:/usr/obj/usr/src/amd64.amd64/tmp/
> usr/bin:/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/bin:/usr/local/sbin
> > > SRC_BASE=/usr/src OSVERSION=1200056
> > > WRKDIRPREFIX=/usr/obj/usr/src/amd64.amd64/sys/THOR make -B deinstall
> reinstall ===>
> > > Deinstalling for nvidia-driver ===> Deinstalling
> nvidia-driver-387.34 Updating
> > > database digests format: . done Checking integrity... done (0
> conflicting)
> > > Deinstallation has been requested for the following 1 packages (of 0
> packages in the
> > > universe):
> > >
> > > Installed packages to be REMOVED:
> > > nvidia-driver-387.34
> > >
> > > Number of packages to be removed: 1
> > >
> > > The operation will free 99 MiB.
> > > [1/1] Deinsta
Re: svn commit: r328489 - head/sys/conf
On 02/04/18 04:33, O. Hartmann wrote:
Am Mon, 29 Jan 2018 05:00:22 -0800
David Wolfskill schrieb:
On Mon, Jan 29, 2018 at 02:10:04AM -0800, Jason Harmening wrote:
This happens now if PORTS_MODULE=x11/nvidia-driver is defined in /etc/src.conf:
[...]
--- kernel-install ---
mkdir -p /boot/kernel
install -p -m 555 -o root -g wheel kernel /boot/kernel/
--- ports-install ---
Variable OBJTOP is recursive.
make[8]: stopped
...
David sent me logs of the failing case; thanks David!
Happy to help! :-)
The failure happens when buildkernel and installkernel are run
separately instead of all-up, e.g. 'make kernel'. The installkernel
step is leaving MK_AUTO_OBJ=no in the env passed to the port build. It
looks like at least one of the install stages of nvidia-driver needs to
generate temporary output, which leads to confusion when the port isn't
built as though it's an in-tree component.
Can you guys try out the attached patch?
I tried it both on my build machine (which does not use kernel modules
from ports, and thus did not exhibit the problem -- but I thought that
verifying that the patch did not break that case worth checking) and on
my laptop (which did exhibit the problem).
It worked in both cases with no issues for me.
Thanks! :-)
Peace,
david
The problem still persists!
I'm on CURRENT, FreeBSD 12.0-CURRENT #32: Sun Feb 4 09:41:39 CET 2018 amd64,
the source
tree is at revision 328839.
I use WITH_META_MODE=YES in /etc/src.conf. My /etc/make.conf consists of a
.include
statement which reels in /usr/local/etc/ports.conf in which I define everything
outside
the source tree for ports (in case of the nvidia driver, its DISTVERSION). This
worked
before and should work again. Today I checked out a completely fresh /usr/src
and gleanced
the /usr/obj folder and rebuilt the system - and get the error again:
[...]
===> Ports module x11/nvidia-driver (install)
cd ${PORTSDIR:-/usr/ports}/x11/nvidia-driver; env -u CC -u CXX -u CPP -u
MAKESYSPATH
MAKEFLAGS="-j 4 -J 15,16 .MAKE.LEVEL.ENV=MAKELEVEL KERNEL=kernel MK_AUTO_OBJ=no
^
Looks like you haven't applied the patch? MK_AUTO_OBJ being left set in
MAKEFLAGS by installkernel was part of the problem.
That said, the fix I have up for review is slightly different:
https://reviews.freebsd.org/D14143
TARGET=amd64 TARGET_ARCH=amd64" SYSDIR=/usr/src/sys
PATH=/usr/obj/usr/src/amd64.amd64/tmp/legacy/usr/sbin:/usr/obj/usr/src/amd64.amd64/tmp/legacy/usr/bin:/usr/obj/usr/src/amd64.amd64/tmp/legacy/bin:/usr/obj/usr/src/amd64.amd64/tmp/usr/sbin:/usr/obj/usr/src/amd64.amd64/tmp/usr/bin:/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/bin:/usr/local/sbin
SRC_BASE=/usr/src OSVERSION=1200056
WRKDIRPREFIX=/usr/obj/usr/src/amd64.amd64/sys/THOR
make -B deinstall reinstall ===> Deinstalling for nvidia-driver ===>
Deinstalling
nvidia-driver-387.34 Updating database digests format: . done Checking
integrity... done
(0 conflicting) Deinstallation has been requested for the following 1 packages
(of 0
packages in the universe):
Installed packages to be REMOVED:
nvidia-driver-387.34
Number of packages to be removed: 1
The operation will free 99 MiB.
[1/1] Deinstalling nvidia-driver-387.34...
[1/1] Deleting files for nvidia-driver-387.34: .. done
===> Staging for nvidia-driver-387.34
===> nvidia-driver-387.34 depends on file: /usr/local/lib/libGL.so - found
===> nvidia-driver-387.34 depends on file:
/usr/local/libdata/pkgconfig/x11.pc - found
===> nvidia-driver-387.34 depends on file:
/usr/local/libdata/pkgconfig/xorg-server.pc
- found ===> nvidia-driver-387.34 depends on file:
/usr/local/libdata/pkgconfig/xext.pc
- found ===> Generating temporary packing list
===> src (install)
===> src/nvidia (install)
Variable OBJTOP is recursive.
make[8]: stopped
in
/usr/obj/usr/src/amd64.amd64/sys/DUMMBOX/usr/ports/x11/nvidia-driver/work/NVIDIA-FreeBSD-x86_64-387.34/src/nvidia
*** Error code 2
Oliver
___
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: r328489 - head/sys/conf
This happens now if PORTS_MODULE=x11/nvidia-driver is defined in /etc/src.conf:
[...]
--- kernel-install ---
mkdir -p /boot/kernel
install -p -m 555 -o root -g wheel kernel /boot/kernel/
--- ports-install ---
Variable OBJTOP is recursive.
make[8]: stopped
in
/usr/obj/usr/src/amd64.amd64/sys/FY/usr/ports/x11/nvidia-driver/work/NVIDIA-FreeBSD-x86_64-387.34/src/nvidia
*** Error code 2
Stop.
make[7]: stopped
in
/usr/obj/usr/src/amd64.amd64/sys/THOR/usr/ports/x11/nvidia-driver/work/NVIDIA-FreeBSD-x86_64-387.34/src
*** Error code 1
David sent me logs of the failing case; thanks David!
The failure happens when buildkernel and installkernel are run
separately instead of all-up, e.g. 'make kernel'. The installkernel
step is leaving MK_AUTO_OBJ=no in the env passed to the port build. It
looks like at least one of the install stages of nvidia-driver needs to
generate temporary output, which leads to confusion when the port isn't
built as though it's an in-tree component.
Can you guys try out the attached patch?
Index: sys/conf/kern.post.mk
===
--- sys/conf/kern.post.mk (revision 328489)
+++ sys/conf/kern.post.mk (working copy)
@@ -70,7 +70,8 @@
-u CXX \
-u CPP \
-u MAKESYSPATH \
- MAKEFLAGS="${MAKEFLAGS:M*:tW:S/^-m /-m_/g:S/ -m / -m_/g:tw:N-m_*}" \
+ MK_AUTO_OBJ=yes \
+ MAKEFLAGS="${MAKEFLAGS:M*:tW:S/^-m /-m_/g:S/ -m /
-m_/g:tw:N-m_*:NMK_AUTO_OBJ=*}" \
SYSDIR=${SYSDIR} \
PATH=${PATH}:${LOCALBASE}/bin:${LOCALBASE}/sbin \
SRC_BASE=${SRC_BASE} \
___
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: r328489 - head/sys/conf
On Sat, Jan 27, 2018 at 12:47 PM, O. Hartmann
wrote:
> Am Sat, 27 Jan 2018 20:13:36 + (UTC)
> "Jason A. Harmening" schrieb:
>
> > Author: jah
> > Date: Sat Jan 27 20:13:36 2018
> > New Revision: 328489
> > URL: https://svnweb.freebsd.org/changeset/base/328489
> >
> > Log:
> > Remove system makefile path directives from env passed to
> PORTS_MODULES step
> >
> > Previously, MAKESYSPATH as well as '-m' directives in MAKEFLAGS would
> cause
> > any port rebuilt during the PORTS_MODULES stage to consume system
> makefiles
> > from $(SRCROOT)/share/mk instead of those installed under
> /usr/share/mk.
> > For kernel modules that need to build against an updated src tree this
> > makes sense; less so for or any userspace library or
> utility
> > the port may also happen to install.
> >
> > Before 11.0, this probably didn't matter much in practice. But the
> addition
> > of src.libnames.mk under $(SRCROOT)/share/mk in 11.0 breaks any
> consumer of
> > bsd.prog.mk and DPADD/LDADD during PORTS_MODULES.
> >
> > Address the build breakage by removing MAKESYSPATH and any occurrence
> of
> > '-m' from MAKEFLAGS in the environment created for the port build.
> > Instead set SYSDIR so that any kmod built by the port will still
> consume
> > conf/kmod.mk from the updated src tree, assuming it uses
> >
> > Reviewed by:bdrewery
> > MFC after: 2 weeks
> > Differential Revision: https://reviews.freebsd.org/D13053
> >
> > Modified:
> > head/sys/conf/kern.post.mk
> >
> > Modified: head/sys/conf/kern.post.mk
> >
> ==
> > --- head/sys/conf/kern.post.mkSat Jan 27 19:23:42 2018
> (r328488)
> > +++ head/sys/conf/kern.post.mkSat Jan 27 20:13:36 2018
> (r328489)
> > @@ -69,6 +69,9 @@ PORTSMODULESENV=\
> > -u CC \
> > -u CXX \
> > -u CPP \
> > + -u MAKESYSPATH \
> > + MAKEFLAGS="${MAKEFLAGS:M*:tW:S/^-m /-m_/g:S/ -m /
> -m_/g:tw:N-m_*}" \
> > + SYSDIR=${SYSDIR} \
> > PATH=${PATH}:${LOCALBASE}/bin:${LOCALBASE}/sbin \
> > SRC_BASE=${SRC_BASE} \
> > OSVERSION=${OSRELDATE} \
> > ___
> > svn-src-h...@freebsd.org mailing list
> > https://lists.freebsd.org/mailman/listinfo/svn-src-head
> > To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
>
> This happens now if PORTS_MODULE=x11/nvidia-driver is defined in
> /etc/src.conf:
>
> [...]
> --- kernel-install ---
> mkdir -p /boot/kernel
> install -p -m 555 -o root -g wheel kernel /boot/kernel/
> --- ports-install ---
> Variable OBJTOP is recursive.
>
> make[8]: stopped
> in /usr/obj/usr/src/amd64.amd64/sys/FY/usr/ports/x11/nvidia-
> driver/work/NVIDIA-FreeBSD-x86_64-387.34/src/nvidia
> *** Error code 2
>
> Stop.
> make[7]: stopped
> in /usr/obj/usr/src/amd64.amd64/sys/THOR/usr/ports/x11/nvidia-
> driver/work/NVIDIA-FreeBSD-x86_64-387.34/src
> *** Error code 1
>
Hmm, I haven't been able to reproduce this locally so far. What command
are you running? Can you post the contents of make.conf and src.conf ?
>
> --
> O. Hartmann
>
> Ich widerspreche der Nutzung oder Übermittlung meiner Daten für
> Werbezwecke oder für die Markt- oder Meinungsforschung (§ 28 Abs. 4 BDSG).
>
___
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: r313037 - in head/sys: amd64/include kern mips/include net powerpc/include sparc64/include
Actually attaching the patch this time ( gmail client) On Sun, Feb 5, 2017 at 10:58 AM, Jason Harmening wrote: > Hmm, it's a good idea to consider the possibility of a barrier issue. It > wouldn't be the first time we've had such a problem on a weakly-ordered > architecture. That said, I don't see a problem in this case. > smp_rendezvous_cpus() takes a spinlock and then issues > atomic_store_rel_int() to ensure the rendezvous params are visible to > other cpus. The latter corresponds to lwsync on powerpc, which AFAIK > should be sufficient to ensure visibility of prior stores. > > For now I'm going with the simpler explanation that I made a bad > assumption in the powerpc get_pcpu() and there is some context in which > the read of sprg0 doesn't return a consistent pointer value. Unfortunately > I don't see where that might be right now. > > On the mips side, Kurt/Alexander can you test the attached patch? It > contains a simple fix to ensure get_pcpu() returns the consistent per-cpu > pointer. > > On Sat, Feb 4, 2017 at 1:34 PM, Svatopluk Kraus wrote: > >> Probably not related. But when I took short look to the patch to see >> what could go wrong, I walked into the following comment in >> _rm_wlock(): "Assumes rm->rm_writecpus update is visible on other CPUs >> before rm_cleanIPI is called." There is no explicit barrier to ensure >> it. However, there might be some barriers inside of >> smp_rendezvous_cpus(). I have no idea what could happened if this >> assumption is not met. Note that rm_cleanIPI() is affected by the >> patch. >> >> >> >> On Sat, Feb 4, 2017 at 9:39 PM, Jason Harmening >> wrote: >> > Can you post an example of such panic? Only 2 MI pieces were changed, >> > netisr and rmlock. I haven't seen problems on my own amd64/i386/arm >> testing >> > of this, so a backtrace might help to narrow down the cause. >> > >> > On Sat, Feb 4, 2017 at 12:22 PM, Andreas Tobler >> > wrote: >> >> >> >> On 04.02.17 20:54, Jason Harmening wrote: >> >>> >> >>> I suspect this broke rmlocks for mips because the rmlock >> implementation >> >>> takes the address of the per-CPU pc_rm_queue when building tracker >> >>> lists. That address may be later accessed from another CPU and will >> >>> then translate to the wrong physical region if the address was taken >> >>> relative to the globally-constant pcpup VA used on mips. >> >>> >> >>> Regardless, for mips get_pcpup() should be implemented as >> >>> pcpu_find(curcpu) since returning an address that may mean something >> >>> different depending on the CPU seems like a big POLA violation if >> >>> nothing else. >> >>> >> >>> I'm more concerned about the report of powerpc breakage. For powerpc >> we >> >>> simply take each pcpu pointer from the pc_allcpu list (which is the >> same >> >>> value stored in the cpuid_to_pcpu array) and pass it through the >> ap_pcpu >> >>> global to each AP's startup code, which then stores it in sprg0. It >> >>> should be globally unique and won't have the variable-translation >> issues >> >>> seen on mips. Andreas, are you certain this change was responsible >> the >> >>> breakage you saw, and was it the same sort of hang observed on mips? >> >> >> >> >> >> I'm really sure. 313036 booted fine, allowed me to execute heavy >> >> compilation jobs, np. 313037 on the other side gave me various >> patterns of >> >> panics. During startup, but I also succeeded to get into multiuser and >> then >> >> the panic happend during port building. >> >> >> >> I have no deeper inside where pcpu data is used. Justin mentioned >> netisr? >> >> >> >> Andreas >> >> >> > >> > > Index: sys/amd64/include/pcpu.h === --- sys/amd64/include/pcpu.h(revision 313193) +++ sys/amd64/include/pcpu.h(working copy) @@ -78,6 +78,7 @@ extern struct pcpu *pcpup; +#defineget_pcpu() (pcpup) #definePCPU_GET(member)(pcpup->pc_ ## member) #definePCPU_ADD(member, val) (pcpup->pc_ ## member += (val)) #definePCPU_INC(member)PCPU_ADD(member, 1) @@ -203,6 +204,15 @@ }
Re: svn commit: r313037 - in head/sys: amd64/include kern mips/include net powerpc/include sparc64/include
Hmm, it's a good idea to consider the possibility of a barrier issue. It wouldn't be the first time we've had such a problem on a weakly-ordered architecture. That said, I don't see a problem in this case. smp_rendezvous_cpus() takes a spinlock and then issues atomic_store_rel_int() to ensure the rendezvous params are visible to other cpus. The latter corresponds to lwsync on powerpc, which AFAIK should be sufficient to ensure visibility of prior stores. For now I'm going with the simpler explanation that I made a bad assumption in the powerpc get_pcpu() and there is some context in which the read of sprg0 doesn't return a consistent pointer value. Unfortunately I don't see where that might be right now. On the mips side, Kurt/Alexander can you test the attached patch? It contains a simple fix to ensure get_pcpu() returns the consistent per-cpu pointer. On Sat, Feb 4, 2017 at 1:34 PM, Svatopluk Kraus wrote: > Probably not related. But when I took short look to the patch to see > what could go wrong, I walked into the following comment in > _rm_wlock(): "Assumes rm->rm_writecpus update is visible on other CPUs > before rm_cleanIPI is called." There is no explicit barrier to ensure > it. However, there might be some barriers inside of > smp_rendezvous_cpus(). I have no idea what could happened if this > assumption is not met. Note that rm_cleanIPI() is affected by the > patch. > > > > On Sat, Feb 4, 2017 at 9:39 PM, Jason Harmening > wrote: > > Can you post an example of such panic? Only 2 MI pieces were changed, > > netisr and rmlock. I haven't seen problems on my own amd64/i386/arm > testing > > of this, so a backtrace might help to narrow down the cause. > > > > On Sat, Feb 4, 2017 at 12:22 PM, Andreas Tobler > > wrote: > >> > >> On 04.02.17 20:54, Jason Harmening wrote: > >>> > >>> I suspect this broke rmlocks for mips because the rmlock implementation > >>> takes the address of the per-CPU pc_rm_queue when building tracker > >>> lists. That address may be later accessed from another CPU and will > >>> then translate to the wrong physical region if the address was taken > >>> relative to the globally-constant pcpup VA used on mips. > >>> > >>> Regardless, for mips get_pcpup() should be implemented as > >>> pcpu_find(curcpu) since returning an address that may mean something > >>> different depending on the CPU seems like a big POLA violation if > >>> nothing else. > >>> > >>> I'm more concerned about the report of powerpc breakage. For powerpc > we > >>> simply take each pcpu pointer from the pc_allcpu list (which is the > same > >>> value stored in the cpuid_to_pcpu array) and pass it through the > ap_pcpu > >>> global to each AP's startup code, which then stores it in sprg0. It > >>> should be globally unique and won't have the variable-translation > issues > >>> seen on mips. Andreas, are you certain this change was responsible > the > >>> breakage you saw, and was it the same sort of hang observed on mips? > >> > >> > >> I'm really sure. 313036 booted fine, allowed me to execute heavy > >> compilation jobs, np. 313037 on the other side gave me various patterns > of > >> panics. During startup, but I also succeeded to get into multiuser and > then > >> the panic happend during port building. > >> > >> I have no deeper inside where pcpu data is used. Justin mentioned > netisr? > >> > >> Andreas > >> > > > ___ 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: r313037 - in head/sys: amd64/include kern mips/include net powerpc/include sparc64/include
Can you post an example of such panic? Only 2 MI pieces were changed, netisr and rmlock. I haven't seen problems on my own amd64/i386/arm testing of this, so a backtrace might help to narrow down the cause. On Sat, Feb 4, 2017 at 12:22 PM, Andreas Tobler wrote: > On 04.02.17 20:54, Jason Harmening wrote: > >> I suspect this broke rmlocks for mips because the rmlock implementation >> takes the address of the per-CPU pc_rm_queue when building tracker >> lists. That address may be later accessed from another CPU and will >> then translate to the wrong physical region if the address was taken >> relative to the globally-constant pcpup VA used on mips. >> >> Regardless, for mips get_pcpup() should be implemented as >> pcpu_find(curcpu) since returning an address that may mean something >> different depending on the CPU seems like a big POLA violation if >> nothing else. >> >> I'm more concerned about the report of powerpc breakage. For powerpc we >> simply take each pcpu pointer from the pc_allcpu list (which is the same >> value stored in the cpuid_to_pcpu array) and pass it through the ap_pcpu >> global to each AP's startup code, which then stores it in sprg0. It >> should be globally unique and won't have the variable-translation issues >> seen on mips. Andreas, are you certain this change was responsible the >> breakage you saw, and was it the same sort of hang observed on mips? >> > > I'm really sure. 313036 booted fine, allowed me to execute heavy > compilation jobs, np. 313037 on the other side gave me various patterns of > panics. During startup, but I also succeeded to get into multiuser and then > the panic happend during port building. > > I have no deeper inside where pcpu data is used. Justin mentioned netisr? > > Andreas > > ___ 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: r313037 - in head/sys: amd64/include kern mips/include net powerpc/include sparc64/include
I suspect this broke rmlocks for mips because the rmlock implementation takes the address of the per-CPU pc_rm_queue when building tracker lists. That address may be later accessed from another CPU and will then translate to the wrong physical region if the address was taken relative to the globally-constant pcpup VA used on mips. Regardless, for mips get_pcpup() should be implemented as pcpu_find(curcpu) since returning an address that may mean something different depending on the CPU seems like a big POLA violation if nothing else. I'm more concerned about the report of powerpc breakage. For powerpc we simply take each pcpu pointer from the pc_allcpu list (which is the same value stored in the cpuid_to_pcpu array) and pass it through the ap_pcpu global to each AP's startup code, which then stores it in sprg0. It should be globally unique and won't have the variable-translation issues seen on mips. Andreas, are you certain this change was responsible the breakage you saw, and was it the same sort of hang observed on mips? On Sat, Feb 4, 2017 at 1:25 AM, Andreas Tobler wrote: > On 04.02.17 07:27, Jason Harmening wrote: > >> It's hard to argue with that:) I've backed it out until we can figure >> out what's going on. >> Sorry for the breakage. >> > > > For the record, powerpc64 was also affected. > > Andreas > >> >> On Fri, Feb 3, 2017 at 9:54 PM, Kurt Lidl > <mailto:l...@pix.net>> wrote: >> >> Having just spent a couple of hours bisecting what broke the kernel on >> my mips64 machine, I can definitively state it was this commit. >> >> With this commit in place, the kernel hangs early in the >> autoconfiguration: >> >> gcc version 4.2.1 20070831 patched [FreeBSD] >> Preloaded elf kernel "kernel" at 0x80aa96a0. >> real memory = 523239424 (510976K bytes) >> Physical memory chunk(s): >> 0x00bf3000 - 0x080d5fff, 122564608 bytes (29923 pages) >> 0x08101000 - 0x0ff00fff, 132120576 bytes (32256 pages) >> 0x41000 - 0x41f196fff, 253325312 bytes (61847 pages) >> avail memory = 504360960 (480MB) >> Create COP2 context zone >> AP #1 started! >> FreeBSD/SMP: Multiprocessor System Detected: 2 CPUs >> hangs here >> >> -Kurt >> >> On 2/4/17 12:29 AM, Jason Harmening wrote: >> >> Hi, >> >> I'm a bit confused as to how this change breaks MIPS. The new >> function, >> get_pcpu() is intended to be used only to access the per-cpu data >> pointer locally. It returns pcpup, which is the per-cpu pointer >> wired >> into the local TLB to translate to the local CPU's physical data >> region, >> correct? >> >> This is the same value used by the per-CPU accessors such as >> PCPU_ADD >> and PCPU_GET. The MI portions of this change only use get_pcpu() >> to >> access the local CPU's data, e.g. under a critical section in the >> rmlock. It is not intended to be used for iterating all CPUs. >> >> If I've missed something and MIPS is truly broken by this, then >> I'll >> gladly revert, but (maybe because it's late) I'm not seeing >> where this >> goes wrong on MIPS. >> >> Thanks, >> Jason >> >> On Fri, Feb 3, 2017 at 8:12 PM, Alexander Kabaev >> mailto:kab...@gmail.com> >> <mailto:kab...@gmail.com <mailto:kab...@gmail.com>>> wrote: >> >> On Wed, 1 Feb 2017 03:32:49 + (UTC) >> "Jason A. Harmening" wrote: >> >> > Author: jah >> > Date: Wed Feb 1 03:32:49 2017 >> > New Revision: 313037 >> > URL: https://svnweb.freebsd.org/changeset/base/313037 >> <https://svnweb.freebsd.org/changeset/base/313037> >> <https://svnweb.freebsd.org/changeset/base/313037 >> <https://svnweb.freebsd.org/changeset/base/313037>> >> > >> > Log: >> > Implement get_pcpu() for the remaining architectures and >> use it to >> > replace pcpu_find(curcpu) in MI code. >> > >> > Modified: >> > head/sys/amd64/include/pcpu.h >> > head/sys/kern/kern_rmlock.c >> > head/sys/mips/include/pcpu.h
Re: svn commit: r313037 - in head/sys: amd64/include kern mips/include net powerpc/include sparc64/include
It's hard to argue with that:) I've backed it out until we can figure out what's going on. Sorry for the breakage. On Fri, Feb 3, 2017 at 9:54 PM, Kurt Lidl wrote: > Having just spent a couple of hours bisecting what broke the kernel on > my mips64 machine, I can definitively state it was this commit. > > With this commit in place, the kernel hangs early in the > autoconfiguration: > > gcc version 4.2.1 20070831 patched [FreeBSD] > Preloaded elf kernel "kernel" at 0x80aa96a0. > real memory = 523239424 (510976K bytes) > Physical memory chunk(s): > 0x00bf3000 - 0x080d5fff, 122564608 bytes (29923 pages) > 0x08101000 - 0x0ff00fff, 132120576 bytes (32256 pages) > 0x41000 - 0x41f196fff, 253325312 bytes (61847 pages) > avail memory = 504360960 (480MB) > Create COP2 context zone > AP #1 started! > FreeBSD/SMP: Multiprocessor System Detected: 2 CPUs > hangs here > > -Kurt > > On 2/4/17 12:29 AM, Jason Harmening wrote: > >> Hi, >> >> I'm a bit confused as to how this change breaks MIPS. The new function, >> get_pcpu() is intended to be used only to access the per-cpu data >> pointer locally. It returns pcpup, which is the per-cpu pointer wired >> into the local TLB to translate to the local CPU's physical data region, >> correct? >> >> This is the same value used by the per-CPU accessors such as PCPU_ADD >> and PCPU_GET. The MI portions of this change only use get_pcpu() to >> access the local CPU's data, e.g. under a critical section in the >> rmlock. It is not intended to be used for iterating all CPUs. >> >> If I've missed something and MIPS is truly broken by this, then I'll >> gladly revert, but (maybe because it's late) I'm not seeing where this >> goes wrong on MIPS. >> >> Thanks, >> Jason >> >> On Fri, Feb 3, 2017 at 8:12 PM, Alexander Kabaev > <mailto:kab...@gmail.com>> wrote: >> >> On Wed, 1 Feb 2017 03:32:49 + (UTC) >> "Jason A. Harmening" wrote: >> >> > Author: jah >> > Date: Wed Feb 1 03:32:49 2017 >> > New Revision: 313037 >> > URL: https://svnweb.freebsd.org/changeset/base/313037 >> <https://svnweb.freebsd.org/changeset/base/313037> >> > >> > Log: >> > Implement get_pcpu() for the remaining architectures and use it to >> > replace pcpu_find(curcpu) in MI code. >> > >> > Modified: >> > head/sys/amd64/include/pcpu.h >> > head/sys/kern/kern_rmlock.c >> > head/sys/mips/include/pcpu.h >> > head/sys/net/netisr.c >> > head/sys/powerpc/include/cpufunc.h >> > head/sys/powerpc/include/pcpu.h >> > head/sys/sparc64/include/pcpu.h >> > >> >> Hi, >> >> this change was not reviewed nor testing was thought for all >> architectures it touches. The change happens to break MIPS quite >> thoroughly, since MIPS is using different pointers when accessing PCPU >> area locally and when doing iterations using cpu_to_cpuid array. I >> therefore officially am requesting this change to be reverted until >> reasonable solution is found to unbreak architectures that use wired >> TLBs to access local per-CPU data. >> >> -- >> Alexander Kabaev >> >> >> > ___ 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: r313037 - in head/sys: amd64/include kern mips/include net powerpc/include sparc64/include
Hi, I'm a bit confused as to how this change breaks MIPS. The new function, get_pcpu() is intended to be used only to access the per-cpu data pointer locally. It returns pcpup, which is the per-cpu pointer wired into the local TLB to translate to the local CPU's physical data region, correct? This is the same value used by the per-CPU accessors such as PCPU_ADD and PCPU_GET. The MI portions of this change only use get_pcpu() to access the local CPU's data, e.g. under a critical section in the rmlock. It is not intended to be used for iterating all CPUs. If I've missed something and MIPS is truly broken by this, then I'll gladly revert, but (maybe because it's late) I'm not seeing where this goes wrong on MIPS. Thanks, Jason On Fri, Feb 3, 2017 at 8:12 PM, Alexander Kabaev wrote: > On Wed, 1 Feb 2017 03:32:49 + (UTC) > "Jason A. Harmening" wrote: > > > Author: jah > > Date: Wed Feb 1 03:32:49 2017 > > New Revision: 313037 > > URL: https://svnweb.freebsd.org/changeset/base/313037 > > > > Log: > > Implement get_pcpu() for the remaining architectures and use it to > > replace pcpu_find(curcpu) in MI code. > > > > Modified: > > head/sys/amd64/include/pcpu.h > > head/sys/kern/kern_rmlock.c > > head/sys/mips/include/pcpu.h > > head/sys/net/netisr.c > > head/sys/powerpc/include/cpufunc.h > > head/sys/powerpc/include/pcpu.h > > head/sys/sparc64/include/pcpu.h > > > > Hi, > > this change was not reviewed nor testing was thought for all > architectures it touches. The change happens to break MIPS quite > thoroughly, since MIPS is using different pointers when accessing PCPU > area locally and when doing iterations using cpu_to_cpuid array. I > therefore officially am requesting this change to be reverted until > reasonable solution is found to unbreak architectures that use wired > TLBs to access local per-CPU data. > > -- > Alexander Kabaev > ___ 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: r312792 - in head/sys/arm: arm include
On Wed, Jan 25, 2017 at 9:23 PM, Jason A. Harmening wrote: > Author: jah > Date: Thu Jan 26 05:23:33 2017 > New Revision: 312792 > URL: https://svnweb.freebsd.org/changeset/base/312792 > > Log: > Further cleanup of per-CPU armv6 pmap data: > > - Replace pcpu_find(curcpu) with get_pcpu(), which is much > more direct. > > - Remove armv4 pcpu fields which I added in r286296 but never > needed to use. > > - armv6 pc_qmap_addr was leftover from the old armv6 pmap > implementation. Rename it and put it to use in the new one. > pc_qmap_addr -> pc_qmap_pte ___ 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: r300258 - head/sys/dev/iicbus
On 05/19/16 20:50, Ravi Pokala wrote:
> -Original Message-
> From: on behalf of "Jason A. Harmening"
>
> Date: 2016-05-19, Thursday at 20:03
> To: , ,
>
> Subject: svn commit: r300258 - head/sys/dev/iicbus
>
>> Author: jah
>> Date: Fri May 20 03:03:04 2016
>> New Revision: 300258
>> URL: https://svnweb.freebsd.org/changeset/base/300258
>>
>> Log:
>> iic_rdwr_data->nmsgs is uint32_t, so limit the allowable number of messages
>> to prevent memory exhaustion and short allocations on 32-bit systems. Since
>> iicrdwr is intended to be a workalike of a Linux i2c-dev call, use the same
>> limit of 42 that Linux uses.
>>
>> Also check the return value of copyin(9) to prevent unnecessary allocation
>> in the failure case.
>>
>> ...
>>
>> error = copyin(d->msgs, buf, sizeof(*d->msgs) * d->nmsgs);
>> +if (error != 0) {
>> +free(buf, M_IIC);
>> +return (error);
>> +}
>>
>
> Hi Jason,
>
> If I’m reading that right, it’s not preventing any allocations, but it is
> preventing a leak. Is that correct?
>
> Thanks,
>
> Ravi (rpokala@)
>
>
Hi Ravi,
There shouldn't be a leak in here, but checking the result prevents
falling through the rest of the function, including the unnecessary
malloc of usrbufs right after this. It also makes the error handling
clearer.
Thanks,
Jason
signature.asc
Description: OpenPGP digital signature
Re: svn commit: r289759 - in head/sys/arm: arm include
On Thu, Nov 5, 2015 at 9:21 AM, Ian Lepore wrote: > Just as an FYI for you guys, since we're all splashing around in busdma > code lately The things on my near-term (next several weeks) to-do > list for busdma are: > > Add "small bounce" support to help reduce the bounce preallocation > overhead for handling bounces due to cacheline alignment. Almost all > arm and mips bounces are triggered by cache alignment restrictions and > involve buffers smaller than a page (usually 256 bytes or less), and we > currently allocate full pages for bouncing. Instead we can use the > same small-buffer uma pools that are used for bus_dmamem_alloc(). > I like that. Along similar (but definitely not the same) lines, I had a suggestion in https://reviews.freebsd.org/D888 to coalesce bounce buffers, similar to how addseg already coalesces adjacent segments. Would something like that help here (in addition to small-bounce) to cut down on bounce buffer overhead, when the total buffer size is still a page or larger? > > The armv6 busdma implementation can be shared by armv4, armv6, and > mips. The only thing blocking this right now is the small-bounce > stuff; armv4 and mips boards often have only 32-128MB of ram, and they > can't afford the bounce-page allocation overhead of the current armv6 > implementation. Once the small-bounce support is in place, we can use > this implementation for all these platforms (and basically for any > platform that comes along that has a similar requirement for software > -assisted cache coherency for DMA). > Do you plan to merge armv6 and mips soonish? If so, then https://reviews.freebsd.org/D3986 can go away. That will also fix the problems mips busdma has with the cacheline copies in sync_buf() not being threadsafe. Code duplication in busdma is a huge pain right now. It seems like we have 3 broad classes of busdma support: bounce/coherent, bounce/non-coherent, and iommu. bounce/coherent could probably share all the same code. You might need a light dusting of platform-specific code (e.g. powerpc issues a sync instruction at the end of bus_dmamap_sync), but otherwise it should be the same. bounce/noncoherent could share the same code, as long as you define a set of macros or inline funcs that abstract the cache maintenance. That'll already be needed to cover the cache differences between armv4, armv6, and mips anyway. iommu is always going to be hardware-specific and need its own implementation for each kind of iommu. Then there's some logic that could probably be shared everywhere: a lot of the bus_dmamem_alloc()/bufalloc code strikes me as falling into that category. You might need MD wrappers that call MI helper functions, so for example the MD code can decide whether BUS_DMA_COHERENT means VM_MEMATTR_UNCACHEABLE, or whether allocated buffers really need to be physically contiguous (not needed for iommu). On top of all that, it makes sense to generalize the vtable approach already used for x86 and arm64. In a lot of cases the vtable would just point to the common bounce/coherent or bounce/noncoherent functions, or in some cases lightweight MD wrappers around those. Then for example it would be easy to add bounce/noncoherent support for arm64 if we end up needing it, and somewhat easier to add arm/mips/ppc iommu support. I'm definitely not suggesting you do all that stuff right now, just throwing ideas out there :) > > -- Ian > > On Thu, 2015-11-05 at 08:08 -0800, Jason Harmening wrote: > > Userspace buffers in load_buffer() also need temporary mappings, so > > it > > might be nice to keep the panic/KASSERT there for completeness. I > > don't > > see how anything coming from userspace would be outside > > vm_page_array > > though, so that is up to you and Michal. > > > > Since Michal's already made the initial patch, I think he should make > > the > > commit. That seems only right, plus I'm still busy trying to get > > everything set up at my new place. I'd like to be on the phabricator > > review, though. > > > > On Thu, Nov 5, 2015 at 3:58 AM, Svatopluk Kraus > > wrote: > > > > > On Thu, Nov 5, 2015 at 1:11 AM, Jason Harmening > > > wrote: > > > > > > > > On Wed, Nov 4, 2015 at 2:17 PM, Svatopluk Kraus > > > > > > > wrote: > > > > > > > > > > On Tue, Nov 3, 2015 at 8:45 AM, Jason Harmening > > > > > wrote: > > > > > > > > > > > > On Sun, Nov 1, 2015 at 8:11 AM, Ian Lepore > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > It's almost certainly not related to sysinit ordering. >
Re: svn commit: r289759 - in head/sys/arm: arm include
Userspace buffers in load_buffer() also need temporary mappings, so it might be nice to keep the panic/KASSERT there for completeness. I don't see how anything coming from userspace would be outside vm_page_array though, so that is up to you and Michal. Since Michal's already made the initial patch, I think he should make the commit. That seems only right, plus I'm still busy trying to get everything set up at my new place. I'd like to be on the phabricator review, though. On Thu, Nov 5, 2015 at 3:58 AM, Svatopluk Kraus wrote: > On Thu, Nov 5, 2015 at 1:11 AM, Jason Harmening > wrote: > > > > On Wed, Nov 4, 2015 at 2:17 PM, Svatopluk Kraus > wrote: > >> > >> On Tue, Nov 3, 2015 at 8:45 AM, Jason Harmening > >> wrote: > >> > > >> > On Sun, Nov 1, 2015 at 8:11 AM, Ian Lepore wrote: > >> >> > >> >> > >> >> It's almost certainly not related to sysinit ordering. This > exception > >> >> is happening during mmc probing after interrupts are enabled. > >> >> > >> >> It appears that the problem is the faulting code is running on one of > >> >> the very early pre-allocated kernel stacks (perhaps in an interrupt > >> >> handler on an idle thread stack), and these stacks are not in memory > >> >> represented in the vm page arrays. The mmc code is using a 64-byte > >> >> buffer on the stack and mapping it for DMA. Normally that causes a > >> >> bounce for cacheline alignment, but unluckily in this case that > buffer > >> >> on the stack just happened to be aligned to a cacheline boundary and > a > >> >> multiple of the cacheline size, so no bounce. That causes the new > sync > >> >> logic that is based on keeping vm_page_t pointers and offsets to get > a > >> >> NULL pointer back from PHYS_TO_VM_PAGE when mapping, then it dies at > >> >> sync time trying to dereference that. It used to work because the > sync > >> >> logic used to use the vaddr, not a page pointer. > >> >> > >> >> Michal was working on a patch yesterday. > >> >> > >> > > >> > Ah, thanks for pointing that out Ian. I was left scratching my head > >> > (admittedly on the road and w/o easy access to the code) wondering > what > >> > on > >> > earth would be trying to do DMA during SI_SUB_CPU. > >> > > >> > >> > >> Using of fictitious pages is not so easy here as in case pointed by > >> kib@ where they are allocated and freed inside one function. For sync > >> list sake, they must be allocated when a buffer is loaded and freed > >> when is unloaded. > >> > >> Michal uses pmap_kextract() in case when KVA of buffer is not zero in > >> his proof-of-concept patch: > >> https://gist.github.com/strejda/d5ca3ed202427a2e0557 > >> When KVA of buffer is not zero, temporary mapping is not used at all, > >> so vm_page_t array is not needed too. > >> > >> IMO, buffer's physical address can be saved in sync list to be used > >> when its KVA is not zero. Thus pmap_kextract() won't be called in > >> dma_dcache_sync(). > > > > > > I like the idea of storing off the physical address. If you want to save > > space in the sync list, I think you can place busaddr and pages in a > union, > > using vaddr == 0 to select which field to use. Some people frown upon > use > > of unions, though. > > > > Any reason the panics on PHYS_TO_VM_PAGE in load_buffer() and load_phys() > > shouldn't be KASSERTs instead? > > > KASSERTs are fine. I have looked at Michal's patch again and only > KASSERT should be in load_phys() as such buffers are unmapped, > temporary mapping must be used and so, they must be in vm_page_array. > And maybe some inline function for getting PA from sync list would be > nice too. I would like to review final patch if you are going to make > it. And it would be nice if Ganbold will test it. Phabricator? > ___ 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: r289759 - in head/sys/arm: arm include
On Wed, Nov 4, 2015 at 2:17 PM, Svatopluk Kraus wrote: > On Tue, Nov 3, 2015 at 8:45 AM, Jason Harmening > wrote: > > > > On Sun, Nov 1, 2015 at 8:11 AM, Ian Lepore wrote: > >> > >> > >> It's almost certainly not related to sysinit ordering. This exception > >> is happening during mmc probing after interrupts are enabled. > >> > >> It appears that the problem is the faulting code is running on one of > >> the very early pre-allocated kernel stacks (perhaps in an interrupt > >> handler on an idle thread stack), and these stacks are not in memory > >> represented in the vm page arrays. The mmc code is using a 64-byte > >> buffer on the stack and mapping it for DMA. Normally that causes a > >> bounce for cacheline alignment, but unluckily in this case that buffer > >> on the stack just happened to be aligned to a cacheline boundary and a > >> multiple of the cacheline size, so no bounce. That causes the new sync > >> logic that is based on keeping vm_page_t pointers and offsets to get a > >> NULL pointer back from PHYS_TO_VM_PAGE when mapping, then it dies at > >> sync time trying to dereference that. It used to work because the sync > >> logic used to use the vaddr, not a page pointer. > >> > >> Michal was working on a patch yesterday. > >> > > > > Ah, thanks for pointing that out Ian. I was left scratching my head > > (admittedly on the road and w/o easy access to the code) wondering what > on > > earth would be trying to do DMA during SI_SUB_CPU. > > > > > Using of fictitious pages is not so easy here as in case pointed by > kib@ where they are allocated and freed inside one function. For sync > list sake, they must be allocated when a buffer is loaded and freed > when is unloaded. > > Michal uses pmap_kextract() in case when KVA of buffer is not zero in > his proof-of-concept patch: > https://gist.github.com/strejda/d5ca3ed202427a2e0557 > When KVA of buffer is not zero, temporary mapping is not used at all, > so vm_page_t array is not needed too. > > IMO, buffer's physical address can be saved in sync list to be used > when its KVA is not zero. Thus pmap_kextract() won't be called in > dma_dcache_sync(). > I like the idea of storing off the physical address. If you want to save space in the sync list, I think you can place busaddr and pages in a union, using vaddr == 0 to select which field to use. Some people frown upon use of unions, though. Any reason the panics on PHYS_TO_VM_PAGE in load_buffer() and load_phys() shouldn't be KASSERTs instead? ___ 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: r289759 - in head/sys/arm: arm include
On Sun, Nov 1, 2015 at 8:11 AM, Ian Lepore wrote: > > It's almost certainly not related to sysinit ordering. This exception > is happening during mmc probing after interrupts are enabled. > > It appears that the problem is the faulting code is running on one of > the very early pre-allocated kernel stacks (perhaps in an interrupt > handler on an idle thread stack), and these stacks are not in memory > represented in the vm page arrays. The mmc code is using a 64-byte > buffer on the stack and mapping it for DMA. Normally that causes a > bounce for cacheline alignment, but unluckily in this case that buffer > on the stack just happened to be aligned to a cacheline boundary and a > multiple of the cacheline size, so no bounce. That causes the new sync > logic that is based on keeping vm_page_t pointers and offsets to get a > NULL pointer back from PHYS_TO_VM_PAGE when mapping, then it dies at > sync time trying to dereference that. It used to work because the sync > logic used to use the vaddr, not a page pointer. > > Michal was working on a patch yesterday. > > Ah, thanks for pointing that out Ian. I was left scratching my head (admittedly on the road and w/o easy access to the code) wondering what on earth would be trying to do DMA during SI_SUB_CPU. ___ 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: r289759 - in head/sys/arm: arm include
On Sat, Oct 31, 2015 at 4:55 AM, Jason Harmening wrote: > > > On 10/31/15 03:21, Ganbold Tsagaankhuu wrote: > > On Fri, Oct 23, 2015 at 12:38 AM, Jason A. Harmening > > wrote: > > > >> Author: jah > >> Date: Thu Oct 22 16:38:01 2015 > >> New Revision: 289759 > >> URL: https://svnweb.freebsd.org/changeset/base/289759 > >> > >> Log: > >> Use pmap_quick* functions in armv6 busdma, for bounce buffers and > cache > >> maintenance. This makes it safe to sync buffers that have no VA mapping > >> associated with the busdma map, but may have other mappings, possibly on > >> different CPUs. This also makes it safe to sync unmapped bounce > buffers in > >> non-sleepable thread contexts. > >> > >> Similar to r286787 for x86, this treats userspace buffers the same as > >> unmapped buffers and no longer borrows the UVA for sync operations. > >> > >> Submitted by: Svatopluk Kraus (earlier > >> revision) > >> Tested by:Svatopluk Kraus > >> Differential Revision:https://reviews.freebsd.org/D3869 > > > > > > > > It seems I can't boot Odroid C1 with this change. > > > > http://pastebin.ca/3227678 > > > > r289758 works for me. > > > > Am I missing something? > > > > thanks, > > > > Ganbold > > > > > > Hmmm, the fault address of 0x20 and the fact that this is happening > during mi_startup() make me wonder if the per-cpu pageframes used by > pmap_quick* haven't been initialized yet. > > I wonder if changing the order of qpages_init in > sys/arm/arm/pmap-v6[-new].c to something other than SI_ORDER_ANY would > help? > > It seems like we'd want SI_ORDER_FOURTH or SI_ORDER_MIDDLE, since > mp_start() is SI_ORDER_THIRD. > > It would be nice to know what's calling bus_dmamap_sync() in this case. > I can't figure that out, but maybe that's because I haven't had coffee > yet. > > Can you build the kernel with 'options VERBOSE_SYSINIT' ? That will add some spew to the boot log, but it should confirm whether this is a sysinit ordering issue. ___ 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: r289759 - in head/sys/arm: arm include
On 10/31/15 03:21, Ganbold Tsagaankhuu wrote: > On Fri, Oct 23, 2015 at 12:38 AM, Jason A. Harmening > wrote: > >> Author: jah >> Date: Thu Oct 22 16:38:01 2015 >> New Revision: 289759 >> URL: https://svnweb.freebsd.org/changeset/base/289759 >> >> Log: >> Use pmap_quick* functions in armv6 busdma, for bounce buffers and cache >> maintenance. This makes it safe to sync buffers that have no VA mapping >> associated with the busdma map, but may have other mappings, possibly on >> different CPUs. This also makes it safe to sync unmapped bounce buffers in >> non-sleepable thread contexts. >> >> Similar to r286787 for x86, this treats userspace buffers the same as >> unmapped buffers and no longer borrows the UVA for sync operations. >> >> Submitted by: Svatopluk Kraus (earlier >> revision) >> Tested by:Svatopluk Kraus >> Differential Revision:https://reviews.freebsd.org/D3869 > > > > It seems I can't boot Odroid C1 with this change. > > http://pastebin.ca/3227678 > > r289758 works for me. > > Am I missing something? > > thanks, > > Ganbold > > Hmmm, the fault address of 0x20 and the fact that this is happening during mi_startup() make me wonder if the per-cpu pageframes used by pmap_quick* haven't been initialized yet. I wonder if changing the order of qpages_init in sys/arm/arm/pmap-v6[-new].c to something other than SI_ORDER_ANY would help? It seems like we'd want SI_ORDER_FOURTH or SI_ORDER_MIDDLE, since mp_start() is SI_ORDER_THIRD. It would be nice to know what's calling bus_dmamap_sync() in this case. I can't figure that out, but maybe that's because I haven't had coffee yet. Unfortunately I'm going to have limited time to help debug this over the next week, as I'm moving across the country starting (hopefully) today. --Jason signature.asc Description: OpenPGP digital signature
Re: svn commit: r286787 - head/sys/x86/x86
On 08/16/15 04:50, Konstantin Belousov wrote:
> On Sun, Aug 16, 2015 at 12:03:58PM +0300, Konstantin Belousov wrote:
>> On Sun, Aug 16, 2015 at 10:16:53AM +0200, Roger Pau Monn?? wrote:
>>> pmap_map_io_transient contains some of this logic, but it uses
>>> vmem_alloc (with M_WAITOK) instead of a pcpu pageframe, which defeats
>>> part of the purpose of this change and cannot be used as-is.
>>
>> This logic can be repeated, but it is probably too much for the purpose.
>> It would be enough to have single frame (we cannot reuse CMAP1),
>> protected by a spin mutex. I do not see much sense in providing
>> optimized per-cpu frames for this case.
>
> Like this. I only compiled the patch.
>
> diff --git a/sys/amd64/amd64/pmap.c b/sys/amd64/amd64/pmap.c
> index 1e64fc8..9d2b2d9 100644
> --- a/sys/amd64/amd64/pmap.c
> +++ b/sys/amd64/amd64/pmap.c
> @@ -390,6 +390,8 @@ static struct md_page *pv_table;
> */
> pt_entry_t *CMAP1 = 0;
> caddr_t CADDR1 = 0;
> +static vm_offset_t qframe = 0;
> +static struct mtx qframe_mtx;
>
> static int pmap_flags = PMAP_PDE_SUPERPAGE; /* flags for x86 pmaps */
>
> @@ -1031,7 +1033,7 @@ pmap_init(void)
> struct pmap_preinit_mapping *ppim;
> vm_page_t mpte;
> vm_size_t s;
> - int i, pv_npg;
> + int error, i, pv_npg;
>
> /*
>* Initialize the vm page array entries for the kernel pmap's
> @@ -1112,6 +1114,12 @@ pmap_init(void)
> printf("PPIM %u: PA=%#lx, VA=%#lx, size=%#lx, mode=%#x\n", i,
> ppim->pa, ppim->va, ppim->sz, ppim->mode);
> }
> +
> + mtx_init(&qframe_mtx, "qfrmlk", NULL, MTX_SPIN);
> + error = vmem_alloc(kernel_arena, PAGE_SIZE, M_BESTFIT | M_WAITOK,
> + (vmem_addr_t *)&qframe);
> + if (error != 0)
> + panic("qframe allocation failed");
> }
>
> static SYSCTL_NODE(_vm_pmap, OID_AUTO, pde, CTLFLAG_RD, 0,
> @@ -7019,13 +7027,28 @@ pmap_unmap_io_transient(vm_page_t page[], vm_offset_t
> vaddr[], int count,
> vm_offset_t
> pmap_quick_enter_page(vm_page_t m)
> {
> + vm_paddr_t paddr;
>
> - return (PHYS_TO_DMAP(VM_PAGE_TO_PHYS(m)));
> + paddr = VM_PAGE_TO_PHYS(m);
> + if (paddr < dmaplimit)
> + return (PHYS_TO_DMAP(paddr));
> + mtx_lock_spin(&qframe_mtx);
> + KASSERT(*vtopte(qframe) == 0, ("qframe busy"));
> + pte_store(vtopte(qframe), paddr | X86_PG_RW | X86_PG_V | X86_PG_A |
> + X86_PG_M | pmap_cache_bits(kernel_pmap, m->md.pat_mode, 0));
> + invlpg(qframe);
> + return (qframe);
> }
>
> void
> pmap_quick_remove_page(vm_offset_t addr)
> {
> +
> + if (addr != qframe)
> + return;
> + pte_store(vtopte(qframe), 0);
> + invlpg(qframe);
> + mtx_unlock_spin(&qframe_mtx);
> }
>
> #include "opt_ddb.h"
>
That looks fine to me.
___
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: r286787 - head/sys/x86/x86
On 08/15/15 08:05, Roger Pau Monné wrote: > El 14/08/15 a les 22.08, Jason A. Harmening ha escrit: >> Author: jah >> Date: Fri Aug 14 20:08:16 2015 >> New Revision: 286787 >> URL: https://svnweb.freebsd.org/changeset/base/286787 >> >> Log: >> Use pmap_quick_enter_page() to handle bouncing of unmapped buffers in the >> x86 busdma_bounce implementation. Also treat user buffers as unmapped. >> This allows two things: >> 1. Sync'ing bounced maps in non-sleepable contexts. The physcopy* calls >> previously used could sleep on sf_buf operations in some cases. >> 2. Sync'ing user buffers outside the context of the owning process > > AFAICT this will break the Xen port. physcopy* uses uiomove_fromphys > that on the amd64 port is able to deal with pages outside of the DMAP. > OTOH pmap_quick_enter_page is not able to deal with pages outside of the > DMAP, and will simply panic. > > Roger. > Is it actually possible for those non-dom0 pages to be used for I/O that passes through busdma? If it is, then it would be easy to make pmap_quick_enter_page() handle them by adding a pcpu pageframe similar to what we do for i386. ___ 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: r286296 - in head/sys: amd64/amd64 arm/arm arm/include arm64/arm64 i386/i386 i386/include mips/mips powerpc/aim powerpc/booke powerpc/include powerpc/powerpc sparc64/include sparc64/sp
On 08/04/15 18:22, Bjoern A. Zeeb wrote: > >> On 04 Aug 2015, at 19:46 , Jason A. Harmening wrote: >> >> Author: jah >> Date: Tue Aug 4 19:46:13 2015 >> New Revision: 286296 >> URL: https://svnweb.freebsd.org/changeset/base/286296 >> >> Log: >> Add two new pmap functions: >> vm_offset_t pmap_quick_enter_page(vm_page_t m) >> void pmap_quick_remove_page(vm_offset_t kva) >> >> These will create and destroy a temporary, CPU-local KVA mapping of a >> specified page. >> >> Guarantees: >> --Will not sleep and will not fail. >> --Safe to call under a non-sleepable lock or from an ithread >> >> Restrictions: >> --Not guaranteed to be safe to call from an interrupt filter or under a >> spin mutex on all platforms >> --Current implementation does not guarantee more than one page of mapping >> space across all platforms. MI code should not make nested calls to >> pmap_quick_enter_page. >> --MI code should not perform locking while holding onto a mapping created >> by pmap_quick_enter_page >> >> The idea is to use this in busdma, for bounce buffer copies as well as >> virtually-indexed cache maintenance on mips and arm. >> >> NOTE: the non-i386, non-amd64 implementations of these functions still need >> review and testing. > > Most of this description should go into a section 9 man page rather than the > commit message ;-) > > /bz > Yep, I'm planning to write one. ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r285270 - head/sys/sys
+2
This has always bugged me. I'll do it if no one else gets to it first. If
the new pmap KPI makes it in, I'll be messing with a bunch of the MD busdma
code anyway.
On Wed, Jul 8, 2015 at 9:47 AM, Adrian Chadd wrote:
> On 8 July 2015 at 07:42, Ian Lepore wrote:
> > On Wed, 2015-07-08 at 07:30 -0700, Adrian Chadd wrote:
> >> Why is this implemented in sys/sys/bus_dma.h, rather than in a machdep
> header?
> >>
> >>
> >
> > Indeed, this stuff is a mess that really needs to be cleaned up. The
> > pre-existing assumption that MI functions don't need to be called, just
> > because one of the parameters of that function has some value that has a
> > special meaning on one platform, is purely x86-MD code in an MI header
> > file. This change just complicates the existing mess.
>
> +1
>
> (So who's going to do it? I'm already taken.)
>
>
>
> -a
>
> >> -a
> >>
> >>
> >> On 8 July 2015 at 06:53, Zbigniew Bodek wrote:
> >> > Author: zbb
> >> > Date: Wed Jul 8 13:52:59 2015
> >> > New Revision: 285270
> >> > URL: https://svnweb.freebsd.org/changeset/base/285270
> >> >
> >> > Log:
> >> > Add memory barrier to bus_dmamap_sync()
> >> >
> >> > On platforms which are fully IO-coherent, the map might be null.
> >> > We need to guarantee that all data is observable after the
> >> > sync operation is called. Add a memory barrier to ensure that on
> ARM.
> >> >
> >> > Reviewed by: andrew, kib
> >> > Obtained from: Semihalf
> >> > Sponsored by: The FreeBSD Foundation
> >> > Differential Revision: https://reviews.freebsd.org/D3012
> >> >
> >> > Modified:
> >> > head/sys/sys/bus_dma.h
> >> >
> >> > Modified: head/sys/sys/bus_dma.h
> >> >
> ==
> >> > --- head/sys/sys/bus_dma.h Wed Jul 8 13:19:13 2015
> (r285269)
> >> > +++ head/sys/sys/bus_dma.h Wed Jul 8 13:52:59 2015
> (r285270)
> >> > @@ -282,13 +282,25 @@ int bus_dmamem_alloc(bus_dma_tag_t dmat,
> >> > void bus_dmamem_free(bus_dma_tag_t dmat, void *vaddr, bus_dmamap_t
> map);
> >> >
> >> > /*
> >> > - * Perform a synchronization operation on the given map.
> >> > + * Perform a synchronization operation on the given map. If the map
> >> > + * is NULL we have a fully IO-coherent system. On every ARM
> architecture
> >> > + * there must be a memory barrier placed to ensure that all data
> >> > + * accesses are visible before going any further.
> >> > */
> >> > void _bus_dmamap_sync(bus_dma_tag_t, bus_dmamap_t, bus_dmasync_op_t);
> >> > +#if defined(__arm__)
> >> > + #define __BUS_DMAMAP_SYNC_DEFAULT mb();
> >> > +#elif defined(__aarch64__)
> >> > + #define __BUS_DMAMAP_SYNC_DEFAULT dmb(sy);
> >> > +#else
> >> > + #define __BUS_DMAMAP_SYNC_DEFAULT {}
> >> > +#endif
> >> > #define bus_dmamap_sync(dmat, dmamap, op) \
> >> > do {\
> >> > if ((dmamap) != NULL) \
> >> > _bus_dmamap_sync(dmat, dmamap, op); \
> >> > + else\
> >> > + __BUS_DMAMAP_SYNC_DEFAULT \
> >> > } while (0)
> >> >
> >> > /*
> >> >
> >>
> >
> >
>
>
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
