Re: [PATCH] intel_uncore.c - Horrible, ugly hack to avoid dmesg spam
> The patch below simply stops printing additional messages after 10 > lines have been printed. > You might want to use ratecheck(9) rather than a simple limit.
[PATCH] intel_uncore.c - Horrible, ugly hack to avoid dmesg spam
Hello tech@
First, I want to say that I'm in no way advocating that this patch
be committed to the tree. I'm sending it solely for the others
who have encountered the same issue I have. It's only purpose is
to suppress dmesg spam on boot and thereby speed up booting by
10-15 seconds, as well as to not flush out potentially important
information from the dmesg buffer.
This patch is ugly, hackish, and (poorly) paints over the underlying
problem. It may cause headaches and nausea in certain
individuals.
In brief, on boot, the system prints out a tremendous amount of
variations of the following error:
error: [drm:pid3322:intel_uncore_check_errors] *ERROR* Unclaimed register
before interrupt
This has been reported to occur on a Thinkpad T440p, Thinkpad T440s,
Dell M3800, and a Toshiba Portege R30-A.
The system otherwise works fine, so it doesn't appear that
whatever is causing the messages to print has any other
deleterious effects on the system.
Enough lines are printed out to completely fill the dmesg buffer
as such:
% dmesg | sort -n | uniq -c
6 error: [drm:pid22060:intel_uncore_check_errors] *ERROR* Unclaimed register
before interrupt
209 error: [drm:pid25769:intel_uncore_check_errors] *ERROR* Unclaimed register
before interrupt
3 error: [drm:pid25817:intel_uncore_check_errors] *ERROR* Unclaimed register
before interrupt
44 error: [drm:pid30074:intel_uncore_check_errors] *ERROR* Unclaimed register
before interrupt
402 error: [drm:pid3322:intel_uncore_check_errors] *ERROR* Unclaimed register
before interrupt
28 error: [drm:pid33672:intel_uncore_check_errors] *ERROR* Unclaimed register
before interrupt
5 error: [drm:pid39271:intel_uncore_check_errors] *ERROR* Unclaimed register
before interrupt
2 error: [drm:pid40435:intel_uncore_check_errors] *ERROR* Unclaimed register
before interrupt
2 error: [drm:pid5194:intel_uncore_check_errors] *ERROR* Unclaimed register
before interrupt
12 error: [drm:pid92159:intel_uncore_check_errors] *ERROR* Unclaimed register
before interrupt
1 pid92159:intel_uncore_check_errors] *ERROR* Unclaimed register before
interrupt
The patch below simply stops printing additional messages after 10
lines have been printed.
--
Bryan
Index: intel_uncore.c
===
RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_uncore.c,v
retrieving revision 1.5
diff -u -r1.5 intel_uncore.c
--- intel_uncore.c 3 Sep 2017 13:38:58 - 1.5
+++ intel_uncore.c 14 Oct 2017 04:05:20 -
@@ -1565,9 +1565,17 @@
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ static long i =0;
+
if (HAS_FPGA_DBG_UNCLAIMED(dev) &&
(__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) {
- DRM_ERROR("Unclaimed register before interrupt\n");
+ if (i < 10)
+ DRM_ERROR("Unclaimed register before interrupt\n");
+ if (i == 10)
+ DRM_ERROR("Suspending printing of more unclaimed"
+ " register errors.\n");
+
__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
+ i++;
}
}
Re: just a few 1<<31 to 1U<<31
On Wed, Sep 27, 2017 at 09:06:01PM +0300, Artturi Alm wrote:
> Hi,
>
> was looking at sdmmc, and then i did remember this rev1.3 commit:
> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/armv7/imx/imxccm.c#rev1.3
>
> also fixes those from under sys/dev/fdt and sys/arch/armv7 i found,
> didn't look elsewhere.
>
> -Artturi
>
ping?
the mail that prompted above mentioned rev1.3 commit,
was this mail to tech@ by Eitan Adler:
https://marc.info/?l=openbsd-tech&m=138527061431038&w=2
-Artturi
diff --git a/sys/arch/armv7/imx/imxccm.c b/sys/arch/armv7/imx/imxccm.c
index 4847d49f92f..b09cae0f9a4 100644
--- a/sys/arch/armv7/imx/imxccm.c
+++ b/sys/arch/armv7/imx/imxccm.c
@@ -129,7 +129,7 @@
#define CCM_ANALOG_PLL_USB1_POWER (1 << 12)
#define CCM_ANALOG_PLL_USB1_ENABLE (1 << 13)
#define CCM_ANALOG_PLL_USB1_BYPASS (1 << 16)
-#define CCM_ANALOG_PLL_USB1_LOCK (1 << 31)
+#define CCM_ANALOG_PLL_USB1_LOCK (1U << 31)
#define CCM_ANALOG_PLL_USB2_DIV_SELECT_MASK0x1
#define CCM_ANALOG_PLL_USB2_EN_USB_CLKS(1 << 6)
#define CCM_ANALOG_PLL_USB2_POWER (1 << 12)
diff --git a/sys/arch/armv7/imx/imxiomuxc.c b/sys/arch/armv7/imx/imxiomuxc.c
index 03adb6e3a47..e4b5e825cda 100644
--- a/sys/arch/armv7/imx/imxiomuxc.c
+++ b/sys/arch/armv7/imx/imxiomuxc.c
@@ -106,7 +106,7 @@
#define IOMUX_CONFIG_SION (1 << 4)
-#define IMX_PINCTRL_NO_PAD_CTL (1 << 31)
+#define IMX_PINCTRL_NO_PAD_CTL (1U << 31)
#define IMX_PINCTRL_SION (1 << 30)
struct imxiomuxc_softc {
diff --git a/sys/dev/fdt/if_dwxe.c b/sys/dev/fdt/if_dwxe.c
index 22a383c06ef..00aa4e76178 100644
--- a/sys/dev/fdt/if_dwxe.c
+++ b/sys/dev/fdt/if_dwxe.c
@@ -184,7 +184,7 @@ struct dwxe_desc {
#define DWXE_TX_PAYLOAD_ERR(1 << 12)
#define DWXE_TX_LENGTH_ERR (1 << 14)
#define DWXE_TX_HEADER_ERR (1 << 16)
-#define DWXE_TX_DESC_CTL (1 << 31)
+#define DWXE_TX_DESC_CTL (1U << 31)
/* Rx status bits */
#define DWXE_RX_PAYLOAD_ERR(1 << 0)
@@ -202,7 +202,7 @@ struct dwxe_desc {
#define DWXE_RX_FRM_LEN_MASK 0x3fff
#define DWXE_RX_FRM_LEN_SHIFT 16
#define DWXE_RX_DAF_FAIL (1 << 30)
-#define DWXE_RX_DESC_CTL (1 << 31)
+#define DWXE_RX_DESC_CTL (1U << 31)
/* Tx size bits */
#define DWXE_TX_BUF_SIZE (0xfff << 0)
@@ -213,11 +213,11 @@ struct dwxe_desc {
#define DWXE_TX_CHECKSUM_CTL_FULL (3 << 27)
#define DWXE_TX_FIR_DESC (1 << 29)
#define DWXE_TX_LAST_DESC (1 << 30)
-#define DWXE_TX_INT_CTL(1 << 31)
+#define DWXE_TX_INT_CTL(1U << 31)
/* Rx size bits */
#define DWXE_RX_BUF_SIZE (0xfff << 0)
-#define DWXE_RX_INT_CTL(1 << 31)
+#define DWXE_RX_INT_CTL(1U << 31)
/* EMAC syscon bits */
#define SYSCON 0x30
diff --git a/sys/dev/sdmmc/sdmmc_ioreg.h b/sys/dev/sdmmc/sdmmc_ioreg.h
index b23edd898d7..f2dc066d5fc 100644
--- a/sys/dev/sdmmc/sdmmc_ioreg.h
+++ b/sys/dev/sdmmc/sdmmc_ioreg.h
@@ -26,7 +26,7 @@
/* CMD52 arguments */
#define SD_ARG_CMD52_READ (0<<31)
-#define SD_ARG_CMD52_WRITE (1<<31)
+#define SD_ARG_CMD52_WRITE (1U<<31)
#define SD_ARG_CMD52_FUNC_SHIFT28
#define SD_ARG_CMD52_FUNC_MASK 0x7
#define SD_ARG_CMD52_EXCHANGE (1<<27)
@@ -38,7 +38,7 @@
/* CMD53 arguments */
#define SD_ARG_CMD53_READ (0<<31)
-#define SD_ARG_CMD53_WRITE (1<<31)
+#define SD_ARG_CMD53_WRITE (1U<<31)
#define SD_ARG_CMD53_FUNC_SHIFT28
#define SD_ARG_CMD53_FUNC_MASK 0x7
#define SD_ARG_CMD53_BLOCK_MODE(1<<27)
@@ -54,7 +54,7 @@
#define MMC_R5(resp) ((resp)[0])
/* SD R4 response (IO OCR) */
-#define SD_IO_OCR_MEM_READY(1<<31)
+#define SD_IO_OCR_MEM_READY(1U<<31)
#define SD_IO_OCR_NUM_FUNCTIONS(ocr) (((ocr) >> 28) & 0x3)
/* XXX big fat memory present "flag" because we don't know better */
#define SD_IO_OCR_MEM_PRESENT (0xf<<24)
diff --git a/sys/dev/sdmmc/sdmmcreg.h b/sys/dev/sdmmc/sdmmcreg.h
index c984afc9f78..87116959652 100644
--- a/sys/dev/sdmmc/sdmmcreg.h
+++ b/sys/dev/sdmmc/sdmmcreg.h
@@ -49,7 +49,7 @@
#define SD_APP_SEND_SCR51 /* R1 */
/* OCR bits */
-#define MMC_OCR_MEM_READY (1<<31) /* memory power-up status bit */
+#define MMC_OCR_MEM_READY (1U<<31)/* memory power-up
status bit */
#define MMC_OCR_ACCESS_MODE_MASK 0x6000 /* bits 30:29 */
#define MMC_OCR_SECTOR_MODE(1<<30)
#define MMC_OCR_BYTE_MODE (1<<29)
armv7/arm64 i2c gates+resets + quirk for >=sun6i-a31-i2c compat
Hi,
in short: everything to enable i2c on newer sunxis.
adds sxitwi* at fdt, and iic* at sxitwi to arm64 GENERIC&RAMDISK,
adds I2C IP gates+resets for a64&h3 to sxiccmu_clocks.h, and
adds sxitwi compatible+quirk for +a31(sun6i+newer).
--Artturi
diff --git a/sys/arch/arm64/conf/GENERIC b/sys/arch/arm64/conf/GENERIC
index 7a5742c3f1e..f3ebbb67843 100644
--- a/sys/arch/arm64/conf/GENERIC
+++ b/sys/arch/arm64/conf/GENERIC
@@ -104,6 +104,8 @@ sxiccmu*at fdt? early 1 # Clock Control Module/Unit
sxirtc*at fdt? # Real Time Clock
sximmc*at fdt? # SD/MMC card controller
sdmmc* at sximmc? # SD/MMC bus
+sxitwi*at fdt? # I2C controller
+iic* at sxitwi? # I2C bus
dwxe* at fdt?
# PCI
diff --git a/sys/arch/arm64/conf/RAMDISK b/sys/arch/arm64/conf/RAMDISK
index a023c37df01..24214378ad7 100644
--- a/sys/arch/arm64/conf/RAMDISK
+++ b/sys/arch/arm64/conf/RAMDISK
@@ -114,6 +114,8 @@ sxiccmu*at fdt? early 1 # Clock Control Module/Unit
sxirtc*at fdt? # Real Time Clock
sximmc*at fdt? # SD/MMC card controller
sdmmc* at sximmc? # SD/MMC bus
+sxitwi*at fdt? # I2C controller
+iic* at sxitwi? # I2C bus
dwxe* at fdt?
# PCI
diff --git a/sys/dev/fdt/sxiccmu_clocks.h b/sys/dev/fdt/sxiccmu_clocks.h
index 8b25ac42bd4..24c063df310 100644
--- a/sys/dev/fdt/sxiccmu_clocks.h
+++ b/sys/dev/fdt/sxiccmu_clocks.h
@@ -28,6 +28,10 @@
#define A64_CLK_BUS_PIO58
+#defineA64_CLK_BUS_I2C063
+#defineA64_CLK_BUS_I2C164
+#defineA64_CLK_BUS_I2C265
+
#define A64_CLK_BUS_UART0 67
#define A64_CLK_BUS_UART1 68
#define A64_CLK_BUS_UART2 69
@@ -56,6 +60,9 @@ struct sxiccmu_ccu_bit sun50i_a64_gates[] = {
[A64_CLK_BUS_OHCI0] = { 0x0060, 28 },
[A64_CLK_BUS_OHCI1] = { 0x0060, 29 },
[A64_CLK_BUS_PIO] = { 0x0068, 5 },
+ [A64_CLK_BUS_I2C0] = { 0x006c, 0, A64_CLK_APB2 },
+ [A64_CLK_BUS_I2C1] = { 0x006c, 1, A64_CLK_APB2 },
+ [A64_CLK_BUS_I2C2] = { 0x006c, 2, A64_CLK_APB2 },
[A64_CLK_BUS_UART0] = { 0x006c, 16, A64_CLK_APB2 },
[A64_CLK_BUS_UART1] = { 0x006c, 17, A64_CLK_APB2 },
[A64_CLK_BUS_UART2] = { 0x006c, 18, A64_CLK_APB2 },
@@ -95,6 +102,10 @@ struct sxiccmu_ccu_bit sun50i_a64_gates[] = {
#define H3_CLK_BUS_PIO 54
+#define H3_CLK_BUS_I2C059
+#define H3_CLK_BUS_I2C160
+#define H3_CLK_BUS_I2C261
+
#define H3_CLK_BUS_UART0 62
#define H3_CLK_BUS_UART1 63
#define H3_CLK_BUS_UART2 64
@@ -128,6 +139,9 @@ struct sxiccmu_ccu_bit sun8i_h3_gates[] = {
[H3_CLK_BUS_OHCI2] = { 0x0060, 30 },
[H3_CLK_BUS_OHCI3] = { 0x0060, 31 },
[H3_CLK_BUS_PIO] = { 0x0068, 5 },
+ [H3_CLK_BUS_I2C0] = { 0x006c, 0, H3_CLK_APB2 },
+ [H3_CLK_BUS_I2C1] = { 0x006c, 1, H3_CLK_APB2 },
+ [H3_CLK_BUS_I2C2] = { 0x006c, 2, H3_CLK_APB2 },
[H3_CLK_BUS_UART0] = { 0x006c, 16, H3_CLK_APB2 },
[H3_CLK_BUS_UART1] = { 0x006c, 17, H3_CLK_APB2 },
[H3_CLK_BUS_UART2] = { 0x006c, 18, H3_CLK_APB2 },
@@ -160,6 +174,10 @@ struct sxiccmu_ccu_bit sun8i_h3_gates[] = {
#define A64_RST_BUS_OHCI0 21
#define A64_RST_BUS_OHCI1 22
+#defineA64_RST_BUS_I2C042
+#defineA64_RST_BUS_I2C143
+#defineA64_RST_BUS_I2C244
+
struct sxiccmu_ccu_bit sun50i_a64_resets[] = {
[A64_RST_USB_PHY0] = { 0x00cc, 0 },
[A64_RST_USB_PHY1] = { 0x00cc, 1 },
@@ -171,6 +189,9 @@ struct sxiccmu_ccu_bit sun50i_a64_resets[] = {
[A64_RST_BUS_EHCI1] = { 0x02c0, 25 },
[A64_RST_BUS_OHCI0] = { 0x02c0, 28 },
[A64_RST_BUS_OHCI1] = { 0x02c0, 29 },
+ [A64_RST_BUS_I2C0] = { 0x02d8, 0 },
+ [A64_RST_BUS_I2C1] = { 0x02d8, 1 },
+ [A64_RST_BUS_I2C2] = { 0x02d8, 2 },
};
#define H3_RST_USB_PHY00
@@ -195,6 +216,10 @@ struct sxiccmu_ccu_bit sun50i_a64_resets[] = {
#define H3_RST_BUS_EPHY39
+#defineH3_RST_BUS_I2C0 46
+#defineH3_RST_BUS_I2C1 47
+#defineH3_RST_BUS_I2C2 48
+
struct sxiccmu_ccu_bit sun8i_h3_resets[] = {
[H3_RST_USB_PHY0] = { 0x00cc, 0 },
[H3_RST_USB_PHY1] = { 0x00cc, 1 },
@@ -213,4 +238,7 @@ struct sxiccmu_ccu_bit sun8i_h3_resets[] = {
[H3_RST_BUS_OHCI2] = { 0x02c0, 30 },
[H3_RST_BUS_OHCI3] = { 0x02c0, 31 },
[H3_RST_BUS_EPHY] = { 0x02c8, 2 },
+ [H3_RST_BUS_I2C0] = { 0x02d8, 0 },
+ [H3_RST_BUS_I2C1] = { 0x02d8, 1 },
+ [H3_RST_BUS_I2C2] = { 0x02d8, 2 },
};
diff --git a/sys/dev/fdt/sxitwi.c b/sys/dev/fdt/sxitwi.c
index f53f2bfd594..9e855728173 100644
--- a/sys/dev/fdt/sxitwi.c
+++ b/sys/dev/fdt/sxitwi.c
@@ -144,6 +144,7 @@ struct sxitwi_softc {
bus_space_h
Re: "max" field in "netstat -m" is ambiguous
> Ian Darwin suggested using fmt_scaled for an output like this: > 6.1M/7.1M/512Mbytes allocated to network (current/peak/max) > > netstat is already linked against libutil. No thank you, it doesn't read well at a glance. Humans can read just fine.
Re: "max" field in "netstat -m" is ambiguous
On Sat, Oct 28, 2017 at 11:06 +0200, Mike Belopuhov wrote:
> On Thu, Oct 26, 2017 at 08:58 +0200, Claudio Jeker wrote:
> > On Wed, Oct 25, 2017 at 11:46:05PM +0200, Mike Belopuhov wrote:
> > > On Wed, Oct 25, 2017 at 21:56 +0200, Claudio Jeker wrote:
> > > > Would be great if netstat could show the current and peak memory usage.
> > > >
> > >
> > > Current is 5876. Maximum is 524288. Do you want to display them in
> > > the x/y/z format?
> > >
> > > 5876//524288 Kbytes allocated to network, 20% in use
> > > (current/peak/max)
> > >
> > > Something like this? Any other ideas?
> >
> > I think that would be an improvement. I normally look for peak values. The
> > current is normally not interesting when tuning systems.
> > Maybe we can even drop the use percentage since it more confusing than
> > anything.
> >
>
> How about this then?
>
> saru:usr.bin/netstat% ./obj/netstat -m
> 532 mbufs in use:
> 379 mbufs allocated to data
> 12 mbufs allocated to packet headers
> 141 mbufs allocated to socket names and addresses
> 18/208 mbuf 2048 byte clusters in use (current/peak)
> 0/45 mbuf 2112 byte clusters in use (current/peak)
> 256/320 mbuf 4096 byte clusters in use (current/peak)
> 0/48 mbuf 8192 byte clusters in use (current/peak)
> 0/42 mbuf 9216 byte clusters in use (current/peak)
> 0/50 mbuf 12288 byte clusters in use (current/peak)
> 0/48 mbuf 16384 byte clusters in use (current/peak)
> 0/48 mbuf 65536 byte clusters in use (current/peak)
> 5952/7236/524288 Kbytes allocated to network (current/peak/max)
> 0 requests for memory denied
> 0 requests for memory delayed
> 0 calls to protocol drain routines
>
> OK?
>
Ian Darwin suggested using fmt_scaled for an output like this:
6.1M/7.1M/512Mbytes allocated to network (current/peak/max)
netstat is already linked against libutil.
Any objections?
diff --git usr.bin/netstat/mbuf.c usr.bin/netstat/mbuf.c
index f7970a57c32..27412f9e217 100644
--- usr.bin/netstat/mbuf.c
+++ usr.bin/netstat/mbuf.c
@@ -42,10 +42,11 @@
#include
#include
#include
#include
#include
+#include
#include "netstat.h"
#defineYES 1
typedef int bool;
@@ -85,13 +86,13 @@ bool seen[256]; /* "have we seen this
type yet?" */
* Print mbuf statistics.
*/
void
mbpr(void)
{
- unsigned long totmem, totused, totmbufs;
- int totpct;
- int i, mib[4], npools;
+ unsigned long totmem, totpeak, totmbufs;
+ int i, maxclusters, mib[4], npools;
+ char fmt[FMT_SCALED_STRSIZE];
struct kinfo_pool pool;
struct mbtypes *mp;
size_t size;
if (nmbtypes != 256) {
@@ -99,10 +100,20 @@ mbpr(void)
"%s: unexpected change to mbstat; check source\n",
__progname);
return;
}
+ mib[0] = CTL_KERN;
+ mib[1] = KERN_MAXCLUSTERS;
+ size = sizeof(maxclusters);
+
+ if (sysctl(mib, 2, &maxclusters, &size, NULL, 0) < 0) {
+ printf("Can't retrieve value of maxclusters from the "
+ "kernel: %s\n", strerror(errno));
+ return;
+ }
+
mib[0] = CTL_KERN;
mib[1] = KERN_MBSTAT;
size = sizeof(mbstat);
if (sysctl(mib, 2, &mbstat, &size, NULL, 0) < 0) {
@@ -174,26 +185,34 @@ mbpr(void)
printf("\t%u mbuf%s allocated to \n",
mbstat.m_mtypes[i],
plural(mbstat.m_mtypes[i]), i);
}
totmem = (mbpool.pr_npages * mbpool.pr_pgsize);
- totused = mbpool.pr_nout * mbpool.pr_size;
+ totpeak = mbpool.pr_hiwat * mbpool.pr_pgsize;
for (i = 0; i < mclp; i++) {
- printf("%u/%lu/%lu mbuf %d byte clusters in use"
- " (current/peak/max)\n",
+ printf("%u/%lu mbuf %d byte clusters in use"
+ " (current/peak)\n",
mclpools[i].pr_nout,
(unsigned long)
(mclpools[i].pr_hiwat * mclpools[i].pr_itemsperpage),
- (unsigned long)
- (mclpools[i].pr_maxpages * mclpools[i].pr_itemsperpage),
mclpools[i].pr_size);
totmem += (mclpools[i].pr_npages * mclpools[i].pr_pgsize);
- totused += mclpools[i].pr_nout * mclpools[i].pr_size;
+ totpeak += mclpools[i].pr_hiwat * mclpools[i].pr_pgsize;
}
- totpct = (totmem == 0) ? 0 : (totused/(totmem / 100));
- printf("%lu Kbytes allocated to network (%d%% in use)\n",
- totmem / 1024, totpct);
+ if (fmt_scaled(totmem, fmt) == 0)
+ printf("%s/", fmt);
+ else
+ printf("%luK/", totmem / 1024);
+ if (fmt_scaled(totpeak, fmt) == 0)
+ printf("%s/", fmt);
+ else
+ printf("%luK/", totpeak / 1024);
+ if (fmt_scaled(maxclusters * MCLBYTES, f
Re: [patch] Remove superfluous seek_filesize() from less
Hi,
On Sat, Sep 16, 2017 at 10:53:47PM +0200, Jesper Wallin wrote:
> I was reading through the code for less/filename.c and noticed that the
> calling for seek_filesize() in filesize() is superfluous? A wild guess
> is that it's remains from when BAD_LSEEK was removed?
There is more that is useless.
* The fstat(2) function is required by POSIX, and we couldn't
care less about non-POSIX systems, so delete the pointless
comment.
* The fstat(2) syscall only fails for EBADF and EIO, and POSIX
would allow failing for EOVERFLOW, too. In all these cases,
trying lseek(2) is useless, so simply return error.
OK?
Ingo
Index: filename.c
===
RCS file: /cvs/src/usr.bin/less/filename.c,v
retrieving revision 1.25
diff -u -p -r1.25 filename.c
--- filename.c 17 Sep 2016 15:06:41 - 1.25
+++ filename.c 28 Oct 2017 14:33:07 -
@@ -363,20 +363,6 @@ bin_file(int f)
}
/*
- * Try to determine the size of a file by seeking to the end.
- */
-static off_t
-seek_filesize(int f)
-{
- off_t spos;
-
- spos = lseek(f, (off_t)0, SEEK_END);
- if (spos == (off_t)-1)
- return (-1);
- return (spos);
-}
-
-/*
* Read a string from a file.
* Return a pointer to the string in memory.
*/
@@ -742,7 +728,6 @@ bad_file(char *filename)
/*
* Return the size of a file, as cheaply as possible.
- * In Unix, we can stat the file.
*/
off_t
filesize(int f)
@@ -751,7 +736,7 @@ filesize(int f)
if (fstat(f, &statbuf) >= 0)
return (statbuf.st_size);
- return (seek_filesize(f));
+ return (-1);
}
/*
ctfconv(1) & forward declaration
Diff below makes ctfconv(1) aware of forward declarations and merge them
with the corresponding type as soon as they are known.
This reduces the number of type entries in a kernel CTF section by ~4K.
After applying this diff, grepping for "(0 bytes)" in the ctfdump(1)
output points to code that generally needs a cleanup.
ok?
Index: itype.h
===
RCS file: /cvs/src/usr.bin/ctfconv/itype.h,v
retrieving revision 1.3
diff -u -p -r1.3 itype.h
--- itype.h 11 Aug 2017 16:28:30 - 1.3
+++ itype.h 28 Oct 2017 13:53:50 -
@@ -58,7 +58,7 @@ struct itype {
#defineITF_UNRES_MEMB 0x02 /* members need to be
resolved */
#defineITF_FUNC 0x04 /* is a function */
#defineITF_OBJ 0x08 /* is an object */
-#defineITF_VARARGS 0x10 /* takes varargs */
+#defineITF_FORWARD 0x10 /* is a forward declaration
*/
#defineITF_INSERTED 0x20 /* already found/inserted */
#defineITF_USED 0x40 /* referenced in the
current CU */
#defineITF_ANON 0x80 /* type without name */
Index: parse.c
===
RCS file: /cvs/src/usr.bin/ctfconv/parse.c,v
retrieving revision 1.9
diff -u -p -r1.9 parse.c
--- parse.c 28 Oct 2017 13:23:26 - 1.9
+++ parse.c 28 Oct 2017 13:53:50 -
@@ -105,6 +105,7 @@ const char *enc2name(unsigned short);
struct itype *it_new(uint64_t, size_t, const char *, uint32_t, uint16_t,
uint64_t, uint16_t, unsigned int);
+voidit_merge(struct itype *, struct itype *);
voidit_reference(struct itype *);
voidit_free(struct itype *);
int it_cmp(struct itype *, struct itype *);
@@ -247,6 +248,30 @@ it_dup(struct itype *it)
return copit;
}
+/*
+ * Merge the content of ``it'', the full type declaration into the
+ * forwarding representation ``fwd''.
+ */
+void
+it_merge(struct itype *fwd, struct itype *it)
+{
+ assert(fwd->it_flags & ITF_FORWARD);
+ assert(fwd->it_type == it->it_type);
+ assert(TAILQ_EMPTY(&fwd->it_members));
+ assert(SIMPLEQ_EMPTY(&it->it_refs));
+
+ fwd->it_off = it->it_off;
+ fwd->it_ref = it->it_ref;
+ fwd->it_refp = it->it_refp;
+ fwd->it_size = it->it_size;
+ fwd->it_nelems = it->it_nelems;
+ fwd->it_enc = it->it_enc;
+ fwd->it_flags = it->it_flags;
+
+ TAILQ_CONCAT(&fwd->it_members, &it->it_members, im_next);
+ assert(TAILQ_EMPTY(&it->it_members));
+}
+
const char *
it_name(struct itype *it)
{
@@ -299,12 +324,6 @@ it_cmp(struct itype *a, struct itype *b)
if ((diff = (a->it_type - b->it_type)) != 0)
return diff;
- if ((diff = (a->it_size - b->it_size)) != 0)
- return diff;
-
- if ((diff = (a->it_nelems - b->it_nelems)) != 0)
- return diff;
-
/* Match by name */
if (!(a->it_flags & ITF_ANON) && !(b->it_flags & ITF_ANON))
return strcmp(it_name(a), it_name(b));
@@ -539,6 +558,11 @@ cu_merge(struct dwcu *dcu, struct itype_
}
}
+ /* If we first got a forward reference, complete it. */
+ if ((prev->it_flags & ITF_FORWARD) &&
+ (old->it_flags & ITF_FORWARD) == 0)
+ it_merge(prev, old);
+
old->it_flags &= ~ITF_USED;
} else if (it->it_flags & ITF_USED) {
RB_INSERT(itype_tree, &itypet[it->it_type], it);
@@ -971,10 +995,15 @@ parse_struct(struct dwdie *die, size_t p
struct itype *it = NULL;
struct dwaval *dav;
const char *name = NULL;
+ unsigned int flags = 0;
size_t size = 0;
+ int forward = 0;
SIMPLEQ_FOREACH(dav, &die->die_avals, dav_next) {
switch (dav->dav_dat->dat_attr) {
+ case DW_AT_declaration:
+ forward = dav2val(dav, psz);
+ break;
case DW_AT_byte_size:
size = dav2val(dav, psz);
assert(size < UINT_MAX);
@@ -988,8 +1017,10 @@ parse_struct(struct dwdie *die, size_t p
}
}
- it = it_new(++tidx, die->die_offset, name, size, 0, 0, type, 0);
+ if (forward)
+ flags = ITF_FORWARD;
+ it = it_new(++tidx, die->die_offset, name, size, 0, 0, type, flags);
subparse_member(die, psz, it, off);
return it;
@@ -1094,7 +1125,7 @@ subparse_arguments(struct dwdie *die, si
uint64_t tag = die->die_dab->dab_tag;
if (tag == DW_TAG_unspecified_parameters) {
- it->
Re: ->
On Sat, Oct 28 2017, Martin Pieuchot wrote: > Another diff to use in userland. Kernel and bootloader are still > untouched. > > ok? ok jca@ -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: "max" field in "netstat -m" is ambiguous
On Sat, Oct 28, 2017 at 11:06:16AM +0200, Mike Belopuhov wrote:
> On Thu, Oct 26, 2017 at 08:58 +0200, Claudio Jeker wrote:
> > On Wed, Oct 25, 2017 at 11:46:05PM +0200, Mike Belopuhov wrote:
> > > On Wed, Oct 25, 2017 at 21:56 +0200, Claudio Jeker wrote:
> > > > Would be great if netstat could show the current and peak memory usage.
> > > >
> > >
> > > Current is 5876. Maximum is 524288. Do you want to display them in
> > > the x/y/z format?
> > >
> > > 5876//524288 Kbytes allocated to network, 20% in use
> > > (current/peak/max)
> > >
> > > Something like this? Any other ideas?
> >
> > I think that would be an improvement. I normally look for peak values. The
> > current is normally not interesting when tuning systems.
> > Maybe we can even drop the use percentage since it more confusing than
> > anything.
> >
>
> How about this then?
>
> saru:usr.bin/netstat% ./obj/netstat -m
> 532 mbufs in use:
> 379 mbufs allocated to data
> 12 mbufs allocated to packet headers
> 141 mbufs allocated to socket names and addresses
> 18/208 mbuf 2048 byte clusters in use (current/peak)
> 0/45 mbuf 2112 byte clusters in use (current/peak)
> 256/320 mbuf 4096 byte clusters in use (current/peak)
> 0/48 mbuf 8192 byte clusters in use (current/peak)
> 0/42 mbuf 9216 byte clusters in use (current/peak)
> 0/50 mbuf 12288 byte clusters in use (current/peak)
> 0/48 mbuf 16384 byte clusters in use (current/peak)
> 0/48 mbuf 65536 byte clusters in use (current/peak)
> 5952/7236/524288 Kbytes allocated to network (current/peak/max)
> 0 requests for memory denied
> 0 requests for memory delayed
> 0 calls to protocol drain routines
>
> OK?
I like it. Ok claudio@
> diff --git usr.bin/netstat/mbuf.c usr.bin/netstat/mbuf.c
> index f7970a57c32..79c2c16a6c3 100644
> --- usr.bin/netstat/mbuf.c
> +++ usr.bin/netstat/mbuf.c
> @@ -85,13 +85,12 @@ bool seen[256]; /* "have we seen this
> type yet?" */
> * Print mbuf statistics.
> */
> void
> mbpr(void)
> {
> - unsigned long totmem, totused, totmbufs;
> - int totpct;
> - int i, mib[4], npools;
> + unsigned long totmem, totpeak, totmbufs;
> + int i, maxclusters, mib[4], npools;
> struct kinfo_pool pool;
> struct mbtypes *mp;
> size_t size;
>
> if (nmbtypes != 256) {
> @@ -99,10 +98,20 @@ mbpr(void)
> "%s: unexpected change to mbstat; check source\n",
> __progname);
> return;
> }
>
> + mib[0] = CTL_KERN;
> + mib[1] = KERN_MAXCLUSTERS;
> + size = sizeof(maxclusters);
> +
> + if (sysctl(mib, 2, &maxclusters, &size, NULL, 0) < 0) {
> + printf("Can't retrieve value of maxclusters from the "
> + "kernel: %s\n", strerror(errno));
> + return;
> + }
> +
> mib[0] = CTL_KERN;
> mib[1] = KERN_MBSTAT;
> size = sizeof(mbstat);
>
> if (sysctl(mib, 2, &mbstat, &size, NULL, 0) < 0) {
> @@ -174,26 +183,24 @@ mbpr(void)
> printf("\t%u mbuf%s allocated to \n",
> mbstat.m_mtypes[i],
> plural(mbstat.m_mtypes[i]), i);
> }
> totmem = (mbpool.pr_npages * mbpool.pr_pgsize);
> - totused = mbpool.pr_nout * mbpool.pr_size;
> + totpeak = mbpool.pr_hiwat * mbpool.pr_pgsize;
> for (i = 0; i < mclp; i++) {
> - printf("%u/%lu/%lu mbuf %d byte clusters in use"
> - " (current/peak/max)\n",
> + printf("%u/%lu mbuf %d byte clusters in use"
> + " (current/peak)\n",
> mclpools[i].pr_nout,
> (unsigned long)
> (mclpools[i].pr_hiwat * mclpools[i].pr_itemsperpage),
> - (unsigned long)
> - (mclpools[i].pr_maxpages * mclpools[i].pr_itemsperpage),
> mclpools[i].pr_size);
> totmem += (mclpools[i].pr_npages * mclpools[i].pr_pgsize);
> - totused += mclpools[i].pr_nout * mclpools[i].pr_size;
> + totpeak += mclpools[i].pr_hiwat * mclpools[i].pr_pgsize;
> }
>
> - totpct = (totmem == 0) ? 0 : (totused/(totmem / 100));
> - printf("%lu Kbytes allocated to network (%d%% in use)\n",
> - totmem / 1024, totpct);
> + printf("%lu/%lu/%lu Kbytes allocated to network "
> + "(current/peak/max)\n", totmem / 1024, totpeak / 1024,
> + (unsigned long)(maxclusters * MCLBYTES) / 1024);
> printf("%lu requests for memory denied\n", mbstat.m_drops);
> printf("%lu requests for memory delayed\n", mbstat.m_wait);
> printf("%lu calls to protocol drain routines\n", mbstat.m_drain);
> }
>
--
:wq Claudio
sysctl.3 w/o
Including doesn't require , is enough. ok? Index: gen/sysctl.3 === RCS file: /cvs/src/lib/libc/gen/sysctl.3,v retrieving revision 1.284 diff -u -p -r1.284 sysctl.3 --- gen/sysctl.316 Oct 2017 15:09:43 - 1.284 +++ gen/sysctl.317 Oct 2017 07:41:01 - @@ -34,7 +34,7 @@ .Nm sysctl .Nd get or set system information .Sh SYNOPSIS -.In sys/param.h +.In sys/types.h .In sys/sysctl.h .Ft int .Fn sysctl "const int *name" "u_int namelen" "void *oldp" "size_t *oldlenp" "void *newp" "size_t newlen"
Re: "max" field in "netstat -m" is ambiguous
On Thu, Oct 26, 2017 at 08:58 +0200, Claudio Jeker wrote:
> On Wed, Oct 25, 2017 at 11:46:05PM +0200, Mike Belopuhov wrote:
> > On Wed, Oct 25, 2017 at 21:56 +0200, Claudio Jeker wrote:
> > > Would be great if netstat could show the current and peak memory usage.
> > >
> >
> > Current is 5876. Maximum is 524288. Do you want to display them in
> > the x/y/z format?
> >
> > 5876//524288 Kbytes allocated to network, 20% in use
> > (current/peak/max)
> >
> > Something like this? Any other ideas?
>
> I think that would be an improvement. I normally look for peak values. The
> current is normally not interesting when tuning systems.
> Maybe we can even drop the use percentage since it more confusing than
> anything.
>
How about this then?
saru:usr.bin/netstat% ./obj/netstat -m
532 mbufs in use:
379 mbufs allocated to data
12 mbufs allocated to packet headers
141 mbufs allocated to socket names and addresses
18/208 mbuf 2048 byte clusters in use (current/peak)
0/45 mbuf 2112 byte clusters in use (current/peak)
256/320 mbuf 4096 byte clusters in use (current/peak)
0/48 mbuf 8192 byte clusters in use (current/peak)
0/42 mbuf 9216 byte clusters in use (current/peak)
0/50 mbuf 12288 byte clusters in use (current/peak)
0/48 mbuf 16384 byte clusters in use (current/peak)
0/48 mbuf 65536 byte clusters in use (current/peak)
5952/7236/524288 Kbytes allocated to network (current/peak/max)
0 requests for memory denied
0 requests for memory delayed
0 calls to protocol drain routines
OK?
diff --git usr.bin/netstat/mbuf.c usr.bin/netstat/mbuf.c
index f7970a57c32..79c2c16a6c3 100644
--- usr.bin/netstat/mbuf.c
+++ usr.bin/netstat/mbuf.c
@@ -85,13 +85,12 @@ bool seen[256]; /* "have we seen this
type yet?" */
* Print mbuf statistics.
*/
void
mbpr(void)
{
- unsigned long totmem, totused, totmbufs;
- int totpct;
- int i, mib[4], npools;
+ unsigned long totmem, totpeak, totmbufs;
+ int i, maxclusters, mib[4], npools;
struct kinfo_pool pool;
struct mbtypes *mp;
size_t size;
if (nmbtypes != 256) {
@@ -99,10 +98,20 @@ mbpr(void)
"%s: unexpected change to mbstat; check source\n",
__progname);
return;
}
+ mib[0] = CTL_KERN;
+ mib[1] = KERN_MAXCLUSTERS;
+ size = sizeof(maxclusters);
+
+ if (sysctl(mib, 2, &maxclusters, &size, NULL, 0) < 0) {
+ printf("Can't retrieve value of maxclusters from the "
+ "kernel: %s\n", strerror(errno));
+ return;
+ }
+
mib[0] = CTL_KERN;
mib[1] = KERN_MBSTAT;
size = sizeof(mbstat);
if (sysctl(mib, 2, &mbstat, &size, NULL, 0) < 0) {
@@ -174,26 +183,24 @@ mbpr(void)
printf("\t%u mbuf%s allocated to \n",
mbstat.m_mtypes[i],
plural(mbstat.m_mtypes[i]), i);
}
totmem = (mbpool.pr_npages * mbpool.pr_pgsize);
- totused = mbpool.pr_nout * mbpool.pr_size;
+ totpeak = mbpool.pr_hiwat * mbpool.pr_pgsize;
for (i = 0; i < mclp; i++) {
- printf("%u/%lu/%lu mbuf %d byte clusters in use"
- " (current/peak/max)\n",
+ printf("%u/%lu mbuf %d byte clusters in use"
+ " (current/peak)\n",
mclpools[i].pr_nout,
(unsigned long)
(mclpools[i].pr_hiwat * mclpools[i].pr_itemsperpage),
- (unsigned long)
- (mclpools[i].pr_maxpages * mclpools[i].pr_itemsperpage),
mclpools[i].pr_size);
totmem += (mclpools[i].pr_npages * mclpools[i].pr_pgsize);
- totused += mclpools[i].pr_nout * mclpools[i].pr_size;
+ totpeak += mclpools[i].pr_hiwat * mclpools[i].pr_pgsize;
}
- totpct = (totmem == 0) ? 0 : (totused/(totmem / 100));
- printf("%lu Kbytes allocated to network (%d%% in use)\n",
- totmem / 1024, totpct);
+ printf("%lu/%lu/%lu Kbytes allocated to network "
+ "(current/peak/max)\n", totmem / 1024, totpeak / 1024,
+ (unsigned long)(maxclusters * MCLBYTES) / 1024);
printf("%lu requests for memory denied\n", mbstat.m_drops);
printf("%lu requests for memory delayed\n", mbstat.m_wait);
printf("%lu calls to protocol drain routines\n", mbstat.m_drain);
}
->
Another diff to use in userland. Kernel and bootloader are still untouched. ok? Index: distrib/common/elfrd_size.c === RCS file: /cvs/src/distrib/common/elfrd_size.c,v retrieving revision 1.6 diff -u -p -r1.6 elfrd_size.c --- distrib/common/elfrd_size.c 31 Jul 2017 01:18:09 - 1.6 +++ distrib/common/elfrd_size.c 27 Oct 2017 17:19:26 - @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -12,8 +13,6 @@ #include #include - -#include #include "elfrdsetroot.h" Index: distrib/common/elfrdsetroot.c === RCS file: /cvs/src/distrib/common/elfrdsetroot.c,v retrieving revision 1.23 diff -u -p -r1.23 elfrdsetroot.c --- distrib/common/elfrdsetroot.c 31 Jul 2017 01:18:09 - 1.23 +++ distrib/common/elfrdsetroot.c 27 Oct 2017 17:19:13 - @@ -39,12 +39,12 @@ #include #include +#include #include #include #include #include -#include #include "elfrdsetroot.h" struct elfhdr head; Index: lib/librthread/rthread.c === RCS file: /cvs/src/lib/librthread/rthread.c,v retrieving revision 1.96 diff -u -p -r1.96 rthread.c --- lib/librthread/rthread.c5 Sep 2017 02:40:54 - 1.96 +++ lib/librthread/rthread.c27 Oct 2017 17:16:53 - @@ -22,7 +22,7 @@ #include #ifndef NO_PIC -#include +#include #pragma weak _DYNAMIC #endif Index: lib/librthread/rthread_fork.c === RCS file: /cvs/src/lib/librthread/rthread_fork.c,v retrieving revision 1.22 diff -u -p -r1.22 rthread_fork.c --- lib/librthread/rthread_fork.c 5 Sep 2017 02:40:54 - 1.22 +++ lib/librthread/rthread_fork.c 27 Oct 2017 17:17:09 - @@ -32,7 +32,7 @@ #ifndef NO_PIC #include -#include +#include #pragma weak _DYNAMIC #endif Index: libexec/ld.so/tib.c === RCS file: /cvs/src/libexec/ld.so/tib.c,v retrieving revision 1.5 diff -u -p -r1.5 tib.c --- libexec/ld.so/tib.c 27 Aug 2017 21:59:49 - 1.5 +++ libexec/ld.so/tib.c 27 Oct 2017 17:17:27 - @@ -22,8 +22,8 @@ #define _DYN_LOADER #include -#include +#include #include #include "archdep.h" Index: usr.bin/gprof/elf.c === RCS file: /cvs/src/usr.bin/gprof/elf.c,v retrieving revision 1.5 diff -u -p -r1.5 elf.c --- usr.bin/gprof/elf.c 20 Aug 2015 22:32:41 - 1.5 +++ usr.bin/gprof/elf.c 27 Oct 2017 17:14:33 - @@ -30,8 +30,8 @@ #include #include #include -#include +#include #include #include #include Index: usr.sbin/config/exec_elf.c === RCS file: /cvs/src/usr.sbin/config/exec_elf.c,v retrieving revision 1.15 diff -u -p -r1.15 exec_elf.c --- usr.sbin/config/exec_elf.c 3 Jun 2017 23:31:37 - 1.15 +++ usr.sbin/config/exec_elf.c 27 Oct 2017 17:14:53 - @@ -26,8 +26,8 @@ #include #include -#include +#include #include #include #include Index: usr.sbin/crunchgen/crunchide.c === RCS file: /cvs/src/usr.sbin/crunchgen/crunchide.c,v retrieving revision 1.11 diff -u -p -r1.11 crunchide.c --- usr.sbin/crunchgen/crunchide.c 18 Oct 2015 17:32:22 - 1.11 +++ usr.sbin/crunchgen/crunchide.c 27 Oct 2017 17:15:31 - @@ -53,10 +53,10 @@ */ #include -#include #include #include +#include #include #include #include Index: usr.sbin/crunchgen/elf_hide.c === RCS file: /cvs/src/usr.sbin/crunchgen/elf_hide.c,v retrieving revision 1.10 diff -u -p -r1.10 elf_hide.c --- usr.sbin/crunchgen/elf_hide.c 20 Aug 2015 22:39:29 - 1.10 +++ usr.sbin/crunchgen/elf_hide.c 27 Oct 2017 17:15:37 - @@ -29,8 +29,8 @@ #include #include #include -#include +#include #include #include #include Index: usr.sbin/mkuboot/copy_elf.c === RCS file: /cvs/src/usr.sbin/mkuboot/copy_elf.c,v retrieving revision 1.5 diff -u -p -r1.5 copy_elf.c --- usr.sbin/mkuboot/copy_elf.c 1 Jan 2014 09:24:54 - 1.5 +++ usr.sbin/mkuboot/copy_elf.c 27 Oct 2017 17:15:44 - @@ -22,7 +22,7 @@ #include #include -#include +#include #if defined(ELFSIZE) && (ELFSIZE == 32) #define elfoff2h(x) letoh32(x) Index: usr.sbin/mkuboot/mkuboot.c === RCS file: /cvs/src/usr.sbin/mkuboot/mkuboot.c,v retrieving revision 1.7 diff -u -p -r1.7 mkuboot.c --- usr.sbin/mkuboot/mkuboot.c 20 Dec 2016 11:27:11 - 1.7 +++ usr.sbin/mkuboot/mkuboot.c 27 Oct 2017 17:15:53 - @@ -18,6 +18,8 @@ #include
Re: fine tuning PF_LOCK in pfioctl()
On 28/10/17(Sat) 00:32, Alexandr Nedvedicky wrote:
> Hello,
>
> patch below fine tunes PF_LOCK in pfioctl() function. Currently pfioctl()
> function grabs PF_LOCK() for all operations at line 1006, right before 'the
> big
> switch' is entered. The PF_LOCK() gets released once we are done with 'the big
> switch' at line 2477:
>
>905 int
>906 pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
>907 {
>908 int error = 0;
>909
>910 /* XXX keep in sync with switch() below */
>911 if (securelevel > 1)
>912 switch (cmd) {
>
> 1004
> 1005 NET_LOCK();
> 1006 PF_LOCK();
> 1007 switch (cmd) {
> 1008
> 1009 case DIOCSTART:
>
> 2472 default:
> 2473 error = ENODEV;
> 2474 break;
> 2475 }
> 2476 fail:
> 2477 PF_UNLOCK();
> 2478 NET_UNLOCK();
> 2479 return (error);
>
> patch below move responsibility for PF_LOCK manipulation to particular ioctl
> commands. The change is investment for future. It will allow us to gradually
> move individual PF subsystems out of PF_LOCK scope.
I'm always afraid when an ioctl(2) logic contains multiple return or break
statements and doesn't fit in a couple of lines. When this happen I'd
suggest to move the code to a function which can assert that the PF_LOCK()
is held.
That said I agree that moving the lock down is some progress. So ok
with me.
>
> OK?
>
> thanks and
> regards
> sasha
>
> 8<---8<---8<--8<
> diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
> index de40e934112..2e954111d03 100644
> --- a/sys/net/pf_ioctl.c
> +++ b/sys/net/pf_ioctl.c
> @@ -1003,10 +1003,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int
> flags, struct proc *p)
> }
>
> NET_LOCK();
> - PF_LOCK();
> switch (cmd) {
>
> case DIOCSTART:
> + PF_LOCK();
> if (pf_status.running)
> error = EEXIST;
> else {
> @@ -1020,9 +1020,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int
> flags, struct proc *p)
> pf_create_queues();
> DPFPRINTF(LOG_NOTICE, "pf: started");
> }
> + PF_UNLOCK();
> break;
>
> case DIOCSTOP:
> + PF_LOCK();
> if (!pf_status.running)
> error = ENOENT;
> else {
> @@ -1031,6 +1033,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
> struct proc *p)
> pf_remove_queues();
> DPFPRINTF(LOG_NOTICE, "pf: stopped");
> }
> + PF_UNLOCK();
> break;
>
> case DIOCGETQUEUES: {
> @@ -1038,6 +1041,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
> struct proc *p)
> struct pf_queuespec *qs;
> u_int32_tnr = 0;
>
> + PF_LOCK();
> pq->ticket = pf_main_ruleset.rules.active.ticket;
>
> /* save state to not run over them all each time? */
> @@ -1047,6 +1051,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
> struct proc *p)
> nr++;
> }
> pq->nr = nr;
> + PF_UNLOCK();
> break;
> }
>
> @@ -1055,8 +1060,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int
> flags, struct proc *p)
> struct pf_queuespec *qs;
> u_int32_tnr = 0;
>
> + PF_LOCK();
> if (pq->ticket != pf_main_ruleset.rules.active.ticket) {
> error = EBUSY;
> + PF_UNLOCK();
> break;
> }
>
> @@ -1066,9 +1073,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int
> flags, struct proc *p)
> qs = TAILQ_NEXT(qs, entries);
> if (qs == NULL) {
> error = EBUSY;
> + PF_UNLOCK();
> break;
> }
> bcopy(qs, &pq->queue, sizeof(pq->queue));
> + PF_UNLOCK();
> break;
> }
>
> @@ -1078,8 +1087,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int
> flags, struct proc *p)
> u_int32_tnr;
> int nbytes;
>
> + PF_LOCK();
> if (pq->ticket != pf_main_ruleset.rules.active.ticket) {
> error = EBUSY;
> + PF_UNLOCK();
> break;
> }
> nbytes = pq->nbytes;
> @@ -1091,6 +1102,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
> struct proc *p)
> qs = TAILQ_NEXT(qs, entries);
>
Re: libfuse: improved command line parsing
On 17/10/17(Tue) 14:26, Helg Bredow wrote:
> [...]
> I've split the patch. This one improves argument and option parsing so that
> almost all sshfs arguments and options will now parse. It won't recognise
> -ocache=no since fuse_opt_match() is incorrect (fuse will treat it the same
> as -ocache=yes). I'll submit a patch for that some other time.
Nice.
> Running check_sym tells me:
>
> No dynamic export changes
> External reference changes:
> added:
> atoi
> strchr
> strsep
> strstr
> strtoul
>
> I take that to mean that there is no need to bump the major or minor version
> for this patch. Is that correct?
>
That's correct.
Regarding fuse options, I'd suggest writing more tests and fuzzing this
code. Complex argument parsing is hard to get right.
Comments inline.
> Index: fuse_opt.c
> ===
> RCS file: /cvs/src/lib/libfuse/fuse_opt.c,v
> retrieving revision 1.18
> diff -u -p -u -p -r1.18 fuse_opt.c
> --- fuse_opt.c4 Jan 2017 12:01:22 - 1.18
> +++ fuse_opt.c17 Oct 2017 13:57:35 -
> @@ -222,57 +222,70 @@ static int
> parse_opt(const struct fuse_opt *o, const char *val, void *data,
> fuse_opt_proc_t f, struct fuse_args *arg)
> {
> - int found, ret, keyval;
> + int keyval;
> size_t idx;
>
> - ret = 0;
> - found = 0;
> - keyval = 0;
> +#define DISCARD 0
> +#define KEEP 1
> +#define NEED_ANOTHER_ARG 2
I'd prefix these defines with _FOPT or w/o underscore if they are part
of the API and move them on the top of the file since you use them in
multiple functions.
>
> - /* check if it is a key=value entry */
> - idx = strcspn(val, "=");
> - if (idx != strlen(val)) {
> - idx++;
> - keyval = 1;
> - }
> + if (o == NULL)
> + return KEEP;
I know it's note your code, but one-letter variable should be avoided.
It's hard to understand what they represent. But let's not clutter this
diff :)
> +
> + keyval = 0;
>
> for(; o->templ; o++) {
> - if ((keyval && strncmp(val, o->templ, idx) == 0) ||
> - (!keyval && strcmp(val, o->templ) == 0)) {
> + /* check key=value or -p n */
> + idx = strcspn(o->templ, "= ");
> +
> + if (strncmp(val, o->templ, idx) == 0) {
> + if (o->templ[idx] == '=') {
> + keyval = 1;
> + val = &val[idx + 1];
How can you be sure that val[idx + 1] is valid? For example:
o->temp = { 'k', 'e', 'y', '=', 'v', 'a', 'l', '\0' }
val = { 'k', '\0' }
This looks like an overflow to me. Well not yet, but when you
dereference `val' below.
> + } else if (o->templ[idx] == ' ') {
> + keyval = 1;
> + if (idx == strlen(val)) {
> + /* ask for next arg to be included */
> + return NEED_ANOTHER_ARG;
> + } else if (strchr(o->templ, '%') != NULL) {
> + val = &val[idx];
Same here, how can you be sure that val[idx] is valid?
> + }
> + }
> +
> if (o->val == FUSE_OPT_KEY_DISCARD)
> - return (1);
> + return (DISCARD);
> +
> + if (o->val == FUSE_OPT_KEY_KEEP)
> + return (KEEP);
>
> if (FUSE_OPT_IS_OPT_KEY(o)) {
> - if (keyval)
> - ret = f(data, &val[idx], o->val, arg);
> - else
> - ret = f(data, val, o->val, arg);
> - }
> + if (f == NULL)
> + return KEEP;
>
> - if (o->off != ULONG_MAX && data && o->val >= 0) {
> - ret = f(data, val, o->val, arg);
> - int *addr = (int *)(data + o->off);
> - *addr = o->val;
> - ret = 0;
> + return f(data, val, o->val, arg);
> + } else if (data == NULL) {
> + return -1;
> + } else if (strchr(o->templ, '%') == NULL) {
> + *((int *)(data + o->off)) = o->val;
Are you sure you can simply deference "data + o->off" w/o sanity check?
> + } else if (strstr(o->templ, "%u") != NULL) {
> + *((unsigned int*)(data + o->off)) = atoi(val);
> + } else if (strstr(o->templ, "%lu") != NULL) {
> + *((unsigned long*)(data + o->off)) =
> + strtoul(v
Re: libfuse: fuse.c null checks and others
On 25/10/17(Wed) 13:27, Helg Bredow wrote:
> I've included different minor patches below as one patch. I haven't split
> into separate patches since the changes are not complex and easy to audit.
>
> Here's what it does:
>
> Almost all functions in fuse.c do not check if the arguments are null. This
> patch adds null checks where appropriate.
>
> Some arguments are incorrectly flagged as unused. Delete "unused" if the
> argument is used in the function.
>
> The wrong version macro is used in dump_version() and is inconsistent with
> that used in fuse_version(). FUSE_USE_VERSION is intended to be defined by
> file system to specify which backward compatible version of libfuse they
> require.
>
> fuse_loop_mt() is not implemented so return -1 rather than success. If a file
> system tries to call it then it should be obvious that it's not going to work.
>
> fuse_main() declared redundant variables due to the lack of NULL checks
> before assignment. We now check for NULL so can safely pass NULL instead.
>
> Tested with a regression test that passes all NULL arguments to exported
> functions.
>
> Something to consider is that fuse_is_lib_option() is deprecated. Should we
> remove it from libfuse to stay strictly at version 26?
Testing for NULL is good. However returning -1 in fuse_loop_mt() and
changing the version number might break ports. So if these changes go
in, you should watch for regression on ports@ at least.
We can remove fuse_is_lib_option() if nothing in ports use it. A good
start to search for it is codesearch.debian.net. You can also ask
porters to do a grep on unpacked port tree.
Could you provide a diff including only the NULL check and the removal
of "unused" markers?
> Index: fuse.c
> ===
> RCS file: /cvs/src/lib/libfuse/fuse.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 fuse.c
> --- fuse.c25 Oct 2017 09:29:46 - 1.31
> +++ fuse.c25 Oct 2017 12:54:48 -
> @@ -71,6 +71,9 @@ fuse_loop(struct fuse *fuse)
> ssize_t n;
> int ret;
>
> + if (fuse == NULL)
> + return (-1);
> +
> fuse->fc->kq = kqueue();
> if (fuse->fc->kq == -1)
> return (-1);
> @@ -156,6 +159,9 @@ fuse_mount(const char *dir, unused struc
> struct fuse_chan *fc;
> const char *errcause;
>
> + if (dir == NULL)
> + return (NULL);
> +
> fc = calloc(1, sizeof(*fc));
> if (fc == NULL)
> return (NULL);
> @@ -197,9 +203,9 @@ bad:
> }
>
> void
> -fuse_unmount(const char *dir, unused struct fuse_chan *ch)
> +fuse_unmount(const char *dir, struct fuse_chan *ch)
> {
> - if (ch->dead)
> + if (ch == NULL || ch->dead)
> return;
>
> if (unmount(dir, MNT_UPDATE) == -1)
> @@ -207,7 +213,7 @@ fuse_unmount(const char *dir, unused str
> }
>
> int
> -fuse_is_lib_option(unused const char *opt)
> +fuse_is_lib_option(const char *opt)
> {
> return (fuse_opt_match(fuse_core_opts, opt));
> }
> @@ -215,6 +221,9 @@ fuse_is_lib_option(unused const char *op
> int
> fuse_chan_fd(struct fuse_chan *ch)
> {
> + if (ch == NULL)
> + return (-1);
> +
> return (ch->fd);
> }
>
> @@ -227,17 +236,20 @@ fuse_get_session(struct fuse *f)
> int
> fuse_loop_mt(unused struct fuse *fuse)
> {
> - return (0);
> + return (-1);
> }
>
> struct fuse *
> fuse_new(struct fuse_chan *fc, unused struct fuse_args *args,
> const struct fuse_operations *ops, unused size_t size,
> -unused void *userdata)
> +void *userdata)
> {
> struct fuse *fuse;
> struct fuse_vnode *root;
>
> + if (fc == NULL || ops == NULL)
> + return (NULL);
> +
> if ((fuse = calloc(1, sizeof(*fuse))) == NULL)
> return (NULL);
>
> @@ -275,8 +287,11 @@ fuse_daemonize(unused int foreground)
> }
>
> void
> -fuse_destroy(unused struct fuse *f)
> +fuse_destroy(struct fuse *f)
> {
> + if (f == NULL)
> + return;
> +
> close(f->fc->fd);
> free(f->fc->dir);
> free(f->fc);
> @@ -322,7 +337,7 @@ fuse_remove_signal_handlers(unused struc
> }
>
> int
> -fuse_set_signal_handlers(unused struct fuse_session *se)
> +fuse_set_signal_handlers(struct fuse_session *se)
> {
> sigse = se;
> signal(SIGHUP, ifuse_get_signal);
> @@ -344,7 +359,7 @@ dump_help(void)
> static void
> dump_version(void)
> {
> - fprintf(stderr, "FUSE library version %i\n", FUSE_USE_VERSION);
> + fprintf(stderr, "FUSE library version %i\n", FUSE_VERSION);
> }
>
> static int
> @@ -453,6 +468,9 @@ fuse_version(void)
> void
> fuse_teardown(struct fuse *fuse, char *mp)
> {
> + if (fuse == NULL || mp == NULL)
> + return;
> +
> fuse_unmount(mp, fuse->fc);
> fuse_destroy(fuse);
> }
> @@ -500,10 +518,8 @@ int
> fuse_main(int argc, char **argv, const struct fuse_operations *ops, void
> *data)
> {
> st
