[PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller
Add a basic driver for the MAX31785, focusing on the fan control features but ignoring the temperature and voltage monitoring features of the device. This driver supports all fan control modes and tachometer / PWM readback where applicable. Signed-off-by: Timothy Pearson Signed-off-by: Andrew Jeffery --- Hello, This is a rework of Timothy Pearson's original patch: https://www.mail-archive.com/linux-hwmon@vger.kernel.org/msg00868.html I've labelled it as v3 to differentiate from Timothy's postings. The original thread had some discussion about the MAX31785 being a PMBus device and that it should thus be a PMBus driver. The implementation still makes use of features not available in the pmbus core, so I've taken up the earlier suggestion and ported it to the devm_hwmon_device_register_with_info() interface. This gave a modest reduction in lines-of-code and at least to me is more aesthetically pleasing. Over and above the features of the original patch is support for a secondary rotor measurement value that is provided by MAX31785 chips with a revised firmware. The feature(s) of the firmware are determined at probe time and extra attributes exposed accordingly. Specifically, the MFR_REVISION 0x3040 of the firmware supports 'slow' and 'fast' rotor reads. The feature is implemented by command 0x90 (READ_FAN_SPEED_1) providing a 4-byte response (with the 'fast' measurement in the second word) rather than the 2-bytes response in the original firmware (MFR_REVISION 0x3030). This feature is not documented in the public datasheet[1]. [1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf The need to read a 4-byte value drives the addition of a helper that is a cut-down version of i2c_smbus_xfer_emulated(), as 4-byte transactions aren't a defined transaction type in the PMBus spec. This seemed more tasteful than hacking the PMBus core to support the quirks of a single device. Also changed from Timothy's original posting is I've massaged the locking a bit and removed what seemed to be a copy/paste bug around max31785_fan_set_pulses() setting the fan_command member. Tested on an IBM Witherspoon machine. Cheers, Andrew Documentation/hwmon/max31785 | 44 +++ drivers/hwmon/Kconfig| 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/max31785.c | 824 +++ 4 files changed, 879 insertions(+) create mode 100644 Documentation/hwmon/max31785 create mode 100644 drivers/hwmon/max31785.c diff --git a/Documentation/hwmon/max31785 b/Documentation/hwmon/max31785 new file mode 100644 index ..dd891c06401e --- /dev/null +++ b/Documentation/hwmon/max31785 @@ -0,0 +1,44 @@ +Kernel driver max31785 +== + +Supported chips: + * Maxim MAX31785 +Prefix: 'max31785' +Addresses scanned: 0x52 0x53 0x54 0x55 +Datasheet: http://pdfserv.maximintegrated.com/en/ds/MAX31785.pdf + +Author: Timothy Pearson + + +Description +--- + +This driver implements support for the Maxim MAX31785 chip. + +The MAX31785 controls the speeds of up to six fans using six independent +PWM outputs. The desired fan speeds (or PWM duty cycles) are written +through the I2C interface. The outputs drive "4-wire" fans directly, +or can be used to modulate the fan's power terminals using an external +pass transistor. + +Tachometer inputs monitor fan tachometer logic outputs for precise (+/-1%) +monitoring and control of fan RPM as well as detection of fan failure. + + +Sysfs entries +- + +fan[1-6]_input RO fan tachometer speed in RPM +fan[1-6]_fault RO fan experienced fault +fan[1-6]_pulses RW tachometer pulses per fan revolution +fan[1-6]_target RW desired fan speed in RPM +pwm[1-6]_enable RW pwm mode, 0=disabled, 1=pwm, 2=rpm, 3=automatic +pwm[1-6] RW fan target duty cycle (0-255) + +Dynamic sysfs entries + + +Whether these entries are present depends on the firmware features detected on +the device during probe. + +fan[1-6]_input_fast RO fan tachometer speed in RPM (fast rotor measurement) diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index e80ca81577f4..c75d6072c823 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -886,6 +886,16 @@ config SENSORS_MAX6697 This driver can also be built as a module. If so, the module will be called max6697. +config SENSORS_MAX31785 + tristate "Maxim MAX31785 sensor chip" + depends on I2C + help + If you say yes here you get support for 6-Channel PWM-Output + Fan RPM Controller. + + This driver can also be built as a module. If so, the module + will be called max31785. + config SENSORS_MAX31790 tristate "Maxim MAX31790 sensor chip" depends on I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index f03dd0a15933..dc55722bee88 100644 --- a/drivers/hwmon/Makefile +++ b/dr
Re: [PATCH v4 0/3] USB Audio Gadget refactoring
On Mon, Jun 05, 2017 at 12:22:13PM +0300, Felipe Balbi wrote: > > Hi, > > Ruslan Bilovol writes: > >> Ruslan Bilovol writes: > >>> I came to this patch series when wanted to do two things: > >>> - use UAC1 as virtual ALSA sound card on gadget side, > >>>just like UAC2 is used so it's possible to do rate > >>>resampling > >>> - have both playback/capture support in UAC1 > >>> > >>> Since I wanted to have same behavior for both UAC1/UAC2, > >>> obviously I've got an utility part (u_audio.c) for > >>> virtual ALSA sound card handling like we have > >>> for ethernet(u_ether) or serial(u_serial) functions. > >>> Function-specific parts (f_uac1/f_uac2) became almost > >>> as storage for class-specific USB descriptors, some > >>> boilerplate for configfs, binding and few USB > >>> config request handling. > >>> > >>> Originally in RFC [1] I've posted before, there was > >>> major change to f_uac1 after that it couldn't do > >>> direct play to existing ALSA sound card anymore, > >>> representing audio on gadget side as virtual > >>> ALSA sound card where audio streams are simply > >>> sinked to and sourced from it, so it may break > >>> current usecase for some people (and that's why > >>> it was RFC). > >>> > >>> During RFC discussion, it was agreed to not touch > >>> existing f_uac1 implementation and create new one > >>> instead. This patchset (v4) introduced new function > >>> named f_uac1_acard and doesn't touch current f_uac1 > >>> implementation, so people still can use old behavior > >> > >> Do you have a pointer to the original RFC discussion where this was > >> discussed? If we really *must* keep the old implementation, I would > >> rather rename that to f_uac1_legacy. Still, I find it unlikely that > >> anybody will care about the old implementation. > > > > It is on LKML (which is down for me) [1] or alternative archive [2] > > > >> > >>> Now, it's possible to use existing user-space > >>> applications for audio routing between Audio Gadget > >>> and real sound card. I personally use alsaloop tool > >>> from alsautils and have ability to create PCM > >>> loopback between two different ALSA cards using > >>> rate resampling, which was not possible with previous > >>> "direct play to ALSA card" approach in f_uac1. > >> > >> this is really good result and will actually make it a lot easier for > >> testing things out. > >> > >>> While here, also dropped redundant platform > >>> driver/device creation in f_uac2 driver (as well as > >>> didn't add "never implemented" volume/mute functionality > >>> in f_uac1 to f_uac1_acard) that made this work even > >>> easier to do. > >>> > >>> This series is tested with both legacy g_audio.ko and > >>> modern configfs approaches under Ubuntu 14.04 (UAC1 and > >>> UAC2) and under Windows7 x64 (UAC1 only) having > >>> perfect results in all cases. > >>> > >>> Comments, testing are welcome. > >>> > >>> v4 changes: > >>> - renamed f_uac1_newapi to f_uac1_acard that is > >>>more meaningful > >> > >> I really don't get why you wanna keep both f_uac1 and f_uac1_acard. Why > >> do we need to maintain the old uac1 implementation? Why two separate > >> files? > > > > In first RFC ([1],[2]) I did exactly what you wrote here (removed > > old uac1 implementation and replaced it by new one) but got feedback > > that it will break things for existing f_uac1 legacy users and it's better > > to > > have separate implementation. > > > > I'm OK with dropping legacy f_uac1 implementation. > > > > Another idea I was thinking about is to implement simple in-kernel > > driver which will do the same as existing alsaloop tool userspace > > tool does (so legacy users will need to load two kernel modules > > and get same functionality). But this seems to be a wrong way, > > since It known that Linux kernel community doesn't like to take drivers > > with same functionality as existing userspace tools already have. > > > > So bottom line: since I'm not a legacy f_uac1 user, there is no > > difference for me how to handle it - remove legacy f_uac1 completely, > > rename it to f_uac1_legacy or add separate f_uac1_acard function. > > > > So if dropping of legacy f_uac1 implementation is OK for you, > > I can do it quickly in next patchset. > > Personally, I don't want duplicated functionality and I think the > virtual sound card approach is much better. Then again, removing > functionality we already support is kind of odd. > > Greg, Alan, what do you guys think? Do we keep a duplicated function > around or do we just tell people to rely on alsaloop? Personally, I > think we're better off with the flexibility of the virtual sound card, > what's your take? If the in-kernel driver will do more things, and we don't break the existing setups using alsaloop, then I don't see the problem, except that we now have to maintain two things :) If you don't mind the maintenance, fine with me... thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the
Re: [PATCH 2/3] Documentation: Move visorbus documentation from staging to Documentation/
On Mon, 05 Jun 2017, David Kershner wrote: > This patch simply does a git mv of the > drivers/staging/unisys/Documentation directory to Documentation. Up to Jon, of course, but I wouldn't add any new files directly under Documentation, and especially not something as specific as this. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/3] watchdog: allow setting deadline for opening /dev/watchdogN
On 2017-05-30 10:56, Rasmus Villemoes wrote: > > v6 tweaks the wording in watchdog-parameters.txt to avoid having to > update it if and when the watchdog core grows new parameters. It also > adds a little more rationale to the commit messages for 2/3 and 3/3, > and adds Reviewed-bys to 1/3 which is unchanged from v5. Ping. Guenther, is there anything I can do on my end to get this in before the 4.13 merge window opens? I realize you're busy, but I'd really appreciate just a brief response to this and the other emails I've sent the past few weeks. -- Rasmus Villemoes Software Developer Prevas A/S Hedeager 1 DK-8200 Aarhus N +45 51210274 rasmus.villem...@prevas.dk www.prevas.dk -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [xfstests PATCH v3 1/5] generic: add a writeback error handling test
On Wed, May 31, 2017 at 09:08:16AM -0400, Jeff Layton wrote:
> I'm working on a set of kernel patches to change how writeback errors
> are handled and reported in the kernel. Instead of reporting a
> writeback error to only the first fsync caller on the file, I aim
> to make the kernel report them once on every file description.
>
> This patch adds a test for the new behavior. Basically, open many fds
> to the same file, turn on dm_error, write to each of the fds, and then
> fsync them all to ensure that they all get an error back.
>
> To do that, I'm adding a new tools/dmerror script that the C program
> can use to load the error table. For now, that's all it can do, but
> we can fill it out with other commands as necessary.
>
> Signed-off-by: Jeff Layton
Thanks for the new tests! And sorry for the late review..
It's testing a new behavior on error reporting on writeback, I'm not
sure if we can call it a new feature or it fixed a bug? But it's more
like a behavior change, I'm not sure how to categorize it.
Because if it's testing a new feature, we usually let test do proper
detection of current test environment (based on actual behavior not
kernel version) and _notrun on filesystems that don't have this feature
yet, instead of failing the test; if it's testing a bug fix, we could
leave the test fail on unfixed filesystems, this also serves as a
reminder that there's bug to fix.
I pulled your test kernel tree, and test passed on EXT4 but failed on
other local filesystems (XFS, btrfs). I assume that's expected.
Besides this kinda high-level question, some minor comments inline.
> ---
> common/dmerror | 13 ++--
> doc/auxiliary-programs.txt | 8 +++
> src/Makefile | 2 +-
> src/fsync-err.c| 161
> +
New binary needs an entry in .gitignore file.
> tests/generic/999 | 76 +
> tests/generic/999.out | 3 +
> tests/generic/group| 1 +
> tools/dmerror | 44 +
This file is used by the test, then it should be in src/ directory and
be installed along with other executable files on "make install".
Because files under tools/ are not installed. Most people will run tests
in the root dir of xfstests and this is not a problem, but there're
still cases people do "make && make install" and run fstests from
/var/lib/xfstests (default installation target).
> 8 files changed, 302 insertions(+), 6 deletions(-)
> create mode 100644 src/fsync-err.c
> create mode 100755 tests/generic/999
> create mode 100644 tests/generic/999.out
> create mode 100755 tools/dmerror
>
> diff --git a/common/dmerror b/common/dmerror
> index d46c5d0b7266..238baa213b1f 100644
> --- a/common/dmerror
> +++ b/common/dmerror
> @@ -23,22 +23,25 @@ if [ $? -eq 0 ]; then
> _notrun "Cannot run tests with DAX on dmerror devices"
> fi
>
> -_dmerror_init()
> +_dmerror_setup()
> {
> local dm_backing_dev=$SCRATCH_DEV
>
> - $DMSETUP_PROG remove error-test > /dev/null 2>&1
> -
> local blk_dev_size=`blockdev --getsz $dm_backing_dev`
>
> DMERROR_DEV='/dev/mapper/error-test'
>
> DMLINEAR_TABLE="0 $blk_dev_size linear $dm_backing_dev 0"
>
> + DMERROR_TABLE="0 $blk_dev_size error $dm_backing_dev 0"
> +}
> +
> +_dmerror_init()
> +{
> + _dmerror_setup
> + $DMSETUP_PROG remove error-test > /dev/null 2>&1
> $DMSETUP_PROG create error-test --table "$DMLINEAR_TABLE" || \
> _fatal "failed to create dm linear device"
> -
> - DMERROR_TABLE="0 $blk_dev_size error $dm_backing_dev 0"
> }
>
> _dmerror_mount()
> diff --git a/doc/auxiliary-programs.txt b/doc/auxiliary-programs.txt
> index 21ef118596b6..191ac0596511 100644
> --- a/doc/auxiliary-programs.txt
> +++ b/doc/auxiliary-programs.txt
> @@ -16,6 +16,7 @@ note the dependency with:
> Contents:
>
> - af_unix -- Create an AF_UNIX socket
> + - fsync-err -- tests fsync error reporting after failed writeback
> - open_by_handle-- open_by_handle_at syscall exercise
> - stat_test -- statx syscall exercise
> - t_dir_type-- print directory entries and their file type
> @@ -30,6 +31,13 @@ af_unix
>
> The af_unix program creates an AF_UNIX socket at the given location.
>
> +fsync-err
> + Specialized program for testing how the kernel reports errors that
> + occur during writeback. Works in conjunction with the dmerror script
> + in tools/ to write data to a device, and then force it to fail
> + writeback and test that errors are reported during fsync and cleared
> + afterward.
> +
> open_by_handle
>
> The open_by_handle program exercises the open_by_handle_at() system
> diff --git a/src/Makefile b/src/Makefile
> index 4ec01975f8f7..b79c4d84d31b 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -13,7 +13,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
> multi_open_
Re: [xfstests PATCH v3 2/5] ext4: allow ext4 to use $SCRATCH_LOGDEV
On Wed, May 31, 2017 at 09:08:17AM -0400, Jeff Layton wrote: > The writeback error handling test requires that you put the journal on a > separate device. This allows us to use dmerror to simulate data > writeback failure, without affecting the journal. > > xfs already has infrastructure for this (a'la $SCRATCH_LOGDEV), so wire > up the ext4 code so that it can do the same thing when _scratch_mkfs is > called. > > Signed-off-by: Jeff Layton > Reviewed-by: Darrick J. Wong > --- > common/rc | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/common/rc b/common/rc > index 743df427c047..391d36f373cd 100644 > --- a/common/rc > +++ b/common/rc > @@ -676,6 +676,9 @@ _scratch_mkfs_ext4() > local tmp=`mktemp` > local mkfs_status > > + [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \ > + $mkfs_cmd -O journal_dev $SCRATCH_LOGDEV && \ > + mkfs_cmd="$mkfs_cmd -J device=$SCRATCH_LOGDEV" Need $MKFS_OPTIONS too when creating journal device, otherwise mkfs will fail when making non-default block size ext4, i.e. journal device has 4k block size, but ext4 has 1k block size if we have MKFS_OPTIONS="-b 1024" Thanks, Eryu > > _scratch_do_mkfs "$mkfs_cmd" "$mkfs_filter" $* 2>$tmp.mkfserr > 1>$tmp.mkfsstd > mkfs_status=$? > -- > 2.9.4 > > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [xfstests PATCH v3 4/5] ext3: allow it to put journal on a separate device when doing scratch_mkfs
On Wed, May 31, 2017 at 09:08:19AM -0400, Jeff Layton wrote: > Signed-off-by: Jeff Layton > --- > common/rc | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/common/rc b/common/rc > index 391d36f373cd..83765aacfb06 100644 > --- a/common/rc > +++ b/common/rc > @@ -832,7 +832,16 @@ _scratch_mkfs() > mkfs_cmd="$MKFS_BTRFS_PROG" > mkfs_filter="cat" > ;; > - ext2|ext3) > + ext3) > + mkfs_cmd="$MKFS_PROG -t $FSTYP -- -F" > + mkfs_filter="grep -v -e ^Warning: -e \"^mke2fs \"" > + > + # put journal on separate device? > + [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \ > + $mkfs_cmd -O journal_dev $SCRATCH_LOGDEV && \ > + mkfs_cmd="$mkfs_cmd -J device=$SCRATCH_LOGDEV" Similar to that ext4 patch, need $MKFS_OPTIONS when creating journal device. Thanks, Eryu > + ;; > + ext2) > mkfs_cmd="$MKFS_PROG -t $FSTYP -- -F" > mkfs_filter="grep -v -e ^Warning: -e \"^mke2fs \"" > ;; > -- > 2.9.4 > > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [xfstests PATCH v3 3/5] generic: test writeback error handling on dmerror devices
On Wed, May 31, 2017 at 09:08:18AM -0400, Jeff Layton wrote:
> Ensure that we get an error back on all fds when a block device is
> open by multiple writers and writeback fails.
>
> Signed-off-by: Jeff Layton
> ---
> tests/generic/998 | 64
> +++
> tests/generic/998.out | 2 ++
> tests/generic/group | 1 +
> 3 files changed, 67 insertions(+)
> create mode 100755 tests/generic/998
> create mode 100644 tests/generic/998.out
>
> diff --git a/tests/generic/998 b/tests/generic/998
> new file mode 100755
> index ..fbadb47507c2
> --- /dev/null
> +++ b/tests/generic/998
> @@ -0,0 +1,64 @@
> +#! /bin/bash
> +# FS QA Test No. 998
> +#
> +# Test writeback error handling when writing to block devices via pagecache.
> +# See src/fsync-err.c for details of what test actually does.
> +#
> +#---
> +# Copyright (c) 2017, Jeff Layton
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> +#---
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +cd /
> +rm -rf $tmp.* $testdir
> +_dmerror_cleanup
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/dmerror
> +
> +# real QA test starts here
> +_supported_os Linux
> +_require_scratch
_require_scratch_nocheck
Then we don't have to re-create a filesystem on SCRATCH_DEV before
exiting.
> +_require_logdev
> +_require_dm_target error
> +_require_test_program fsync-err
> +
> +rm -f $seqres.full
> +
> +$XFS_IO_PROG -d -c "pwrite -S 0x7c -b 1048576 0 $((64 * 1048576))"
> $SCRATCH_DEV >> $seqres.full
I don't see why this is needed, add some comments? Or remove it if it's
not needed?
> +_dmerror_init
> +
> +$here/src/fsync-err $DMERROR_DEV
> +
> +# success, all done
> +_dmerror_load_working_table
> +_dmerror_cleanup
> +_scratch_mkfs > $seqres.full 2>&1
> +status=0
> +exit
> diff --git a/tests/generic/998.out b/tests/generic/998.out
> new file mode 100644
> index ..658c438820e2
> --- /dev/null
> +++ b/tests/generic/998.out
> @@ -0,0 +1,2 @@
> +QA output created by 998
> +Test passed!
> diff --git a/tests/generic/group b/tests/generic/group
> index 39f7b14657f1..9fc384363ca7 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -440,4 +440,5 @@
> 435 auto encrypt
> 436 auto quick rw
> 437 auto quick
> +998 auto quick
This is a test for block device, not filesystems, I'd remove it from
auto and quick group, but add it to 'blockdev' group. So it won't be run
if someone wants to test filesystems.
Thanks,
Eryu
> 999 auto quick
> --
> 2.9.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [xfstests PATCH v3 5/5] btrfs: allow it to use $SCRATCH_LOGDEV
On Wed, May 31, 2017 at 09:08:20AM -0400, Jeff Layton wrote: > With btrfs, we can't really put the log on a separate device. What we > can do however is mirror the metadata across two devices and make the > data striped across all devices. When we turn on dmerror then the > metadata can fall back to using the other mirror while the data errors > out. > > Note that the current incarnation of btrfs has a fixed 64k stripe > width. If that ever changes or becomes settable, we may need to adjust > the amount of data that the test program writes. > > Signed-off-by: Jeff Layton > --- > common/rc | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/common/rc b/common/rc > index 83765aacfb06..078270451b53 100644 > --- a/common/rc > +++ b/common/rc > @@ -830,6 +830,8 @@ _scratch_mkfs() > ;; > btrfs) > mkfs_cmd="$MKFS_BTRFS_PROG" > + [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \ > + mkfs_cmd="$mkfs_cmd -d raid0 -m raid1 $SCRATCH_LOGDEV" I don't think this is the correct way to do it. If btrfs doesn't support external log device, then this test doesn't fit btrfs, or we need other ways to test btrfs. One of the problems of this hack is that raid1 requires all devices are in the same size, we have a _require_scratch_dev_pool_equal_size() rule to check on it, but this hack doesn't do the proper check and test fails if SCRATCH_LOGDEV is smaller or bigger in size. If btrfs "-d raid0 -m raid1" is capable to do this writeback error test, perhaps you can write a new btrfs test and mkfs with "-d raid0 -m raid1" explicitly. e.g. ... _require_scratch_dev_pool 2 _require_scratch_dev_pool_equal_size ... _scratch_mkfs "-d raid0 -m raid1" ... Thanks, Eryu > mkfs_filter="cat" > ;; > ext3) > -- > 2.9.4 > > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/3] USB Audio Gadget refactoring
Hi, Greg KH writes: >> > I'm OK with dropping legacy f_uac1 implementation. >> > >> > Another idea I was thinking about is to implement simple in-kernel >> > driver which will do the same as existing alsaloop tool userspace >> > tool does (so legacy users will need to load two kernel modules >> > and get same functionality). But this seems to be a wrong way, >> > since It known that Linux kernel community doesn't like to take drivers >> > with same functionality as existing userspace tools already have. >> > >> > So bottom line: since I'm not a legacy f_uac1 user, there is no >> > difference for me how to handle it - remove legacy f_uac1 completely, >> > rename it to f_uac1_legacy or add separate f_uac1_acard function. >> > >> > So if dropping of legacy f_uac1 implementation is OK for you, >> > I can do it quickly in next patchset. >> >> Personally, I don't want duplicated functionality and I think the >> virtual sound card approach is much better. Then again, removing >> functionality we already support is kind of odd. >> >> Greg, Alan, what do you guys think? Do we keep a duplicated function >> around or do we just tell people to rely on alsaloop? Personally, I >> think we're better off with the flexibility of the virtual sound card, >> what's your take? > > If the in-kernel driver will do more things, and we don't break the > existing setups using alsaloop, then I don't see the problem, except > that we now have to maintain two things :) > > If you don't mind the maintenance, fine with me... Okay, I don't think many will continue to use f_uac1.c. Ruslan, can you update your series so that current f_uac1.c gets renamed to f_uac1_legacy.c and you introduce a *new* f_uac1.c instead? Thanks a lot -- balbi signature.asc Description: PGP signature
Re: [xfstests PATCH v3 1/5] generic: add a writeback error handling test
On Tue, 2017-06-06 at 16:58 +0800, Eryu Guan wrote:
> On Wed, May 31, 2017 at 09:08:16AM -0400, Jeff Layton wrote:
> > I'm working on a set of kernel patches to change how writeback errors
> > are handled and reported in the kernel. Instead of reporting a
> > writeback error to only the first fsync caller on the file, I aim
> > to make the kernel report them once on every file description.
> >
> > This patch adds a test for the new behavior. Basically, open many fds
> > to the same file, turn on dm_error, write to each of the fds, and then
> > fsync them all to ensure that they all get an error back.
> >
> > To do that, I'm adding a new tools/dmerror script that the C program
> > can use to load the error table. For now, that's all it can do, but
> > we can fill it out with other commands as necessary.
> >
> > Signed-off-by: Jeff Layton
>
> Thanks for the new tests! And sorry for the late review..
>
> It's testing a new behavior on error reporting on writeback, I'm not
> sure if we can call it a new feature or it fixed a bug? But it's more
> like a behavior change, I'm not sure how to categorize it.
>
> Because if it's testing a new feature, we usually let test do proper
> detection of current test environment (based on actual behavior not
> kernel version) and _notrun on filesystems that don't have this feature
> yet, instead of failing the test; if it's testing a bug fix, we could
> leave the test fail on unfixed filesystems, this also serves as a
> reminder that there's bug to fix.
>
Thanks for the review! I'm not sure how to categorize this either. Since
the plan is to convert all the filesystems piecemeal, maybe we should
just consider it a new feature.
> I pulled your test kernel tree, and test passed on EXT4 but failed on
> other local filesystems (XFS, btrfs). I assume that's expected.
>
> Besides this kinda high-level question, some minor comments inline.
>
Yes, ext4 should pass on my latest kernel tree, but everything else
should fail.
> > ---
> > common/dmerror | 13 ++--
> > doc/auxiliary-programs.txt | 8 +++
> > src/Makefile | 2 +-
> > src/fsync-err.c| 161
> > +
>
> New binary needs an entry in .gitignore file.
>
OK, thanks. Will fix.
> > tests/generic/999 | 76 +
> > tests/generic/999.out | 3 +
> > tests/generic/group| 1 +
> > tools/dmerror | 44 +
>
> This file is used by the test, then it should be in src/ directory and
> be installed along with other executable files on "make install".
> Because files under tools/ are not installed. Most people will run tests
> in the root dir of xfstests and this is not a problem, but there're
> still cases people do "make && make install" and run fstests from
> /var/lib/xfstests (default installation target).
>
Ok, no problem. I'll move it. I wasn't sure here since dmerror is a
shell script, and most of the stuff in src/ is stuff that needs to be
built.
> > 8 files changed, 302 insertions(+), 6 deletions(-)
> > create mode 100644 src/fsync-err.c
> > create mode 100755 tests/generic/999
> > create mode 100644 tests/generic/999.out
> > create mode 100755 tools/dmerror
> >
> > diff --git a/common/dmerror b/common/dmerror
> > index d46c5d0b7266..238baa213b1f 100644
> > --- a/common/dmerror
> > +++ b/common/dmerror
> > @@ -23,22 +23,25 @@ if [ $? -eq 0 ]; then
> > _notrun "Cannot run tests with DAX on dmerror devices"
> > fi
> >
> > -_dmerror_init()
> > +_dmerror_setup()
> > {
> > local dm_backing_dev=$SCRATCH_DEV
> >
> > - $DMSETUP_PROG remove error-test > /dev/null 2>&1
> > -
> > local blk_dev_size=`blockdev --getsz $dm_backing_dev`
> >
> > DMERROR_DEV='/dev/mapper/error-test'
> >
> > DMLINEAR_TABLE="0 $blk_dev_size linear $dm_backing_dev 0"
> >
> > + DMERROR_TABLE="0 $blk_dev_size error $dm_backing_dev 0"
> > +}
> > +
> > +_dmerror_init()
> > +{
> > + _dmerror_setup
> > + $DMSETUP_PROG remove error-test > /dev/null 2>&1
> > $DMSETUP_PROG create error-test --table "$DMLINEAR_TABLE" || \
> > _fatal "failed to create dm linear device"
> > -
> > - DMERROR_TABLE="0 $blk_dev_size error $dm_backing_dev 0"
> > }
> >
> > _dmerror_mount()
> > diff --git a/doc/auxiliary-programs.txt b/doc/auxiliary-programs.txt
> > index 21ef118596b6..191ac0596511 100644
> > --- a/doc/auxiliary-programs.txt
> > +++ b/doc/auxiliary-programs.txt
> > @@ -16,6 +16,7 @@ note the dependency with:
> > Contents:
> >
> > - af_unix -- Create an AF_UNIX socket
> > + - fsync-err -- tests fsync error reporting after failed
> > writeback
> > - open_by_handle -- open_by_handle_at syscall exercise
> > - stat_test -- statx syscall exercise
> > - t_dir_type -- print directory entries and their file type
> > @@ -30,6 +31,13 @@ af_unix
> >
> > The af_unix program creates an AF_UNIX s
Re: [xfstests PATCH v3 1/5] generic: add a writeback error handling test
On Tue, Jun 06, 2017 at 06:15:57AM -0400, Jeff Layton wrote:
> On Tue, 2017-06-06 at 16:58 +0800, Eryu Guan wrote:
> > On Wed, May 31, 2017 at 09:08:16AM -0400, Jeff Layton wrote:
> > > I'm working on a set of kernel patches to change how writeback errors
> > > are handled and reported in the kernel. Instead of reporting a
> > > writeback error to only the first fsync caller on the file, I aim
> > > to make the kernel report them once on every file description.
> > >
> > > This patch adds a test for the new behavior. Basically, open many fds
> > > to the same file, turn on dm_error, write to each of the fds, and then
> > > fsync them all to ensure that they all get an error back.
> > >
> > > To do that, I'm adding a new tools/dmerror script that the C program
> > > can use to load the error table. For now, that's all it can do, but
> > > we can fill it out with other commands as necessary.
> > >
> > > Signed-off-by: Jeff Layton
> >
> > Thanks for the new tests! And sorry for the late review..
> >
> > It's testing a new behavior on error reporting on writeback, I'm not
> > sure if we can call it a new feature or it fixed a bug? But it's more
> > like a behavior change, I'm not sure how to categorize it.
> >
> > Because if it's testing a new feature, we usually let test do proper
> > detection of current test environment (based on actual behavior not
> > kernel version) and _notrun on filesystems that don't have this feature
> > yet, instead of failing the test; if it's testing a bug fix, we could
> > leave the test fail on unfixed filesystems, this also serves as a
> > reminder that there's bug to fix.
> >
>
> Thanks for the review! I'm not sure how to categorize this either. Since
> the plan is to convert all the filesystems piecemeal, maybe we should
> just consider it a new feature.
Then we need a new _require rule to properly detect for the 'feature'
support. I'm not sure if this is doable, but something like
_require_statx, _require_seek_data_hole would be good.
>
> > I pulled your test kernel tree, and test passed on EXT4 but failed on
> > other local filesystems (XFS, btrfs). I assume that's expected.
> >
> > Besides this kinda high-level question, some minor comments inline.
> >
>
> Yes, ext4 should pass on my latest kernel tree, but everything else
> should fail.
With the new _require rule, test should _notrun on XFS and btrfs then.
>
> > > ---
> > > common/dmerror | 13 ++--
> > > doc/auxiliary-programs.txt | 8 +++
> > > src/Makefile | 2 +-
> > > src/fsync-err.c| 161
> > > +
> >
> > New binary needs an entry in .gitignore file.
> >
>
> OK, thanks. Will fix.
>
> > > tests/generic/999 | 76 +
> > > tests/generic/999.out | 3 +
> > > tests/generic/group| 1 +
> > > tools/dmerror | 44 +
> >
> > This file is used by the test, then it should be in src/ directory and
> > be installed along with other executable files on "make install".
> > Because files under tools/ are not installed. Most people will run tests
> > in the root dir of xfstests and this is not a problem, but there're
> > still cases people do "make && make install" and run fstests from
> > /var/lib/xfstests (default installation target).
> >
>
> Ok, no problem. I'll move it. I wasn't sure here since dmerror is a
> shell script, and most of the stuff in src/ is stuff that needs to be
> built.
We do have a few perl or shell scripts in src/ dir, we can see this from
src/Makefile
$(LTINSTALL) -m 755 fill2attr fill2fs fill2fs_check scaleread.sh
$(PKG_LIB_DIR)/src
>
> > > 8 files changed, 302 insertions(+), 6 deletions(-)
> > > create mode 100644 src/fsync-err.c
> > > create mode 100755 tests/generic/999
> > > create mode 100644 tests/generic/999.out
> > > create mode 100755 tools/dmerror
> > >
> > > diff --git a/common/dmerror b/common/dmerror
> > > index d46c5d0b7266..238baa213b1f 100644
> > > --- a/common/dmerror
> > > +++ b/common/dmerror
> > > @@ -23,22 +23,25 @@ if [ $? -eq 0 ]; then
> > > _notrun "Cannot run tests with DAX on dmerror devices"
> > > fi
> > >
> > > -_dmerror_init()
> > > +_dmerror_setup()
> > > {
> > > local dm_backing_dev=$SCRATCH_DEV
> > >
> > > - $DMSETUP_PROG remove error-test > /dev/null 2>&1
> > > -
> > > local blk_dev_size=`blockdev --getsz $dm_backing_dev`
> > >
> > > DMERROR_DEV='/dev/mapper/error-test'
> > >
> > > DMLINEAR_TABLE="0 $blk_dev_size linear $dm_backing_dev 0"
> > >
> > > + DMERROR_TABLE="0 $blk_dev_size error $dm_backing_dev 0"
> > > +}
> > > +
> > > +_dmerror_init()
> > > +{
> > > + _dmerror_setup
> > > + $DMSETUP_PROG remove error-test > /dev/null 2>&1
> > > $DMSETUP_PROG create error-test --table "$DMLINEAR_TABLE" || \
> > > _fatal "failed to create dm linear device"
> > > -
> > > - DMERROR_TABLE="0 $blk_dev_size error $dm_backing_dev 0"
> > > }
> > >
> > > _dmer
kernel-doc mishandles declarations split into lines
Hi,
Apologies for the long CC list, wasn't sure who really feels like they
understand this script anymore ... Markus, I think you had a rewrite of
the script in python?
If you have something like this:
/**
* struct something
* @very_long_member_name: abcde
*/
struct something {
struct this_is_a_very_long_struct_name_so_need_to_break_for_the
very_long_member_name;
};
then kernel-doc will mishandle it:
$ ./scripts/kernel-doc -rst -internal /tmp/test.c
/tmp/test.c:9: warning: No description found for parameter
'this_is_a_very_long_struct_name_so_need_to_break_for_thevery_long_member_name'
/tmp/test.c:9: warning: Excess struct/union/enum/typedef member
'very_long_member_name' description in 'something'
.. c:type:: struct something
**Definition**
::
struct something {
};
**Members**
Clearly, the code that strips trailing and leading whitespace (in
process_proto_type) is a bit *too* eager to do so, but so far I haven't
found anything that makes it work.
Anyone have any thoughts?
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller
On 06/06/2017 12:02 AM, Andrew Jeffery wrote: Add a basic driver for the MAX31785, focusing on the fan control features but ignoring the temperature and voltage monitoring features of the device. This driver supports all fan control modes and tachometer / PWM readback where applicable. Signed-off-by: Timothy Pearson Signed-off-by: Andrew Jeffery --- Hello, This is a rework of Timothy Pearson's original patch: https://www.mail-archive.com/linux-hwmon@vger.kernel.org/msg00868.html I've labelled it as v3 to differentiate from Timothy's postings. The original thread had some discussion about the MAX31785 being a PMBus device and that it should thus be a PMBus driver. The implementation still makes use of features not available in the pmbus core, so I've taken up the earlier suggestion and ported it to the devm_hwmon_device_register_with_info() interface. This gave a modest reduction in lines-of-code and at least to me is more aesthetically pleasing. Over and above the features of the original patch is support for a secondary rotor measurement value that is provided by MAX31785 chips with a revised firmware. The feature(s) of the firmware are determined at probe time and extra attributes exposed accordingly. Specifically, the MFR_REVISION 0x3040 of the firmware supports 'slow' and 'fast' rotor reads. The feature is implemented by command 0x90 (READ_FAN_SPEED_1) providing a 4-byte response (with the 'fast' measurement in the second word) rather than the 2-bytes response in the original firmware (MFR_REVISION 0x3030). Taking the pmbus driver question out, why would this warrant another non-standard attribute outside the ABI ? I could see the desire to replace the 'slow' access with the 'fast' one, but provide two attributes ? No, I don't see the point, sorry, even more so without detailed explanation why the second attribute in addition to the first one would add any value. This feature is not documented in the public datasheet[1]. [1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf The need to read a 4-byte value drives the addition of a helper that is a cut-down version of i2c_smbus_xfer_emulated(), as 4-byte transactions aren't a defined transaction type in the PMBus spec. This seemed more tasteful than hacking the PMBus core to support the quirks of a single device. That is why we have PMBus helper drivers. Guenter Also changed from Timothy's original posting is I've massaged the locking a bit and removed what seemed to be a copy/paste bug around max31785_fan_set_pulses() setting the fan_command member. Tested on an IBM Witherspoon machine. Cheers, Andrew Documentation/hwmon/max31785 | 44 +++ drivers/hwmon/Kconfig| 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/max31785.c | 824 +++ 4 files changed, 879 insertions(+) create mode 100644 Documentation/hwmon/max31785 create mode 100644 drivers/hwmon/max31785.c diff --git a/Documentation/hwmon/max31785 b/Documentation/hwmon/max31785 new file mode 100644 index ..dd891c06401e --- /dev/null +++ b/Documentation/hwmon/max31785 @@ -0,0 +1,44 @@ +Kernel driver max31785 +== + +Supported chips: + * Maxim MAX31785 +Prefix: 'max31785' +Addresses scanned: 0x52 0x53 0x54 0x55 +Datasheet: http://pdfserv.maximintegrated.com/en/ds/MAX31785.pdf + +Author: Timothy Pearson + + +Description +--- + +This driver implements support for the Maxim MAX31785 chip. + +The MAX31785 controls the speeds of up to six fans using six independent +PWM outputs. The desired fan speeds (or PWM duty cycles) are written +through the I2C interface. The outputs drive "4-wire" fans directly, +or can be used to modulate the fan's power terminals using an external +pass transistor. + +Tachometer inputs monitor fan tachometer logic outputs for precise (+/-1%) +monitoring and control of fan RPM as well as detection of fan failure. + + +Sysfs entries +- + +fan[1-6]_input RO fan tachometer speed in RPM +fan[1-6]_fault RO fan experienced fault +fan[1-6]_pulses RW tachometer pulses per fan revolution +fan[1-6]_target RW desired fan speed in RPM +pwm[1-6]_enable RW pwm mode, 0=disabled, 1=pwm, 2=rpm, 3=automatic +pwm[1-6] RW fan target duty cycle (0-255) + +Dynamic sysfs entries + + +Whether these entries are present depends on the firmware features detected on +the device during probe. + +fan[1-6]_input_fast RO fan tachometer speed in RPM (fast rotor measurement) diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index e80ca81577f4..c75d6072c823 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -886,6 +886,16 @@ config SENSORS_MAX6697 This driver can also be built as a module. If so, the module will be called max6697. +config SENSORS_MAX31785 + tristate "Maxim MAX31785 sensor chip
Re: [PATCH v6 0/3] watchdog: allow setting deadline for opening /dev/watchdogN
On 06/06/2017 01:08 AM, Rasmus Villemoes wrote: On 2017-05-30 10:56, Rasmus Villemoes wrote: v6 tweaks the wording in watchdog-parameters.txt to avoid having to update it if and when the watchdog core grows new parameters. It also adds a little more rationale to the commit messages for 2/3 and 3/3, and adds Reviewed-bys to 1/3 which is unchanged from v5. Ping. Guenther, is there anything I can do on my end to get this in before the 4.13 merge window opens? I realize you're busy, but I'd really appreciate just a brief response to this and the other emails I've sent the past few weeks. You made a suggestion that this overlaps with another option, which forces me to go back and check the entire exchange from the start, for which I did not have time yet, sorry. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel-doc mishandles declarations split into lines
Em Tue, 06 Jun 2017 15:28:34 +0200
Johannes Berg escreveu:
> Hi,
>
> Apologies for the long CC list, wasn't sure who really feels like they
> understand this script anymore ... Markus, I think you had a rewrite of
> the script in python?
>
> If you have something like this:
>
> /**
> * struct something
> * @very_long_member_name: abcde
> */
> struct something {
> struct this_is_a_very_long_struct_name_so_need_to_break_for_the
> very_long_member_name;
> };
>
> then kernel-doc will mishandle it:
>
> $ ./scripts/kernel-doc -rst -internal /tmp/test.c
> /tmp/test.c:9: warning: No description found for parameter
> 'this_is_a_very_long_struct_name_so_need_to_break_for_thevery_long_member_name'
> /tmp/test.c:9: warning: Excess struct/union/enum/typedef member
> 'very_long_member_name' description in 'something'
>
>
> .. c:type:: struct something
>
>
> **Definition**
>
> ::
>
> struct something {
> };
>
> **Members**
>
>
>
> Clearly, the code that strips trailing and leading whitespace (in
> process_proto_type) is a bit *too* eager to do so, but so far I haven't
> found anything that makes it work.
>
> Anyone have any thoughts?
A trivial "fix" would be to use just one line for the struct field :-)
-
The logic that handle structs is at sub dump_struct() function at
kernel-doc, with is called by dump_declaration().
I added some debug prints at kernel-doc...
I guess the problem you're noticing is at process_proto_type():
$ ./scripts/kernel-doc -rst -internal /tmp/test.c
process_proto_type - original:struct something {
process_proto_type - parsed: struct something {
process_proto_type - original: struct
this_is_a_very_long_struct_name_so_need_to_break_for_the
process_proto_type - parsed: struct
this_is_a_very_long_struct_name_so_need_to_break_for_the
process_proto_type - original: very_long_member_name;
process_proto_type - parsed: very_long_member_name;
process_proto_type - original:};
process_proto_type - parsed: };
process_proto_type - to_dump: struct something {struct
this_is_a_very_long_struct_name_so_need_to_break_for_thevery_long_member_name;};
dump_struct: struct something {struct
this_is_a_very_long_struct_name_so_need_to_break_for_thevery_long_member_name;};
members: struct
this_is_a_very_long_struct_name_so_need_to_break_for_thevery_long_member_name;
Basically, that while(1) loop there seems to be misinterpreting the line with
"very_long_member_name;"
Thanks,
Mauro
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index a26a5f2dce39..55001278a2af 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -464,7 +464,7 @@ my $doc_com = '\s*\*\s*';
my $doc_com_body = '\s*\* ?';
my $doc_decl = $doc_com . '(\w+)';
# @params and a strictly limited set of supported section names
-my $doc_sect = $doc_com .
+my $doc_sect = $doc_com .
'\s*(\@[.\w]+|\@\.\.\.|description|context|returns?|notes?|examples?)\s*:(.*)';
my $doc_content = $doc_com_body . '(.*)';
my $doc_block = $doc_com . 'DOC:\s*(.*)?';
@@ -2167,6 +2167,8 @@ sub dump_struct($$) {
my $nested;
if ($x =~ /(struct|union)\s+(\w+)\s*{(.*)}/) {
+
+printf("dump_struct: $x\n");
#my $decl_type = $1;
$declaration_name = $2;
my $members = $3;
@@ -2190,6 +2192,9 @@ sub dump_struct($$) {
# replace DECLARE_BITMAP
$members =~ s/DECLARE_BITMAP\s*\(([^,)]+), ([^,)]+)\)/unsigned long
$1\[BITS_TO_LONGS($2)\]/gos;
+printf("members: $members\n");
+
+
create_parameterlist($members, ';', $file);
check_sections($file, $declaration_name, "struct", $sectcheck,
$struct_actual, $nested);
@@ -2751,6 +2756,8 @@ sub process_proto_type($$) {
my $x = shift;
my $file = shift;
+printf("process_proto_type - original:$x\n");
+
$x =~ s@[\r\n]+@ @gos; # strip newlines/cr's.
$x =~ s@^\s+@@gos; # strip leading spaces
$x =~ s@\s+$@@gos; # strip trailing spaces
@@ -2761,12 +2768,15 @@ sub process_proto_type($$) {
$x .= ";";
}
+printf("process_proto_type - parsed: $x\n");
+
while (1) {
if ( $x =~ /([^{};]*)([{};])(.*)/ ) {
$prototype .= $1 . $2;
($2 eq '{') && $brcount++;
($2 eq '}') && $brcount--;
if (($2 eq ';') && ($brcount == 0)) {
+printf("process_proto_type - to_dump: $prototype\n");
dump_declaration($prototype, $file);
reset_state();
last;
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel-doc mishandles declarations split into lines
On Tue, 2017-06-06 at 10:59 -0300, Mauro Carvalho Chehab wrote: > > A trivial "fix" would be to use just one line for the struct field :- > ) Sure, we did this, but it makes checkpatch unhappy. We have a relatively long struct name, a relatively long member name, and then it's also an array so you have another constant that needs to fit ... (I didn't puth the array part into the example, but without that it'd actually fit on 80 cols) > The logic that handle structs is at sub dump_struct() function at > kernel-doc, with is called by dump_declaration(). > > I added some debug prints at kernel-doc... > I guess the problem you're noticing is at process_proto_type(): > [...] > Basically, that while(1) loop there seems to be misinterpreting the > line with "very_long_member_name;" Oh, that's possible - I may have been looking in the wrong place. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel-doc mishandles declarations split into lines
Em Tue, 06 Jun 2017 16:12:30 +0200
Johannes Berg escreveu:
> On Tue, 2017-06-06 at 10:59 -0300, Mauro Carvalho Chehab wrote:
> >
> > A trivial "fix" would be to use just one line for the struct field :-
> > )
>
> Sure, we did this, but it makes checkpatch unhappy. We have a
> relatively long struct name, a relatively long member name, and then
> it's also an array so you have another constant that needs to fit ...
> (I didn't puth the array part into the example, but without that it'd
> actually fit on 80 cols)
>
> > The logic that handle structs is at sub dump_struct() function at
> > kernel-doc, with is called by dump_declaration().
> >
> > I added some debug prints at kernel-doc...
> > I guess the problem you're noticing is at process_proto_type():
> >
> [...]
> > Basically, that while(1) loop there seems to be misinterpreting the
> > line with "very_long_member_name;"
>
> Oh, that's possible - I may have been looking in the wrong place.
>
> johannes
The enclosed patch should fix the issue, hopefully not introducing
any regressions ;-)
-
[PATCH] be sure that multiline definitions will be properly espaced
When handling comments from structs with multiple lines, like:
/**
* struct something
* @very_long_member_name: abcde
*/
struct something {
struct this_is_a_very_long_struct_name_so_need_to_break_for_the
very_long_member_name;
};
The logic adds the continuation line without a proper space, causing
it to be misinterpreted. Be sure to add an space to replace the
end of line.
Reported-by: Johannes Berg
Signed-off-by: Mauro Carvalho Chehab
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index a26a5f2dce39..1aa44c299f78 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -2763,17 +2763,18 @@ sub process_proto_type($$) {
while (1) {
if ( $x =~ /([^{};]*)([{};])(.*)/ ) {
- $prototype .= $1 . $2;
+ $prototype .= $1 . $2 . " ";
($2 eq '{') && $brcount++;
($2 eq '}') && $brcount--;
if (($2 eq ';') && ($brcount == 0)) {
+ $prototype =~ s/\s+/ /g;
dump_declaration($prototype, $file);
reset_state();
last;
}
$x = $3;
} else {
- $prototype .= $x;
+ $prototype .= $x . " ";
last;
}
}
Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel-doc mishandles declarations split into lines
>
> [PATCH] be sure that multiline definitions will be properly espaced
>
> When handling comments from structs with multiple lines, like:
> /**
>* struct something
>* @very_long_member_name: abcde
>*/
> struct something {
> struct
> this_is_a_very_long_struct_name_so_need_to_break_for_the
> very_long_member_name;
> };
>
> The logic adds the continuation line without a proper space, causing
> it to be misinterpreted. Be sure to add an space to replace the
> end of line.
>
> Reported-by: Johannes Berg
> Signed-off-by: Mauro Carvalho Chehab
>
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index a26a5f2dce39..1aa44c299f78 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -2763,17 +2763,18 @@ sub process_proto_type($$) {
>
> while (1) {
> if ( $x =~ /([^{};]*)([{};])(.*)/ ) {
> - $prototype .= $1 . $2;
> + $prototype .= $1 . $2 . " ";
> ($2 eq '{') && $brcount++;
> ($2 eq '}') && $brcount--;
> if (($2 eq ';') && ($brcount == 0)) {
> + $prototype =~ s/\s+/ /g;
> dump_declaration($prototype, $file);
> reset_state();
> last;
> }
> $x = $3;
> } else {
> - $prototype .= $x;
> + $prototype .= $x . " ";
> last;
Interesting. I just ended with almost the same patch, but didn't have
the line inside the brcount if, so it didn't work. However, your
version also introduces regressions:
/**
* enum foo - foo
* @F1: f1
* @F2: f2
*/
enum foo {
F1,
F2,
};
now generates a warning:
/tmp/test.c:20: warning: Enum value ' ' not described in enum 'foo'
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus
On Mon, Jun 05, 2017 at 04:07:29PM -0400, David Kershner wrote: > This patchset moves drivers/staging/unisys/include to > include/linux/visorbus, and moves drivers/staging/unisys/visorbus to > drivers/virt/visorbus. Um, are you thinking it is ready to be moved? Have you asked for another review? In a totally random chance, I was doing some driver core work today and I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you have 2 tabs for your 'struct attribute' variables, which is really odd. Also, you should be using the ATTRIBUTE_GROUPS() macro for them instead of having to "open code" the struct attribute_group lists. So either you all have horrible luck in that I just happened to find the only remaining problem, or that you should proabably ask for a good code audit, I haven't looked at the code before today since the last round of "fun" I found in just one other random file :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus
On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> On Mon, Jun 05, 2017 at 04:07:29PM -0400, David Kershner wrote:
> > This patchset moves drivers/staging/unisys/include to
> > include/linux/visorbus, and moves drivers/staging/unisys/visorbus to
> > drivers/virt/visorbus.
>
> Um, are you thinking it is ready to be moved? Have you asked for
> another review?
>
> In a totally random chance, I was doing some driver core work today and
> I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> have 2 tabs for your 'struct attribute' variables, which is really odd.
>
> Also, you should be using the ATTRIBUTE_GROUPS() macro for them instead
> of having to "open code" the struct attribute_group lists.
>
> So either you all have horrible luck in that I just happened to find the
> only remaining problem, or that you should proabably ask for a good code
> audit, I haven't looked at the code before today since the last round of
> "fun" I found in just one other random file :)
Also, many of the attribute callbacks in that file seem to all have
their leading '{' in the wrong place. Odd that checkpatch.pl doesn't
catch that...
partition_handle_show() is one such example that is obviously wrong.
There's also one checkpatch.pl warning for it, which should probably be
resolved as well.
I strongly suggest you go through everything again on your own...
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus
On Tue, Jun 06, 2017 at 04:53:22PM +0200, Greg KH wrote:
> On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > On Mon, Jun 05, 2017 at 04:07:29PM -0400, David Kershner wrote:
> > > This patchset moves drivers/staging/unisys/include to
> > > include/linux/visorbus, and moves drivers/staging/unisys/visorbus to
> > > drivers/virt/visorbus.
> >
> > Um, are you thinking it is ready to be moved? Have you asked for
> > another review?
> >
> > In a totally random chance, I was doing some driver core work today and
> > I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> > have 2 tabs for your 'struct attribute' variables, which is really odd.
> >
> > Also, you should be using the ATTRIBUTE_GROUPS() macro for them instead
> > of having to "open code" the struct attribute_group lists.
> >
> > So either you all have horrible luck in that I just happened to find the
> > only remaining problem, or that you should proabably ask for a good code
> > audit, I haven't looked at the code before today since the last round of
> > "fun" I found in just one other random file :)
>
> Also, many of the attribute callbacks in that file seem to all have
> their leading '{' in the wrong place. Odd that checkpatch.pl doesn't
> catch that...
>
> partition_handle_show() is one such example that is obviously wrong.
>
> There's also one checkpatch.pl warning for it, which should probably be
> resolved as well.
drivers/staging/unisys/visorbus/visorbus_main.c:1035: WARNING: Prefer using
'"%s...", __func__' to using 'create_bus_instance', this function's name, in a
string
to be specific, something you should have caught, right?
Are you sure this is ready to be moved out of staging? :(
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus
On Tue, Jun 06, 2017 at 04:54:30PM +0200, Greg KH wrote:
> On Tue, Jun 06, 2017 at 04:53:22PM +0200, Greg KH wrote:
> > On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > > On Mon, Jun 05, 2017 at 04:07:29PM -0400, David Kershner wrote:
> > > > This patchset moves drivers/staging/unisys/include to
> > > > include/linux/visorbus, and moves drivers/staging/unisys/visorbus to
> > > > drivers/virt/visorbus.
> > >
> > > Um, are you thinking it is ready to be moved? Have you asked for
> > > another review?
> > >
> > > In a totally random chance, I was doing some driver core work today and
> > > I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> > > have 2 tabs for your 'struct attribute' variables, which is really odd.
> > >
> > > Also, you should be using the ATTRIBUTE_GROUPS() macro for them instead
> > > of having to "open code" the struct attribute_group lists.
> > >
> > > So either you all have horrible luck in that I just happened to find the
> > > only remaining problem, or that you should proabably ask for a good code
> > > audit, I haven't looked at the code before today since the last round of
> > > "fun" I found in just one other random file :)
> >
> > Also, many of the attribute callbacks in that file seem to all have
> > their leading '{' in the wrong place. Odd that checkpatch.pl doesn't
> > catch that...
> >
> > partition_handle_show() is one such example that is obviously wrong.
> >
> > There's also one checkpatch.pl warning for it, which should probably be
> > resolved as well.
>
> drivers/staging/unisys/visorbus/visorbus_main.c:1035: WARNING: Prefer using
> '"%s...", __func__' to using 'create_bus_instance', this function's name, in
> a string
>
> to be specific, something you should have caught, right?
>
> Are you sure this is ready to be moved out of staging? :(
Eek, I can't look away...
You do this a bunch:
if (dev->visorchannel) {
visorchannel_destroy(dev->visorchannel);
yet the first thing that visorchannel_destroy() does is check for null.
So, no need to test this twice, right, only do so in the function, that
will make your code flow a lot "smoother" where ever you are calling
this.
Ok, I'll stop now, gotta go find some dinner...
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus
On Tue, 2017-06-06 at 16:53 +0200, Greg KH wrote:
> On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> > have 2 tabs for your 'struct attribute' variables, which is really odd.
[]
> Also, many of the attribute callbacks in that file seem to all have
> their leading '{' in the wrong place. Odd that checkpatch.pl doesn't
> catch that...
checkpatch doesn't really check much about inconsistent
indentation. I believe the only new statement indentation
check is after an if.
For instance, checkpatch doesn't emit a warning on this code:
struct foo {
int bar;
};
struct foo *alloc_foo(void)
{
struct foo *baz = malloc(sizeof(struct foo));
if (baz)
baz->bar = 1;
return baz;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus
On Tue, Jun 06, 2017 at 08:33:49AM -0700, Joe Perches wrote:
> On Tue, 2017-06-06 at 16:53 +0200, Greg KH wrote:
> > On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > > I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> > > have 2 tabs for your 'struct attribute' variables, which is really odd.
> []
> > Also, many of the attribute callbacks in that file seem to all have
> > their leading '{' in the wrong place. Odd that checkpatch.pl doesn't
> > catch that...
>
> checkpatch doesn't really check much about inconsistent
> indentation. I believe the only new statement indentation
> check is after an if.
>
> For instance, checkpatch doesn't emit a warning on this code:
>
> struct foo {
> int bar;
> };
>
> struct foo *alloc_foo(void)
> {
> struct foo *baz = malloc(sizeof(struct foo));
> if (baz)
> baz->bar = 1;
> return baz;
> }
Ok, but the following code in that file should be caught, right:
static ssize_t partition_handle_show(struct device *dev,
struct device_attribute *attr,
char *buf) {
struct visor_device *vdev = to_visor_device(dev);
u64 handle = visorchannel_get_clientpartition(vdev->visorchannel);
return sprintf(buf, "0x%llx\n", handle);
}
static DEVICE_ATTR_RO(partition_handle);
The initial { is in the wrong place...
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus
On Tue, 2017-06-06 at 17:39 +0200, Greg KH wrote:
> On Tue, Jun 06, 2017 at 08:33:49AM -0700, Joe Perches wrote:
> > On Tue, 2017-06-06 at 16:53 +0200, Greg KH wrote:
> > > On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > > > I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> > > > have 2 tabs for your 'struct attribute' variables, which is really odd.
> >
> > []
> > > Also, many of the attribute callbacks in that file seem to all have
> > > their leading '{' in the wrong place. Odd that checkpatch.pl doesn't
> > > catch that...
[]
> the following code in that file should be caught, right:
>
> static ssize_t partition_handle_show(struct device *dev,
> struct device_attribute *attr,
> char *buf) {
> struct visor_device *vdev = to_visor_device(dev);
> u64 handle = visorchannel_get_clientpartition(vdev->visorchannel);
>
> return sprintf(buf, "0x%llx\n", handle);
> }
> static DEVICE_ATTR_RO(partition_handle);
Not really.
> The initial { is in the wrong place...
True.
Please understand that checkpatch looks at patches one line
at a time. It's not very smart about function definitions
or context.
checkpatch's function definition code is pretty limited.
It can miss a lot of style misuses.
Single line function definitions brace tests work well.
Multiple line function definitions do not.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 6/7] mm, oom: cgroup-aware OOM killer
On Sun, Jun 04, 2017 at 11:43:33PM +0300, Vladimir Davydov wrote:
> On Thu, Jun 01, 2017 at 07:35:14PM +0100, Roman Gushchin wrote:
> ...
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index f979ac7..855d335 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -2625,6 +2625,184 @@ static inline bool memcg_has_children(struct
> > mem_cgroup *memcg)
> > return ret;
> > }
> >
> > +static long mem_cgroup_oom_badness(struct mem_cgroup *memcg,
> > + const nodemask_t *nodemask)
> > +{
> > + long points = 0;
> > + int nid;
> > + struct mem_cgroup *iter;
> > +
> > + for_each_mem_cgroup_tree(iter, memcg) {
>
> AFAIU this function is called on every iteration over the cgroup tree,
> which might be costly in case of a deep hierarchy, as it has quadratic
> complexity at worst. We could eliminate the nested loop by computing
> badness of all eligible cgroups before starting looking for a victim and
> saving the values in struct mem_cgroup. Not sure if it's worth it, as
> OOM is a pretty cold path.
I've thought about it, but it really not obvious that we want to pay
with some additional memory usage (and code complexity) for optimization
of this path. So, I decided to leave it simple now, and postpone
all optimizations after we'll agree on everything else.
>
> > + for_each_node_state(nid, N_MEMORY) {
> > + if (nodemask && !node_isset(nid, *nodemask))
> > + continue;
> > +
> > + points += mem_cgroup_node_nr_lru_pages(iter, nid,
> > + LRU_ALL_ANON | BIT(LRU_UNEVICTABLE));
>
> Hmm, is there a reason why we shouldn't take into account file pages?
Because under the OOM conditions we should not have too much pagecache,
and killing a process will unlikely help us to release any additional memory.
But maybe I'm missing something... Lazy free?
>
> > + }
> > +
> > + points += mem_cgroup_get_nr_swap_pages(iter);
>
> AFAICS mem_cgroup_get_nr_swap_pages() returns the number of pages that
> can still be charged to the cgroup. IIUC we want to account pages that
> have already been charged to the cgroup, i.e. the value of the 'swap'
> page counter or MEMCG_SWAP stat counter.
Ok, I'll check it. Thank you!
>
> > + points += memcg_page_state(iter, MEMCG_KERNEL_STACK_KB) /
> > + (PAGE_SIZE / 1024);
> > + points += memcg_page_state(iter, MEMCG_SLAB_UNRECLAIMABLE);
> > + points += memcg_page_state(iter, MEMCG_SOCK);
> > + }
> > +
> > + return points;
> > +}
> > +
> > +bool mem_cgroup_select_oom_victim(struct oom_control *oc)
> > +{
> > + struct cgroup_subsys_state *css = NULL;
> > + struct mem_cgroup *iter = NULL;
> > + struct mem_cgroup *chosen_memcg = NULL;
> > + struct mem_cgroup *parent = root_mem_cgroup;
> > + unsigned long totalpages = oc->totalpages;
> > + long chosen_memcg_points = 0;
> > + long points = 0;
> > +
> > + oc->chosen = NULL;
> > + oc->chosen_memcg = NULL;
> > +
> > + if (mem_cgroup_disabled())
> > + return false;
> > +
> > + if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
> > + return false;
> > +
> > + pr_info("Choosing a victim memcg because of the %s",
> > + oc->memcg ?
> > + "memory limit reached of cgroup " :
> > + "system-wide OOM\n");
> > + if (oc->memcg) {
> > + pr_cont_cgroup_path(oc->memcg->css.cgroup);
> > + pr_cont("\n");
> > +
> > + chosen_memcg = oc->memcg;
> > + parent = oc->memcg;
> > + }
> > +
> > + rcu_read_lock();
> > +
> > + for (;;) {
> > + css = css_next_child(css, &parent->css);
> > + if (css) {
> > + iter = mem_cgroup_from_css(css);
> > +
> > + points = mem_cgroup_oom_badness(iter, oc->nodemask);
> > + points += iter->oom_score_adj * (totalpages / 1000);
> > +
> > + pr_info("Cgroup ");
> > + pr_cont_cgroup_path(iter->css.cgroup);
> > + pr_cont(": %ld\n", points);
>
> Not sure if everyone wants to see these messages in the log.
What do you suggest? Remove debug output at all (probably, we still want some),
ratelimit it, make optional?
>
> > +
> > + if (points > chosen_memcg_points) {
> > + chosen_memcg = iter;
> > + chosen_memcg_points = points;
> > + oc->chosen_points = points;
> > + }
> > +
> > + continue;
> > + }
> > +
> > + if (chosen_memcg && !chosen_memcg->oom_kill_all_tasks) {
> > + /* Go deeper in the cgroup hierarchy */
> > + totalpages = chosen_memcg_points;
>
> We set 'totalpages' to the target cgroup limit (or the total RAM
> size) when computing a victim score. Why do you prefer to use
> chosen_memcg_p
Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus
On Tue, Jun 06, 2017 at 08:52:27AM -0700, Joe Perches wrote:
> On Tue, 2017-06-06 at 17:39 +0200, Greg KH wrote:
> > On Tue, Jun 06, 2017 at 08:33:49AM -0700, Joe Perches wrote:
> > > On Tue, 2017-06-06 at 16:53 +0200, Greg KH wrote:
> > > > On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > > > > I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> > > > > have 2 tabs for your 'struct attribute' variables, which is really
> > > > > odd.
> > >
> > > []
> > > > Also, many of the attribute callbacks in that file seem to all have
> > > > their leading '{' in the wrong place. Odd that checkpatch.pl doesn't
> > > > catch that...
> []
> > the following code in that file should be caught, right:
> >
> > static ssize_t partition_handle_show(struct device *dev,
> > struct device_attribute *attr,
> > char *buf) {
> > struct visor_device *vdev = to_visor_device(dev);
> > u64 handle = visorchannel_get_clientpartition(vdev->visorchannel);
> >
> > return sprintf(buf, "0x%llx\n", handle);
> > }
> > static DEVICE_ATTR_RO(partition_handle);
>
> Not really.
>
> > The initial { is in the wrong place...
>
> True.
>
> Please understand that checkpatch looks at patches one line
> at a time. It's not very smart about function definitions
> or context.
>
> checkpatch's function definition code is pretty limited.
> It can miss a lot of style misuses.
>
> Single line function definitions brace tests work well.
> Multiple line function definitions do not.
Ok, that makes sense why this is missed. No big deal, a simple visual
inspection shows stuff like this up really easily, which obviously no
one did yet on this file :)
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/7] Add kselftest_harness.h
On 06/05/2017 12:37 PM, Mickaël Salaün wrote:
> Hi,
>
> This patch series make the seccomp/test_harness.h more generally available [1]
> and update the kselftest documentation in the Sphinx format. It also improve
> the Makefile of seccomp tests to take into account any kselftest_harness.h
> update.
>
> [1]
> https://lkml.kernel.org/r/CAGXu5j+8CVz8vL51DRYXqOY=xc3zuKFf=ptene88xyhzfyi...@mail.gmail.com
>
> Regards,
>
> Mickaël Salaün (7):
> selftests: Make test_harness.h more generally available
> selftests: Cosmetic renames in kselftest_harness.h
> selftests/seccomp: Force rebuild according to dependencies
> Documentation/dev-tools: Add kselftest
> Documentation/dev-tools: Use reStructuredText markups for kselftest
> selftests: Remove the TEST_API() wrapper from kselftest_harness.h
> Documentation/dev-tools: Add kselftest_harness documentation
>
> Documentation/00-INDEX | 2 -
> Documentation/dev-tools/index.rst | 1 +
> .../{kselftest.txt => dev-tools/kselftest.rst} | 100 ++-
> MAINTAINERS| 1 +
> .../test_harness.h => kselftest_harness.h} | 691
> +
> tools/testing/selftests/seccomp/Makefile | 2 +
> tools/testing/selftests/seccomp/seccomp_bpf.c | 2 +-
> 7 files changed, 519 insertions(+), 280 deletions(-)
> rename Documentation/{kselftest.txt => dev-tools/kselftest.rst} (52%)
> rename tools/testing/selftests/{seccomp/test_harness.h =>
> kselftest_harness.h} (52%)
>
Applied the series to linux-kselftest next for 4.13-rc1
thanks for the series.
-- Shuah
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller
On 06/06/17 8:33 AM, Guenter Roeck wrote: On 06/06/2017 12:02 AM, Andrew Jeffery wrote: Add a basic driver for the MAX31785, focusing on the fan control features but ignoring the temperature and voltage monitoring features of the device. This driver supports all fan control modes and tachometer / PWM readback where applicable. Signed-off-by: Timothy Pearson Signed-off-by: Andrew Jeffery --- Hello, This is a rework of Timothy Pearson's original patch: https://www.mail-archive.com/linux-hwmon@vger.kernel.org/msg00868.html I've labelled it as v3 to differentiate from Timothy's postings. The original thread had some discussion about the MAX31785 being a PMBus device and that it should thus be a PMBus driver. The implementation still makes use of features not available in the pmbus core, so I've taken up the earlier suggestion and ported it to the devm_hwmon_device_register_with_info() interface. This gave a modest reduction in lines-of-code and at least to me is more aesthetically pleasing. Over and above the features of the original patch is support for a secondary rotor measurement value that is provided by MAX31785 chips with a revised firmware. The feature(s) of the firmware are determined at probe time and extra attributes exposed accordingly. Specifically, the MFR_REVISION 0x3040 of the firmware supports 'slow' and 'fast' rotor reads. The feature is implemented by command 0x90 (READ_FAN_SPEED_1) providing a 4-byte response (with the 'fast' measurement in the second word) rather than the 2-bytes response in the original firmware (MFR_REVISION 0x3030). Taking the pmbus driver question out, why would this warrant another non-standard attribute outside the ABI ? I could see the desire to replace the 'slow' access with the 'fast' one, but provide two attributes ? No, I don't see the point, sorry, even more so without detailed explanation why the second attribute in addition to the first one would add any value. In the case of counter-rotating(CR) fans which contain two rotors to provide more airflow there are then two tach feedbacks. These CR fans take a single target speed and provide individual feedbacks for each rotor contained within the fan enclosure. Providing these individual feedbacks assists in fan fault driven speed changes, improved thermal characterization among other things. Maxim provided this as a 'slow' / 'fast' set of bytes to be user compatible(so the 'slow' rotor speed, regardless of which rotor, is in the first 2 bytes with the 'slow' version of firmware as well). In some cases, mfg systems could have a mix of these revisions. This feature is not documented in the public datasheet[1]. [1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf The need to read a 4-byte value drives the addition of a helper that is a cut-down version of i2c_smbus_xfer_emulated(), as 4-byte transactions aren't a defined transaction type in the PMBus spec. This seemed more tasteful than hacking the PMBus core to support the quirks of a single device. That is why we have PMBus helper drivers. Guenter Also changed from Timothy's original posting is I've massaged the locking a bit and removed what seemed to be a copy/paste bug around max31785_fan_set_pulses() setting the fan_command member. Tested on an IBM Witherspoon machine. Cheers, Andrew Documentation/hwmon/max31785 | 44 +++ drivers/hwmon/Kconfig| 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/max31785.c | 824 +++ 4 files changed, 879 insertions(+) create mode 100644 Documentation/hwmon/max31785 create mode 100644 drivers/hwmon/max31785.c diff --git a/Documentation/hwmon/max31785 b/Documentation/hwmon/max31785 new file mode 100644 index ..dd891c06401e --- /dev/null +++ b/Documentation/hwmon/max31785 @@ -0,0 +1,44 @@ +Kernel driver max31785 +== + +Supported chips: + * Maxim MAX31785 +Prefix: 'max31785' +Addresses scanned: 0x52 0x53 0x54 0x55 +Datasheet: http://pdfserv.maximintegrated.com/en/ds/MAX31785.pdf + +Author: Timothy Pearson + + +Description +--- + +This driver implements support for the Maxim MAX31785 chip. + +The MAX31785 controls the speeds of up to six fans using six independent +PWM outputs. The desired fan speeds (or PWM duty cycles) are written +through the I2C interface. The outputs drive "4-wire" fans directly, +or can be used to modulate the fan's power terminals using an external +pass transistor. + +Tachometer inputs monitor fan tachometer logic outputs for precise (+/-1%) +monitoring and control of fan RPM as well as detection of fan failure. + + +Sysfs entries +- + +fan[1-6]_input RO fan tachometer speed in RPM +fan[1-6]_fault RO fan experienced fault +fan[1-6]_pulses RW tachometer pulses per fan revolution +fan[1-6]_target RW desired fan speed in RPM +pwm[1-6]_enable RW
Re: [RFC PATCH v2 1/7] mm, oom: refactor select_bad_process() to take memcg as an argument
On Sun, Jun 04, 2017 at 03:50:37PM -0700, David Rientjes wrote: > We use a heavily modified system and memcg oom killer and I'm wondering > if there is some opportunity for collaboration because we may have some > shared goals. > > I can summarize how we currently use the oom killer at a high level so > that it is not overwhelming with implementation details and give some > rationale for why we have converged onto this strategy over the period of > a few years. > > For victim selection, we have strict priority based oom killing both at > the memcg level and the process level. > > Each process has its own "badness" value that is independent of > oom_score_adj, although some conversion is done if a third-party thread > chooses to disable itself from oom killing for backwards compatibility. > Lower values are more preferable victims, but that detail should not > matter significantly. If two processes share the same badness value, > tiebreaks are done by selecting the largest rss. > > Each memcg in a hierarchy also has its own badness value which > semantically means the same as the per-process value, although it > considers the entire memcg as a unit, similar to your approach, when > iterating the hierarchy to choose a process. The benefit of the > per-memcg and per-process approach is that you can kill the lowest > priority process from the lowest priority memcg. > > The above scoring is enabled with a VM sysctl for the system and is used > for both system (global) and memcg oom kills. For system overcommit, > this means we can kill the lowest priority job on the system; for memcg, > we can allow users to define their oom kill priorities at each level of > their own hierarchy. > > When the system or root of an oom memcg hierarchy encounters its limit, > we iterate each level of the memcg hierarchy to find the lowest priority > job. This is done by comparing the badness of the sibling memcgs at > each level, finding the lowest, and iterating that subtree. If there are > lower priority processes per the per-process badness value compared to > all sibling memcgs, that process is killed. > > We also have complete userspace oom handling support. This complements > the existing memory.oom_control notification when a memcg is oom with a > separate notifier that notifies when the kernel has oom killed a process. > It is possible to delay the oom killer from killing a process for memcg > oom kills with a configurable, per-memcg, oom delay. If set, the kernel > will wait for userspace to respond to its oom notification and effect its > own policy decisions until memory is uncharged to that memcg hierarchy, > or the oom delay expires. If the oom delay expires, the kernel oom > killer kills a process based on badness. > > Our oom kill notification file used to get an fd to register with > cgroup.event_control also provides oom kill statistics based on system, > memcg, local, hierarchical, and user-induced oom kills when read(). > > We also have a convenient "kill all" knob that userspace can write when > handling oom conditions to iterate all threads attached to a particular > memcg and kill them. This is merely to prevent races where userspace > does the oom killing itself, which is not problematic in itself, but > additional tasks continue to be attached to an oom memcg. > > A caveat here is that we also support fully inclusive kmem accounting to > memcg hierarchies, so we call the oom killer as part of the memcg charge > path rather than only upon returning from fault with VM_FAULT_OOM. We > have our own oom killer livelock detection that isn't necessarily > important in this thread, but we haven't encountered a situation where we > livelock by calling the oom killer during charge, and this is a > requirement for memcg charging as part of slab allocation. > > I could post many patches to implement all of this functionality that we > have used for a few years, but I first wanted to send this email to see > if there is any common ground or to better understand your methodology > for using the kernel oom killer for both system and memcg oom kills. > > Otherwise, very interesting stuff! Hi David! Thank you for sharing this! It's very interesting, and it looks like, it's not that far from what I've suggested. So we definitily need to come up with some common solution. Thanks! Roman -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [xfstests PATCH v3 1/5] generic: add a writeback error handling test
On Tue, Jun 06, 2017 at 08:23:25PM +0800, Eryu Guan wrote: > On Tue, Jun 06, 2017 at 06:15:57AM -0400, Jeff Layton wrote: > > On Tue, 2017-06-06 at 16:58 +0800, Eryu Guan wrote: > > > On Wed, May 31, 2017 at 09:08:16AM -0400, Jeff Layton wrote: > > > > I'm working on a set of kernel patches to change how writeback errors > > > > are handled and reported in the kernel. Instead of reporting a > > > > writeback error to only the first fsync caller on the file, I aim > > > > to make the kernel report them once on every file description. > > > > > > > > This patch adds a test for the new behavior. Basically, open many fds > > > > to the same file, turn on dm_error, write to each of the fds, and then > > > > fsync them all to ensure that they all get an error back. > > > > > > > > To do that, I'm adding a new tools/dmerror script that the C program > > > > can use to load the error table. For now, that's all it can do, but > > > > we can fill it out with other commands as necessary. > > > > > > > > Signed-off-by: Jeff Layton > > > > > > Thanks for the new tests! And sorry for the late review.. > > > > > > It's testing a new behavior on error reporting on writeback, I'm not > > > sure if we can call it a new feature or it fixed a bug? But it's more > > > like a behavior change, I'm not sure how to categorize it. > > > > > > Because if it's testing a new feature, we usually let test do proper > > > detection of current test environment (based on actual behavior not > > > kernel version) and _notrun on filesystems that don't have this feature > > > yet, instead of failing the test; if it's testing a bug fix, we could > > > leave the test fail on unfixed filesystems, this also serves as a > > > reminder that there's bug to fix. > > > > > > > Thanks for the review! I'm not sure how to categorize this either. Since > > the plan is to convert all the filesystems piecemeal, maybe we should > > just consider it a new feature. > > Then we need a new _require rule to properly detect for the 'feature' > support. I'm not sure if this is doable, but something like > _require_statx, _require_seek_data_hole would be good. > > > > > > I pulled your test kernel tree, and test passed on EXT4 but failed on > > > other local filesystems (XFS, btrfs). I assume that's expected. > > > > > > Besides this kinda high-level question, some minor comments inline. > > > > > > > Yes, ext4 should pass on my latest kernel tree, but everything else > > should fail. > > With the new _require rule, test should _notrun on XFS and btrfs then. Frankly I personally prefer that upstream XFS fails until someone fixes it. :) (But that's just my opinion.) That said, I'm not 100% sure what's required of XFS to play nicely with this new mechanism -- glancing at the ext* patches it looks like we'd need to set a fs flag and possibly change some or all of the "write cached dirty buffers out to disk" calls to their _since variants? Metadata writeback errors are handled by retrying writes and/or shutting down the fs, so I think the f_md_wb_error case is already covered. That said, I agree that it's useful to detect that the kernel simply lacks any of the new wb error reporting at all, so therefore we can skip the tests. --D > > > > > > > --- > > > > common/dmerror | 13 ++-- > > > > doc/auxiliary-programs.txt | 8 +++ > > > > src/Makefile | 2 +- > > > > src/fsync-err.c| 161 > > > > + > > > > > > New binary needs an entry in .gitignore file. > > > > > > > OK, thanks. Will fix. > > > > > > tests/generic/999 | 76 + > > > > tests/generic/999.out | 3 + > > > > tests/generic/group| 1 + > > > > tools/dmerror | 44 + > > > > > > This file is used by the test, then it should be in src/ directory and > > > be installed along with other executable files on "make install". > > > Because files under tools/ are not installed. Most people will run tests > > > in the root dir of xfstests and this is not a problem, but there're > > > still cases people do "make && make install" and run fstests from > > > /var/lib/xfstests (default installation target). > > > > > > > Ok, no problem. I'll move it. I wasn't sure here since dmerror is a > > shell script, and most of the stuff in src/ is stuff that needs to be > > built. > > We do have a few perl or shell scripts in src/ dir, we can see this from > src/Makefile > > $(LTINSTALL) -m 755 fill2attr fill2fs fill2fs_check scaleread.sh > $(PKG_LIB_DIR)/src > > > > > > > 8 files changed, 302 insertions(+), 6 deletions(-) > > > > create mode 100644 src/fsync-err.c > > > > create mode 100755 tests/generic/999 > > > > create mode 100644 tests/generic/999.out > > > > create mode 100755 tools/dmerror > > > > > > > > diff --git a/common/dmerror b/common/dmerror > > > > index d46c5d0b7266..238baa213b1f 100644 > > > > --- a/common/dmerror > > >
[PATCH v3 3/3] perf: xgene: Add support for SoC PMU version 3
This patch adds support for SoC-wide (AKA uncore) Performance Monitoring
Unit version 3.
It can support up to
- 2 IOB PMU instances
- 8 L3C PMU instances
- 2 MCB PMU instances
- 8 MCU PMU instances
and these PMUs support 64 bit counter
Signed-off-by: Hoan Tran
---
drivers/perf/xgene_pmu.c | 558 ---
1 file changed, 529 insertions(+), 29 deletions(-)
diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
index f34fc78..84c32e0 100644
--- a/drivers/perf/xgene_pmu.c
+++ b/drivers/perf/xgene_pmu.c
@@ -37,6 +37,8 @@
#define CSW_CSWCR 0x
#define CSW_CSWCR_DUALMCB_MASK BIT(0)
+#define CSW_CSWCR_MCB0_ROUTING(x) (((x) & 0x0C) >> 2)
+#define CSW_CSWCR_MCB1_ROUTING(x) (((x) & 0x30) >> 4)
#define MCBADDRMR 0x
#define MCBADDRMR_DUALMCU_MODE_MASKBIT(2)
@@ -50,8 +52,17 @@
#define PCPPMU_INT_L3CBIT(2)
#define PCPPMU_INT_IOBBIT(3)
+#define PCPPMU_V3_INTMASK 0x00FF33FF
+#define PCPPMU_V3_INTENMASK 0x
+#define PCPPMU_V3_INTCLRMASK 0xFF00CC00
+#define PCPPMU_V3_INT_MCU 0x00FF
+#define PCPPMU_V3_INT_MCB 0x0300
+#define PCPPMU_V3_INT_L3C 0x00FF
+#define PCPPMU_V3_INT_IOB 0x3000
+
#define PMU_MAX_COUNTERS 4
-#define PMU_CNT_MAX_PERIOD 0x1ULL
+#define PMU_CNT_MAX_PERIOD 0xULL
+#define PMU_V3_CNT_MAX_PERIOD 0xULL
#define PMU_OVERFLOW_MASK 0xF
#define PMU_PMCR_E BIT(0)
#define PMU_PMCR_P BIT(1)
@@ -73,6 +84,10 @@
#define PMU_PMOVSR 0xC80
#define PMU_PMCR 0xE04
+/* PMU registers for V3 */
+#define PMU_PMOVSCLR 0xC80
+#define PMU_PMOVSSET 0xCC0
+
#define to_pmu_dev(p) container_of(p, struct xgene_pmu_dev, pmu)
#define GET_CNTR(ev) (ev->hw.idx)
#define GET_EVENTID(ev) (ev->hw.config & 0xFFULL)
@@ -119,6 +134,7 @@ struct xgene_pmu {
void __iomem *pcppmu_csr;
u32 mcb_active_mask;
u32 mc_active_mask;
+ u32 l3c_active_mask;
cpumask_t cpu;
raw_spinlock_t lock;
const struct xgene_pmu_ops *ops;
@@ -143,11 +159,13 @@ struct xgene_pmu_data {
enum xgene_pmu_version {
PCP_PMU_V1 = 1,
PCP_PMU_V2,
+ PCP_PMU_V3,
};
enum xgene_pmu_dev_type {
PMU_TYPE_L3C = 0,
PMU_TYPE_IOB,
+ PMU_TYPE_IOB_SLOW,
PMU_TYPE_MCB,
PMU_TYPE_MC,
};
@@ -213,6 +231,56 @@ static ssize_t xgene_pmu_format_show(struct device *dev,
.attrs = mc_pmu_format_attrs,
};
+static struct attribute *l3c_pmu_v3_format_attrs[] = {
+ XGENE_PMU_FORMAT_ATTR(l3c_eventid, "config:0-39"),
+ NULL,
+};
+
+static struct attribute *iob_pmu_v3_format_attrs[] = {
+ XGENE_PMU_FORMAT_ATTR(iob_eventid, "config:0-47"),
+ NULL,
+};
+
+static struct attribute *iob_slow_pmu_v3_format_attrs[] = {
+ XGENE_PMU_FORMAT_ATTR(iob_slow_eventid, "config:0-16"),
+ NULL,
+};
+
+static struct attribute *mcb_pmu_v3_format_attrs[] = {
+ XGENE_PMU_FORMAT_ATTR(mcb_eventid, "config:0-35"),
+ NULL,
+};
+
+static struct attribute *mc_pmu_v3_format_attrs[] = {
+ XGENE_PMU_FORMAT_ATTR(mc_eventid, "config:0-44"),
+ NULL,
+};
+
+static const struct attribute_group l3c_pmu_v3_format_attr_group = {
+ .name = "format",
+ .attrs = l3c_pmu_v3_format_attrs,
+};
+
+static const struct attribute_group iob_pmu_v3_format_attr_group = {
+ .name = "format",
+ .attrs = iob_pmu_v3_format_attrs,
+};
+
+static const struct attribute_group iob_slow_pmu_v3_format_attr_group = {
+ .name = "format",
+ .attrs = iob_slow_pmu_v3_format_attrs,
+};
+
+static const struct attribute_group mcb_pmu_v3_format_attr_group = {
+ .name = "format",
+ .attrs = mcb_pmu_v3_format_attrs,
+};
+
+static const struct attribute_group mc_pmu_v3_format_attr_group = {
+ .name = "format",
+ .attrs = mc_pmu_v3_format_attrs,
+};
+
/*
* sysfs event attributes
*/
@@ -329,6 +397,219 @@ static ssize_t xgene_pmu_event_show(struct device *dev,
.attrs = mc_pmu_events_attrs,
};
+static struct attribute *l3c_pmu_v3_events_attrs[] = {
+ XGENE_PMU_EVENT_ATTR(cycle-count, 0x00),
+ XGENE_PMU_EVENT_ATTR(read-hit, 0x01),
+ XGENE_PMU_EVENT_ATTR(read-miss, 0x02),
+ XGENE_PMU_EVENT_ATTR(index-flush-eviction, 0x03),
+ XGENE_PMU_EVENT_ATTR(write-caused-replacement, 0x04),
+ XGENE_PMU_EVENT_ATTR(write-not-caused-replacement, 0x05),
+ XGENE_PMU_EVENT_ATTR(clean-eviction,0x06),
+ XGENE_PMU_EVENT_ATTR(dirty-eviction,0x07),
+ XGENE_PMU_EVENT_ATTR(read, 0x08),
+ XGENE_PMU_EVENT_ATTR(write, 0x09),
+ XGENE_PMU_EVENT_
[PATCH v3 0/3] perf: xgene: Add support for SoC PMU version 3
This patch set adds support for SoC-wide (AKA uncore) Performance Monitoring Unit version 3. It can support up to - 2 IOB PMU instances - 8 L3C PMU instances - 2 MCB PMU instances - 8 MCU PMU instances and these PMUs support 64 bit counter v3: * Seperate acpi_pmu_probe_active_mcb_mcu_l3c for v3 * Consistent PMU event name * Update comment * Correct active MCU detection v2: * Split into separate patches * Use the function pointers for the PMU leaf functions * Parse PMU subnode by the match table * Dont allow user change agent id by config1 for SoC PMU v3 Hoan Tran (3): perf: xgene: Parse PMU subnode from the match table perf: xgene: Move PMU leaf functions into function pointer structure perf: xgene: Add support for SoC PMU version 3 drivers/perf/xgene_pmu.c | 677 +++ 1 file changed, 622 insertions(+), 55 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/3] perf: xgene: Move PMU leaf functions into function pointer structure
This patch moves PMU leaf functions into a function pointer structure.
It helps code maintain and expasion easier.
Signed-off-by: Hoan Tran
---
drivers/perf/xgene_pmu.c | 85 +---
1 file changed, 66 insertions(+), 19 deletions(-)
diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
index 5ffd580..f34fc78 100644
--- a/drivers/perf/xgene_pmu.c
+++ b/drivers/perf/xgene_pmu.c
@@ -96,6 +96,23 @@ struct xgene_pmu_dev {
struct perf_event *pmu_counter_event[PMU_MAX_COUNTERS];
};
+struct xgene_pmu_ops {
+ void (*mask_int)(struct xgene_pmu *pmu);
+ void (*unmask_int)(struct xgene_pmu *pmu);
+ u64 (*read_counter)(struct xgene_pmu_dev *pmu, int idx);
+ void (*write_counter)(struct xgene_pmu_dev *pmu, int idx, u64 val);
+ void (*write_evttype)(struct xgene_pmu_dev *pmu_dev, int idx, u32 val);
+ void (*write_agentmsk)(struct xgene_pmu_dev *pmu_dev, u32 val);
+ void (*write_agent1msk)(struct xgene_pmu_dev *pmu_dev, u32 val);
+ void (*enable_counter)(struct xgene_pmu_dev *pmu_dev, int idx);
+ void (*disable_counter)(struct xgene_pmu_dev *pmu_dev, int idx);
+ void (*enable_counter_int)(struct xgene_pmu_dev *pmu_dev, int idx);
+ void (*disable_counter_int)(struct xgene_pmu_dev *pmu_dev, int idx);
+ void (*reset_counters)(struct xgene_pmu_dev *pmu_dev);
+ void (*start_counters)(struct xgene_pmu_dev *pmu_dev);
+ void (*stop_counters)(struct xgene_pmu_dev *pmu_dev);
+};
+
struct xgene_pmu {
struct device *dev;
int version;
@@ -104,6 +121,7 @@ struct xgene_pmu {
u32 mc_active_mask;
cpumask_t cpu;
raw_spinlock_t lock;
+ const struct xgene_pmu_ops *ops;
struct list_head l3cpmus;
struct list_head iobpmus;
struct list_head mcbpmus;
@@ -392,13 +410,14 @@ static inline void xgene_pmu_unmask_int(struct xgene_pmu
*xgene_pmu)
writel(PCPPMU_INTCLRMASK, xgene_pmu->pcppmu_csr + PCPPMU_INTMASK_REG);
}
-static inline u32 xgene_pmu_read_counter(struct xgene_pmu_dev *pmu_dev, int
idx)
+static inline u64 xgene_pmu_read_counter32(struct xgene_pmu_dev *pmu_dev,
+ int idx)
{
- return readl(pmu_dev->inf->csr + PMU_PMEVCNTR0 + (4 * idx));
+ return (u64)readl(pmu_dev->inf->csr + PMU_PMEVCNTR0 + (4 * idx));
}
static inline void
-xgene_pmu_write_counter(struct xgene_pmu_dev *pmu_dev, int idx, u32 val)
+xgene_pmu_write_counter32(struct xgene_pmu_dev *pmu_dev, int idx, u64 val)
{
writel(val, pmu_dev->inf->csr + PMU_PMEVCNTR0 + (4 * idx));
}
@@ -491,20 +510,22 @@ static inline void xgene_pmu_stop_counters(struct
xgene_pmu_dev *pmu_dev)
static void xgene_perf_pmu_enable(struct pmu *pmu)
{
struct xgene_pmu_dev *pmu_dev = to_pmu_dev(pmu);
+ struct xgene_pmu *xgene_pmu = pmu_dev->parent;
int enabled = bitmap_weight(pmu_dev->cntr_assign_mask,
pmu_dev->max_counters);
if (!enabled)
return;
- xgene_pmu_start_counters(pmu_dev);
+ xgene_pmu->ops->start_counters(pmu_dev);
}
static void xgene_perf_pmu_disable(struct pmu *pmu)
{
struct xgene_pmu_dev *pmu_dev = to_pmu_dev(pmu);
+ struct xgene_pmu *xgene_pmu = pmu_dev->parent;
- xgene_pmu_stop_counters(pmu_dev);
+ xgene_pmu->ops->stop_counters(pmu_dev);
}
static int xgene_perf_event_init(struct perf_event *event)
@@ -572,27 +593,32 @@ static int xgene_perf_event_init(struct perf_event *event)
static void xgene_perf_enable_event(struct perf_event *event)
{
struct xgene_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
+ struct xgene_pmu *xgene_pmu = pmu_dev->parent;
- xgene_pmu_write_evttype(pmu_dev, GET_CNTR(event), GET_EVENTID(event));
- xgene_pmu_write_agentmsk(pmu_dev, ~((u32)GET_AGENTID(event)));
+ xgene_pmu->ops->write_evttype(pmu_dev, GET_CNTR(event),
+ GET_EVENTID(event));
+ xgene_pmu->ops->write_agentmsk(pmu_dev, ~((u32)GET_AGENTID(event)));
if (pmu_dev->inf->type == PMU_TYPE_IOB)
- xgene_pmu_write_agent1msk(pmu_dev, ~((u32)GET_AGENT1ID(event)));
+ xgene_pmu->ops->write_agent1msk(pmu_dev,
+ ~((u32)GET_AGENT1ID(event)));
- xgene_pmu_enable_counter(pmu_dev, GET_CNTR(event));
- xgene_pmu_enable_counter_int(pmu_dev, GET_CNTR(event));
+ xgene_pmu->ops->enable_counter(pmu_dev, GET_CNTR(event));
+ xgene_pmu->ops->enable_counter_int(pmu_dev, GET_CNTR(event));
}
static void xgene_perf_disable_event(struct perf_event *event)
{
struct xgene_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
+ struct xgene_pmu *xgene_pmu = pmu_dev->parent;
- xgene_pmu_disable_counter(pmu_dev, GET_CNTR(event));
- xgene_pmu_disable_counter_int(pmu_dev, GET_CNTR(event));
+ xgene_pmu->ops->disable_counter(pmu_dev,
[PATCH v3 1/3] perf: xgene: Parse PMU subnode from the match table
This patch parses PMU Subnode from a match table.
Signed-off-by: Hoan Tran
---
drivers/perf/xgene_pmu.c | 40 ++--
1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
index 35b5289..5ffd580 100644
--- a/drivers/perf/xgene_pmu.c
+++ b/drivers/perf/xgene_pmu.c
@@ -1047,9 +1047,35 @@ xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu
*xgene_pmu,
return NULL;
}
+static const struct acpi_device_id xgene_pmu_acpi_type_match[] = {
+ {"APMC0D5D", PMU_TYPE_L3C},
+ {"APMC0D5E", PMU_TYPE_IOB},
+ {"APMC0D5F", PMU_TYPE_MCB},
+ {"APMC0D60", PMU_TYPE_MC},
+ {},
+};
+
+static const struct acpi_device_id *xgene_pmu_acpi_match_type(
+ const struct acpi_device_id *ids,
+ struct acpi_device *adev)
+{
+ const struct acpi_device_id *match_id = NULL;
+ const struct acpi_device_id *id;
+
+ for (id = ids; id->id[0] || id->cls; id++) {
+ if (!acpi_match_device_ids(adev, id))
+ match_id = id;
+ else if (match_id)
+ break;
+ }
+
+ return match_id;
+}
+
static acpi_status acpi_pmu_dev_add(acpi_handle handle, u32 level,
void *data, void **return_value)
{
+ const struct acpi_device_id *acpi_id;
struct xgene_pmu *xgene_pmu = data;
struct xgene_pmu_dev_ctx *ctx;
struct acpi_device *adev;
@@ -1059,17 +1085,11 @@ static acpi_status acpi_pmu_dev_add(acpi_handle handle,
u32 level,
if (acpi_bus_get_status(adev) || !adev->status.present)
return AE_OK;
- if (!strcmp(acpi_device_hid(adev), "APMC0D5D"))
- ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_L3C);
- else if (!strcmp(acpi_device_hid(adev), "APMC0D5E"))
- ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_IOB);
- else if (!strcmp(acpi_device_hid(adev), "APMC0D5F"))
- ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_MCB);
- else if (!strcmp(acpi_device_hid(adev), "APMC0D60"))
- ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_MC);
- else
- ctx = NULL;
+ acpi_id = xgene_pmu_acpi_match_type(xgene_pmu_acpi_type_match, adev);
+ if (!acpi_id)
+ return AE_OK;
+ ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, (u32)acpi_id->driver_data);
if (!ctx)
return AE_OK;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus
> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, June 6, 2017 11:06 AM
> To: Kershner, David A
> Cc: cor...@lwn.net; t...@linutronix.de; mi...@kernel.org; akpm@linux-
> foundation.org; jes.soren...@gmail.com; linux-ker...@vger.kernel.org;
> linux-doc@vger.kernel.org; driverdev-de...@linuxdriverproject.org; *S-Par-
> Maintainer
> Subject: Re: [PATCH 0/3] move visorbus out of staging to
> drivers/virt/visorbus
>
> On Tue, Jun 06, 2017 at 04:54:30PM +0200, Greg KH wrote:
> > On Tue, Jun 06, 2017 at 04:53:22PM +0200, Greg KH wrote:
> > > On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > > > On Mon, Jun 05, 2017 at 04:07:29PM -0400, David Kershner wrote:
> > > > > This patchset moves drivers/staging/unisys/include to
> > > > > include/linux/visorbus, and moves drivers/staging/unisys/visorbus to
> > > > > drivers/virt/visorbus.
> > > >
> > > > Um, are you thinking it is ready to be moved? Have you asked for
> > > > another review?
> > > >
Thank you for taking a quick look at our patch series. Part of the motivation
behind this submission was, in fact, to initiate another code review. What is
the formal procedure for initiating a code review?
> > > > In a totally random chance, I was doing some driver core work today
> and
> > > > I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> > > > have 2 tabs for your 'struct attribute' variables, which is really odd.
> > > >
Sorry I missed that; I guess my eyes glazed over by the time I got to that file,
and I was expecting checkpatch to catch that. Now I know better, and I will be
looking for more things. Thanks for catching.
> > > > Also, you should be using the ATTRIBUTE_GROUPS() macro for them
> instead
> > > > of having to "open code" the struct attribute_group lists.
> > > >
> > > > So either you all have horrible luck in that I just happened to find the
> > > > only remaining problem, or that you should proabably ask for a good
> code
> > > > audit, I haven't looked at the code before today since the last round of
> > > > "fun" I found in just one other random file :)
> > >
> > > Also, many of the attribute callbacks in that file seem to all have
> > > their leading '{' in the wrong place. Odd that checkpatch.pl doesn't
> > > catch that...
> > >
> > > partition_handle_show() is one such example that is obviously wrong.
> > >
> > > There's also one checkpatch.pl warning for it, which should probably be
> > > resolved as well.
> >
> > drivers/staging/unisys/visorbus/visorbus_main.c:1035: WARNING: Prefer
> using '"%s...", __func__' to using 'create_bus_instance', this function's
> name,
> in a string
> >
> > to be specific, something you should have caught, right?
> >
> > Are you sure this is ready to be moved out of staging? :(
>
> Eek, I can't look away...
>
> You do this a bunch:
> if (dev->visorchannel) {
> visorchannel_destroy(dev->visorchannel);
>
> yet the first thing that visorchannel_destroy() does is check for null.
> So, no need to test this twice, right, only do so in the function, that
> will make your code flow a lot "smoother" where ever you are calling
> this.
>
> Ok, I'll stop now, gotta go find some dinner...
>
We will do some more internal reviews, and send out fixes for things
we find. I hope you enjoyed your dinner.
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus
On Tue, Jun 06, 2017 at 06:20:17PM +, Kershner, David A wrote: > > > > -Original Message- > > From: Greg KH [mailto:gre...@linuxfoundation.org] > > Sent: Tuesday, June 6, 2017 11:06 AM > > To: Kershner, David A > > Cc: cor...@lwn.net; t...@linutronix.de; mi...@kernel.org; akpm@linux- > > foundation.org; jes.soren...@gmail.com; linux-ker...@vger.kernel.org; > > linux-doc@vger.kernel.org; driverdev-de...@linuxdriverproject.org; *S-Par- > > Maintainer > > Subject: Re: [PATCH 0/3] move visorbus out of staging to > > drivers/virt/visorbus > > > > On Tue, Jun 06, 2017 at 04:54:30PM +0200, Greg KH wrote: > > > On Tue, Jun 06, 2017 at 04:53:22PM +0200, Greg KH wrote: > > > > On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote: > > > > > On Mon, Jun 05, 2017 at 04:07:29PM -0400, David Kershner wrote: > > > > > > This patchset moves drivers/staging/unisys/include to > > > > > > include/linux/visorbus, and moves drivers/staging/unisys/visorbus to > > > > > > drivers/virt/visorbus. > > > > > > > > > > Um, are you thinking it is ready to be moved? Have you asked for > > > > > another review? > > > > > > > Thank you for taking a quick look at our patch series. Part of the motivation > behind this submission was, in fact, to initiate another code review. What is > the formal procedure for initiating a code review? Send an email that says, "Hey Greg, we think the code is ready to be moved out of staging, can you review it to see if we have missed anything?" Of course, do it _AFTER_ you have fixed up the checkpatch.pl issues. That's not even done yet... thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] net: phy: Delete unused function phy_ethtool_gset
From: Yuval Shaia Date: Mon, 5 Jun 2017 10:18:40 +0300 > It's unused, so remove it. > > Signed-off-by: Yuval Shaia > --- > v0 -> v1: > * Add commit message > * Update Documentation/networking/phy.txt > * Modify commit header message Applied to net-next, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/3] USB Audio Gadget refactoring
On Tue, Jun 6, 2017 at 12:41 PM, Felipe Balbi wrote: > > Hi, > > Greg KH writes: >>> > I'm OK with dropping legacy f_uac1 implementation. >>> > >>> > Another idea I was thinking about is to implement simple in-kernel >>> > driver which will do the same as existing alsaloop tool userspace >>> > tool does (so legacy users will need to load two kernel modules >>> > and get same functionality). But this seems to be a wrong way, >>> > since It known that Linux kernel community doesn't like to take drivers >>> > with same functionality as existing userspace tools already have. >>> > >>> > So bottom line: since I'm not a legacy f_uac1 user, there is no >>> > difference for me how to handle it - remove legacy f_uac1 completely, >>> > rename it to f_uac1_legacy or add separate f_uac1_acard function. >>> > >>> > So if dropping of legacy f_uac1 implementation is OK for you, >>> > I can do it quickly in next patchset. >>> >>> Personally, I don't want duplicated functionality and I think the >>> virtual sound card approach is much better. Then again, removing >>> functionality we already support is kind of odd. >>> >>> Greg, Alan, what do you guys think? Do we keep a duplicated function >>> around or do we just tell people to rely on alsaloop? Personally, I >>> think we're better off with the flexibility of the virtual sound card, >>> what's your take? >> >> If the in-kernel driver will do more things, and we don't break the >> existing setups using alsaloop, then I don't see the problem, except >> that we now have to maintain two things :) >> >> If you don't mind the maintenance, fine with me... > > Okay, I don't think many will continue to use f_uac1.c. Ruslan, can you > update your series so that current f_uac1.c gets renamed to > f_uac1_legacy.c and you introduce a *new* f_uac1.c instead? Yes sure, I'll post an updated patch series soon Best regards, Ruslan -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [xfstests PATCH v3 1/5] generic: add a writeback error handling test
On Tue, 2017-06-06 at 10:17 -0700, Darrick J. Wong wrote: > On Tue, Jun 06, 2017 at 08:23:25PM +0800, Eryu Guan wrote: > > On Tue, Jun 06, 2017 at 06:15:57AM -0400, Jeff Layton wrote: > > > On Tue, 2017-06-06 at 16:58 +0800, Eryu Guan wrote: > > > > On Wed, May 31, 2017 at 09:08:16AM -0400, Jeff Layton wrote: > > > > > I'm working on a set of kernel patches to change how writeback errors > > > > > are handled and reported in the kernel. Instead of reporting a > > > > > writeback error to only the first fsync caller on the file, I aim > > > > > to make the kernel report them once on every file description. > > > > > > > > > > This patch adds a test for the new behavior. Basically, open many fds > > > > > to the same file, turn on dm_error, write to each of the fds, and then > > > > > fsync them all to ensure that they all get an error back. > > > > > > > > > > To do that, I'm adding a new tools/dmerror script that the C program > > > > > can use to load the error table. For now, that's all it can do, but > > > > > we can fill it out with other commands as necessary. > > > > > > > > > > Signed-off-by: Jeff Layton > > > > > > > > Thanks for the new tests! And sorry for the late review.. > > > > > > > > It's testing a new behavior on error reporting on writeback, I'm not > > > > sure if we can call it a new feature or it fixed a bug? But it's more > > > > like a behavior change, I'm not sure how to categorize it. > > > > > > > > Because if it's testing a new feature, we usually let test do proper > > > > detection of current test environment (based on actual behavior not > > > > kernel version) and _notrun on filesystems that don't have this feature > > > > yet, instead of failing the test; if it's testing a bug fix, we could > > > > leave the test fail on unfixed filesystems, this also serves as a > > > > reminder that there's bug to fix. > > > > > > > > > > Thanks for the review! I'm not sure how to categorize this either. Since > > > the plan is to convert all the filesystems piecemeal, maybe we should > > > just consider it a new feature. > > > > Then we need a new _require rule to properly detect for the 'feature' > > support. I'm not sure if this is doable, but something like > > _require_statx, _require_seek_data_hole would be good. > > > > > > > > > I pulled your test kernel tree, and test passed on EXT4 but failed on > > > > other local filesystems (XFS, btrfs). I assume that's expected. > > > > > > > > Besides this kinda high-level question, some minor comments inline. > > > > > > > > > > Yes, ext4 should pass on my latest kernel tree, but everything else > > > should fail. Oh, and I should mention that ext2/3 also pass when mounted using ext4 driver. Legacy ext2 driver sort of works, but it reports a few too many errors because of the way the ext2_error macro works. That shouldn't be too hard to fix, I just need some guidance on that one. I had xfs and btrfs working with an earlier iteration of the patches, but now that we're converting a fs at a time, it's a little more work to get there. It shouldn't be too hard to do though. I'll probably re-post in a few days, and will try to take a stab at XFS and btrfs conversion too. > > > > With the new _require rule, test should _notrun on XFS and btrfs then. > > Frankly I personally prefer that upstream XFS fails until someone fixes it. :) > (But that's just my opinion.) > > That said, I'm not 100% sure what's required of XFS to play nicely with > this new mechanism -- glancing at the ext* patches it looks like we'd > need to set a fs flag and possibly change some or all of the "write > cached dirty buffers out to disk" calls to their _since variants? Yeah, that's pretty much the size of it. In fact, the latter part (changing to the _since variants) is somewhat optional. We can have the errseq_t based tracking coexist with the AS_EIO/AS_ENOSPC flags. It's weird but I don't see a real downside to preserving them until we've got more of this converted over. In the latest branch I'm working on, I'm breaking up those changes into different patches so it should be a little clearer for other fs maintainers to see what I'm doing and why. Stay tuned... > Metadata writeback errors are handled by retrying writes and/or shutting > down the fs, so I think the f_md_wb_error case is already covered. Thanks. I think we do need f_md_wb_err for ext2/4 though, IIUC? > > That said, I agree that it's useful to detect that the kernel simply > lacks any of the new wb error reporting at all, so therefore we can skip > the tests. > Suggestions on ways to implement such a check would be welcome. Maybe a file in /sys or in debugfs? -- Jeff Layton -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 1/7] mm, oom: refactor select_bad_process() to take memcg as an argument
On Tue, 6 Jun 2017, Roman Gushchin wrote: > Hi David! > > Thank you for sharing this! > > It's very interesting, and it looks like, > it's not that far from what I've suggested. > > So we definitily need to come up with some common solution. > Hi Roman, Yes, definitely. I could post a series of patches to do everything that was listed in my email sans the fully inclusive kmem accounting, which may be pursued at a later date, if it would be helpful to see where there is common ground? Another question is what you think about userspace oom handling? We implement our own oom kill policies in userspace for both the system and for user-controlled memcg hierarchies because it often does not match the kernel implementation and there is some action that can be taken other than killing a process. Have you tried to implement functionality to do userspace oom handling, or are you considering it? This is the main motivation behind allowing an oom delay to be configured. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Make the main documentation title less Geocities
This is probably the lamest patch ever, but then again "Welcome to The Linux Kernel's documentation" is nearly equally lame. Really, we don't need to "Welcome" people to the documentation, just tell them what the site is about. Signed-off-by: Konstantin Ryabitsev --- Documentation/index.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/index.rst b/Documentation/index.rst index bc67dbf..0536b3f 100644 --- a/Documentation/index.rst +++ b/Documentation/index.rst @@ -3,8 +3,8 @@ You can adapt this file completely to your liking, but it should at least contain the root `toctree` directive. -Welcome to The Linux Kernel's documentation -=== +The Linux Kernel documentation +== This is the top level of the kernel's documentation tree. Kernel documentation, like the kernel itself, is very much a work in progress; -- 2.7.5 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] drivers: virt: Add visorbus to the drivers/virt directory
Hi David, [auto build test ERROR on staging/staging-testing] [also build test ERROR on v4.12-rc4 next-20170606] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/David-Kershner/move-visorbus-out-of-staging-to-drivers-virt-visorbus/20170606-070850 config: m68k-allyesconfig (attached as .config) compiler: m68k-linux-gcc (GCC) 4.9.0 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=m68k All errors (new ones prefixed by >>): drivers/virt/visorbus/visorchipset.c: In function 'save_crash_message': >> drivers/virt/visorbus/visorchipset.c:479:36: error: dereferencing pointer to >> incomplete type dev_err(&chipset_dev->acpi_device->dev, ^ drivers/virt/visorbus/visorchipset.c:485:36: error: dereferencing pointer to incomplete type dev_err(&chipset_dev->acpi_device->dev, ^ drivers/virt/visorbus/visorchipset.c:495:36: error: dereferencing pointer to incomplete type dev_err(&chipset_dev->acpi_device->dev, ^ drivers/virt/visorbus/visorchipset.c:508:37: error: dereferencing pointer to incomplete type dev_err(&chipset_dev->acpi_device->dev, ^ drivers/virt/visorbus/visorchipset.c:519:37: error: dereferencing pointer to incomplete type dev_err(&chipset_dev->acpi_device->dev, ^ drivers/virt/visorbus/visorchipset.c:525:36: error: dereferencing pointer to incomplete type dev_err(&chipset_dev->acpi_device->dev, ^ drivers/virt/visorbus/visorchipset.c: In function 'visorbus_create': drivers/virt/visorbus/visorchipset.c:582:36: error: dereferencing pointer to incomplete type dev_err(&chipset_dev->acpi_device->dev, ^ drivers/virt/visorbus/visorchipset.c: In function 'visorbus_configure': drivers/virt/visorbus/visorchipset.c:734:35: error: dereferencing pointer to incomplete type dev_err(&chipset_dev->acpi_device->dev, ^ drivers/virt/visorbus/visorchipset.c: In function 'visorbus_device_create': drivers/virt/visorbus/visorchipset.c:755:36: error: dereferencing pointer to incomplete type dev_err(&chipset_dev->acpi_device->dev, ^ drivers/virt/visorbus/visorchipset.c:762:36: error: dereferencing pointer to incomplete type dev_err(&chipset_dev->acpi_device->dev, ^ drivers/virt/visorbus/visorchipset.c:770:36: error: dereferencing pointer to incomplete type dev_err(&chipset_dev->acpi_device->dev, ^ drivers/virt/visorbus/visorchipset.c:796:36: error: dereferencing pointer to incomplete type dev_err(&chipset_dev->acpi_device->dev, ^ drivers/virt/visorbus/visorchipset.c: In function 'visorbus_device_changestate': drivers/virt/visorbus/visorchipset.c:896:35: error: dereferencing pointer to incomplete type dev_err(&chipset_dev->acpi_device->dev, "failed: %d\n", err); ^ drivers/virt/visorbus/visorchipset.c: In function 'parahotplug_request_kickoff': drivers/virt/visorbus/visorchipset.c:1188:53: error: dereferencing pointer to incomplete type return kobject_uevent_env(&chipset_dev->acpi_device->dev.kobj, ^ drivers/virt/visorbus/visorchipset.c: In function 'chipset_ready_uevent': drivers/virt/visorbus/visorchipset.c:1255:48: error: dereferencing pointer to incomplete type res = kobject_uevent(&chipset_dev->acpi_device->dev.kobj, ^ drivers/virt/visorbus/visorchipset.c: In function 'chipset_selftest_uevent': drivers/virt/visorbus/visorchipset.c:1279:52: error: dereferencing pointer to incomplete type res = kobject_uevent_env(&chipset_dev->acpi_device->dev.kobj, ^ drivers/virt/visorbus/visorchipset.c: In function 'chipset_notready_uevent': drivers/virt/visorbus/visorchipset.c:1300:48: error: dereferencing pointer to incomplete type res = kobject_uevent(&chipset_dev->acpi_device->dev.kobj,
Re: [xfstests PATCH v3 1/5] generic: add a writeback error handling test
On Tue, Jun 06, 2017 at 04:12:58PM -0400, Jeff Layton wrote: > On Tue, 2017-06-06 at 10:17 -0700, Darrick J. Wong wrote: > > On Tue, Jun 06, 2017 at 08:23:25PM +0800, Eryu Guan wrote: > > > On Tue, Jun 06, 2017 at 06:15:57AM -0400, Jeff Layton wrote: > > > > On Tue, 2017-06-06 at 16:58 +0800, Eryu Guan wrote: > > > > > On Wed, May 31, 2017 at 09:08:16AM -0400, Jeff Layton wrote: > > > > > > I'm working on a set of kernel patches to change how writeback > > > > > > errors > > > > > > are handled and reported in the kernel. Instead of reporting a > > > > > > writeback error to only the first fsync caller on the file, I aim > > > > > > to make the kernel report them once on every file description. > > > > > > > > > > > > This patch adds a test for the new behavior. Basically, open many > > > > > > fds > > > > > > to the same file, turn on dm_error, write to each of the fds, and > > > > > > then > > > > > > fsync them all to ensure that they all get an error back. > > > > > > > > > > > > To do that, I'm adding a new tools/dmerror script that the C program > > > > > > can use to load the error table. For now, that's all it can do, but > > > > > > we can fill it out with other commands as necessary. > > > > > > > > > > > > Signed-off-by: Jeff Layton > > > > > > > > > > Thanks for the new tests! And sorry for the late review.. > > > > > > > > > > It's testing a new behavior on error reporting on writeback, I'm not > > > > > sure if we can call it a new feature or it fixed a bug? But it's more > > > > > like a behavior change, I'm not sure how to categorize it. > > > > > > > > > > Because if it's testing a new feature, we usually let test do proper > > > > > detection of current test environment (based on actual behavior not > > > > > kernel version) and _notrun on filesystems that don't have this > > > > > feature > > > > > yet, instead of failing the test; if it's testing a bug fix, we could > > > > > leave the test fail on unfixed filesystems, this also serves as a > > > > > reminder that there's bug to fix. > > > > > > > > > > > > > Thanks for the review! I'm not sure how to categorize this either. Since > > > > the plan is to convert all the filesystems piecemeal, maybe we should > > > > just consider it a new feature. > > > > > > Then we need a new _require rule to properly detect for the 'feature' > > > support. I'm not sure if this is doable, but something like > > > _require_statx, _require_seek_data_hole would be good. > > > > > > > > > > > > I pulled your test kernel tree, and test passed on EXT4 but failed on > > > > > other local filesystems (XFS, btrfs). I assume that's expected. > > > > > > > > > > Besides this kinda high-level question, some minor comments inline. > > > > > > > > > > > > > Yes, ext4 should pass on my latest kernel tree, but everything else > > > > should fail. > > Oh, and I should mention that ext2/3 also pass when mounted using ext4 > driver. Legacy ext2 driver sort of works, but it reports a few too many > errors because of the way the ext2_error macro works. That shouldn't be > too hard to fix, I just need some guidance on that one. > > I had xfs and btrfs working with an earlier iteration of the patches, > but now that we're converting a fs at a time, it's a little more work to > get there. It shouldn't be too hard to do though. I'll probably re-post > in a few days, and will try to take a stab at XFS and btrfs conversion > too. > > > > > > > With the new _require rule, test should _notrun on XFS and btrfs then. > > > > Frankly I personally prefer that upstream XFS fails until someone fixes it. > > :) > > (But that's just my opinion.) > > > > That said, I'm not 100% sure what's required of XFS to play nicely with > > this new mechanism -- glancing at the ext* patches it looks like we'd > > need to set a fs flag and possibly change some or all of the "write > > cached dirty buffers out to disk" calls to their _since variants? > > Yeah, that's pretty much the size of it. > > In fact, the latter part (changing to the _since variants) is somewhat > optional. We can have the errseq_t based tracking coexist with the > AS_EIO/AS_ENOSPC flags. It's weird but I don't see a real downside to > preserving them until we've got more of this converted over. > > In the latest branch I'm working on, I'm breaking up those changes into > different patches so it should be a little clearer for other fs > maintainers to see what I'm doing and why. Stay tuned... Ok. > > Metadata writeback errors are handled by retrying writes and/or shutting > > down the fs, so I think the f_md_wb_error case is already covered. > > Thanks. I think we do need f_md_wb_err for ext2/4 though, IIUC? Yes. Sorry, the previous statement applies only to XFS. > > That said, I agree that it's useful to detect that the kernel simply > > lacks any of the new wb error reporting at all, so therefore we can skip > > the tests. > > > > Suggestions on ways to implement such a check would be welcom
Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller
Hi Andrew, [auto build test ERROR on hwmon/hwmon-next] [also build test ERROR on v4.12-rc4 next-20170606] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Andrew-Jeffery/hwmon-Add-support-for-MAX31785-intelligent-fan-controller/20170607-020015 base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next config: ia64-allmodconfig (attached as .config) compiler: ia64-linux-gcc (GCC) 6.2.0 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=ia64 All error/warnings (new ones prefixed by >>): In file included from include/linux/kobject.h:21:0, from include/linux/device.h:17, from include/linux/hwmon-sysfs.h:23, from drivers/hwmon/max31785.c:20: >> drivers/hwmon/max31785.c:727:50: error: initialization from incompatible >> pointer type [-Werror=incompatible-pointer-types] static SENSOR_DEVICE_ATTR(fan1_input_fast, 0444, max31785_fan_get_fast, ^ include/linux/sysfs.h:103:10: note: in definition of macro '__ATTR' .show = _show, \ ^ >> include/linux/hwmon-sysfs.h:38:4: note: in expansion of macro 'SENSOR_ATTR' = SENSOR_ATTR(_name, _mode, _show, _store, _index) ^~~ >> drivers/hwmon/max31785.c:727:8: note: in expansion of macro >> 'SENSOR_DEVICE_ATTR' static SENSOR_DEVICE_ATTR(fan1_input_fast, 0444, max31785_fan_get_fast, ^~ drivers/hwmon/max31785.c:727:50: note: (near initialization for 'sensor_dev_attr_fan1_input_fast.dev_attr.show') static SENSOR_DEVICE_ATTR(fan1_input_fast, 0444, max31785_fan_get_fast, ^ include/linux/sysfs.h:103:10: note: in definition of macro '__ATTR' .show = _show, \ ^ >> include/linux/hwmon-sysfs.h:38:4: note: in expansion of macro 'SENSOR_ATTR' = SENSOR_ATTR(_name, _mode, _show, _store, _index) ^~~ >> drivers/hwmon/max31785.c:727:8: note: in expansion of macro >> 'SENSOR_DEVICE_ATTR' static SENSOR_DEVICE_ATTR(fan1_input_fast, 0444, max31785_fan_get_fast, ^~ drivers/hwmon/max31785.c:729:50: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] static SENSOR_DEVICE_ATTR(fan2_input_fast, 0444, max31785_fan_get_fast, ^ include/linux/sysfs.h:103:10: note: in definition of macro '__ATTR' .show = _show, \ ^ >> include/linux/hwmon-sysfs.h:38:4: note: in expansion of macro 'SENSOR_ATTR' = SENSOR_ATTR(_name, _mode, _show, _store, _index) ^~~ drivers/hwmon/max31785.c:729:8: note: in expansion of macro 'SENSOR_DEVICE_ATTR' static SENSOR_DEVICE_ATTR(fan2_input_fast, 0444, max31785_fan_get_fast, ^~ drivers/hwmon/max31785.c:729:50: note: (near initialization for 'sensor_dev_attr_fan2_input_fast.dev_attr.show') static SENSOR_DEVICE_ATTR(fan2_input_fast, 0444, max31785_fan_get_fast, ^ include/linux/sysfs.h:103:10: note: in definition of macro '__ATTR' .show = _show, \ ^ >> include/linux/hwmon-sysfs.h:38:4: note: in expansion of macro 'SENSOR_ATTR' = SENSOR_ATTR(_name, _mode, _show, _store, _index) ^~~ drivers/hwmon/max31785.c:729:8: note: in expansion of macro 'SENSOR_DEVICE_ATTR' static SENSOR_DEVICE_ATTR(fan2_input_fast, 0444, max31785_fan_get_fast, ^~ drivers/hwmon/max31785.c:731:50: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] static SENSOR_DEVICE_ATTR(fan3_input_fast, 0444, max31785_fan_get_fast, ^ include/linux/sysfs.h:103:10: note: in definition of macro '__ATTR' .show = _show, \ ^ >> include/linux/hwmon-sysfs.h:38:4: note: in expansion of macro 'SENSOR_ATTR' = SENSOR_ATTR(_name, _mode, _show, _store, _index) ^~~ drivers/hwmon/max31785.c:731:8: note: in expansion of macro 'SENSOR_DEVICE_ATTR' static SENSOR_DEVICE_ATTR(fan3_input_fast, 0444, max31785_fan_get_fast, ^~ drivers/hwmon/max31785.c:731:50: note: (near initialization for 'sensor_dev_attr_fan3_input_fast.dev_attr.show') st
Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller
On Wed, Jun 7, 2017 at 1:50 AM, Matthew Barth wrote: > > On 06/06/17 8:33 AM, Guenter Roeck wrote: >> >> On 06/06/2017 12:02 AM, Andrew Jeffery wrote: >>> Over and above the features of the original patch is support for a >>> secondary >>> rotor measurement value that is provided by MAX31785 chips with a revised >>> firmware. The feature(s) of the firmware are determined at probe time and >>> extra >>> attributes exposed accordingly. Specifically, the MFR_REVISION 0x3040 of >>> the >>> firmware supports 'slow' and 'fast' rotor reads. The feature is >>> implemented by >>> command 0x90 (READ_FAN_SPEED_1) providing a 4-byte response (with the >>> 'fast' >>> measurement in the second word) rather than the 2-bytes response in the >>> original firmware (MFR_REVISION 0x3030). >>> >> >> Taking the pmbus driver question out, why would this warrant another >> non-standard >> attribute outside the ABI ? I could see the desire to replace the 'slow' >> access >> with the 'fast' one, but provide two attributes ? No, I don't see the >> point, sorry, >> even more so without detailed explanation why the second attribute in >> addition >> to the first one would add any value. > > In the case of counter-rotating(CR) fans which contain two rotors to provide > more airflow there are then two tach feedbacks. These CR fans take a single > target speed and provide individual feedbacks for each rotor contained > within the fan enclosure. Providing these individual feedbacks assists in > fan fault driven speed changes, improved thermal characterization among > other things. > > Maxim provided this as a 'slow' / 'fast' set of bytes to be user > compatible(so the 'slow' rotor speed, regardless of which rotor, is in the > first 2 bytes with the 'slow' version of firmware as well). In some cases, > mfg systems could have a mix of these revisions. Andrew, instead of creating the _fast sysfs nodes, I think you could expose the extra rotors are separate fan devices in sysfs. This would not introduce new ABI. Guenter, would this be acceptable to you? Cheers, Joel > >> >>> This feature is not documented in the public datasheet[1]. >>> >>> [1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf >>> >>> The need to read a 4-byte value drives the addition of a helper that is a >>> cut-down version of i2c_smbus_xfer_emulated(), as 4-byte transactions >>> aren't a >>> defined transaction type in the PMBus spec. This seemed more tasteful >>> than >>> hacking the PMBus core to support the quirks of a single device. >>> >> >> That is why we have PMBus helper drivers. >> >> Guenter >> >>> Also changed from Timothy's original posting is I've massaged the locking >>> a bit >>> and removed what seemed to be a copy/paste bug around >>> max31785_fan_set_pulses() >>> setting the fan_command member. >>> >>> Tested on an IBM Witherspoon machine. >>> >>> Cheers, >>> >>> Andrew >>> >>> Documentation/hwmon/max31785 | 44 +++ >>> drivers/hwmon/Kconfig| 10 + >>> drivers/hwmon/Makefile | 1 + >>> drivers/hwmon/max31785.c | 824 >>> +++ >>> 4 files changed, 879 insertions(+) >>> create mode 100644 Documentation/hwmon/max31785 >>> create mode 100644 drivers/hwmon/max31785.c >>> >>> diff --git a/Documentation/hwmon/max31785 b/Documentation/hwmon/max31785 >>> new file mode 100644 >>> index ..dd891c06401e >>> --- /dev/null >>> +++ b/Documentation/hwmon/max31785 >>> @@ -0,0 +1,44 @@ >>> +Kernel driver max31785 >>> +== >>> + >>> +Supported chips: >>> + * Maxim MAX31785 >>> +Prefix: 'max31785' >>> +Addresses scanned: 0x52 0x53 0x54 0x55 >>> +Datasheet: http://pdfserv.maximintegrated.com/en/ds/MAX31785.pdf >>> + >>> +Author: Timothy Pearson >>> + >>> + >>> +Description >>> +--- >>> + >>> +This driver implements support for the Maxim MAX31785 chip. >>> + >>> +The MAX31785 controls the speeds of up to six fans using six independent >>> +PWM outputs. The desired fan speeds (or PWM duty cycles) are written >>> +through the I2C interface. The outputs drive "4-wire" fans directly, >>> +or can be used to modulate the fan's power terminals using an external >>> +pass transistor. >>> + >>> +Tachometer inputs monitor fan tachometer logic outputs for precise >>> (+/-1%) >>> +monitoring and control of fan RPM as well as detection of fan failure. >>> + >>> + >>> +Sysfs entries >>> +- >>> + >>> +fan[1-6]_input RO fan tachometer speed in RPM >>> +fan[1-6]_fault RO fan experienced fault >>> +fan[1-6]_pulses RW tachometer pulses per fan revolution >>> +fan[1-6]_target RW desired fan speed in RPM >>> +pwm[1-6]_enable RW pwm mode, 0=disabled, 1=pwm, 2=rpm, >>> 3=automatic >>> +pwm[1-6] RW fan target duty cycle (0-255) >>> + >>> +Dynamic sysfs entries >>> + >>> + >>> +Whether these entries are present depends on the firmware features >>> detected on >>> +
Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller
On Wed, 2017-06-07 at 12:18 +0930, Joel Stanley wrote: > On Wed, Jun 7, 2017 at 1:50 AM, Matthew Barth > > wrote: > > > > On 06/06/17 8:33 AM, Guenter Roeck wrote: > > > > > > On 06/06/2017 12:02 AM, Andrew Jeffery wrote: > > > > Over and above the features of the original patch is support for a > > > > secondary > > > > rotor measurement value that is provided by MAX31785 chips with a > > > > revised > > > > firmware. The feature(s) of the firmware are determined at probe time > > > > and > > > > extra > > > > attributes exposed accordingly. Specifically, the MFR_REVISION 0x3040 of > > > > the > > > > firmware supports 'slow' and 'fast' rotor reads. The feature is > > > > implemented by > > > > command 0x90 (READ_FAN_SPEED_1) providing a 4-byte response (with the > > > > 'fast' > > > > measurement in the second word) rather than the 2-bytes response in the > > > > original firmware (MFR_REVISION 0x3030). > > > > > > > > > > Taking the pmbus driver question out, why would this warrant another > > > non-standard > > > attribute outside the ABI ? I could see the desire to replace the 'slow' > > > access > > > with the 'fast' one, but provide two attributes ? No, I don't see the > > > point, sorry, > > > even more so without detailed explanation why the second attribute in > > > addition > > > to the first one would add any value. > > > > In the case of counter-rotating(CR) fans which contain two rotors to provide > > more airflow there are then two tach feedbacks. These CR fans take a single > > target speed and provide individual feedbacks for each rotor contained > > within the fan enclosure. Providing these individual feedbacks assists in > > fan fault driven speed changes, improved thermal characterization among > > other things. > > > > Maxim provided this as a 'slow' / 'fast' set of bytes to be user > > compatible(so the 'slow' rotor speed, regardless of which rotor, is in the > > first 2 bytes with the 'slow' version of firmware as well). In some cases, > > mfg systems could have a mix of these revisions. > > Andrew, instead of creating the _fast sysfs nodes, I think you could > expose the extra rotors are separate fan devices in sysfs. This would > not introduce new ABI. I considered this approach: I debated whether to consider the fan unit as a single entity with two inputs, or just separate fans, and ended up leaning towards the former. To go the latter path we need to consider whether or not to expose the writeable properties for the second input (given they also affect the first) and how to sensibly arrange the pairs given the functionality is determined at probe-time. Not rocket science but decisions we need to make. There's also the issue that the physical fan that each element of an input pair represents will change as the fan speeds vary, based on the behaviour that Matt outlined. I don't feel this maps well onto the expectations of the sysfs interface, but then I'm not sure there's much we can do to alleviate it either other than designating one of the input attributes of a fan pair as the fastest input. Regardless, I'll look into it in the anticipation that this is a viable way forward. Cheers, Andrew signature.asc Description: This is a digitally signed message part
