Re: [REGRESSION][4.13.y][4.14.y][v4.15.y] net: reduce skb_warn_bad_offload() noise

[Willem de Bruijn]

On Mon, Dec 11, 2017 at 4:44 PM, Greg Kroah-Hartman
 wrote:
> On Mon, Dec 11, 2017 at 04:25:26PM -0500, Willem de Bruijn wrote:
>> Note that UFO was removed in 4.14 and that skb_warn_bad_offload
>> can happen for various types of packets, so there may be multiple
>> independent bug reports. I'm investigating two other non-UFO reports
>> just now.
>
> Meta-comment, now that UFO is gone from mainline, I'm wondering if I
> should just delete it from 4.4 and 4.9 as well.  Any objections for
> that?  I'd like to make it easy to maintain these kernels for a while,
> and having them diverge like this, with all of the issues around UFO,
> seems like it will just make life harder for myself if I leave it in.
>
> Any opinions?

Some of that removal had to be reverted with commit 0c19f846d582
("net: accept UFO datagrams from tuntap and packet") for VM live
migration between kernels.

Any backports probably should squash that in at the least. Just today
another thread discussed that that patch may not address all open
issues still, so it may be premature to backport at this point.
http://lkml.kernel.org/r/

Re: [REGRESSION][4.13.y][4.14.y][v4.15.y] net: reduce skb_warn_bad_offload() noise

[Willem de Bruijn]

On Mon, Dec 11, 2017 at 4:44 PM, Greg Kroah-Hartman
 wrote:
> On Mon, Dec 11, 2017 at 04:25:26PM -0500, Willem de Bruijn wrote:
>> Note that UFO was removed in 4.14 and that skb_warn_bad_offload
>> can happen for various types of packets, so there may be multiple
>> independent bug reports. I'm investigating two other non-UFO reports
>> just now.
>
> Meta-comment, now that UFO is gone from mainline, I'm wondering if I
> should just delete it from 4.4 and 4.9 as well.  Any objections for
> that?  I'd like to make it easy to maintain these kernels for a while,
> and having them diverge like this, with all of the issues around UFO,
> seems like it will just make life harder for myself if I leave it in.
>
> Any opinions?

Some of that removal had to be reverted with commit 0c19f846d582
("net: accept UFO datagrams from tuntap and packet") for VM live
migration between kernels.

Any backports probably should squash that in at the least. Just today
another thread discussed that that patch may not address all open
issues still, so it may be premature to backport at this point.
http://lkml.kernel.org/r/

Re: [PATCH] Documentation: Better document the hardlockup_panic sysctl

[Jonathan Corbet]

On Sun, 10 Dec 2017 01:48:46 -0600
Scott Wood  wrote:

> Commit ac1f591249d95372f ("kernel/watchdog.c: add sysctl knob
> hardlockup_panic") added the hardlockup_panic sysctl, but did not add it
> to Documentation/sysctl/kernel.txt.  Add this, and reference it from the
> corresponding entry in Documentation/admin-guide/kernel-parameters.txt.

Applied to the docs tree, thanks.

jon

Re: [PATCH] Documentation: Better document the hardlockup_panic sysctl

[Jonathan Corbet]

On Sun, 10 Dec 2017 01:48:46 -0600
Scott Wood  wrote:

> Commit ac1f591249d95372f ("kernel/watchdog.c: add sysctl knob
> hardlockup_panic") added the hardlockup_panic sysctl, but did not add it
> to Documentation/sysctl/kernel.txt.  Add this, and reference it from the
> corresponding entry in Documentation/admin-guide/kernel-parameters.txt.

Applied to the docs tree, thanks.

jon

Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray

[Dave Chinner]

On Sun, Dec 10, 2017 at 08:23:15PM -0800, Matthew Wilcox wrote:
> On Mon, Dec 11, 2017 at 10:57:45AM +1100, Dave Chinner wrote:
> > i.e.  the fact the cmpxchg failed may not have anything to do with a
> > race condtion - it failed because the slot wasn't empty like we
> > expected it to be. There can be any number of reasons the slot isn't
> > empty - the API should not "document" that the reason the insert
> > failed was a race condition. It should document the case that we
> > "couldn't insert because there was an existing entry in the slot".
> > Let the surrounding code document the reason why that might have
> > happened - it's not for the API to assume reasons for failure.
> > 
> > i.e. this API and potential internal implementation makes much
> > more sense:
> > 
> > int
> > xa_store_iff_empty(...)
> > {
> > curr = xa_cmpxchg(>pag_ici_xa, agino, NULL, ip, GFP_NOFS);
> > if (!curr)
> > return 0;   /* success! */
> > if (!IS_ERR(curr))
> > return -EEXIST; /* failed - slot not empty */
> > return PTR_ERR(curr);   /* failed - XA internal issue */
> > }
> > 
> > as it replaces the existing preload and insert code in the XFS code
> > paths whilst letting us handle and document the "insert failed
> > because slot not empty" case however we want. It implies nothing
> > about *why* the slot wasn't empty, just that we couldn't do the
> > insert because it wasn't empty.
> 
> Yeah, OK.  So, over the top of the recent changes I'm looking at this:
> 
> I'm not in love with xa_store_empty() as a name.  I almost want
> xa_store_weak(), but after my MAP_FIXED_WEAK proposed name got shot
> down, I'm leery of it.  "empty" is at least a concept we already have
> in the API with the comment for xa_init() talking about an empty array
> and xa_empty().  I also considered xa_store_null and xa_overwrite_null
> and xa_replace_null().  Naming remains hard.
> 
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 941f38bb94a4..586b43836905 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -451,7 +451,7 @@ xfs_iget_cache_miss(
>   int flags,
>   int lock_flags)
>  {
> - struct xfs_inode*ip, *curr;
> + struct xfs_inode*ip;
>   int error;
>   xfs_agino_t agino = XFS_INO_TO_AGINO(mp, ino);
>   int iflags;
> @@ -498,8 +498,7 @@ xfs_iget_cache_miss(
>   xfs_iflags_set(ip, iflags);
>  
>   /* insert the new inode */
> - curr = xa_cmpxchg(>pag_ici_xa, agino, NULL, ip, GFP_NOFS);
> - error = __xa_race(curr, -EAGAIN);
> + error = xa_store_empty(>pag_ici_xa, agino, ip, GFP_NOFS, -EAGAIN);
>   if (error)
>   goto out_unlock;

Can we avoid encoding the error to be returned into the function
call? No other generic/library API does this, so this seems like a
highly unusual special snowflake approach. I just don't see how
creating a whole new error specification convention is justified
for the case where it *might* save a line or two of error checking
code in a caller?

I'd much prefer that the API defines the error on failure, and let
the caller handle that specific error however they need to like
every other library interface we have...

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com

Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray

[Dave Chinner]

On Sun, Dec 10, 2017 at 08:23:15PM -0800, Matthew Wilcox wrote:
> On Mon, Dec 11, 2017 at 10:57:45AM +1100, Dave Chinner wrote:
> > i.e.  the fact the cmpxchg failed may not have anything to do with a
> > race condtion - it failed because the slot wasn't empty like we
> > expected it to be. There can be any number of reasons the slot isn't
> > empty - the API should not "document" that the reason the insert
> > failed was a race condition. It should document the case that we
> > "couldn't insert because there was an existing entry in the slot".
> > Let the surrounding code document the reason why that might have
> > happened - it's not for the API to assume reasons for failure.
> > 
> > i.e. this API and potential internal implementation makes much
> > more sense:
> > 
> > int
> > xa_store_iff_empty(...)
> > {
> > curr = xa_cmpxchg(>pag_ici_xa, agino, NULL, ip, GFP_NOFS);
> > if (!curr)
> > return 0;   /* success! */
> > if (!IS_ERR(curr))
> > return -EEXIST; /* failed - slot not empty */
> > return PTR_ERR(curr);   /* failed - XA internal issue */
> > }
> > 
> > as it replaces the existing preload and insert code in the XFS code
> > paths whilst letting us handle and document the "insert failed
> > because slot not empty" case however we want. It implies nothing
> > about *why* the slot wasn't empty, just that we couldn't do the
> > insert because it wasn't empty.
> 
> Yeah, OK.  So, over the top of the recent changes I'm looking at this:
> 
> I'm not in love with xa_store_empty() as a name.  I almost want
> xa_store_weak(), but after my MAP_FIXED_WEAK proposed name got shot
> down, I'm leery of it.  "empty" is at least a concept we already have
> in the API with the comment for xa_init() talking about an empty array
> and xa_empty().  I also considered xa_store_null and xa_overwrite_null
> and xa_replace_null().  Naming remains hard.
> 
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 941f38bb94a4..586b43836905 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -451,7 +451,7 @@ xfs_iget_cache_miss(
>   int flags,
>   int lock_flags)
>  {
> - struct xfs_inode*ip, *curr;
> + struct xfs_inode*ip;
>   int error;
>   xfs_agino_t agino = XFS_INO_TO_AGINO(mp, ino);
>   int iflags;
> @@ -498,8 +498,7 @@ xfs_iget_cache_miss(
>   xfs_iflags_set(ip, iflags);
>  
>   /* insert the new inode */
> - curr = xa_cmpxchg(>pag_ici_xa, agino, NULL, ip, GFP_NOFS);
> - error = __xa_race(curr, -EAGAIN);
> + error = xa_store_empty(>pag_ici_xa, agino, ip, GFP_NOFS, -EAGAIN);
>   if (error)
>   goto out_unlock;

Can we avoid encoding the error to be returned into the function
call? No other generic/library API does this, so this seems like a
highly unusual special snowflake approach. I just don't see how
creating a whole new error specification convention is justified
for the case where it *might* save a line or two of error checking
code in a caller?

I'd much prefer that the API defines the error on failure, and let
the caller handle that specific error however they need to like
every other library interface we have...

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com

Re: [PATCH v5] leds: trigger: Introduce a NETDEV trigger

[Jacek Anaszewski]

Hi Ben,

Thanks for the update.

On 12/10/2017 10:17 PM, Ben Whitten wrote:
> This commit introduces a NETDEV trigger for named device
> activity. Available triggers are link, rx, and tx.
> 
> Signed-off-by: Ben Whitten 
> 
> ---
> Changes in v5:
> Adjust header comment style to be consistent
> Changes in v4:
> Adopt SPDX licence header
> Changes in v3:
> Cancel the software blink prior to a oneshot re-queue
> Changes in v2:
> Sort includes and redate documentation
> Correct licence
> Remove macro and replace with generic function using enums
> Convert blink logic in stats work to use led_blink_oneshot
> Uses configured brightness instead of FULL
> ---
>  .../ABI/testing/sysfs-class-led-trigger-netdev |  45 ++
>  drivers/leds/trigger/Kconfig   |   7 +
>  drivers/leds/trigger/Makefile  |   1 +
>  drivers/leds/trigger/ledtrig-netdev.c  | 496 
> +
>  4 files changed, 549 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-netdev
>  create mode 100644 drivers/leds/trigger/ledtrig-netdev.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-netdev 
> b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev
> new file mode 100644
> index 000..451af6d
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev
> @@ -0,0 +1,45 @@
> +What:/sys/class/leds//device_name
> +Date:Dec 2017
> +KernelVersion:   4.16
> +Contact: linux-l...@vger.kernel.org
> +Description:
> + Specifies the network device name to monitor.
> +
> +What:/sys/class/leds//interval
> +Date:Dec 2017
> +KernelVersion:   4.16
> +Contact: linux-l...@vger.kernel.org
> +Description:
> + Specifies the duration of the LED blink in milliseconds.
> + Defaults to 50 ms.
> +
> +What:/sys/class/leds//link
> +Date:Dec 2017
> +KernelVersion:   4.16
> +Contact: linux-l...@vger.kernel.org
> +Description:
> + Signal the link state of the named network device.
> + If set to 0 (default), the LED's normal state is off.
> + If set to 1, the LED's normal state reflects the link state
> + of the named network device.
> + Setting this value also immediately changes the LED state.
> +
> +What:/sys/class/leds//tx
> +Date:Dec 2017
> +KernelVersion:   4.16
> +Contact: linux-l...@vger.kernel.org
> +Description:
> + Signal transmission of data on the named network device.
> + If set to 0 (default), the LED will not blink on transmission.
> + If set to 1, the LED will blink for the milliseconds specified
> + in interval to signal transmission.
> +
> +What:/sys/class/leds//rx
> +Date:Dec 2017
> +KernelVersion:   4.16
> +Contact: linux-l...@vger.kernel.org
> +Description:
> + Signal reception of data on the named network device.
> + If set to 0 (default), the LED will not blink on reception.
> + If set to 1, the LED will blink for the milliseconds specified
> + in interval to signal reception.
> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
> index 3f9ddb9..4ec1853 100644
> --- a/drivers/leds/trigger/Kconfig
> +++ b/drivers/leds/trigger/Kconfig
> @@ -126,4 +126,11 @@ config LEDS_TRIGGER_PANIC
> a different trigger.
> If unsure, say Y.
>  
> +config LEDS_TRIGGER_NETDEV
> + tristate "LED Netdev Trigger"
> + depends on NET && LEDS_TRIGGERS
> + help
> +   This allows LEDs to be controlled by network device activity.
> +   If unsure, say Y.
> +
>  endif # LEDS_TRIGGERS
> diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
> index 9f2e868..59e163d 100644
> --- a/drivers/leds/trigger/Makefile
> +++ b/drivers/leds/trigger/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON)   += 
> ledtrig-default-on.o
>  obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT) += ledtrig-transient.o
>  obj-$(CONFIG_LEDS_TRIGGER_CAMERA)+= ledtrig-camera.o
>  obj-$(CONFIG_LEDS_TRIGGER_PANIC) += ledtrig-panic.o
> +obj-$(CONFIG_LEDS_TRIGGER_NETDEV)+= ledtrig-netdev.o
> diff --git a/drivers/leds/trigger/ledtrig-netdev.c 
> b/drivers/leds/trigger/ledtrig-netdev.c
> new file mode 100644
> index 000..6df4781
> --- /dev/null
> +++ b/drivers/leds/trigger/ledtrig-netdev.c
> @@ -0,0 +1,496 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright 2017 Ben Whitten 
> +// Copyright 2007 Oliver Jowett 
> +//
> +// LED Kernel Netdev Trigger
> +//
> +// Toggles the LED to reflect the link and traffic state of a named net 
> device
> +//
> +// Derived from ledtrig-timer.c which is:
> +//  Copyright 2005-2006 Openedhand 

Re: [PATCH v5] leds: trigger: Introduce a NETDEV trigger

[Jacek Anaszewski]

Hi Ben,

Thanks for the update.

On 12/10/2017 10:17 PM, Ben Whitten wrote:
> This commit introduces a NETDEV trigger for named device
> activity. Available triggers are link, rx, and tx.
> 
> Signed-off-by: Ben Whitten 
> 
> ---
> Changes in v5:
> Adjust header comment style to be consistent
> Changes in v4:
> Adopt SPDX licence header
> Changes in v3:
> Cancel the software blink prior to a oneshot re-queue
> Changes in v2:
> Sort includes and redate documentation
> Correct licence
> Remove macro and replace with generic function using enums
> Convert blink logic in stats work to use led_blink_oneshot
> Uses configured brightness instead of FULL
> ---
>  .../ABI/testing/sysfs-class-led-trigger-netdev |  45 ++
>  drivers/leds/trigger/Kconfig   |   7 +
>  drivers/leds/trigger/Makefile  |   1 +
>  drivers/leds/trigger/ledtrig-netdev.c  | 496 
> +
>  4 files changed, 549 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-netdev
>  create mode 100644 drivers/leds/trigger/ledtrig-netdev.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-netdev 
> b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev
> new file mode 100644
> index 000..451af6d
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev
> @@ -0,0 +1,45 @@
> +What:/sys/class/leds//device_name
> +Date:Dec 2017
> +KernelVersion:   4.16
> +Contact: linux-l...@vger.kernel.org
> +Description:
> + Specifies the network device name to monitor.
> +
> +What:/sys/class/leds//interval
> +Date:Dec 2017
> +KernelVersion:   4.16
> +Contact: linux-l...@vger.kernel.org
> +Description:
> + Specifies the duration of the LED blink in milliseconds.
> + Defaults to 50 ms.
> +
> +What:/sys/class/leds//link
> +Date:Dec 2017
> +KernelVersion:   4.16
> +Contact: linux-l...@vger.kernel.org
> +Description:
> + Signal the link state of the named network device.
> + If set to 0 (default), the LED's normal state is off.
> + If set to 1, the LED's normal state reflects the link state
> + of the named network device.
> + Setting this value also immediately changes the LED state.
> +
> +What:/sys/class/leds//tx
> +Date:Dec 2017
> +KernelVersion:   4.16
> +Contact: linux-l...@vger.kernel.org
> +Description:
> + Signal transmission of data on the named network device.
> + If set to 0 (default), the LED will not blink on transmission.
> + If set to 1, the LED will blink for the milliseconds specified
> + in interval to signal transmission.
> +
> +What:/sys/class/leds//rx
> +Date:Dec 2017
> +KernelVersion:   4.16
> +Contact: linux-l...@vger.kernel.org
> +Description:
> + Signal reception of data on the named network device.
> + If set to 0 (default), the LED will not blink on reception.
> + If set to 1, the LED will blink for the milliseconds specified
> + in interval to signal reception.
> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
> index 3f9ddb9..4ec1853 100644
> --- a/drivers/leds/trigger/Kconfig
> +++ b/drivers/leds/trigger/Kconfig
> @@ -126,4 +126,11 @@ config LEDS_TRIGGER_PANIC
> a different trigger.
> If unsure, say Y.
>  
> +config LEDS_TRIGGER_NETDEV
> + tristate "LED Netdev Trigger"
> + depends on NET && LEDS_TRIGGERS
> + help
> +   This allows LEDs to be controlled by network device activity.
> +   If unsure, say Y.
> +
>  endif # LEDS_TRIGGERS
> diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
> index 9f2e868..59e163d 100644
> --- a/drivers/leds/trigger/Makefile
> +++ b/drivers/leds/trigger/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON)   += 
> ledtrig-default-on.o
>  obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT) += ledtrig-transient.o
>  obj-$(CONFIG_LEDS_TRIGGER_CAMERA)+= ledtrig-camera.o
>  obj-$(CONFIG_LEDS_TRIGGER_PANIC) += ledtrig-panic.o
> +obj-$(CONFIG_LEDS_TRIGGER_NETDEV)+= ledtrig-netdev.o
> diff --git a/drivers/leds/trigger/ledtrig-netdev.c 
> b/drivers/leds/trigger/ledtrig-netdev.c
> new file mode 100644
> index 000..6df4781
> --- /dev/null
> +++ b/drivers/leds/trigger/ledtrig-netdev.c
> @@ -0,0 +1,496 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright 2017 Ben Whitten 
> +// Copyright 2007 Oliver Jowett 
> +//
> +// LED Kernel Netdev Trigger
> +//
> +// Toggles the LED to reflect the link and traffic state of a named net 
> device
> +//
> +// Derived from ledtrig-timer.c which is:
> +//  Copyright 2005-2006 Openedhand Ltd.
> +//  Author: Richard Purdie 
> +
> +#include 
> +#include 
> 

Re: [PATCH 0/2] mux: add overview and add to driver-api docs

[Jonathan Corbet]

On Mon, 11 Dec 2017 09:29:41 +0100
Peter Rosin  wrote:

> Don't know if this is worth adding, but it might answer at least a few
> questions.
> 
> It looks ok when I view the htmldocs output, but I'm not all that certain
> this is good to go?

Almost, but I have one request: the new mux.rst file is rather
unenlightening for somebody who reads it directly in the docs tree.  If
you want to keep the bulk of the text in the source I can live with that,
but can mux.rst at least get an introductory paragraph saying what it
covers (what *is* the mux subsystem?) and why readers might want to go
find the rest?

Thanks,

jon

Re: [PATCH 0/2] mux: add overview and add to driver-api docs

[Jonathan Corbet]

On Mon, 11 Dec 2017 09:29:41 +0100
Peter Rosin  wrote:

> Don't know if this is worth adding, but it might answer at least a few
> questions.
> 
> It looks ok when I view the htmldocs output, but I'm not all that certain
> this is good to go?

Almost, but I have one request: the new mux.rst file is rather
unenlightening for somebody who reads it directly in the docs tree.  If
you want to keep the bulk of the text in the source I can live with that,
but can mux.rst at least get an introductory paragraph saying what it
covers (what *is* the mux subsystem?) and why readers might want to go
find the rest?

Thanks,

jon

Re: [PATCH 1/4] fs/notify: fdinfo can report unsupported file handles.

[NeilBrown]

On Mon, Dec 11 2017, Amir Goldstein wrote:

> On Mon, Dec 11, 2017 at 8:04 AM, NeilBrown  wrote:
>> If a filesystem does not set sb->s_export_op, then it
>> does not support filehandles and export_fs_encode_fh()
>> and exportfs_encode_inode_fh() should not be called.
>> They will use export_encode_fh() is which is a default
>> that uses inode number generation number, but in general
>> they may not be stable.
>>
>> So change exportfs_encode_inode_fh() to return FILEID_INVALID
>> if called on an unsupported Filesystem.  Currently only
>> notify/fdinfo can do that.
>>
>
> I wish you would leave this check to the caller, maybe add a helper
> exportfs_can_decode_fh() for callers to use.
>
> Although there are no current uses for it in-tree, there is value in
> being able to encode a unique file handle even when it cannot be
> decoded back to an open file.
>
> I am using this property in my fanotify super block watch patches,
> where the object identifier on the event is an encoded file handle
> of the object, which delegates tracking filesystem objects to
> userspace and prevents fanotify from keeping elevated refcounts
> on inodes and dentries.
>
> There are quite a few userspace tools out there that are checking
> that st_ino hasn't changed on a file between non atomic operations.
> Those tools (or others) could benefit from a unique file handle if
> we ever decide to provide a relaxed version of name_to_handle_at().

If the filesystem doesn't define ->s_export_op, then you really cannot
trust anything beyond the inode number (and maybe not even that), and
the inode number is already easily available.
What actual value do you think you get from this pretend-file-handle
on filesystems that don't support file handles?

If there is a demonstrated need for some sort of identifier that is
stronger than an inode number, but not strong enough for
open_by_handle_at(), then you should explain that need and propose a
well defined interface.  You shouldn't use a back-door and hope no-one
notices.

Thanks,
NeilBrown


signature.asc
Description: PGP signature

Re: [PATCH 1/4] fs/notify: fdinfo can report unsupported file handles.

[NeilBrown]

On Mon, Dec 11 2017, Amir Goldstein wrote:

> On Mon, Dec 11, 2017 at 8:04 AM, NeilBrown  wrote:
>> If a filesystem does not set sb->s_export_op, then it
>> does not support filehandles and export_fs_encode_fh()
>> and exportfs_encode_inode_fh() should not be called.
>> They will use export_encode_fh() is which is a default
>> that uses inode number generation number, but in general
>> they may not be stable.
>>
>> So change exportfs_encode_inode_fh() to return FILEID_INVALID
>> if called on an unsupported Filesystem.  Currently only
>> notify/fdinfo can do that.
>>
>
> I wish you would leave this check to the caller, maybe add a helper
> exportfs_can_decode_fh() for callers to use.
>
> Although there are no current uses for it in-tree, there is value in
> being able to encode a unique file handle even when it cannot be
> decoded back to an open file.
>
> I am using this property in my fanotify super block watch patches,
> where the object identifier on the event is an encoded file handle
> of the object, which delegates tracking filesystem objects to
> userspace and prevents fanotify from keeping elevated refcounts
> on inodes and dentries.
>
> There are quite a few userspace tools out there that are checking
> that st_ino hasn't changed on a file between non atomic operations.
> Those tools (or others) could benefit from a unique file handle if
> we ever decide to provide a relaxed version of name_to_handle_at().

If the filesystem doesn't define ->s_export_op, then you really cannot
trust anything beyond the inode number (and maybe not even that), and
the inode number is already easily available.
What actual value do you think you get from this pretend-file-handle
on filesystems that don't support file handles?

If there is a demonstrated need for some sort of identifier that is
stronger than an inode number, but not strong enough for
open_by_handle_at(), then you should explain that need and propose a
well defined interface.  You shouldn't use a back-door and hope no-one
notices.

Thanks,
NeilBrown


signature.asc
Description: PGP signature

Re: [GIT PULL] workqueue fixes for v4.15-rc1

[Tejun Heo]

Oops, $SUBJ should have been "workqueue fixes for v4.15-rc3", not rc1.

Thanks.

-- 
tejun

Re: [GIT PULL] workqueue fixes for v4.15-rc1

[Tejun Heo]

Oops, $SUBJ should have been "workqueue fixes for v4.15-rc3", not rc1.

Thanks.

-- 
tejun

[GIT PULL] libata fixes for v4.15-rc3

[Tejun Heo]

Hello, Linus.

Nothing too interesting.  David Milburn improved a corner case
misbehavior during hotplug.  Other than that, minor driver-specific
fixes.

Thanks.

The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:

  Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git for-4.15-fixes

for you to fetch changes up to 2dc0b46b5ea30f169b0b272253ea846a5a281731:

  libata: sata_down_spd_limit should return if driver has not recorded sstatus 
speed (2017-12-04 13:57:03 -0800)


Albert Pool (1):
  ata: mediatek: Fix typo in module description

Arvind Yadav (2):
  pata_pdc2027x: Remove unnecessary error check
  pata_pdc2027x : make pdc2027x_*_timing structures const

David Milburn (1):
  libata: sata_down_spd_limit should return if driver has not recorded 
sstatus speed

Matthias Brugger (1):
  ahci: mtk: Change driver name to ahci-mtk

Yuantian Tang (1):
  ahci: qoriq: refine port register configuration

 drivers/ata/ahci_mtk.c  |  6 +++---
 drivers/ata/ahci_qoriq.c| 12 
 drivers/ata/libata-core.c   | 12 +---
 drivers/ata/pata_pdc2027x.c | 16 ++--
 4 files changed, 30 insertions(+), 16 deletions(-)

-- 
tejun

[GIT PULL] libata fixes for v4.15-rc3

[Tejun Heo]

Hello, Linus.

Nothing too interesting.  David Milburn improved a corner case
misbehavior during hotplug.  Other than that, minor driver-specific
fixes.

Thanks.

The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:

  Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git for-4.15-fixes

for you to fetch changes up to 2dc0b46b5ea30f169b0b272253ea846a5a281731:

  libata: sata_down_spd_limit should return if driver has not recorded sstatus 
speed (2017-12-04 13:57:03 -0800)


Albert Pool (1):
  ata: mediatek: Fix typo in module description

Arvind Yadav (2):
  pata_pdc2027x: Remove unnecessary error check
  pata_pdc2027x : make pdc2027x_*_timing structures const

David Milburn (1):
  libata: sata_down_spd_limit should return if driver has not recorded 
sstatus speed

Matthias Brugger (1):
  ahci: mtk: Change driver name to ahci-mtk

Yuantian Tang (1):
  ahci: qoriq: refine port register configuration

 drivers/ata/ahci_mtk.c  |  6 +++---
 drivers/ata/ahci_qoriq.c| 12 
 drivers/ata/libata-core.c   | 12 +---
 drivers/ata/pata_pdc2027x.c | 16 ++--
 4 files changed, 30 insertions(+), 16 deletions(-)

-- 
tejun

Re: [PATCH RESEND] KVM: X86: Fix load bad host fpu state

[Wanpeng Li]

2017-12-12 4:48 GMT+08:00 David Hildenbrand :
> On 10.12.2017 22:44, Wanpeng Li wrote:
>> From: Wanpeng Li 
>>
>> [ cut here ]
>>  Bad FPU state detected at kvm_put_guest_fpu+0xd8/0x2d0 [kvm], 
>> reinitializing FPU registers.
>>  WARNING: CPU: 1 PID: 4594 at arch/x86/mm/extable.c:103 
>> ex_handler_fprestore+0x88/0x90
>>  CPU: 1 PID: 4594 Comm: qemu-system-x86 Tainted: GB  OE
>> 4.15.0-rc2+ #10
>>  RIP: 0010:ex_handler_fprestore+0x88/0x90
>>  Call Trace:
>>   fixup_exception+0x4e/0x60
>>   do_general_protection+0xff/0x270
>>   general_protection+0x22/0x30
>>  RIP: 0010:kvm_put_guest_fpu+0xd8/0x2d0 [kvm]
>>  RSP: 0018:8803d5627810 EFLAGS: 00010246
>>   kvm_vcpu_reset+0x3b4/0x3c0 [kvm]
>>   kvm_apic_accept_events+0x1c0/0x240 [kvm]
>>   kvm_arch_vcpu_ioctl_run+0x1658/0x2fb0 [kvm]
>>   kvm_vcpu_ioctl+0x479/0x880 [kvm]
>>   do_vfs_ioctl+0x142/0x9a0
>>   SyS_ioctl+0x74/0x80
>>   do_syscall_64+0x15f/0x600
>>
>> This can be reproduced by running any testcase in kvm-unit-tests since
>> the qemu userspace FPU context is not initialized, which results in the
>> init path from kvm_apic_accept_events() will load/put qemu userspace
>> FPU context w/o initialized. In addition, w/o this splatting we still
>> should initialize vcpu->arch.user_fpu instead of current->thread.fpu.
>> This patch fixes it by initializing qemu user FPU context if it is
>> uninitialized before KVM_RUN.
>>
>> Cc: Paolo Bonzini 
>> Cc: Radim Krčmář 
>> Cc: Rik van Riel 
>> Cc: sta...@vger.kernel.org
>> Fixes: f775b13eedee (x86,kvm: move qemu/guest FPU switching out to vcpu_run)
>> Signed-off-by: Wanpeng Li 
>> ---
>>  arch/x86/kvm/x86.c | 7 +--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index a92b22f..063a643 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -7273,10 +7273,13 @@ static int complete_emulated_mmio(struct kvm_vcpu 
>> *vcpu)
>>
>>  int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>  {
>> - struct fpu *fpu = >thread.fpu;
>> + struct fpu *fpu = >arch.user_fpu;
>>   int r;
>>
>> - fpu__initialize(fpu);
>> + if (!fpu->initialized) {
>> + fpstate_init(>state);
>> + fpu->initialized = 1;
>> + }
>
> Is there a chance of keeping using fpu__initialize() ? Duplicating the
> code is ugly.

There is a warning in fpu__initialize() which results in just
current->thread.fpu can take advantage of.

>
> E.g. can't we simply initialize that in kvm_load_guest_fpu?

We still miss to initialize qemu user FPU context for the above calltrace.

Regards,
Wanpeng Li

Re: [PATCH RESEND] KVM: X86: Fix load bad host fpu state

[Wanpeng Li]

2017-12-12 4:48 GMT+08:00 David Hildenbrand :
> On 10.12.2017 22:44, Wanpeng Li wrote:
>> From: Wanpeng Li 
>>
>> [ cut here ]
>>  Bad FPU state detected at kvm_put_guest_fpu+0xd8/0x2d0 [kvm], 
>> reinitializing FPU registers.
>>  WARNING: CPU: 1 PID: 4594 at arch/x86/mm/extable.c:103 
>> ex_handler_fprestore+0x88/0x90
>>  CPU: 1 PID: 4594 Comm: qemu-system-x86 Tainted: GB  OE
>> 4.15.0-rc2+ #10
>>  RIP: 0010:ex_handler_fprestore+0x88/0x90
>>  Call Trace:
>>   fixup_exception+0x4e/0x60
>>   do_general_protection+0xff/0x270
>>   general_protection+0x22/0x30
>>  RIP: 0010:kvm_put_guest_fpu+0xd8/0x2d0 [kvm]
>>  RSP: 0018:8803d5627810 EFLAGS: 00010246
>>   kvm_vcpu_reset+0x3b4/0x3c0 [kvm]
>>   kvm_apic_accept_events+0x1c0/0x240 [kvm]
>>   kvm_arch_vcpu_ioctl_run+0x1658/0x2fb0 [kvm]
>>   kvm_vcpu_ioctl+0x479/0x880 [kvm]
>>   do_vfs_ioctl+0x142/0x9a0
>>   SyS_ioctl+0x74/0x80
>>   do_syscall_64+0x15f/0x600
>>
>> This can be reproduced by running any testcase in kvm-unit-tests since
>> the qemu userspace FPU context is not initialized, which results in the
>> init path from kvm_apic_accept_events() will load/put qemu userspace
>> FPU context w/o initialized. In addition, w/o this splatting we still
>> should initialize vcpu->arch.user_fpu instead of current->thread.fpu.
>> This patch fixes it by initializing qemu user FPU context if it is
>> uninitialized before KVM_RUN.
>>
>> Cc: Paolo Bonzini 
>> Cc: Radim Krčmář 
>> Cc: Rik van Riel 
>> Cc: sta...@vger.kernel.org
>> Fixes: f775b13eedee (x86,kvm: move qemu/guest FPU switching out to vcpu_run)
>> Signed-off-by: Wanpeng Li 
>> ---
>>  arch/x86/kvm/x86.c | 7 +--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index a92b22f..063a643 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -7273,10 +7273,13 @@ static int complete_emulated_mmio(struct kvm_vcpu 
>> *vcpu)
>>
>>  int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>  {
>> - struct fpu *fpu = >thread.fpu;
>> + struct fpu *fpu = >arch.user_fpu;
>>   int r;
>>
>> - fpu__initialize(fpu);
>> + if (!fpu->initialized) {
>> + fpstate_init(>state);
>> + fpu->initialized = 1;
>> + }
>
> Is there a chance of keeping using fpu__initialize() ? Duplicating the
> code is ugly.

There is a warning in fpu__initialize() which results in just
current->thread.fpu can take advantage of.

>
> E.g. can't we simply initialize that in kvm_load_guest_fpu?

We still miss to initialize qemu user FPU context for the above calltrace.

Regards,
Wanpeng Li

Re: [PATCH] Bluetooth: hci_serdev: Init hci_uart proto_lock to avoid oops

[John Stultz]

On Thu, Nov 16, 2017 at 10:07 PM, Marcel Holtmann  wrote:
> Hi Lukas,
>
>> John Stultz reports a boot time crash with the HiKey board (which uses
>> hci_serdev) occurring in hci_uart_tx_wakeup().  That function is
>> contained in hci_ldisc.c, but also called from the newer hci_serdev.c.
>> It acquires the proto_lock in struct hci_uart and it turns out that we
>> forgot to init the lock in the serdev code path, thus causing the crash.
>>
>> John bisected the crash to commit 67d2f8781b9f ("Bluetooth: hci_ldisc:
>> Allow sleeping while proto locks are held"), but the issue was present
>> before and the commit merely exposed it.  (Perhaps by luck, the crash
>> did not occur with rwlocks.)
>>
>> Init the proto_lock in the serdev code path to avoid the oops.
>>
[snip]
> patch has been applied to bluetooth-next tree.

Sorry to be a nuisance if its just a timing thing, but I wanted to
follow up on this just to make sure it didn't fall through the cracks,
as I noticed w/ -rc3 it hasn't landed yet.

thanks
-john

Re: [PATCH] Bluetooth: hci_serdev: Init hci_uart proto_lock to avoid oops

[John Stultz]

On Thu, Nov 16, 2017 at 10:07 PM, Marcel Holtmann  wrote:
> Hi Lukas,
>
>> John Stultz reports a boot time crash with the HiKey board (which uses
>> hci_serdev) occurring in hci_uart_tx_wakeup().  That function is
>> contained in hci_ldisc.c, but also called from the newer hci_serdev.c.
>> It acquires the proto_lock in struct hci_uart and it turns out that we
>> forgot to init the lock in the serdev code path, thus causing the crash.
>>
>> John bisected the crash to commit 67d2f8781b9f ("Bluetooth: hci_ldisc:
>> Allow sleeping while proto locks are held"), but the issue was present
>> before and the commit merely exposed it.  (Perhaps by luck, the crash
>> did not occur with rwlocks.)
>>
>> Init the proto_lock in the serdev code path to avoid the oops.
>>
[snip]
> patch has been applied to bluetooth-next tree.

Sorry to be a nuisance if its just a timing thing, but I wanted to
follow up on this just to make sure it didn't fall through the cracks,
as I noticed w/ -rc3 it hasn't landed yet.

thanks
-john

[GIT PULL] workqueue fixes for v4.15-rc1

[Tejun Heo]

Hello,

* Lai's hotplug simplifications inadvertently fix a possible deadlock
  involving cpuset and workqueue.

* CPU isolation fix which was reverted due to the changes in the
  housekeeping code resurrected.

* A trivial unused include removal.

Thanks.

The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:

  Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-4.15-fixes

for you to fetch changes up to 01dfee9582d9b4403c4902df096ed8b43d55181c:

  workqueue: remove unneeded kallsyms include (2017-12-11 07:15:43 -0800)


Lai Jiangshan (2):
  workqueue/hotplug: simplify workqueue_offline_cpu()
  workqueue/hotplug: remove the workaround in rebind_workers()

Sergey Senozhatsky (1):
  workqueue: remove unneeded kallsyms include

Tal Shorer (2):
  main: kernel_start: move housekeeping_init() before workqueue_init_early()
  workqueue: respect isolated cpus when queueing an unbound work

 init/main.c|  7 ++-
 kernel/workqueue.c | 33 -
 2 files changed, 18 insertions(+), 22 deletions(-)

-- 
tejun

[GIT PULL] workqueue fixes for v4.15-rc1

[Tejun Heo]

Hello,

* Lai's hotplug simplifications inadvertently fix a possible deadlock
  involving cpuset and workqueue.

* CPU isolation fix which was reverted due to the changes in the
  housekeeping code resurrected.

* A trivial unused include removal.

Thanks.

The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:

  Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-4.15-fixes

for you to fetch changes up to 01dfee9582d9b4403c4902df096ed8b43d55181c:

  workqueue: remove unneeded kallsyms include (2017-12-11 07:15:43 -0800)


Lai Jiangshan (2):
  workqueue/hotplug: simplify workqueue_offline_cpu()
  workqueue/hotplug: remove the workaround in rebind_workers()

Sergey Senozhatsky (1):
  workqueue: remove unneeded kallsyms include

Tal Shorer (2):
  main: kernel_start: move housekeeping_init() before workqueue_init_early()
  workqueue: respect isolated cpus when queueing an unbound work

 init/main.c|  7 ++-
 kernel/workqueue.c | 33 -
 2 files changed, 18 insertions(+), 22 deletions(-)

-- 
tejun

[PATCH] phy: rockchip-typec: Try to turn the PHY on several times

[Douglas Anderson]

Bind / unbind stress testing of the USB controller on rk3399 found
that we'd often end up with lots of failures that looked like this:

  phy phy-ff80.phy.9: phy poweron failed --> -110
  dwc3 fe90.dwc3: failed to initialize core
  dwc3: probe of fe90.dwc3 failed with error -110

Those errors were sometimes seen at bootup too, in which case USB
peripherals wouldn't work until unplugged and re-plugged in.

I spent some time trying to figure out why the PHY was failing to
power on but I wasn't able to.  Possibly this has to do with the fact
that the PHY docs say that the USB controller "needs to be held in
reset to hold pipe power state in P2 before initializing the Type C
PHY" but that doesn't appear to be easy to do with the dwc3 driver
today.  Messing around with the ordering of the reset vs. the PHY
initialization in the dwc3 driver didn't seem to fix things.

I did, however, find that if I simply retry the power on it seems to
have a good chance of working.  So let's add some retries.  I ran a
pretty tight bind/unbind loop overnight.  When I did so, I found that
I need to retry between 1% and 2% of the time.  Overnight I found only
a small handful of times where I needed 2 retries.  I never found a
case where I needed 3 retries.

I'm completely aware of the fact that this is quite an ugly hack and I
wish I didn't have to resort to it, but I have no other real idea how
to make this hardware reliable.  If Rockchip in the future can come up
with a solution we can always revert this hack.  Until then, let's at
least have something that works.

This patch is tested atop Enric's latest dwc3 patch series ending at:
  https://patchwork.kernel.org/patch/10095527/
...but it could be applied independently of that series without any
bad effects.

For some more details on this bug, you can refer to:
  https://bugs.chromium.org/p/chromium/issues/detail?id=783464

Signed-off-by: Douglas Anderson 
---

 drivers/phy/rockchip/phy-rockchip-typec.c | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c 
b/drivers/phy/rockchip/phy-rockchip-typec.c
index ee85fa0ca4b0..5c2157156ce1 100644
--- a/drivers/phy/rockchip/phy-rockchip-typec.c
+++ b/drivers/phy/rockchip/phy-rockchip-typec.c
@@ -349,6 +349,8 @@
 #define MODE_DFP_USB   BIT(1)
 #define MODE_DFP_DPBIT(2)
 
+#define POWER_ON_TRIES 5
+
 struct usb3phy_reg {
u32 offset;
u32 enable_bit;
@@ -818,9 +820,8 @@ static int tcphy_get_mode(struct rockchip_typec_phy *tcphy)
return mode;
 }
 
-static int rockchip_usb3_phy_power_on(struct phy *phy)
+static int _rockchip_usb3_phy_power_on(struct rockchip_typec_phy *tcphy)
 {
-   struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
struct rockchip_usb3phy_port_cfg *cfg = >port_cfgs;
const struct usb3phy_reg *reg = >pipe_status;
int timeout, new_mode, ret = 0;
@@ -867,6 +868,25 @@ static int rockchip_usb3_phy_power_on(struct phy *phy)
return ret;
 }
 
+static int rockchip_usb3_phy_power_on(struct phy *phy)
+{
+   struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
+   int ret;
+   int tries;
+
+   for (tries = 0; tries < POWER_ON_TRIES; tries++) {
+   ret = _rockchip_usb3_phy_power_on(tcphy);
+   if (!ret)
+   break;
+   }
+
+   if (tries && !ret)
+   dev_info(tcphy->dev, "Needed %d loops to turn on\n", tries);
+
+   return ret;
+}
+
+
 static int rockchip_usb3_phy_power_off(struct phy *phy)
 {
struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
-- 
2.15.1.424.g9478a66081-goog

[PATCH] phy: rockchip-typec: Try to turn the PHY on several times

[Douglas Anderson]

Bind / unbind stress testing of the USB controller on rk3399 found
that we'd often end up with lots of failures that looked like this:

  phy phy-ff80.phy.9: phy poweron failed --> -110
  dwc3 fe90.dwc3: failed to initialize core
  dwc3: probe of fe90.dwc3 failed with error -110

Those errors were sometimes seen at bootup too, in which case USB
peripherals wouldn't work until unplugged and re-plugged in.

I spent some time trying to figure out why the PHY was failing to
power on but I wasn't able to.  Possibly this has to do with the fact
that the PHY docs say that the USB controller "needs to be held in
reset to hold pipe power state in P2 before initializing the Type C
PHY" but that doesn't appear to be easy to do with the dwc3 driver
today.  Messing around with the ordering of the reset vs. the PHY
initialization in the dwc3 driver didn't seem to fix things.

I did, however, find that if I simply retry the power on it seems to
have a good chance of working.  So let's add some retries.  I ran a
pretty tight bind/unbind loop overnight.  When I did so, I found that
I need to retry between 1% and 2% of the time.  Overnight I found only
a small handful of times where I needed 2 retries.  I never found a
case where I needed 3 retries.

I'm completely aware of the fact that this is quite an ugly hack and I
wish I didn't have to resort to it, but I have no other real idea how
to make this hardware reliable.  If Rockchip in the future can come up
with a solution we can always revert this hack.  Until then, let's at
least have something that works.

This patch is tested atop Enric's latest dwc3 patch series ending at:
  https://patchwork.kernel.org/patch/10095527/
...but it could be applied independently of that series without any
bad effects.

For some more details on this bug, you can refer to:
  https://bugs.chromium.org/p/chromium/issues/detail?id=783464

Signed-off-by: Douglas Anderson 
---

 drivers/phy/rockchip/phy-rockchip-typec.c | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c 
b/drivers/phy/rockchip/phy-rockchip-typec.c
index ee85fa0ca4b0..5c2157156ce1 100644
--- a/drivers/phy/rockchip/phy-rockchip-typec.c
+++ b/drivers/phy/rockchip/phy-rockchip-typec.c
@@ -349,6 +349,8 @@
 #define MODE_DFP_USB   BIT(1)
 #define MODE_DFP_DPBIT(2)
 
+#define POWER_ON_TRIES 5
+
 struct usb3phy_reg {
u32 offset;
u32 enable_bit;
@@ -818,9 +820,8 @@ static int tcphy_get_mode(struct rockchip_typec_phy *tcphy)
return mode;
 }
 
-static int rockchip_usb3_phy_power_on(struct phy *phy)
+static int _rockchip_usb3_phy_power_on(struct rockchip_typec_phy *tcphy)
 {
-   struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
struct rockchip_usb3phy_port_cfg *cfg = >port_cfgs;
const struct usb3phy_reg *reg = >pipe_status;
int timeout, new_mode, ret = 0;
@@ -867,6 +868,25 @@ static int rockchip_usb3_phy_power_on(struct phy *phy)
return ret;
 }
 
+static int rockchip_usb3_phy_power_on(struct phy *phy)
+{
+   struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
+   int ret;
+   int tries;
+
+   for (tries = 0; tries < POWER_ON_TRIES; tries++) {
+   ret = _rockchip_usb3_phy_power_on(tcphy);
+   if (!ret)
+   break;
+   }
+
+   if (tries && !ret)
+   dev_info(tcphy->dev, "Needed %d loops to turn on\n", tries);
+
+   return ret;
+}
+
+
 static int rockchip_usb3_phy_power_off(struct phy *phy)
 {
struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
-- 
2.15.1.424.g9478a66081-goog

Re: [PATCH v3] binder: fix proc->files use-after-free

[Greg KH]

On Mon, Dec 11, 2017 at 01:23:28PM -0800, Todd Kjos wrote:
> Greg- when this is in, we'll want it in 4.14 as well.

Ugh, missed that, I'll be sure to mark it for stable, thanks for letting
me know.

greg k-h

Re: [PATCH v3] binder: fix proc->files use-after-free

[Greg KH]

On Mon, Dec 11, 2017 at 01:23:28PM -0800, Todd Kjos wrote:
> Greg- when this is in, we'll want it in 4.14 as well.

Ugh, missed that, I'll be sure to mark it for stable, thanks for letting
me know.

greg k-h

Re: [REGRESSION][4.13.y][4.14.y][v4.15.y] net: reduce skb_warn_bad_offload() noise

[Greg Kroah-Hartman]

On Mon, Dec 11, 2017 at 04:25:26PM -0500, Willem de Bruijn wrote:
> Note that UFO was removed in 4.14 and that skb_warn_bad_offload
> can happen for various types of packets, so there may be multiple
> independent bug reports. I'm investigating two other non-UFO reports
> just now.

Meta-comment, now that UFO is gone from mainline, I'm wondering if I
should just delete it from 4.4 and 4.9 as well.  Any objections for
that?  I'd like to make it easy to maintain these kernels for a while,
and having them diverge like this, with all of the issues around UFO,
seems like it will just make life harder for myself if I leave it in.

Any opinions?

thanks,

greg k-h

Re: [REGRESSION][4.13.y][4.14.y][v4.15.y] net: reduce skb_warn_bad_offload() noise

[Greg Kroah-Hartman]

On Mon, Dec 11, 2017 at 04:25:26PM -0500, Willem de Bruijn wrote:
> Note that UFO was removed in 4.14 and that skb_warn_bad_offload
> can happen for various types of packets, so there may be multiple
> independent bug reports. I'm investigating two other non-UFO reports
> just now.

Meta-comment, now that UFO is gone from mainline, I'm wondering if I
should just delete it from 4.4 and 4.9 as well.  Any objections for
that?  I'd like to make it easy to maintain these kernels for a while,
and having them diverge like this, with all of the issues around UFO,
seems like it will just make life harder for myself if I leave it in.

Any opinions?

thanks,

greg k-h

Re: [PATCH v5] kernel: make groups_sort calling a responsibility group_info allocators

[NeilBrown]

On Mon, Dec 11 2017, Thiago Rafael Becker wrote:

> In testing, we found that nfsd threads may call set_groups in parallel for
> the same entry cached in auth.unix.gid, racing in the call of groups_sort,
> corrupting the groups for that entry and leading to permission denials for
> the client.
>
> This patch:
>  - Make groups_sort globally visible.
>  - Move the call to groups_sort to the modifiers of group_info
>  - Remove the call to groups_sort from set_groups
>
> Signed-off-by: Thiago Rafael Becker 

Reviewed-by: NeilBrown 

Thanks.  I like this better as a single patch.

I'd probably suggest "Cc: sta...@vger.kernel.org".
Less important bugfixes have gone to stable...

It might be nice to enhance
  Documentation/security/credentials.rst 
to explain that the groups list is always sorted, and must be sorted
before set_groups() or set_current_groups() is called, but
as the document doesn't mention any of this at all, this is purely
an extra enhancement and doesn't need to be included in the same patch.

David: do you agree that this sort of content would be appropriate in
that file (which you apparently authored).  Would you like to update it,
or shall I?

Thanks,
NeilBrown



> ---
>  arch/s390/kernel/compat_linux.c   | 1 +
>  fs/nfsd/auth.c| 3 +++
>  include/linux/cred.h  | 1 +
>  kernel/groups.c   | 5 +++--
>  kernel/uid16.c| 1 +
>  net/sunrpc/auth_gss/gss_rpc_xdr.c | 1 +
>  net/sunrpc/auth_gss/svcauth_gss.c | 1 +
>  net/sunrpc/svcauth_unix.c | 2 ++
>  8 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/arch/s390/kernel/compat_linux.c b/arch/s390/kernel/compat_linux.c
> index f04db37..59eea9c 100644
> --- a/arch/s390/kernel/compat_linux.c
> +++ b/arch/s390/kernel/compat_linux.c
> @@ -263,6 +263,7 @@ COMPAT_SYSCALL_DEFINE2(s390_setgroups16, int, gidsetsize, 
> u16 __user *, grouplis
>   return retval;
>   }
>  
> + groups_sort(group_info);
>   retval = set_current_groups(group_info);
>   put_group_info(group_info);
>  
> diff --git a/fs/nfsd/auth.c b/fs/nfsd/auth.c
> index 697f8ae..f650e47 100644
> --- a/fs/nfsd/auth.c
> +++ b/fs/nfsd/auth.c
> @@ -60,6 +60,9 @@ int nfsd_setuser(struct svc_rqst *rqstp, struct svc_export 
> *exp)
>   gi->gid[i] = exp->ex_anon_gid;
>   else
>   gi->gid[i] = rqgi->gid[i];
> +
> + /* Each thread allocates its own gi, no race */
> + groups_sort(gi);
>   }
>   } else {
>   gi = get_group_info(rqgi);
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index 099058e..6312865 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -83,6 +83,7 @@ extern int set_current_groups(struct group_info *);
>  extern void set_groups(struct cred *, struct group_info *);
>  extern int groups_search(const struct group_info *, kgid_t);
>  extern bool may_setgroups(void);
> +extern void groups_sort(struct group_info *);
>  
>  /*
>   * The security context of a task
> diff --git a/kernel/groups.c b/kernel/groups.c
> index e357bc8..daae2f2 100644
> --- a/kernel/groups.c
> +++ b/kernel/groups.c
> @@ -86,11 +86,12 @@ static int gid_cmp(const void *_a, const void *_b)
>   return gid_gt(a, b) - gid_lt(a, b);
>  }
>  
> -static void groups_sort(struct group_info *group_info)
> +void groups_sort(struct group_info *group_info)
>  {
>   sort(group_info->gid, group_info->ngroups, sizeof(*group_info->gid),
>gid_cmp, NULL);
>  }
> +EXPORT_SYMBOL(groups_sort);
>  
>  /* a simple bsearch */
>  int groups_search(const struct group_info *group_info, kgid_t grp)
> @@ -122,7 +123,6 @@ int groups_search(const struct group_info *group_info, 
> kgid_t grp)
>  void set_groups(struct cred *new, struct group_info *group_info)
>  {
>   put_group_info(new->group_info);
> - groups_sort(group_info);
>   get_group_info(group_info);
>   new->group_info = group_info;
>  }
> @@ -206,6 +206,7 @@ SYSCALL_DEFINE2(setgroups, int, gidsetsize, gid_t __user 
> *, grouplist)
>   return retval;
>   }
>  
> + groups_sort(group_info);
>   retval = set_current_groups(group_info);
>   put_group_info(group_info);
>  
> diff --git a/kernel/uid16.c b/kernel/uid16.c
> index ce74a49..ef1da2a 100644
> --- a/kernel/uid16.c
> +++ b/kernel/uid16.c
> @@ -192,6 +192,7 @@ SYSCALL_DEFINE2(setgroups16, int, gidsetsize, old_gid_t 
> __user *, grouplist)
>   return retval;
>   }
>  
> + groups_sort(group_info);
>   retval = set_current_groups(group_info);
>   put_group_info(group_info);
>  
> diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c 
> b/net/sunrpc/auth_gss/gss_rpc_xdr.c
> index c4778ca..444380f 100644
> --- a/net/sunrpc/auth_gss/gss_rpc_xdr.c
> +++ b/net/sunrpc/auth_gss/gss_rpc_xdr.c
> @@ -231,6 +231,7 @@ static int 

Re: [PATCH v5] kernel: make groups_sort calling a responsibility group_info allocators

[NeilBrown]

On Mon, Dec 11 2017, Thiago Rafael Becker wrote:

> In testing, we found that nfsd threads may call set_groups in parallel for
> the same entry cached in auth.unix.gid, racing in the call of groups_sort,
> corrupting the groups for that entry and leading to permission denials for
> the client.
>
> This patch:
>  - Make groups_sort globally visible.
>  - Move the call to groups_sort to the modifiers of group_info
>  - Remove the call to groups_sort from set_groups
>
> Signed-off-by: Thiago Rafael Becker 

Reviewed-by: NeilBrown 

Thanks.  I like this better as a single patch.

I'd probably suggest "Cc: sta...@vger.kernel.org".
Less important bugfixes have gone to stable...

It might be nice to enhance
  Documentation/security/credentials.rst 
to explain that the groups list is always sorted, and must be sorted
before set_groups() or set_current_groups() is called, but
as the document doesn't mention any of this at all, this is purely
an extra enhancement and doesn't need to be included in the same patch.

David: do you agree that this sort of content would be appropriate in
that file (which you apparently authored).  Would you like to update it,
or shall I?

Thanks,
NeilBrown



> ---
>  arch/s390/kernel/compat_linux.c   | 1 +
>  fs/nfsd/auth.c| 3 +++
>  include/linux/cred.h  | 1 +
>  kernel/groups.c   | 5 +++--
>  kernel/uid16.c| 1 +
>  net/sunrpc/auth_gss/gss_rpc_xdr.c | 1 +
>  net/sunrpc/auth_gss/svcauth_gss.c | 1 +
>  net/sunrpc/svcauth_unix.c | 2 ++
>  8 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/arch/s390/kernel/compat_linux.c b/arch/s390/kernel/compat_linux.c
> index f04db37..59eea9c 100644
> --- a/arch/s390/kernel/compat_linux.c
> +++ b/arch/s390/kernel/compat_linux.c
> @@ -263,6 +263,7 @@ COMPAT_SYSCALL_DEFINE2(s390_setgroups16, int, gidsetsize, 
> u16 __user *, grouplis
>   return retval;
>   }
>  
> + groups_sort(group_info);
>   retval = set_current_groups(group_info);
>   put_group_info(group_info);
>  
> diff --git a/fs/nfsd/auth.c b/fs/nfsd/auth.c
> index 697f8ae..f650e47 100644
> --- a/fs/nfsd/auth.c
> +++ b/fs/nfsd/auth.c
> @@ -60,6 +60,9 @@ int nfsd_setuser(struct svc_rqst *rqstp, struct svc_export 
> *exp)
>   gi->gid[i] = exp->ex_anon_gid;
>   else
>   gi->gid[i] = rqgi->gid[i];
> +
> + /* Each thread allocates its own gi, no race */
> + groups_sort(gi);
>   }
>   } else {
>   gi = get_group_info(rqgi);
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index 099058e..6312865 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -83,6 +83,7 @@ extern int set_current_groups(struct group_info *);
>  extern void set_groups(struct cred *, struct group_info *);
>  extern int groups_search(const struct group_info *, kgid_t);
>  extern bool may_setgroups(void);
> +extern void groups_sort(struct group_info *);
>  
>  /*
>   * The security context of a task
> diff --git a/kernel/groups.c b/kernel/groups.c
> index e357bc8..daae2f2 100644
> --- a/kernel/groups.c
> +++ b/kernel/groups.c
> @@ -86,11 +86,12 @@ static int gid_cmp(const void *_a, const void *_b)
>   return gid_gt(a, b) - gid_lt(a, b);
>  }
>  
> -static void groups_sort(struct group_info *group_info)
> +void groups_sort(struct group_info *group_info)
>  {
>   sort(group_info->gid, group_info->ngroups, sizeof(*group_info->gid),
>gid_cmp, NULL);
>  }
> +EXPORT_SYMBOL(groups_sort);
>  
>  /* a simple bsearch */
>  int groups_search(const struct group_info *group_info, kgid_t grp)
> @@ -122,7 +123,6 @@ int groups_search(const struct group_info *group_info, 
> kgid_t grp)
>  void set_groups(struct cred *new, struct group_info *group_info)
>  {
>   put_group_info(new->group_info);
> - groups_sort(group_info);
>   get_group_info(group_info);
>   new->group_info = group_info;
>  }
> @@ -206,6 +206,7 @@ SYSCALL_DEFINE2(setgroups, int, gidsetsize, gid_t __user 
> *, grouplist)
>   return retval;
>   }
>  
> + groups_sort(group_info);
>   retval = set_current_groups(group_info);
>   put_group_info(group_info);
>  
> diff --git a/kernel/uid16.c b/kernel/uid16.c
> index ce74a49..ef1da2a 100644
> --- a/kernel/uid16.c
> +++ b/kernel/uid16.c
> @@ -192,6 +192,7 @@ SYSCALL_DEFINE2(setgroups16, int, gidsetsize, old_gid_t 
> __user *, grouplist)
>   return retval;
>   }
>  
> + groups_sort(group_info);
>   retval = set_current_groups(group_info);
>   put_group_info(group_info);
>  
> diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c 
> b/net/sunrpc/auth_gss/gss_rpc_xdr.c
> index c4778ca..444380f 100644
> --- a/net/sunrpc/auth_gss/gss_rpc_xdr.c
> +++ b/net/sunrpc/auth_gss/gss_rpc_xdr.c
> @@ -231,6 +231,7 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
>

Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray

[Dave Chinner]

On Sat, Dec 09, 2017 at 09:00:18AM -0800, Joe Perches wrote:
> On Sat, 2017-12-09 at 09:36 +1100, Dave Chinner wrote:
> > 1. Using lockdep_set_novalidate_class() for anything other
> > than device->mutex will throw checkpatch warnings. Nice. (*)
> []
> > (*) checkpatch.pl is considered mostly harmful round here, too,
> > but that's another rant
> 
> How so?

Short story is that it barfs all over the slightly non-standard
coding style used in XFS.  It generates enough noise on incidental
things we aren't important that it complicates simple things. e.g. I
just moved a block of defines from one header to another, and
checkpatch throws about 10 warnings on that because of whitespace.
I'm just moving code - I don't want to change it and it doesn't need
to be modified because it is neat and easy to read and is obviously
correct. A bunch of prototypes I added another parameter to gets
warnings because it uses "unsigned" for an existing parameter that
I'm not changing. And so on.

This sort of stuff is just lowest-common-denominator noise - great
for new code and/or inexperienced developers, but not for working
with large bodies of existing code with slightly non-standard
conventions. Churning *lots* of code we otherwise wouldn't touch
just to shut up checkpatch warnings takes time, risks regressions
and makes it harder to trace the history of the code when we are
trying to track down bugs.

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com

Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray

[Dave Chinner]

On Sat, Dec 09, 2017 at 09:00:18AM -0800, Joe Perches wrote:
> On Sat, 2017-12-09 at 09:36 +1100, Dave Chinner wrote:
> > 1. Using lockdep_set_novalidate_class() for anything other
> > than device->mutex will throw checkpatch warnings. Nice. (*)
> []
> > (*) checkpatch.pl is considered mostly harmful round here, too,
> > but that's another rant
> 
> How so?

Short story is that it barfs all over the slightly non-standard
coding style used in XFS.  It generates enough noise on incidental
things we aren't important that it complicates simple things. e.g. I
just moved a block of defines from one header to another, and
checkpatch throws about 10 warnings on that because of whitespace.
I'm just moving code - I don't want to change it and it doesn't need
to be modified because it is neat and easy to read and is obviously
correct. A bunch of prototypes I added another parameter to gets
warnings because it uses "unsigned" for an existing parameter that
I'm not changing. And so on.

This sort of stuff is just lowest-common-denominator noise - great
for new code and/or inexperienced developers, but not for working
with large bodies of existing code with slightly non-standard
conventions. Churning *lots* of code we otherwise wouldn't touch
just to shut up checkpatch warnings takes time, risks regressions
and makes it harder to trace the history of the code when we are
trying to track down bugs.

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com

Re: [PATCH] media: pxa_camera: disable and unprepare the clock source on error

[Laurent Pinchart]

Hi Flavio,

On Monday, 11 December 2017 23:05:46 EET Flavio Ceolin wrote:
> > On Wednesday, 6 December 2017 18:38:50 EET Flavio Ceolin wrote:
> >> pxa_camera_probe() was not calling pxa_camera_deactivate(),
> >> responsible to call clk_disable_unprepare(), on the failure path. This
> >> was leading to unbalancing source clock.
> >> 
> >> Found by Linux Driver Verification project (linuxtesting.org).
> > 
> > Any chance I could sign you up for more work on this driver ? :-)
> 
> Definetely, this would be great :)

Actually it looks like the work I thought was needed has already been 
performed. The pxa-camera driver used to make use of the soc-camera framework, 
which we are trying to remove, and occurrences of soc_camera in the code gave 
me the wrong idea that the driver was still based on that framework. It seems 
this isn't the case. Thank you for making me happy :-)

> >> Signed-off-by: Flavio Ceolin 
> > 
> > Reviewed-by: Laurent Pinchart 
> > 
> > I expect Hans Verkuil to pick up the patch.
> > 
> >> ---
> >> 
> >>  drivers/media/platform/pxa_camera.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/media/platform/pxa_camera.c
> >> b/drivers/media/platform/pxa_camera.c index 9d3f0cb..7877037 100644
> >> --- a/drivers/media/platform/pxa_camera.c
> >> +++ b/drivers/media/platform/pxa_camera.c
> >> @@ -2489,7 +2489,7 @@ static int pxa_camera_probe(struct platform_device
> >> *pdev)
> >>dev_set_drvdata(>dev, pcdev);
> >>err = v4l2_device_register(>dev, >v4l2_dev);
> >>if (err)
> >> -  goto exit_free_dma;
> >> +  goto exit_deactivate;
> >> 
> >>pcdev->asds[0] = >asd;
> >>pcdev->notifier.subdevs = pcdev->asds;
> >> @@ -2525,6 +2525,8 @@ static int pxa_camera_probe(struct platform_device
> >> *pdev)
> >>v4l2_clk_unregister(pcdev->mclk_clk);
> >>  exit_free_v4l2dev:
> >>v4l2_device_unregister(>v4l2_dev);
> >> +exit_deactivate:
> >> +  pxa_camera_deactivate(pcdev);
> >>  exit_free_dma:
> >>dma_release_channel(pcdev->dma_chans[2]);
> >>  exit_free_dma_u:

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: pxa_camera: disable and unprepare the clock source on error

[Laurent Pinchart]

Hi Flavio,

On Monday, 11 December 2017 23:05:46 EET Flavio Ceolin wrote:
> > On Wednesday, 6 December 2017 18:38:50 EET Flavio Ceolin wrote:
> >> pxa_camera_probe() was not calling pxa_camera_deactivate(),
> >> responsible to call clk_disable_unprepare(), on the failure path. This
> >> was leading to unbalancing source clock.
> >> 
> >> Found by Linux Driver Verification project (linuxtesting.org).
> > 
> > Any chance I could sign you up for more work on this driver ? :-)
> 
> Definetely, this would be great :)

Actually it looks like the work I thought was needed has already been 
performed. The pxa-camera driver used to make use of the soc-camera framework, 
which we are trying to remove, and occurrences of soc_camera in the code gave 
me the wrong idea that the driver was still based on that framework. It seems 
this isn't the case. Thank you for making me happy :-)

> >> Signed-off-by: Flavio Ceolin 
> > 
> > Reviewed-by: Laurent Pinchart 
> > 
> > I expect Hans Verkuil to pick up the patch.
> > 
> >> ---
> >> 
> >>  drivers/media/platform/pxa_camera.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/media/platform/pxa_camera.c
> >> b/drivers/media/platform/pxa_camera.c index 9d3f0cb..7877037 100644
> >> --- a/drivers/media/platform/pxa_camera.c
> >> +++ b/drivers/media/platform/pxa_camera.c
> >> @@ -2489,7 +2489,7 @@ static int pxa_camera_probe(struct platform_device
> >> *pdev)
> >>dev_set_drvdata(>dev, pcdev);
> >>err = v4l2_device_register(>dev, >v4l2_dev);
> >>if (err)
> >> -  goto exit_free_dma;
> >> +  goto exit_deactivate;
> >> 
> >>pcdev->asds[0] = >asd;
> >>pcdev->notifier.subdevs = pcdev->asds;
> >> @@ -2525,6 +2525,8 @@ static int pxa_camera_probe(struct platform_device
> >> *pdev)
> >>v4l2_clk_unregister(pcdev->mclk_clk);
> >>  exit_free_v4l2dev:
> >>v4l2_device_unregister(>v4l2_dev);
> >> +exit_deactivate:
> >> +  pxa_camera_deactivate(pcdev);
> >>  exit_free_dma:
> >>dma_release_channel(pcdev->dma_chans[2]);
> >>  exit_free_dma_u:

-- 
Regards,

Laurent Pinchart

Re: [PATCH] docs: refcount_t documentation

[Jonathan Corbet]

On Tue,  5 Dec 2017 12:46:35 +0200
Elena Reshetova  wrote:

> Some functions from refcount_t API provide different
> memory ordering guarantees that their atomic counterparts.
> This adds a document outlining these differences (
> Documentation/core-api/refcount-vs-atomic.rst) as well as
> some other minor improvements.

I've applied this, thanks, it looks good.

One thing I noticed, though, is that this landed in the core-api manual,
while the refcount_t documentation is in the driver-api manual.  The
cross-references work just fine and such, but this still doesn't seem quite
right.  Probably what should be done is to have all the refcount_t
material in core-api; I may do that if I get a moment.

Thanks,

jon

Re: [PATCH] docs: refcount_t documentation

[Jonathan Corbet]

On Tue,  5 Dec 2017 12:46:35 +0200
Elena Reshetova  wrote:

> Some functions from refcount_t API provide different
> memory ordering guarantees that their atomic counterparts.
> This adds a document outlining these differences (
> Documentation/core-api/refcount-vs-atomic.rst) as well as
> some other minor improvements.

I've applied this, thanks, it looks good.

One thing I noticed, though, is that this landed in the core-api manual,
while the refcount_t documentation is in the driver-api manual.  The
cross-references work just fine and such, but this still doesn't seem quite
right.  Probably what should be done is to have all the refcount_t
material in core-api; I may do that if I get a moment.

Thanks,

jon

Re: [PATCH v9 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver

[Jacek Anaszewski]

Dan,

On 12/11/2017 07:26 PM, Dan Murphy wrote:
> This adds the devicetree bindings for the LM3692x
> I2C LED string driver.
> 
> Acked-by: Pavel Machek 
> Signed-off-by: Dan Murphy 
> ---
> 
> v9 - Moved 2 nodes to Optional Child and renamed node names to device type
> https://patchwork.kernel.org/patch/10093757/
> 
> v8 - Added address-cells and size-cells as well as child node reg - 
> https://patchwork.kernel.org/patch/10091259/
> v7 - No changes - https://patchwork.kernel.org/patch/10087475/
> v6 - No changes -https://patchwork.kernel.org/patch/10085567/
> v5 - No Changes - https://patchwork.kernel.org/patch/10081071/
> v4 - Fix example node, added trigger entry, removed ambiguous x for 
> compatible and
> added common.txt pointer for label - 
> https://patchwork.kernel.org/patch/10060107
> v3 - No changes
> v2 - No changes - https://patchwork.kernel.org/patch/10056677/
> 
>  .../devicetree/bindings/leds/leds-lm3692x.txt  | 49 
> ++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt 
> b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
> new file mode 100644
> index ..a93e19edfb42
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
> @@ -0,0 +1,49 @@
> +* Texas Instruments - LM3692x Highly Efficient White LED Driver
> +
> +The LM3692x is an ultra-compact, highly efficient,
> +white-LED driver designed for LCD display backlighting.
> +
> +The main difference between the LM36922 and LM36923 is the number of
> +LED strings it supports.  The LM36922 supports two strings while the LM36923
> +supports three strings.
> +
> +Required properties:
> + - compatible:
> + "ti,lm36922"
> + "ti,lm36923"
> + - reg :  I2C slave address
> + - #address-cells : 1
> + - #size-cells : 0
> +
> +Optional properties:
> + - enable-gpios : gpio pin to enable/disable the device.
> + - vled-supply : LED supply
> +
> +Required child properties:
> + - reg : 0
> +
> +Optional child properties:
> + - label : see Documentation/devicetree/bindings/leds/common.txt
> + - linux,default-trigger :
> +see Documentation/devicetree/bindings/leds/common.txt
> +
> +Example:
> +
> +led-controller@36 {
> + compatible = "ti,lm3692x";
> + reg = <0x36>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + enable-gpios = < 28 GPIO_ACTIVE_HIGH>;
> + vled-supply = <>;
> +
> + led@0 {
> + reg = <0>;
> + label = "backlight_cluster";

We need to consequently adhere to LED class device naming
convention, so:

label = "white:backlight_cluster";

> + linux,default-trigger = "backlight";
> + };
> +}
> +
> +For more product information please see the link below:
> +http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf
> 

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v9 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver

[Jacek Anaszewski]

Dan,

On 12/11/2017 07:26 PM, Dan Murphy wrote:
> This adds the devicetree bindings for the LM3692x
> I2C LED string driver.
> 
> Acked-by: Pavel Machek 
> Signed-off-by: Dan Murphy 
> ---
> 
> v9 - Moved 2 nodes to Optional Child and renamed node names to device type
> https://patchwork.kernel.org/patch/10093757/
> 
> v8 - Added address-cells and size-cells as well as child node reg - 
> https://patchwork.kernel.org/patch/10091259/
> v7 - No changes - https://patchwork.kernel.org/patch/10087475/
> v6 - No changes -https://patchwork.kernel.org/patch/10085567/
> v5 - No Changes - https://patchwork.kernel.org/patch/10081071/
> v4 - Fix example node, added trigger entry, removed ambiguous x for 
> compatible and
> added common.txt pointer for label - 
> https://patchwork.kernel.org/patch/10060107
> v3 - No changes
> v2 - No changes - https://patchwork.kernel.org/patch/10056677/
> 
>  .../devicetree/bindings/leds/leds-lm3692x.txt  | 49 
> ++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt 
> b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
> new file mode 100644
> index ..a93e19edfb42
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
> @@ -0,0 +1,49 @@
> +* Texas Instruments - LM3692x Highly Efficient White LED Driver
> +
> +The LM3692x is an ultra-compact, highly efficient,
> +white-LED driver designed for LCD display backlighting.
> +
> +The main difference between the LM36922 and LM36923 is the number of
> +LED strings it supports.  The LM36922 supports two strings while the LM36923
> +supports three strings.
> +
> +Required properties:
> + - compatible:
> + "ti,lm36922"
> + "ti,lm36923"
> + - reg :  I2C slave address
> + - #address-cells : 1
> + - #size-cells : 0
> +
> +Optional properties:
> + - enable-gpios : gpio pin to enable/disable the device.
> + - vled-supply : LED supply
> +
> +Required child properties:
> + - reg : 0
> +
> +Optional child properties:
> + - label : see Documentation/devicetree/bindings/leds/common.txt
> + - linux,default-trigger :
> +see Documentation/devicetree/bindings/leds/common.txt
> +
> +Example:
> +
> +led-controller@36 {
> + compatible = "ti,lm3692x";
> + reg = <0x36>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + enable-gpios = < 28 GPIO_ACTIVE_HIGH>;
> + vled-supply = <>;
> +
> + led@0 {
> + reg = <0>;
> + label = "backlight_cluster";

We need to consequently adhere to LED class device naming
convention, so:

label = "white:backlight_cluster";

> + linux,default-trigger = "backlight";
> + };
> +}
> +
> +For more product information please see the link below:
> +http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf
> 

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v4 5/5] ARM: ep93xx: ts72xx: Add support for BK3 board - ts72xx derivative

[Lukasz Majewski]

Hi Hartley,

> On Thursday, November 30, 2017 4:52 PM, Lukasz Majewski wrote:
> >
> > The BK3 board is a derivative of the ts72xx reference design.  
> 
> Lukasz,
> 
> I was just reviewing the other TS-72xx boards and noticed this:
> 
> 
> 
> > +/* BK3 specific defines */
> > +#define BK3_CPLDVER_PHYS_BASE  0x2340
> > +#define BK3_CPLDVER_VIRT_BASE  0xfebfd000
> > +#define BK3_CPLDVER_SIZE   0x1000
> > +  
> 
> 
> 
> > +static struct map_desc bk3_io_desc[] __initdata = {
> > +   {
> > +   .virtual= BK3_CPLDVER_VIRT_BASE,
> > +   .pfn=
> > __phys_to_pfn(BK3_CPLDVER_PHYS_BASE),
> > +   .length = BK3_CPLDVER_SIZE,
> > +   .type   = MT_DEVICE,
> > +   }
> > +};
> > +  
> 
> This register appears to be common to all the TS-72xx boards.

The CPLD was used on the reference ts-72xx boards, but support for it
seems to not be present in the mainline kernel.

Do you have a ts72xx board with CPLD embedded? Is any of your design
using it?


My another concern - is it safe to perform IO mapping on memory regions
which are not used / specified? When I do a single ts72xx mapping - for
all boards - then we may end up with some mappings which are not needed.

With the code as it is - I only map regions which are already used on
relevant boards.

> 
> I don't think Arnd has pulled the series yet. Would you mind renaming
> the defines and rebasing this patch? 

If needed I can resend the patch series, or prepare a single fix patch.
No problem.

> The BK3 board and other TS-72xx
> boards can then have a common .map_io.
> 
> Thanks,
> Hartley
> 



Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de


pgpc4g3ZF32Fi.pgp
Description: OpenPGP digital signature

Re: [PATCH v4 5/5] ARM: ep93xx: ts72xx: Add support for BK3 board - ts72xx derivative

[Lukasz Majewski]

Hi Hartley,

> On Thursday, November 30, 2017 4:52 PM, Lukasz Majewski wrote:
> >
> > The BK3 board is a derivative of the ts72xx reference design.  
> 
> Lukasz,
> 
> I was just reviewing the other TS-72xx boards and noticed this:
> 
> 
> 
> > +/* BK3 specific defines */
> > +#define BK3_CPLDVER_PHYS_BASE  0x2340
> > +#define BK3_CPLDVER_VIRT_BASE  0xfebfd000
> > +#define BK3_CPLDVER_SIZE   0x1000
> > +  
> 
> 
> 
> > +static struct map_desc bk3_io_desc[] __initdata = {
> > +   {
> > +   .virtual= BK3_CPLDVER_VIRT_BASE,
> > +   .pfn=
> > __phys_to_pfn(BK3_CPLDVER_PHYS_BASE),
> > +   .length = BK3_CPLDVER_SIZE,
> > +   .type   = MT_DEVICE,
> > +   }
> > +};
> > +  
> 
> This register appears to be common to all the TS-72xx boards.

The CPLD was used on the reference ts-72xx boards, but support for it
seems to not be present in the mainline kernel.

Do you have a ts72xx board with CPLD embedded? Is any of your design
using it?


My another concern - is it safe to perform IO mapping on memory regions
which are not used / specified? When I do a single ts72xx mapping - for
all boards - then we may end up with some mappings which are not needed.

With the code as it is - I only map regions which are already used on
relevant boards.

> 
> I don't think Arnd has pulled the series yet. Would you mind renaming
> the defines and rebasing this patch? 

If needed I can resend the patch series, or prepare a single fix patch.
No problem.

> The BK3 board and other TS-72xx
> boards can then have a common .map_io.
> 
> Thanks,
> Hartley
> 



Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de


pgpc4g3ZF32Fi.pgp
Description: OpenPGP digital signature

[PATCH v3 2/2] hwmon (pmbus): cffps: Add debugfs entries

[Eddie James]

From: "Edward A. James" 

Add debugfs entries for additional power supply data, including part
number, serial number, FRU number, firmware revision, ccin, and the
input history of the power supply. The input history is 10 minutes of
input power data in the form of twenty 30-second packets. Each packet
contains average and maximum power for that 30 second period.

Signed-off-by: Edward A. James 
---
 drivers/hwmon/pmbus/ibm-cffps.c | 202 +++-
 1 file changed, 201 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c
index cb56da6..5376f04 100644
--- a/drivers/hwmon/pmbus/ibm-cffps.c
+++ b/drivers/hwmon/pmbus/ibm-cffps.c
@@ -8,12 +8,26 @@
  */
 
 #include 
+#include 
 #include 
+#include 
 #include 
+#include 
 #include 
+#include 
 
 #include "pmbus.h"
 
+#define CFFPS_FRU_CMD  0x9A
+#define CFFPS_PN_CMD   0x9B
+#define CFFPS_SN_CMD   0x9E
+#define CFFPS_CCIN_CMD 0xBD
+#define CFFPS_FW_CMD_START 0xFA
+#define CFFPS_FW_NUM_BYTES 4
+
+#define CFFPS_INPUT_HISTORY_CMD0xD6
+#define CFFPS_INPUT_HISTORY_SIZE   100
+
 /* STATUS_MFR_SPECIFIC bits */
 #define CFFPS_MFR_FAN_FAULTBIT(0)
 #define CFFPS_MFR_THERMAL_FAULTBIT(1)
@@ -24,6 +38,144 @@
 #define CFFPS_MFR_VAUX_FAULT   BIT(6)
 #define CFFPS_MFR_CURRENT_SHARE_WARNINGBIT(7)
 
+enum {
+   CFFPS_DEBUGFS_INPUT_HISTORY = 0,
+   CFFPS_DEBUGFS_FRU,
+   CFFPS_DEBUGFS_PN,
+   CFFPS_DEBUGFS_SN,
+   CFFPS_DEBUGFS_CCIN,
+   CFFPS_DEBUGFS_FW,
+   CFFPS_DEBUGFS_NUM_ENTRIES
+};
+
+struct ibm_cffps_input_history {
+   struct mutex update_lock;
+   unsigned long last_update;
+
+   u8 byte_count;
+   u8 data[CFFPS_INPUT_HISTORY_SIZE];
+};
+
+struct ibm_cffps {
+   struct i2c_client *client;
+
+   struct ibm_cffps_input_history input_history;
+
+   int debugfs_entries[CFFPS_DEBUGFS_NUM_ENTRIES];
+};
+
+#define to_psu(x, y) container_of((x), struct ibm_cffps, debugfs_entries[(y)])
+
+static ssize_t ibm_cffps_read_input_history(struct ibm_cffps *psu,
+   char __user *buf, size_t count,
+   loff_t *ppos)
+{
+   int rc;
+   u8 msgbuf0[1] = { CFFPS_INPUT_HISTORY_CMD };
+   u8 msgbuf1[CFFPS_INPUT_HISTORY_SIZE + 1] = { 0 };
+   struct i2c_msg msg[2] = {
+   {
+   .addr = psu->client->addr,
+   .flags = psu->client->flags,
+   .len = 1,
+   .buf = msgbuf0,
+   }, {
+   .addr = psu->client->addr,
+   .flags = psu->client->flags | I2C_M_RD,
+   .len = CFFPS_INPUT_HISTORY_SIZE + 1,
+   .buf = msgbuf1,
+   },
+   };
+
+   if (!*ppos) {
+   mutex_lock(>input_history.update_lock);
+   if (time_after(jiffies, psu->input_history.last_update + HZ)) {
+   /*
+* Use a raw i2c transfer, since we need more bytes
+* than Linux I2C supports through smbus xfr (only 32).
+*/
+   rc = i2c_transfer(psu->client->adapter, msg, 2);
+   if (rc < 0) {
+   mutex_unlock(>input_history.update_lock);
+   return rc;
+   }
+
+   psu->input_history.byte_count = msgbuf1[0];
+   memcpy(psu->input_history.data, [1],
+  CFFPS_INPUT_HISTORY_SIZE);
+   psu->input_history.last_update = jiffies;
+   }
+
+   mutex_unlock(>input_history.update_lock);
+   }
+
+   return simple_read_from_buffer(buf, count, ppos,
+  psu->input_history.data,
+  psu->input_history.byte_count);
+}
+
+static ssize_t ibm_cffps_debugfs_op(struct file *file, char __user *buf,
+   size_t count, loff_t *ppos)
+{
+   u8 cmd;
+   int i, rc;
+   int *idxp = file->private_data;
+   int idx = *idxp;
+   struct ibm_cffps *psu = to_psu(idxp, idx);
+   char data[I2C_SMBUS_BLOCK_MAX] = { 0 };
+
+   switch (idx) {
+   case CFFPS_DEBUGFS_INPUT_HISTORY:
+   return ibm_cffps_read_input_history(psu, buf, count, ppos);
+   case CFFPS_DEBUGFS_FRU:
+   cmd = CFFPS_FRU_CMD;
+   break;
+   case CFFPS_DEBUGFS_PN:
+   cmd = CFFPS_PN_CMD;
+   break;
+   case CFFPS_DEBUGFS_SN:
+   cmd = 

[PATCH v3 1/2] hwmon (pmbus): Export pmbus device debugfs directory entry

[Eddie James]

From: "Edward A. James" 

Pmbus client drivers, if they want to use debugfs, should use the same
root directory as the pmbus debugfs entries are using. Therefore, export
the device dentry for the pmbus client.

Signed-off-by: Edward A. James 
---
 drivers/hwmon/pmbus/pmbus.h  | 2 ++
 drivers/hwmon/pmbus/pmbus_core.c | 8 
 2 files changed, 10 insertions(+)

diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index d39d506..1d24397 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -461,4 +461,6 @@ int pmbus_get_fan_rate_cached(struct i2c_client *client, 
int page, int id,
  enum pmbus_fan_mode mode);
 int pmbus_update_fan(struct i2c_client *client, int page, int id,
 u8 config, u8 mask, u16 command);
+struct dentry *pmbus_get_debugfs_dir(struct i2c_client *client);
+
 #endif /* PMBUS_H */
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 99ab39f..f7c47d7 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -2381,6 +2381,14 @@ int pmbus_do_remove(struct i2c_client *client)
 }
 EXPORT_SYMBOL_GPL(pmbus_do_remove);
 
+struct dentry *pmbus_get_debugfs_dir(struct i2c_client *client)
+{
+   struct pmbus_data *data = i2c_get_clientdata(client);
+
+   return data->debugfs;
+}
+EXPORT_SYMBOL_GPL(pmbus_get_debugfs_dir);
+
 static int __init pmbus_core_init(void)
 {
pmbus_debugfs_dir = debugfs_create_dir("pmbus", NULL);
-- 
1.8.3.1

[PATCH v3 2/2] hwmon (pmbus): cffps: Add debugfs entries

[Eddie James]

From: "Edward A. James" 

Add debugfs entries for additional power supply data, including part
number, serial number, FRU number, firmware revision, ccin, and the
input history of the power supply. The input history is 10 minutes of
input power data in the form of twenty 30-second packets. Each packet
contains average and maximum power for that 30 second period.

Signed-off-by: Edward A. James 
---
 drivers/hwmon/pmbus/ibm-cffps.c | 202 +++-
 1 file changed, 201 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c
index cb56da6..5376f04 100644
--- a/drivers/hwmon/pmbus/ibm-cffps.c
+++ b/drivers/hwmon/pmbus/ibm-cffps.c
@@ -8,12 +8,26 @@
  */
 
 #include 
+#include 
 #include 
+#include 
 #include 
+#include 
 #include 
+#include 
 
 #include "pmbus.h"
 
+#define CFFPS_FRU_CMD  0x9A
+#define CFFPS_PN_CMD   0x9B
+#define CFFPS_SN_CMD   0x9E
+#define CFFPS_CCIN_CMD 0xBD
+#define CFFPS_FW_CMD_START 0xFA
+#define CFFPS_FW_NUM_BYTES 4
+
+#define CFFPS_INPUT_HISTORY_CMD0xD6
+#define CFFPS_INPUT_HISTORY_SIZE   100
+
 /* STATUS_MFR_SPECIFIC bits */
 #define CFFPS_MFR_FAN_FAULTBIT(0)
 #define CFFPS_MFR_THERMAL_FAULTBIT(1)
@@ -24,6 +38,144 @@
 #define CFFPS_MFR_VAUX_FAULT   BIT(6)
 #define CFFPS_MFR_CURRENT_SHARE_WARNINGBIT(7)
 
+enum {
+   CFFPS_DEBUGFS_INPUT_HISTORY = 0,
+   CFFPS_DEBUGFS_FRU,
+   CFFPS_DEBUGFS_PN,
+   CFFPS_DEBUGFS_SN,
+   CFFPS_DEBUGFS_CCIN,
+   CFFPS_DEBUGFS_FW,
+   CFFPS_DEBUGFS_NUM_ENTRIES
+};
+
+struct ibm_cffps_input_history {
+   struct mutex update_lock;
+   unsigned long last_update;
+
+   u8 byte_count;
+   u8 data[CFFPS_INPUT_HISTORY_SIZE];
+};
+
+struct ibm_cffps {
+   struct i2c_client *client;
+
+   struct ibm_cffps_input_history input_history;
+
+   int debugfs_entries[CFFPS_DEBUGFS_NUM_ENTRIES];
+};
+
+#define to_psu(x, y) container_of((x), struct ibm_cffps, debugfs_entries[(y)])
+
+static ssize_t ibm_cffps_read_input_history(struct ibm_cffps *psu,
+   char __user *buf, size_t count,
+   loff_t *ppos)
+{
+   int rc;
+   u8 msgbuf0[1] = { CFFPS_INPUT_HISTORY_CMD };
+   u8 msgbuf1[CFFPS_INPUT_HISTORY_SIZE + 1] = { 0 };
+   struct i2c_msg msg[2] = {
+   {
+   .addr = psu->client->addr,
+   .flags = psu->client->flags,
+   .len = 1,
+   .buf = msgbuf0,
+   }, {
+   .addr = psu->client->addr,
+   .flags = psu->client->flags | I2C_M_RD,
+   .len = CFFPS_INPUT_HISTORY_SIZE + 1,
+   .buf = msgbuf1,
+   },
+   };
+
+   if (!*ppos) {
+   mutex_lock(>input_history.update_lock);
+   if (time_after(jiffies, psu->input_history.last_update + HZ)) {
+   /*
+* Use a raw i2c transfer, since we need more bytes
+* than Linux I2C supports through smbus xfr (only 32).
+*/
+   rc = i2c_transfer(psu->client->adapter, msg, 2);
+   if (rc < 0) {
+   mutex_unlock(>input_history.update_lock);
+   return rc;
+   }
+
+   psu->input_history.byte_count = msgbuf1[0];
+   memcpy(psu->input_history.data, [1],
+  CFFPS_INPUT_HISTORY_SIZE);
+   psu->input_history.last_update = jiffies;
+   }
+
+   mutex_unlock(>input_history.update_lock);
+   }
+
+   return simple_read_from_buffer(buf, count, ppos,
+  psu->input_history.data,
+  psu->input_history.byte_count);
+}
+
+static ssize_t ibm_cffps_debugfs_op(struct file *file, char __user *buf,
+   size_t count, loff_t *ppos)
+{
+   u8 cmd;
+   int i, rc;
+   int *idxp = file->private_data;
+   int idx = *idxp;
+   struct ibm_cffps *psu = to_psu(idxp, idx);
+   char data[I2C_SMBUS_BLOCK_MAX] = { 0 };
+
+   switch (idx) {
+   case CFFPS_DEBUGFS_INPUT_HISTORY:
+   return ibm_cffps_read_input_history(psu, buf, count, ppos);
+   case CFFPS_DEBUGFS_FRU:
+   cmd = CFFPS_FRU_CMD;
+   break;
+   case CFFPS_DEBUGFS_PN:
+   cmd = CFFPS_PN_CMD;
+   break;
+   case CFFPS_DEBUGFS_SN:
+   cmd = CFFPS_SN_CMD;
+   break;
+   case 

[PATCH v3 1/2] hwmon (pmbus): Export pmbus device debugfs directory entry

[Eddie James]

From: "Edward A. James" 

Pmbus client drivers, if they want to use debugfs, should use the same
root directory as the pmbus debugfs entries are using. Therefore, export
the device dentry for the pmbus client.

Signed-off-by: Edward A. James 
---
 drivers/hwmon/pmbus/pmbus.h  | 2 ++
 drivers/hwmon/pmbus/pmbus_core.c | 8 
 2 files changed, 10 insertions(+)

diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index d39d506..1d24397 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -461,4 +461,6 @@ int pmbus_get_fan_rate_cached(struct i2c_client *client, 
int page, int id,
  enum pmbus_fan_mode mode);
 int pmbus_update_fan(struct i2c_client *client, int page, int id,
 u8 config, u8 mask, u16 command);
+struct dentry *pmbus_get_debugfs_dir(struct i2c_client *client);
+
 #endif /* PMBUS_H */
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 99ab39f..f7c47d7 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -2381,6 +2381,14 @@ int pmbus_do_remove(struct i2c_client *client)
 }
 EXPORT_SYMBOL_GPL(pmbus_do_remove);
 
+struct dentry *pmbus_get_debugfs_dir(struct i2c_client *client)
+{
+   struct pmbus_data *data = i2c_get_clientdata(client);
+
+   return data->debugfs;
+}
+EXPORT_SYMBOL_GPL(pmbus_get_debugfs_dir);
+
 static int __init pmbus_core_init(void)
 {
pmbus_debugfs_dir = debugfs_create_dir("pmbus", NULL);
-- 
1.8.3.1

[PATCH v3 0/2] hwmon (pmbus): cffps: Add debugfs entries

[Eddie James]

From: "Edward A. James" 

This series enables debugfs entries in the IBM common form factor power supply
driver. In order to avoid multiple debugfs directories for the same driver, the
first patch in the series allows a pmbus client driver to query pmbus core for
it's debugfs directory.

The second patch in the series adds debugfs files for a number of attributes of
the power supply, including FRU (field replaceable unit) number, firmware
version, part number, serial number, and CCIN. There is also an entry for the
psu "input history" in binary format. This data represents previous 10 minutes
of power supply data.

Changes since v2:
 - CCIN is big-endian and has to be swapped.

Changes since v1:
 - Check for debugfs directory before allocating memory.
 - Add cffps1 sub-directory for the debugfs entries.
 - Always set last_update to jiffies - HZ.
 - Add EXPORT_SYMBOL_GPL for pmbus_get_debugfs_dir().

Edward A. James (2):
  hwmon (pmbus): Export pmbus device debugfs directory entry
  hwmon (pmbus): cffps: Add debugfs entries

 drivers/hwmon/pmbus/ibm-cffps.c  | 202 ++-
 drivers/hwmon/pmbus/pmbus.h  |   2 +
 drivers/hwmon/pmbus/pmbus_core.c |   8 ++
 3 files changed, 211 insertions(+), 1 deletion(-)

-- 
1.8.3.1

[PATCH v3 0/2] hwmon (pmbus): cffps: Add debugfs entries

[Eddie James]

From: "Edward A. James" 

This series enables debugfs entries in the IBM common form factor power supply
driver. In order to avoid multiple debugfs directories for the same driver, the
first patch in the series allows a pmbus client driver to query pmbus core for
it's debugfs directory.

The second patch in the series adds debugfs files for a number of attributes of
the power supply, including FRU (field replaceable unit) number, firmware
version, part number, serial number, and CCIN. There is also an entry for the
psu "input history" in binary format. This data represents previous 10 minutes
of power supply data.

Changes since v2:
 - CCIN is big-endian and has to be swapped.

Changes since v1:
 - Check for debugfs directory before allocating memory.
 - Add cffps1 sub-directory for the debugfs entries.
 - Always set last_update to jiffies - HZ.
 - Add EXPORT_SYMBOL_GPL for pmbus_get_debugfs_dir().

Edward A. James (2):
  hwmon (pmbus): Export pmbus device debugfs directory entry
  hwmon (pmbus): cffps: Add debugfs entries

 drivers/hwmon/pmbus/ibm-cffps.c  | 202 ++-
 drivers/hwmon/pmbus/pmbus.h  |   2 +
 drivers/hwmon/pmbus/pmbus_core.c |   8 ++
 3 files changed, 211 insertions(+), 1 deletion(-)

-- 
1.8.3.1

Re: [REGRESSION][4.13.y][4.14.y][v4.15.y] net: reduce skb_warn_bad_offload() noise

[David Miller]

From: Joseph Salisbury 
Date: Mon, 11 Dec 2017 15:35:34 -0500

> A kernel bug report was opened against Ubuntu [0].  It was found that
> reverting the following commit resolved this bug:
> 
> commit b2504a5dbef3305ef41988ad270b0e8ec289331c
> Author: Eric Dumazet 
> Date:   Tue Jan 31 10:20:32 2017 -0800
> 
>     net: reduce skb_warn_bad_offload() noise
>    
> 
> The regression was introduced as of v4.11-rc1 and still exists in
> current mainline.
>    
> I was hoping to get your feedback, since you are the patch author.  Do
> you think gathering any additional data will help diagnose this issue,
> or would it be best to submit a revert request?
>    
> This commit did in fact resolve another bug[1], but in the process
> introduced this regression.

It helps if you can consolidate the information obtained in your bug
tracking here in the email so that people on this list can get an idea
of what the problem scope might be without having to go to your
special bug tracking site.

This is really not about us being snobs about this mailing list, it's
about you wanting to get a result.  And you'll get a better result
faster if you post the details here on the lsit because most
developers are not going to go to your bug tracking site to read the
bug comments.

Also, this isn't a functional regression, it is just that we are
generating warnings that we didn't before.  It doesn't mean that
Eric's patch is wrong, it could just be that his new check is
triggering for a bug that has always been there.

Scanning the bug myself it seems that the critical required component
is IPSEC, and IPSEC has it's own way of doing segmentation offload.

Thanks.

Re: [REGRESSION][4.13.y][4.14.y][v4.15.y] net: reduce skb_warn_bad_offload() noise

[David Miller]

From: Joseph Salisbury 
Date: Mon, 11 Dec 2017 15:35:34 -0500

> A kernel bug report was opened against Ubuntu [0].  It was found that
> reverting the following commit resolved this bug:
> 
> commit b2504a5dbef3305ef41988ad270b0e8ec289331c
> Author: Eric Dumazet 
> Date:   Tue Jan 31 10:20:32 2017 -0800
> 
>     net: reduce skb_warn_bad_offload() noise
>    
> 
> The regression was introduced as of v4.11-rc1 and still exists in
> current mainline.
>    
> I was hoping to get your feedback, since you are the patch author.  Do
> you think gathering any additional data will help diagnose this issue,
> or would it be best to submit a revert request?
>    
> This commit did in fact resolve another bug[1], but in the process
> introduced this regression.

It helps if you can consolidate the information obtained in your bug
tracking here in the email so that people on this list can get an idea
of what the problem scope might be without having to go to your
special bug tracking site.

This is really not about us being snobs about this mailing list, it's
about you wanting to get a result.  And you'll get a better result
faster if you post the details here on the lsit because most
developers are not going to go to your bug tracking site to read the
bug comments.

Also, this isn't a functional regression, it is just that we are
generating warnings that we didn't before.  It doesn't mean that
Eric's patch is wrong, it could just be that his new check is
triggering for a bug that has always been there.

Scanning the bug myself it seems that the critical required component
is IPSEC, and IPSEC has it's own way of doing segmentation offload.

Thanks.

[PATCH] arch: define weak abort

[Sudip Mukherjee]

gcc was generating abort due to 'divide by zero' and if it is not
defined in the toolchain the build fails.

Currently 'frv' and 'arc' are failing. Previously other arch was also
broken like m32r was fixed by d22e3d69ee1a ("m32r: fix build failure").

Lets define this weak function which is common for all arch and fix the
problem permanently. We can even remove the arch specific 'abort' after
this is done.

Cc: Alexey Brodkin 
Signed-off-by: Sudip Mukherjee 
---
 kernel/exit.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/kernel/exit.c b/kernel/exit.c
index af6c245..90c6869 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1759,3 +1759,11 @@ long kernel_wait4(pid_t upid, int __user *stat_addr, int 
options,
return -EFAULT;
 }
 #endif
+
+__weak void abort(void)
+{
+   BUG();
+
+   /* if that doesn't kill us, halt */
+   panic("Oops failed to kill thread");
+}
-- 
1.9.1

[PATCH] arch: define weak abort

[Sudip Mukherjee]

gcc was generating abort due to 'divide by zero' and if it is not
defined in the toolchain the build fails.

Currently 'frv' and 'arc' are failing. Previously other arch was also
broken like m32r was fixed by d22e3d69ee1a ("m32r: fix build failure").

Lets define this weak function which is common for all arch and fix the
problem permanently. We can even remove the arch specific 'abort' after
this is done.

Cc: Alexey Brodkin 
Signed-off-by: Sudip Mukherjee 
---
 kernel/exit.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/kernel/exit.c b/kernel/exit.c
index af6c245..90c6869 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1759,3 +1759,11 @@ long kernel_wait4(pid_t upid, int __user *stat_addr, int 
options,
return -EFAULT;
 }
 #endif
+
+__weak void abort(void)
+{
+   BUG();
+
+   /* if that doesn't kill us, halt */
+   panic("Oops failed to kill thread");
+}
-- 
1.9.1

Re: [REGRESSION][4.13.y][4.14.y][v4.15.y] net: reduce skb_warn_bad_offload() noise

[Willem de Bruijn]

On Mon, Dec 11, 2017 at 3:35 PM, Joseph Salisbury
 wrote:
> Hi Eric,
>
> A kernel bug report was opened against Ubuntu [0].  It was found that
> reverting the following commit resolved this bug:

The recorded trace in that bug is against 4.10.0 with some backports.
Given that commit b2504a5dbef3 ("net: reduce skb_warn_bad_offload()
noise") is implicated, I guess that that was backported from 4.11-rc1.

The WARN shows

  e1000e: caps=(0x0030002149a9, 0x)
  len=1701 data_len=1659 gso_size=1480 gso_type=2 ip_summed=0

The numbering changed in 4.14, but for this kernel

  SKB_GSO_UDP = 1 << 1,

so this is a UFO packet with CHECKSUM_NONE. The stack shows

kernel: [570943.494549] skb_warn_bad_offload+0xd1/0x120
kernel: [570943.494550] __skb_gso_segment+0x17d/0x190
kernel: [570943.494564] validate_xmit_skb+0x14f/0x2a0
kernel: [570943.494565] validate_xmit_skb_list+0x43/0x70

so if that patch has been backported, then this must trigger in
__skb_gso_segment on the return path from skb_mac_gso_segment.

Did you backport

commit 8d63bee643f1fb53e472f0e135cae4eb99d62d19
Author: Willem de Bruijn 
Date:   Tue Aug 8 14:22:55 2017 -0400

net: avoid skb_warn_bad_offload false positives on UFO

skb_warn_bad_offload triggers a warning when an skb enters the GSO
stack at __skb_gso_segment that does not have CHECKSUM_PARTIAL
checksum offload set.

Commit b2504a5dbef3 ("net: reduce skb_warn_bad_offload() noise")
observed that SKB_GSO_DODGY producers can trigger the check and
that passing those packets through the GSO handlers will fix it
up. But, the software UFO handler will set ip_summed to
CHECKSUM_NONE.

When __skb_gso_segment is called from the receive path, this
triggers the warning again.

Make UFO set CHECKSUM_UNNECESSARY instead of CHECKSUM_NONE. On
Tx these two are equivalent. On Rx, this better matches the
skb state (checksum computed), as CHECKSUM_NONE here means no
checksum computed.

See also this thread for context:
http://patchwork.ozlabs.org/patch/799015/

Fixes: b2504a5dbef3 ("net: reduce skb_warn_bad_offload() noise")
Signed-off-by: Willem de Bruijn 
Signed-off-by: David S. Miller 

Note that UFO was removed in 4.14 and that skb_warn_bad_offload
can happen for various types of packets, so there may be multiple
independent bug reports. I'm investigating two other non-UFO reports
just now.

Re: [REGRESSION][4.13.y][4.14.y][v4.15.y] net: reduce skb_warn_bad_offload() noise

[Willem de Bruijn]

On Mon, Dec 11, 2017 at 3:35 PM, Joseph Salisbury
 wrote:
> Hi Eric,
>
> A kernel bug report was opened against Ubuntu [0].  It was found that
> reverting the following commit resolved this bug:

The recorded trace in that bug is against 4.10.0 with some backports.
Given that commit b2504a5dbef3 ("net: reduce skb_warn_bad_offload()
noise") is implicated, I guess that that was backported from 4.11-rc1.

The WARN shows

  e1000e: caps=(0x0030002149a9, 0x)
  len=1701 data_len=1659 gso_size=1480 gso_type=2 ip_summed=0

The numbering changed in 4.14, but for this kernel

  SKB_GSO_UDP = 1 << 1,

so this is a UFO packet with CHECKSUM_NONE. The stack shows

kernel: [570943.494549] skb_warn_bad_offload+0xd1/0x120
kernel: [570943.494550] __skb_gso_segment+0x17d/0x190
kernel: [570943.494564] validate_xmit_skb+0x14f/0x2a0
kernel: [570943.494565] validate_xmit_skb_list+0x43/0x70

so if that patch has been backported, then this must trigger in
__skb_gso_segment on the return path from skb_mac_gso_segment.

Did you backport

commit 8d63bee643f1fb53e472f0e135cae4eb99d62d19
Author: Willem de Bruijn 
Date:   Tue Aug 8 14:22:55 2017 -0400

net: avoid skb_warn_bad_offload false positives on UFO

skb_warn_bad_offload triggers a warning when an skb enters the GSO
stack at __skb_gso_segment that does not have CHECKSUM_PARTIAL
checksum offload set.

Commit b2504a5dbef3 ("net: reduce skb_warn_bad_offload() noise")
observed that SKB_GSO_DODGY producers can trigger the check and
that passing those packets through the GSO handlers will fix it
up. But, the software UFO handler will set ip_summed to
CHECKSUM_NONE.

When __skb_gso_segment is called from the receive path, this
triggers the warning again.

Make UFO set CHECKSUM_UNNECESSARY instead of CHECKSUM_NONE. On
Tx these two are equivalent. On Rx, this better matches the
skb state (checksum computed), as CHECKSUM_NONE here means no
checksum computed.

See also this thread for context:
http://patchwork.ozlabs.org/patch/799015/

Fixes: b2504a5dbef3 ("net: reduce skb_warn_bad_offload() noise")
Signed-off-by: Willem de Bruijn 
Signed-off-by: David S. Miller 

Note that UFO was removed in 4.14 and that skb_warn_bad_offload
can happen for various types of packets, so there may be multiple
independent bug reports. I'm investigating two other non-UFO reports
just now.

Re: [ANNOUNCE] 4.1.46-rt52

[Julia Cartwright]

On Mon, Dec 11, 2017 at 03:01:39PM -0600, Corey Minyard wrote:
> On 12/11/2017 02:20 PM, Julia Cartwright wrote:
> > On Mon, Dec 11, 2017 at 01:04:58PM -0600, Corey Minyard wrote:
[..]
> > > > 
> > > > Sebastian Andrzej Siewior (2):
> > > > PM / CPU: replace raw_notifier with atomic_notifier (fixup)
> > > This change breaks the compile of 4.1 on ARM (and other things using
> > > kernel/cpu_pm.c).
> > > 
> > > rcu_irq_enter_irqson() and friends doesn't exist in 4.1.  In fact,
> > > rcu_irq_enter() doesn't even exist.
> > > 
> > > Sorry I didn't get to testing this earlier, I've been focused on other 
> > > things.
> > Thanks!  My current builds apparently don't currently cover CPU_PM
> > builds, so I didn't see this :(.  Thanks for the report.
> > 
> > v4.1 does contain rcu_irq_enter(), and it looks like what we should be
> > using in 4.1 instead.
> 
> Ah, it does, I looked in the wrong place.
> 
> > I'm thinking the below should be okay; I'll be standing up a proper cpu
> > hotplugging test to give it some runtime.
> 
> The below should work.  You'll need an ARM64, ARM or MIPS system with
> the proper config settings to exercise this code, though.

Shouldn't be a problem, I've got an ARM64 board that I've been meaning
to integrate into my testing anyway.

Thanks,
   Julia

Re: [ANNOUNCE] 4.1.46-rt52

[Julia Cartwright]

On Mon, Dec 11, 2017 at 03:01:39PM -0600, Corey Minyard wrote:
> On 12/11/2017 02:20 PM, Julia Cartwright wrote:
> > On Mon, Dec 11, 2017 at 01:04:58PM -0600, Corey Minyard wrote:
[..]
> > > > 
> > > > Sebastian Andrzej Siewior (2):
> > > > PM / CPU: replace raw_notifier with atomic_notifier (fixup)
> > > This change breaks the compile of 4.1 on ARM (and other things using
> > > kernel/cpu_pm.c).
> > > 
> > > rcu_irq_enter_irqson() and friends doesn't exist in 4.1.  In fact,
> > > rcu_irq_enter() doesn't even exist.
> > > 
> > > Sorry I didn't get to testing this earlier, I've been focused on other 
> > > things.
> > Thanks!  My current builds apparently don't currently cover CPU_PM
> > builds, so I didn't see this :(.  Thanks for the report.
> > 
> > v4.1 does contain rcu_irq_enter(), and it looks like what we should be
> > using in 4.1 instead.
> 
> Ah, it does, I looked in the wrong place.
> 
> > I'm thinking the below should be okay; I'll be standing up a proper cpu
> > hotplugging test to give it some runtime.
> 
> The below should work.  You'll need an ARM64, ARM or MIPS system with
> the proper config settings to exercise this code, though.

Shouldn't be a problem, I've got an ARM64 board that I've been meaning
to integrate into my testing anyway.

Thanks,
   Julia

Re: [PATCH v3] binder: fix proc->files use-after-free

[Todd Kjos]

Greg- when this is in, we'll want it in 4.14 as well.

On Mon, Nov 27, 2017 at 9:32 AM, Todd Kjos  wrote:
> proc->files cleanup is initiated by binder_vma_close. Therefore
> a reference on the binder_proc is not enough to prevent the
> files_struct from being released while the binder_proc still has
> a reference. This can lead to an attempt to dereference the
> stale pointer obtained from proc->files prior to proc->files
> cleanup. This has been seen once in task_get_unused_fd_flags()
> when __alloc_fd() is called with a stale "files".
>
> The fix is to protect proc->files with a mutex to prevent cleanup
> while in use.
>
> Signed-off-by: Todd Kjos 
> ---
> v2: declare binder_get_files_struct as static
> v3: rework to protect proc->files with a mutex instead of using 
> get_files_struct
>
> Also needed in 4.14
>
>  drivers/android/binder.c | 44 +++-
>  1 file changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index a73596a4f804..7c027ee61375 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -482,7 +482,8 @@ enum binder_deferred_state {
>   * @tsk   task_struct for group_leader of process
>   *(invariant after initialized)
>   * @files files_struct for process
> - *(invariant after initialized)
> + *(protected by @files_lock)
> + * @files_lockmutex to protect @files
>   * @deferred_work_node:   element for binder_deferred_list
>   *(protected by binder_deferred_lock)
>   * @deferred_work:bitmap of deferred work to perform
> @@ -530,6 +531,7 @@ struct binder_proc {
> int pid;
> struct task_struct *tsk;
> struct files_struct *files;
> +   struct mutex files_lock;
> struct hlist_node deferred_work_node;
> int deferred_work;
> bool is_dead;
> @@ -877,20 +879,26 @@ static void binder_inc_node_tmpref_ilocked(struct 
> binder_node *node);
>
>  static int task_get_unused_fd_flags(struct binder_proc *proc, int flags)
>  {
> -   struct files_struct *files = proc->files;
> unsigned long rlim_cur;
> unsigned long irqs;
> +   int ret;
>
> -   if (files == NULL)
> -   return -ESRCH;
> -
> -   if (!lock_task_sighand(proc->tsk, ))
> -   return -EMFILE;
> -
> +   mutex_lock(>files_lock);
> +   if (proc->files == NULL) {
> +   ret = -ESRCH;
> +   goto err;
> +   }
> +   if (!lock_task_sighand(proc->tsk, )) {
> +   ret = -EMFILE;
> +   goto err;
> +   }
> rlim_cur = task_rlimit(proc->tsk, RLIMIT_NOFILE);
> unlock_task_sighand(proc->tsk, );
>
> -   return __alloc_fd(files, 0, rlim_cur, flags);
> +   ret = __alloc_fd(proc->files, 0, rlim_cur, flags);
> +err:
> +   mutex_unlock(>files_lock);
> +   return ret;
>  }
>
>  /*
> @@ -899,8 +907,10 @@ static int task_get_unused_fd_flags(struct binder_proc 
> *proc, int flags)
>  static void task_fd_install(
> struct binder_proc *proc, unsigned int fd, struct file *file)
>  {
> +   mutex_lock(>files_lock);
> if (proc->files)
> __fd_install(proc->files, fd, file);
> +   mutex_unlock(>files_lock);
>  }
>
>  /*
> @@ -910,9 +920,11 @@ static long task_close_fd(struct binder_proc *proc, 
> unsigned int fd)
>  {
> int retval;
>
> -   if (proc->files == NULL)
> -   return -ESRCH;
> -
> +   mutex_lock(>files_lock);
> +   if (proc->files == NULL) {
> +   retval = -ESRCH;
> +   goto err;
> +   }
> retval = __close_fd(proc->files, fd);
> /* can't restart close syscall because file table entry was cleared */
> if (unlikely(retval == -ERESTARTSYS ||
> @@ -920,7 +932,8 @@ static long task_close_fd(struct binder_proc *proc, 
> unsigned int fd)
>  retval == -ERESTARTNOHAND ||
>  retval == -ERESTART_RESTARTBLOCK))
> retval = -EINTR;
> -
> +err:
> +   mutex_unlock(>files_lock);
> return retval;
>  }
>
> @@ -4605,7 +4618,9 @@ static int binder_mmap(struct file *filp, struct 
> vm_area_struct *vma)
> ret = binder_alloc_mmap_handler(>alloc, vma);
> if (ret)
> return ret;
> +   mutex_lock(>files_lock);
> proc->files = get_files_struct(current);
> +   mutex_unlock(>files_lock);
> return 0;
>
>  err_bad_arg:
> @@ -4629,6 +4644,7 @@ static int binder_open(struct inode *nodp, struct file 
> *filp)
> spin_lock_init(>outer_lock);
> get_task_struct(current->group_leader);
> proc->tsk = current->group_leader;
> +   mutex_init(>files_lock);
> INIT_LIST_HEAD(>todo);
> proc->default_priority = task_nice(current);
>   

Re: [PATCH v3] binder: fix proc->files use-after-free

[Todd Kjos]

Greg- when this is in, we'll want it in 4.14 as well.

On Mon, Nov 27, 2017 at 9:32 AM, Todd Kjos  wrote:
> proc->files cleanup is initiated by binder_vma_close. Therefore
> a reference on the binder_proc is not enough to prevent the
> files_struct from being released while the binder_proc still has
> a reference. This can lead to an attempt to dereference the
> stale pointer obtained from proc->files prior to proc->files
> cleanup. This has been seen once in task_get_unused_fd_flags()
> when __alloc_fd() is called with a stale "files".
>
> The fix is to protect proc->files with a mutex to prevent cleanup
> while in use.
>
> Signed-off-by: Todd Kjos 
> ---
> v2: declare binder_get_files_struct as static
> v3: rework to protect proc->files with a mutex instead of using 
> get_files_struct
>
> Also needed in 4.14
>
>  drivers/android/binder.c | 44 +++-
>  1 file changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index a73596a4f804..7c027ee61375 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -482,7 +482,8 @@ enum binder_deferred_state {
>   * @tsk   task_struct for group_leader of process
>   *(invariant after initialized)
>   * @files files_struct for process
> - *(invariant after initialized)
> + *(protected by @files_lock)
> + * @files_lockmutex to protect @files
>   * @deferred_work_node:   element for binder_deferred_list
>   *(protected by binder_deferred_lock)
>   * @deferred_work:bitmap of deferred work to perform
> @@ -530,6 +531,7 @@ struct binder_proc {
> int pid;
> struct task_struct *tsk;
> struct files_struct *files;
> +   struct mutex files_lock;
> struct hlist_node deferred_work_node;
> int deferred_work;
> bool is_dead;
> @@ -877,20 +879,26 @@ static void binder_inc_node_tmpref_ilocked(struct 
> binder_node *node);
>
>  static int task_get_unused_fd_flags(struct binder_proc *proc, int flags)
>  {
> -   struct files_struct *files = proc->files;
> unsigned long rlim_cur;
> unsigned long irqs;
> +   int ret;
>
> -   if (files == NULL)
> -   return -ESRCH;
> -
> -   if (!lock_task_sighand(proc->tsk, ))
> -   return -EMFILE;
> -
> +   mutex_lock(>files_lock);
> +   if (proc->files == NULL) {
> +   ret = -ESRCH;
> +   goto err;
> +   }
> +   if (!lock_task_sighand(proc->tsk, )) {
> +   ret = -EMFILE;
> +   goto err;
> +   }
> rlim_cur = task_rlimit(proc->tsk, RLIMIT_NOFILE);
> unlock_task_sighand(proc->tsk, );
>
> -   return __alloc_fd(files, 0, rlim_cur, flags);
> +   ret = __alloc_fd(proc->files, 0, rlim_cur, flags);
> +err:
> +   mutex_unlock(>files_lock);
> +   return ret;
>  }
>
>  /*
> @@ -899,8 +907,10 @@ static int task_get_unused_fd_flags(struct binder_proc 
> *proc, int flags)
>  static void task_fd_install(
> struct binder_proc *proc, unsigned int fd, struct file *file)
>  {
> +   mutex_lock(>files_lock);
> if (proc->files)
> __fd_install(proc->files, fd, file);
> +   mutex_unlock(>files_lock);
>  }
>
>  /*
> @@ -910,9 +920,11 @@ static long task_close_fd(struct binder_proc *proc, 
> unsigned int fd)
>  {
> int retval;
>
> -   if (proc->files == NULL)
> -   return -ESRCH;
> -
> +   mutex_lock(>files_lock);
> +   if (proc->files == NULL) {
> +   retval = -ESRCH;
> +   goto err;
> +   }
> retval = __close_fd(proc->files, fd);
> /* can't restart close syscall because file table entry was cleared */
> if (unlikely(retval == -ERESTARTSYS ||
> @@ -920,7 +932,8 @@ static long task_close_fd(struct binder_proc *proc, 
> unsigned int fd)
>  retval == -ERESTARTNOHAND ||
>  retval == -ERESTART_RESTARTBLOCK))
> retval = -EINTR;
> -
> +err:
> +   mutex_unlock(>files_lock);
> return retval;
>  }
>
> @@ -4605,7 +4618,9 @@ static int binder_mmap(struct file *filp, struct 
> vm_area_struct *vma)
> ret = binder_alloc_mmap_handler(>alloc, vma);
> if (ret)
> return ret;
> +   mutex_lock(>files_lock);
> proc->files = get_files_struct(current);
> +   mutex_unlock(>files_lock);
> return 0;
>
>  err_bad_arg:
> @@ -4629,6 +4644,7 @@ static int binder_open(struct inode *nodp, struct file 
> *filp)
> spin_lock_init(>outer_lock);
> get_task_struct(current->group_leader);
> proc->tsk = current->group_leader;
> +   mutex_init(>files_lock);
> INIT_LIST_HEAD(>todo);
> proc->default_priority = task_nice(current);
> binder_dev = 

Re: [PATCH] Documentation: kernel-hacking: corrected a typo

[Jonathan Corbet]

On Fri,  8 Dec 2017 19:10:54 +0100
Marco Donato Torsello  wrote:

> Corrected a typo.
> 
> Signed-off-by: Marco Donato Torsello 
> ---
>  Documentation/kernel-hacking/hacking.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to the docs tree, thanks.

jon

Re: [PATCH] Documentation: kernel-hacking: corrected a typo

[Jonathan Corbet]

On Fri,  8 Dec 2017 19:10:54 +0100
Marco Donato Torsello  wrote:

> Corrected a typo.
> 
> Signed-off-by: Marco Donato Torsello 
> ---
>  Documentation/kernel-hacking/hacking.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to the docs tree, thanks.

jon

Re: [PATCH v2 2/2] drm/tinydrm: add driver for ST7735R panels

[Noralf Trønnes]

Den 10.12.2017 23.10, skrev David Lechner:

This adds a new driver for Sitronix ST7735R display panels.

This has been tested using an Adafruit 1.8" TFT.

Signed-off-by: David Lechner 
---



+static void st7735r_pipe_enable(struct drm_simple_display_pipe *pipe,
+   struct drm_crtc_state *crtc_state)
+{
+   struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
+   struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+   struct device *dev = tdev->drm->dev;
+   int ret;
+   u8 addr_mode;
+
+   DRM_DEBUG_KMS("\n");
+
+   mipi_dbi_hw_reset(mipi);
+
+   ret = mipi_dbi_command(mipi, MIPI_DCS_SOFT_RESET);
+   if (ret) {
+   DRM_DEV_ERROR(dev, "Error sending command %d\n", ret);
+   return;
+   }
+
+   msleep(150);
+
+   mipi_dbi_command(mipi, MIPI_DCS_EXIT_SLEEP_MODE);
+   msleep(500);
+
+   mipi_dbi_command(mipi, ST7735R_FRMCTR1, 0x01, 0x2c, 0x2d);
+   mipi_dbi_command(mipi, ST7735R_FRMCTR2, 0x01, 0x2c, 0x2d);
+   mipi_dbi_command(mipi, ST7735R_FRMCTR3, 0x01, 0x2c, 0x2d, 0x01, 0x2c,
+0x2d);
+   mipi_dbi_command(mipi, ST7735R_INVCTR, 0x07);
+   mipi_dbi_command(mipi, ST7735R_PWCTR1, 0xa2, 0x02, 0x84);
+   mipi_dbi_command(mipi, ST7735R_PWCTR2, 0xc5);
+   mipi_dbi_command(mipi, ST7735R_PWCTR3, 0x0a, 0x00);
+   mipi_dbi_command(mipi, ST7735R_PWCTR4, 0x8a, 0x2a);
+   mipi_dbi_command(mipi, ST7735R_PWCTR5, 0x8a, 0xee);
+   mipi_dbi_command(mipi, ST7735R_VMCTR1, 0x0e);
+   mipi_dbi_command(mipi, MIPI_DCS_EXIT_INVERT_MODE);
+   switch (mipi->rotation) {
+   default:
+   addr_mode = ST7735R_MX | ST7735R_MY;
+   break;
+   case 90:
+   addr_mode = ST7735R_MX | ST7735R_MV;
+   break;
+   case 180:
+   addr_mode = 0;
+   break;
+   case 270:
+   addr_mode = ST7735R_MY | ST7735R_MV;
+   break;
+   }
+   mipi_dbi_command(mipi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
+   mipi_dbi_command(mipi, MIPI_DCS_SET_PIXEL_FORMAT,
+MIPI_DCS_PIXEL_FMT_16BIT);
+   mipi_dbi_command(mipi, ST7735R_GAMCTRP1, 0x0f, 0x1a, 0x0f, 0x18, 0x2f,
+0x28, 0x20, 0x22, 0x1f, 0x1b, 0x23, 0x37, 0x00, 0x07,
+0x02, 0x10);
+   mipi_dbi_command(mipi, ST7735R_GAMCTRN1, 0x0f, 0x1b, 0x0f, 0x17, 0x33,
+0x2c, 0x29, 0x2e, 0x30, 0x30, 0x39, 0x3f, 0x00, 0x07,
+0x03, 0x10);
+   mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
+
+   msleep(100);
+
+   mipi_dbi_command(mipi, MIPI_DCS_ENTER_NORMAL_MODE);
+
+   msleep(20);
+
+   mipi_dbi_pipe_enable(pipe, crtc_state);
+}
+
+static const struct drm_simple_display_pipe_funcs st7735r_pipe_funcs = {
+   .enable = st7735r_pipe_enable,
+   .disable= mipi_dbi_pipe_disable,
+   .update = tinydrm_display_pipe_update,
+   .prepare_fb = tinydrm_display_pipe_prepare_fb,
+};
+
+static const struct drm_display_mode st7735r_mode = {
+   TINYDRM_MODE(128, 160, 28, 35),
+};


This naming talk has made me realise that these should be named after
the panel since they're not generic controller functions.
Maybe the mode can be generic, not sure if the physical size can/will
differ, the resolution is very likely to be the same for all.

What's your view on my descision to split the MIPI DBI compatible
controllers into per controller drivers instead of having one driver
that supports them all?
I've been afraid that it will be a very big file with macro definitions
per controller and enable functions per panel.

This driver has 15 macros and ~80 panel specific lines.
With one panel supported, half the driver is boilerplate.
The mi0283qt panel (MIPI DBI compatible) is in the same ballpark.

Adding helpers for panel reset/exit_sleep, for MIPI_DCS_SET_ADDRESS_MODE
and for display_on/enter_normal/flush could cut it down some more if we
made one driver to rule them all. Maybe the enable function per panel
could be cut in half that way.

I'm starting to wonder if one driver isn't such a bad solution after all...

Noralf.

Re: [PATCH v2 2/2] drm/tinydrm: add driver for ST7735R panels

[Noralf Trønnes]

Den 10.12.2017 23.10, skrev David Lechner:

This adds a new driver for Sitronix ST7735R display panels.

This has been tested using an Adafruit 1.8" TFT.

Signed-off-by: David Lechner 
---



+static void st7735r_pipe_enable(struct drm_simple_display_pipe *pipe,
+   struct drm_crtc_state *crtc_state)
+{
+   struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
+   struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+   struct device *dev = tdev->drm->dev;
+   int ret;
+   u8 addr_mode;
+
+   DRM_DEBUG_KMS("\n");
+
+   mipi_dbi_hw_reset(mipi);
+
+   ret = mipi_dbi_command(mipi, MIPI_DCS_SOFT_RESET);
+   if (ret) {
+   DRM_DEV_ERROR(dev, "Error sending command %d\n", ret);
+   return;
+   }
+
+   msleep(150);
+
+   mipi_dbi_command(mipi, MIPI_DCS_EXIT_SLEEP_MODE);
+   msleep(500);
+
+   mipi_dbi_command(mipi, ST7735R_FRMCTR1, 0x01, 0x2c, 0x2d);
+   mipi_dbi_command(mipi, ST7735R_FRMCTR2, 0x01, 0x2c, 0x2d);
+   mipi_dbi_command(mipi, ST7735R_FRMCTR3, 0x01, 0x2c, 0x2d, 0x01, 0x2c,
+0x2d);
+   mipi_dbi_command(mipi, ST7735R_INVCTR, 0x07);
+   mipi_dbi_command(mipi, ST7735R_PWCTR1, 0xa2, 0x02, 0x84);
+   mipi_dbi_command(mipi, ST7735R_PWCTR2, 0xc5);
+   mipi_dbi_command(mipi, ST7735R_PWCTR3, 0x0a, 0x00);
+   mipi_dbi_command(mipi, ST7735R_PWCTR4, 0x8a, 0x2a);
+   mipi_dbi_command(mipi, ST7735R_PWCTR5, 0x8a, 0xee);
+   mipi_dbi_command(mipi, ST7735R_VMCTR1, 0x0e);
+   mipi_dbi_command(mipi, MIPI_DCS_EXIT_INVERT_MODE);
+   switch (mipi->rotation) {
+   default:
+   addr_mode = ST7735R_MX | ST7735R_MY;
+   break;
+   case 90:
+   addr_mode = ST7735R_MX | ST7735R_MV;
+   break;
+   case 180:
+   addr_mode = 0;
+   break;
+   case 270:
+   addr_mode = ST7735R_MY | ST7735R_MV;
+   break;
+   }
+   mipi_dbi_command(mipi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
+   mipi_dbi_command(mipi, MIPI_DCS_SET_PIXEL_FORMAT,
+MIPI_DCS_PIXEL_FMT_16BIT);
+   mipi_dbi_command(mipi, ST7735R_GAMCTRP1, 0x0f, 0x1a, 0x0f, 0x18, 0x2f,
+0x28, 0x20, 0x22, 0x1f, 0x1b, 0x23, 0x37, 0x00, 0x07,
+0x02, 0x10);
+   mipi_dbi_command(mipi, ST7735R_GAMCTRN1, 0x0f, 0x1b, 0x0f, 0x17, 0x33,
+0x2c, 0x29, 0x2e, 0x30, 0x30, 0x39, 0x3f, 0x00, 0x07,
+0x03, 0x10);
+   mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
+
+   msleep(100);
+
+   mipi_dbi_command(mipi, MIPI_DCS_ENTER_NORMAL_MODE);
+
+   msleep(20);
+
+   mipi_dbi_pipe_enable(pipe, crtc_state);
+}
+
+static const struct drm_simple_display_pipe_funcs st7735r_pipe_funcs = {
+   .enable = st7735r_pipe_enable,
+   .disable= mipi_dbi_pipe_disable,
+   .update = tinydrm_display_pipe_update,
+   .prepare_fb = tinydrm_display_pipe_prepare_fb,
+};
+
+static const struct drm_display_mode st7735r_mode = {
+   TINYDRM_MODE(128, 160, 28, 35),
+};


This naming talk has made me realise that these should be named after
the panel since they're not generic controller functions.
Maybe the mode can be generic, not sure if the physical size can/will
differ, the resolution is very likely to be the same for all.

What's your view on my descision to split the MIPI DBI compatible
controllers into per controller drivers instead of having one driver
that supports them all?
I've been afraid that it will be a very big file with macro definitions
per controller and enable functions per panel.

This driver has 15 macros and ~80 panel specific lines.
With one panel supported, half the driver is boilerplate.
The mi0283qt panel (MIPI DBI compatible) is in the same ballpark.

Adding helpers for panel reset/exit_sleep, for MIPI_DCS_SET_ADDRESS_MODE
and for display_on/enter_normal/flush could cut it down some more if we
made one driver to rule them all. Maybe the enable function per panel
could be cut in half that way.

I'm starting to wonder if one driver isn't such a bad solution after all...

Noralf.

Re: [PATCH] Documentation: mono: Update links and s/CVS/Git/

[Jonathan Corbet]

On Sat,  9 Dec 2017 17:21:04 +0100
Jonathan Neuschäfer  wrote:

> The URLs in mono.rst redirect to pages on www.mono-project.com, so let's
> update them. I took the liberty to update the compilation instructions
> to the Linux-specific version, because readers of the kernel
> documentation will most likely use Linux.

Applied to the docs tree, thanks.  I do wonder if anybody actually does
this, though...

jon

Re: [PATCH] Documentation: mono: Update links and s/CVS/Git/

[Jonathan Corbet]

On Sat,  9 Dec 2017 17:21:04 +0100
Jonathan Neuschäfer  wrote:

> The URLs in mono.rst redirect to pages on www.mono-project.com, so let's
> update them. I took the liberty to update the compilation instructions
> to the Linux-specific version, because readers of the kernel
> documentation will most likely use Linux.

Applied to the docs tree, thanks.  I do wonder if anybody actually does
this, though...

jon

Re: [PATCH] tuners: tda8290: reduce stack usage with kasan

[Michael Ira Krufky]

On Mon, Dec 11, 2017 at 2:34 PM, Joe Perches  wrote:
> On Mon, 2017-12-11 at 13:06 +0100, Arnd Bergmann wrote:
>> With CONFIG_KASAN enabled, we get a relatively large stack frame in one 
>> function
>>
>> drivers/media/tuners/tda8290.c: In function 'tda8290_set_params':
>> drivers/media/tuners/tda8290.c:310:1: warning: the frame size of 1520 bytes 
>> is larger than 1024 bytes [-Wframe-larger-than=]
>>
>> With CONFIG_KASAN_EXTRA this goes up to
>>
>> drivers/media/tuners/tda8290.c: In function 'tda8290_set_params':
>> drivers/media/tuners/tda8290.c:310:1: error: the frame size of 3200 bytes is 
>> larger than 3072 bytes [-Werror=frame-larger-than=]
>>
>> We can significantly reduce this by marking local arrays as 'static const', 
>> and
>> this should result in better compiled code for everyone.
> []
>> diff --git a/drivers/media/tuners/tda8290.c b/drivers/media/tuners/tda8290.c
> []
>> @@ -63,8 +63,8 @@ static int tda8290_i2c_bridge(struct dvb_frontend *fe, int 
>> close)
>>  {
>>   struct tda8290_priv *priv = fe->analog_demod_priv;
>>
>> - unsigned char  enable[2] = { 0x21, 0xC0 };
>> - unsigned char disable[2] = { 0x21, 0x00 };
>> + static unsigned char  enable[2] = { 0x21, 0xC0 };
>> + static unsigned char disable[2] = { 0x21, 0x00 };
>
> Doesn't match commit message.
>
> static const or just static?
>
>> @@ -84,9 +84,9 @@ static int tda8295_i2c_bridge(struct dvb_frontend *fe, int 
>> close)
>>  {
>>   struct tda8290_priv *priv = fe->analog_demod_priv;
>>
>> - unsigned char  enable[2] = { 0x45, 0xc1 };
>> - unsigned char disable[2] = { 0x46, 0x00 };
>> - unsigned char buf[3] = { 0x45, 0x01, 0x00 };
>> + static unsigned char  enable[2] = { 0x45, 0xc1 };
>> + static unsigned char disable[2] = { 0x46, 0x00 };
>
> etc.
>
>


Joe is correct - they can be CONSTified. My bad -- a lot of the code I
wrote many years ago has this problem -- I wasn't so stack-conscious
back then.

The bytes in `enable` / `disable` don't get changed, but they may be
copied to another byte array that does get changed.  If would be best
to make these `static const`

Best regards,

Michael Ira Krufky

Re: [PATCH] tuners: tda8290: reduce stack usage with kasan

[Michael Ira Krufky]

On Mon, Dec 11, 2017 at 2:34 PM, Joe Perches  wrote:
> On Mon, 2017-12-11 at 13:06 +0100, Arnd Bergmann wrote:
>> With CONFIG_KASAN enabled, we get a relatively large stack frame in one 
>> function
>>
>> drivers/media/tuners/tda8290.c: In function 'tda8290_set_params':
>> drivers/media/tuners/tda8290.c:310:1: warning: the frame size of 1520 bytes 
>> is larger than 1024 bytes [-Wframe-larger-than=]
>>
>> With CONFIG_KASAN_EXTRA this goes up to
>>
>> drivers/media/tuners/tda8290.c: In function 'tda8290_set_params':
>> drivers/media/tuners/tda8290.c:310:1: error: the frame size of 3200 bytes is 
>> larger than 3072 bytes [-Werror=frame-larger-than=]
>>
>> We can significantly reduce this by marking local arrays as 'static const', 
>> and
>> this should result in better compiled code for everyone.
> []
>> diff --git a/drivers/media/tuners/tda8290.c b/drivers/media/tuners/tda8290.c
> []
>> @@ -63,8 +63,8 @@ static int tda8290_i2c_bridge(struct dvb_frontend *fe, int 
>> close)
>>  {
>>   struct tda8290_priv *priv = fe->analog_demod_priv;
>>
>> - unsigned char  enable[2] = { 0x21, 0xC0 };
>> - unsigned char disable[2] = { 0x21, 0x00 };
>> + static unsigned char  enable[2] = { 0x21, 0xC0 };
>> + static unsigned char disable[2] = { 0x21, 0x00 };
>
> Doesn't match commit message.
>
> static const or just static?
>
>> @@ -84,9 +84,9 @@ static int tda8295_i2c_bridge(struct dvb_frontend *fe, int 
>> close)
>>  {
>>   struct tda8290_priv *priv = fe->analog_demod_priv;
>>
>> - unsigned char  enable[2] = { 0x45, 0xc1 };
>> - unsigned char disable[2] = { 0x46, 0x00 };
>> - unsigned char buf[3] = { 0x45, 0x01, 0x00 };
>> + static unsigned char  enable[2] = { 0x45, 0xc1 };
>> + static unsigned char disable[2] = { 0x46, 0x00 };
>
> etc.
>
>


Joe is correct - they can be CONSTified. My bad -- a lot of the code I
wrote many years ago has this problem -- I wasn't so stack-conscious
back then.

The bytes in `enable` / `disable` don't get changed, but they may be
copied to another byte array that does get changed.  If would be best
to make these `static const`

Best regards,

Michael Ira Krufky

Re: [PATCH v2] mm: terminate shrink_slab loop if signal is pending

[Suren Baghdasaryan]

On Sun, Dec 10, 2017 at 2:13 AM, Michal Hocko  wrote:
> On Fri 08-12-17 10:06:26, Suren Baghdasaryan wrote:
>> On Fri, Dec 8, 2017 at 6:03 AM, Tetsuo Handa
>>  wrote:
>> > Michal Hocko wrote:
>> >> On Fri 08-12-17 20:36:16, Tetsuo Handa wrote:
>> >> > On 2017/12/08 17:22, Michal Hocko wrote:
>> >> > > On Thu 07-12-17 17:23:05, Suren Baghdasaryan wrote:
>> >> > >> Slab shrinkers can be quite time consuming and when signal
>> >> > >> is pending they can delay handling of the signal. If fatal
>> >> > >> signal is pending there is no point in shrinking that process
>> >> > >> since it will be killed anyway.
>> >> > >
>> >> > > The thing is that we are _not_ shrinking _that_ process. We are
>> >> > > shrinking globally shared objects and the fact that the memory 
>> >> > > pressure
>> >> > > is so large that the kswapd doesn't keep pace with it means that we 
>> >> > > have
>> >> > > to throttle all allocation sites by doing this direct reclaim. I agree
>> >> > > that expediting killed task is a good thing in general because such a
>> >> > > process should free at least some memory.
>>
>> Agree, wording here is inaccurate. My original intent was to have a
>> safeguard against slow shrinkers but I understand your concern that
>> this can mask a real problem in a shrinker. In essence expediting the
>> killing is the ultimate goal here but as you mentioned it's not as
>> simple as this change.
>
> Moreover it doesn't work if the SIGKILL can be delivered asynchronously
> (which is your case AFAICU).  You can be already running the slow
> shrinker at that time...

That is true but at least my change would not allow more than one
shrinker to run while SIGKILL is pending. And that's why this change
is not a fix for slow shrinkers, they have to be properly fixed with
or without this change.

>
> [...]
>> > I agree that making waits/loops killable is generally good. But be sure to 
>> > be
>> > prepared for the worst case. For example, start __GFP_KILLABLE from "best 
>> > effort"
>> > basis (i.e. no guarantee that the allocating thread will leave the page 
>> > allocator
>> > slowpath immediately) and check for fatal_signal_pending() only if
>> > __GFP_KILLABLE is set. That is,
>> >
>> > +   /*
>> > +* We are about to die and free our memory.
>> > +* Stop shrinking which might delay signal handling.
>> > +*/
>> > +   if (unlikely((gfp_mask & __GFP_KILLABLE) && 
>> > fatal_signal_pending(current)))
>> > +   break;
>> >
>> > at shrink_slab() etc. and
>> >
>> > +   if ((gfp_mask & __GFP_KILLABLE) && 
>> > fatal_signal_pending(current))
>> > +   goto nopage;
>> >
>> > at __alloc_pages_slowpath().
>>
>> I was thinking about something similar and will experiment to see if
>> this solves the problem and if it has any side effects. Anyone sees
>> any obvious problems with this approach?
>
> Tetsuo has been proposing this flag in the past and I've had objections
> why this is not a great idea. I do not have any link handy but the core
> objection was that the semantic would be too fuzzy. All the allocations
> in the same context would have to be killable for this flag to have any
> effect. Spreading it all over the kernel is simply not feasible.

Sounds like I'll have to do much more reading before touching this code :)

>
> --
> Michal Hocko
> SUSE Labs

Re: [PATCH v2] mm: terminate shrink_slab loop if signal is pending

[Suren Baghdasaryan]

On Sun, Dec 10, 2017 at 2:13 AM, Michal Hocko  wrote:
> On Fri 08-12-17 10:06:26, Suren Baghdasaryan wrote:
>> On Fri, Dec 8, 2017 at 6:03 AM, Tetsuo Handa
>>  wrote:
>> > Michal Hocko wrote:
>> >> On Fri 08-12-17 20:36:16, Tetsuo Handa wrote:
>> >> > On 2017/12/08 17:22, Michal Hocko wrote:
>> >> > > On Thu 07-12-17 17:23:05, Suren Baghdasaryan wrote:
>> >> > >> Slab shrinkers can be quite time consuming and when signal
>> >> > >> is pending they can delay handling of the signal. If fatal
>> >> > >> signal is pending there is no point in shrinking that process
>> >> > >> since it will be killed anyway.
>> >> > >
>> >> > > The thing is that we are _not_ shrinking _that_ process. We are
>> >> > > shrinking globally shared objects and the fact that the memory 
>> >> > > pressure
>> >> > > is so large that the kswapd doesn't keep pace with it means that we 
>> >> > > have
>> >> > > to throttle all allocation sites by doing this direct reclaim. I agree
>> >> > > that expediting killed task is a good thing in general because such a
>> >> > > process should free at least some memory.
>>
>> Agree, wording here is inaccurate. My original intent was to have a
>> safeguard against slow shrinkers but I understand your concern that
>> this can mask a real problem in a shrinker. In essence expediting the
>> killing is the ultimate goal here but as you mentioned it's not as
>> simple as this change.
>
> Moreover it doesn't work if the SIGKILL can be delivered asynchronously
> (which is your case AFAICU).  You can be already running the slow
> shrinker at that time...

That is true but at least my change would not allow more than one
shrinker to run while SIGKILL is pending. And that's why this change
is not a fix for slow shrinkers, they have to be properly fixed with
or without this change.

>
> [...]
>> > I agree that making waits/loops killable is generally good. But be sure to 
>> > be
>> > prepared for the worst case. For example, start __GFP_KILLABLE from "best 
>> > effort"
>> > basis (i.e. no guarantee that the allocating thread will leave the page 
>> > allocator
>> > slowpath immediately) and check for fatal_signal_pending() only if
>> > __GFP_KILLABLE is set. That is,
>> >
>> > +   /*
>> > +* We are about to die and free our memory.
>> > +* Stop shrinking which might delay signal handling.
>> > +*/
>> > +   if (unlikely((gfp_mask & __GFP_KILLABLE) && 
>> > fatal_signal_pending(current)))
>> > +   break;
>> >
>> > at shrink_slab() etc. and
>> >
>> > +   if ((gfp_mask & __GFP_KILLABLE) && 
>> > fatal_signal_pending(current))
>> > +   goto nopage;
>> >
>> > at __alloc_pages_slowpath().
>>
>> I was thinking about something similar and will experiment to see if
>> this solves the problem and if it has any side effects. Anyone sees
>> any obvious problems with this approach?
>
> Tetsuo has been proposing this flag in the past and I've had objections
> why this is not a great idea. I do not have any link handy but the core
> objection was that the semantic would be too fuzzy. All the allocations
> in the same context would have to be killable for this flag to have any
> effect. Spreading it all over the kernel is simply not feasible.

Sounds like I'll have to do much more reading before touching this code :)

>
> --
> Michal Hocko
> SUSE Labs

[PATCH v2] mm: Release a semaphore in 'get_vaddr_frames()'

[Christophe JAILLET]

A semaphore is acquired before this check, so we must release it before
leaving.

Fixes: b7f0554a56f2 ("mm: fail get_vaddr_frames() for filesystem-dax mappings")
Signed-off-by: Christophe JAILLET 
---
-- Untested --

v1 -> v2: 'goto out' instead of duplicating code
---
 mm/frame_vector.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index 297c7238f7d4..c64dca6e27c2 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -62,8 +62,10 @@ int get_vaddr_frames(unsigned long start, unsigned int 
nr_frames,
 * get_user_pages_longterm() and disallow it for filesystem-dax
 * mappings.
 */
-   if (vma_is_fsdax(vma))
-   return -EOPNOTSUPP;
+   if (vma_is_fsdax(vma)) {
+   ret = -EOPNOTSUPP;
+   goto out;
+   }
 
if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) {
vec->got_ref = true;
-- 
2.14.1

[PATCH v2] mm: Release a semaphore in 'get_vaddr_frames()'

[Christophe JAILLET]

A semaphore is acquired before this check, so we must release it before
leaving.

Fixes: b7f0554a56f2 ("mm: fail get_vaddr_frames() for filesystem-dax mappings")
Signed-off-by: Christophe JAILLET 
---
-- Untested --

v1 -> v2: 'goto out' instead of duplicating code
---
 mm/frame_vector.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index 297c7238f7d4..c64dca6e27c2 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -62,8 +62,10 @@ int get_vaddr_frames(unsigned long start, unsigned int 
nr_frames,
 * get_user_pages_longterm() and disallow it for filesystem-dax
 * mappings.
 */
-   if (vma_is_fsdax(vma))
-   return -EOPNOTSUPP;
+   if (vma_is_fsdax(vma)) {
+   ret = -EOPNOTSUPP;
+   goto out;
+   }
 
if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) {
vec->got_ref = true;
-- 
2.14.1

Re: [PATCH] media: pxa_camera: disable and unprepare the clock source on error

[Flavio Ceolin]

Hi Laurent,

> Hi Flavio,
>
> Thank you for the patch.
>
> On Wednesday, 6 December 2017 18:38:50 EET Flavio Ceolin wrote:
>> pxa_camera_probe() was not calling pxa_camera_deactivate(),
>> responsible to call clk_disable_unprepare(), on the failure path. This
>> was leading to unbalancing source clock.
>> 
>> Found by Linux Driver Verification project (linuxtesting.org).
>
> Any chance I could sign you up for more work on this driver ? :-)

Definetely, this would be great :)

>
>> Signed-off-by: Flavio Ceolin 
>
> Reviewed-by: Laurent Pinchart 
>
> I expect Hans Verkuil to pick up the patch.
>
>> ---
>>  drivers/media/platform/pxa_camera.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/media/platform/pxa_camera.c
>> b/drivers/media/platform/pxa_camera.c index 9d3f0cb..7877037 100644
>> --- a/drivers/media/platform/pxa_camera.c
>> +++ b/drivers/media/platform/pxa_camera.c
>> @@ -2489,7 +2489,7 @@ static int pxa_camera_probe(struct platform_device
>> *pdev) dev_set_drvdata(>dev, pcdev);
>>  err = v4l2_device_register(>dev, >v4l2_dev);
>>  if (err)
>> -goto exit_free_dma;
>> +goto exit_deactivate;
>> 
>>  pcdev->asds[0] = >asd;
>>  pcdev->notifier.subdevs = pcdev->asds;
>> @@ -2525,6 +2525,8 @@ static int pxa_camera_probe(struct platform_device
>> *pdev) v4l2_clk_unregister(pcdev->mclk_clk);
>>  exit_free_v4l2dev:
>>  v4l2_device_unregister(>v4l2_dev);
>> +exit_deactivate:
>> +pxa_camera_deactivate(pcdev);
>>  exit_free_dma:
>>  dma_release_channel(pcdev->dma_chans[2]);
>>  exit_free_dma_u:
>
> -- 
> Regards,
>
> Laurent Pinchart

Regards,
Flavio Ceolin

Re: [PATCH] media: pxa_camera: disable and unprepare the clock source on error

[Flavio Ceolin]

Hi Laurent,

> Hi Flavio,
>
> Thank you for the patch.
>
> On Wednesday, 6 December 2017 18:38:50 EET Flavio Ceolin wrote:
>> pxa_camera_probe() was not calling pxa_camera_deactivate(),
>> responsible to call clk_disable_unprepare(), on the failure path. This
>> was leading to unbalancing source clock.
>> 
>> Found by Linux Driver Verification project (linuxtesting.org).
>
> Any chance I could sign you up for more work on this driver ? :-)

Definetely, this would be great :)

>
>> Signed-off-by: Flavio Ceolin 
>
> Reviewed-by: Laurent Pinchart 
>
> I expect Hans Verkuil to pick up the patch.
>
>> ---
>>  drivers/media/platform/pxa_camera.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/media/platform/pxa_camera.c
>> b/drivers/media/platform/pxa_camera.c index 9d3f0cb..7877037 100644
>> --- a/drivers/media/platform/pxa_camera.c
>> +++ b/drivers/media/platform/pxa_camera.c
>> @@ -2489,7 +2489,7 @@ static int pxa_camera_probe(struct platform_device
>> *pdev) dev_set_drvdata(>dev, pcdev);
>>  err = v4l2_device_register(>dev, >v4l2_dev);
>>  if (err)
>> -goto exit_free_dma;
>> +goto exit_deactivate;
>> 
>>  pcdev->asds[0] = >asd;
>>  pcdev->notifier.subdevs = pcdev->asds;
>> @@ -2525,6 +2525,8 @@ static int pxa_camera_probe(struct platform_device
>> *pdev) v4l2_clk_unregister(pcdev->mclk_clk);
>>  exit_free_v4l2dev:
>>  v4l2_device_unregister(>v4l2_dev);
>> +exit_deactivate:
>> +pxa_camera_deactivate(pcdev);
>>  exit_free_dma:
>>  dma_release_channel(pcdev->dma_chans[2]);
>>  exit_free_dma_u:
>
> -- 
> Regards,
>
> Laurent Pinchart

Regards,
Flavio Ceolin

Re: [PATCH v2] mm: terminate shrink_slab loop if signal is pending

[Suren Baghdasaryan]

On Sat, Dec 9, 2017 at 12:08 AM, Tetsuo Handa
 wrote:
> Suren Baghdasaryan wrote:
>> On Fri, Dec 8, 2017 at 6:03 AM, Tetsuo Handa
>>  wrote:
>> >> > >> This change checks for pending
>> >> > >> fatal signals inside shrink_slab loop and if one is detected
>> >> > >> terminates this loop early.
>> >> > >
>> >> > > This changelog doesn't really address my previous review feedback, I 
>> >> > > am
>> >> > > afraid. You should mention more details about problems you are seeing
>> >> > > and what causes them.
>>
>> The problem I'm facing is that a SIGKILL sent from user space to kill
>> the least important process is delayed enough for OOM-killer to get a
>> chance to kill something else, possibly a more important process. Here
>> "important" is from user's point of view. So the delay in SIGKILL
>> delivery effectively causes extra kills. Traces indicate that this
>> delay happens when process being killed is in direct reclaim and
>> shrinkers (before I fixed them) were the biggest cause for the delay.
>
> Sending SIGKILL from userspace is not releasing memory fast enough to prevent
> the OOM killer from invoking? Yes, under memory pressure, even an attempt to
> send SIGKILL from userspace could be delayed due to e.g. page fault.
>

I understand that there will be cases when OOM is unavoidable. I'm
trying to minimize the cases when SIGKILL processing is delayed.

> Unless it is memcg OOM, you could try OOM notifier callback for checking
> whether there are SIGKILL pending processes and wait for timeout if any.
> This situation resembles timeout-based OOM killing discussion, where the OOM
> killer is enabled again (based on an assumption that the OOM victim got stuck
> due to OOM lockup) after some timeout from previous OOM-killing. And since
> we did not merge timeout-based approach, there is no timestamp field for
> remembering when the SIGKILL was delivered. Therefore, an attempt to wait for
> timeout would become messy.
>
> Regardless of whether you try OOM notifier callback, I think that adding
> __GFP_KILLABLE and allow bailing out without calling out_of_memory() and
> warn_alloc() will help terminating killed processes faster. I think that
> majority of memory allocation requests can give up upon SIGKILL. Important
> allocation requests which should not give up upon SIGKILL (e.g. committing
> to filesystem metadata and storage) can be offloaded to kernel threads.
>
>>
>> >> > > If we have a shrinker which takes considerable
>> >> > > amount of time them we should be addressing that. If that is not
>> >> > > possible then it should be documented at least.
>>
>> I already submitted patches for couple shrinkers. Problem became less
>> pronounced and less frequent but the retry loop Tetsuo mentioned still
>> visibly delays the delivery. The worst case I've seen after fixing
>> shrinkers is 43ms.
>
> You meant "delays the termination (of the killed process)" rather than
> "delays the delivery (of SIGKILL)", didn't you?
>

To be more precise, I'm talking about the delay between send_signal()
and get_signal() or in other words the delay between the moment when
we send the SIGKILL and the moment we handle it.

>> > I agree that making waits/loops killable is generally good. But be sure to 
>> > be
>> > prepared for the worst case. For example, start __GFP_KILLABLE from "best 
>> > effort"
>> > basis (i.e. no guarantee that the allocating thread will leave the page 
>> > allocator
>> > slowpath immediately) and check for fatal_signal_pending() only if
>> > __GFP_KILLABLE is set. That is,
>> >
>> > +   /*
>> > +* We are about to die and free our memory.
>> > +* Stop shrinking which might delay signal handling.
>> > +*/
>> > +   if (unlikely((gfp_mask & __GFP_KILLABLE) && 
>> > fatal_signal_pending(current)))
>> > +   break;
>> >
>> > at shrink_slab() etc. and
>> >
>> > +   if ((gfp_mask & __GFP_KILLABLE) && 
>> > fatal_signal_pending(current))
>> > +   goto nopage;
>> >
>> > at __alloc_pages_slowpath().
>>
>> I was thinking about something similar and will experiment to see if
>> this solves the problem and if it has any side effects. Anyone sees
>> any obvious problems with this approach?
>>
>
> It is Michal who thinks that "killability for a particular allocation request 
> sounds
> like a hack" ( 
> http://lkml.kernel.org/r/201606112335.hbg09891.olfjoftvmoq...@i-love.sakura.ne.jp
>  ).
> I'm willing to give up memory allocations from functions which are called by
> system calls if SIGKILL is pending. Thus, it should be time to try 
> __GFP_KILLABLE.

Re: [PATCH v2] mm: terminate shrink_slab loop if signal is pending

[Suren Baghdasaryan]

On Sat, Dec 9, 2017 at 12:08 AM, Tetsuo Handa
 wrote:
> Suren Baghdasaryan wrote:
>> On Fri, Dec 8, 2017 at 6:03 AM, Tetsuo Handa
>>  wrote:
>> >> > >> This change checks for pending
>> >> > >> fatal signals inside shrink_slab loop and if one is detected
>> >> > >> terminates this loop early.
>> >> > >
>> >> > > This changelog doesn't really address my previous review feedback, I 
>> >> > > am
>> >> > > afraid. You should mention more details about problems you are seeing
>> >> > > and what causes them.
>>
>> The problem I'm facing is that a SIGKILL sent from user space to kill
>> the least important process is delayed enough for OOM-killer to get a
>> chance to kill something else, possibly a more important process. Here
>> "important" is from user's point of view. So the delay in SIGKILL
>> delivery effectively causes extra kills. Traces indicate that this
>> delay happens when process being killed is in direct reclaim and
>> shrinkers (before I fixed them) were the biggest cause for the delay.
>
> Sending SIGKILL from userspace is not releasing memory fast enough to prevent
> the OOM killer from invoking? Yes, under memory pressure, even an attempt to
> send SIGKILL from userspace could be delayed due to e.g. page fault.
>

I understand that there will be cases when OOM is unavoidable. I'm
trying to minimize the cases when SIGKILL processing is delayed.

> Unless it is memcg OOM, you could try OOM notifier callback for checking
> whether there are SIGKILL pending processes and wait for timeout if any.
> This situation resembles timeout-based OOM killing discussion, where the OOM
> killer is enabled again (based on an assumption that the OOM victim got stuck
> due to OOM lockup) after some timeout from previous OOM-killing. And since
> we did not merge timeout-based approach, there is no timestamp field for
> remembering when the SIGKILL was delivered. Therefore, an attempt to wait for
> timeout would become messy.
>
> Regardless of whether you try OOM notifier callback, I think that adding
> __GFP_KILLABLE and allow bailing out without calling out_of_memory() and
> warn_alloc() will help terminating killed processes faster. I think that
> majority of memory allocation requests can give up upon SIGKILL. Important
> allocation requests which should not give up upon SIGKILL (e.g. committing
> to filesystem metadata and storage) can be offloaded to kernel threads.
>
>>
>> >> > > If we have a shrinker which takes considerable
>> >> > > amount of time them we should be addressing that. If that is not
>> >> > > possible then it should be documented at least.
>>
>> I already submitted patches for couple shrinkers. Problem became less
>> pronounced and less frequent but the retry loop Tetsuo mentioned still
>> visibly delays the delivery. The worst case I've seen after fixing
>> shrinkers is 43ms.
>
> You meant "delays the termination (of the killed process)" rather than
> "delays the delivery (of SIGKILL)", didn't you?
>

To be more precise, I'm talking about the delay between send_signal()
and get_signal() or in other words the delay between the moment when
we send the SIGKILL and the moment we handle it.

>> > I agree that making waits/loops killable is generally good. But be sure to 
>> > be
>> > prepared for the worst case. For example, start __GFP_KILLABLE from "best 
>> > effort"
>> > basis (i.e. no guarantee that the allocating thread will leave the page 
>> > allocator
>> > slowpath immediately) and check for fatal_signal_pending() only if
>> > __GFP_KILLABLE is set. That is,
>> >
>> > +   /*
>> > +* We are about to die and free our memory.
>> > +* Stop shrinking which might delay signal handling.
>> > +*/
>> > +   if (unlikely((gfp_mask & __GFP_KILLABLE) && 
>> > fatal_signal_pending(current)))
>> > +   break;
>> >
>> > at shrink_slab() etc. and
>> >
>> > +   if ((gfp_mask & __GFP_KILLABLE) && 
>> > fatal_signal_pending(current))
>> > +   goto nopage;
>> >
>> > at __alloc_pages_slowpath().
>>
>> I was thinking about something similar and will experiment to see if
>> this solves the problem and if it has any side effects. Anyone sees
>> any obvious problems with this approach?
>>
>
> It is Michal who thinks that "killability for a particular allocation request 
> sounds
> like a hack" ( 
> http://lkml.kernel.org/r/201606112335.hbg09891.olfjoftvmoq...@i-love.sakura.ne.jp
>  ).
> I'm willing to give up memory allocations from functions which are called by
> system calls if SIGKILL is pending. Thus, it should be time to try 
> __GFP_KILLABLE.

RE: [PATCH 1/2] vmbus: unregister device_obj->channels_kset

[Dexuan Cui]

> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Monday, December 11, 2017 12:59 PM
> To: Dexuan Cui 
> Cc: Stephen Hemminger ; o...@aepfle.de;
> Stephen Hemminger ; jasow...@redhat.com;
> linux-kernel@vger.kernel.org; sta...@vger.kernel.org; a...@canonical.com;
> marcelo.ce...@canonical.com; de...@linuxdriverproject.org;
> vkuzn...@redhat.com; leann.ogasaw...@canonical.com
> Subject: Re: [PATCH 1/2] vmbus: unregister device_obj->channels_kset
> 
> On Mon, Dec 11, 2017 at 08:41:44PM +, Dexuan Cui wrote:
> > > From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> > > Behalf Of Stephen Hemminger
> > > Sent: Tuesday, November 28, 2017 9:30 AM
> > > To: Greg KH 
> > > Cc: o...@aepfle.de; Stephen Hemminger ;
> > > jasow...@redhat.com; linux-kernel@vger.kernel.org;
> sta...@vger.kernel.org;
> > > a...@canonical.com; marcelo.ce...@canonical.com;
> > > de...@linuxdriverproject.org; vkuzn...@redhat.com;
> > > leann.ogasaw...@canonical.com
> > > Subject: Re: [PATCH 1/2] vmbus: unregister device_obj->channels_kset
> > >
> > > On Tue, 28 Nov 2017 16:56:05 +0100
> > > Greg KH  wrote:
> > >
> > > > On Tue, Nov 14, 2017 at 06:53:32AM -0700,
> k...@exchange.microsoft.com
> > > wrote:
> > > > > From: Dexuan Cui 
> > > > >
> > > > > Fixes: c2e5df616e1a ("vmbus: add per-channel sysfs info")
> > > > >
> > > > > Without the patch, a device can't be thoroughly destroyed, because
> > > > > vmbus_device_register() -> kset_create_and_add() still holds a 
> > > > > reference
> > > > > to the hv_device's device.kobj.
> > > > >
> > > > > Signed-off-by: Dexuan Cui 
> > > > > Cc: Stephen Hemminger 
> > > > > Cc: sta...@vger.kernel.org
> > > >
> > > > Why is this marked for stable when the patch it "fixes" is in 4.15-rc1?
> > >
> > > It doesn't need to go to stable.
> >
> > Hi Greg,
> > May I know the status of the patch?
> 
> It's still in my "to-apply" queue.  As it's only a 4.15-final thing,
> don't worry, it will get there...
> 
> thanks,
> 
> greg k-h

Thanks a lot, Greg!

-- Dexuan

RE: [PATCH 1/2] vmbus: unregister device_obj->channels_kset

[Dexuan Cui]

> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Monday, December 11, 2017 12:59 PM
> To: Dexuan Cui 
> Cc: Stephen Hemminger ; o...@aepfle.de;
> Stephen Hemminger ; jasow...@redhat.com;
> linux-kernel@vger.kernel.org; sta...@vger.kernel.org; a...@canonical.com;
> marcelo.ce...@canonical.com; de...@linuxdriverproject.org;
> vkuzn...@redhat.com; leann.ogasaw...@canonical.com
> Subject: Re: [PATCH 1/2] vmbus: unregister device_obj->channels_kset
> 
> On Mon, Dec 11, 2017 at 08:41:44PM +, Dexuan Cui wrote:
> > > From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> > > Behalf Of Stephen Hemminger
> > > Sent: Tuesday, November 28, 2017 9:30 AM
> > > To: Greg KH 
> > > Cc: o...@aepfle.de; Stephen Hemminger ;
> > > jasow...@redhat.com; linux-kernel@vger.kernel.org;
> sta...@vger.kernel.org;
> > > a...@canonical.com; marcelo.ce...@canonical.com;
> > > de...@linuxdriverproject.org; vkuzn...@redhat.com;
> > > leann.ogasaw...@canonical.com
> > > Subject: Re: [PATCH 1/2] vmbus: unregister device_obj->channels_kset
> > >
> > > On Tue, 28 Nov 2017 16:56:05 +0100
> > > Greg KH  wrote:
> > >
> > > > On Tue, Nov 14, 2017 at 06:53:32AM -0700,
> k...@exchange.microsoft.com
> > > wrote:
> > > > > From: Dexuan Cui 
> > > > >
> > > > > Fixes: c2e5df616e1a ("vmbus: add per-channel sysfs info")
> > > > >
> > > > > Without the patch, a device can't be thoroughly destroyed, because
> > > > > vmbus_device_register() -> kset_create_and_add() still holds a 
> > > > > reference
> > > > > to the hv_device's device.kobj.
> > > > >
> > > > > Signed-off-by: Dexuan Cui 
> > > > > Cc: Stephen Hemminger 
> > > > > Cc: sta...@vger.kernel.org
> > > >
> > > > Why is this marked for stable when the patch it "fixes" is in 4.15-rc1?
> > >
> > > It doesn't need to go to stable.
> >
> > Hi Greg,
> > May I know the status of the patch?
> 
> It's still in my "to-apply" queue.  As it's only a 4.15-final thing,
> don't worry, it will get there...
> 
> thanks,
> 
> greg k-h

Thanks a lot, Greg!

-- Dexuan

Re: linux-next: Signed-off-by missing for commit in the nand tree

[Boris Brezillon]

On Tue, 12 Dec 2017 07:50:34 +1100
Stephen Rothwell  wrote:

> Hi Boris,
> 
> Commit
> 
>   b8267c7ef4f1 ("mtd: nand: add ->exec_op() implementation")
> 
> is missing a Signed-off-by from its committer.
> 

Just fixed it.

Thanks,

Boris

Re: linux-next: Signed-off-by missing for commit in the nand tree

[Boris Brezillon]

On Tue, 12 Dec 2017 07:50:34 +1100
Stephen Rothwell  wrote:

> Hi Boris,
> 
> Commit
> 
>   b8267c7ef4f1 ("mtd: nand: add ->exec_op() implementation")
> 
> is missing a Signed-off-by from its committer.
> 

Just fixed it.

Thanks,

Boris

Re: [ANNOUNCE] 4.1.46-rt52

[Corey Minyard]

On 12/11/2017 02:20 PM, Julia Cartwright wrote:

On Mon, Dec 11, 2017 at 01:04:58PM -0600, Corey Minyard wrote:

On 11/29/2017 05:13 PM, Julia Cartwright wrote:

Hello RT Folks!

I'm pleased to announce the 4.1.46-rt52 stable release.

You can get this release via the git tree at:

git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git

branch: v4.1-rt
Head SHA1: 6e737a91c1ce923d4e10db831e14cfef9a2459cc

Or to build 4.1.46-rt52 directly, the following patches should be applied:


https://urldefense.proofpoint.com/v2/url?u=http-3A__www.kernel.org_pub_linux_kernel_v4.x_linux-2D4.1.tar.xz=DwICaQ=I_0YwoKy7z5LMTVdyO6YCiE2uzI1jjZZuIPelcSjixA=cAXq_8W9Othb2h8ZcWv8Vw=a5KTpX9A0-pHwmQkOOLr5_xeWPavI-lgN7Z0Goes2So=tGmSNpZKEKvhTpeVu_ktmD4I5PIXfIJhZ79DxwbCjtQ=


https://urldefense.proofpoint.com/v2/url?u=http-3A__www.kernel.org_pub_linux_kernel_v4.x_patch-2D4.1.46.xz=DwICaQ=I_0YwoKy7z5LMTVdyO6YCiE2uzI1jjZZuIPelcSjixA=cAXq_8W9Othb2h8ZcWv8Vw=a5KTpX9A0-pHwmQkOOLr5_xeWPavI-lgN7Z0Goes2So=hx9pHgRy7tWhUeeiJhPOH0T8qdDrhtZvWxOWuIBZZgs=


https://urldefense.proofpoint.com/v2/url?u=http-3A__www.kernel.org_pub_linux_kernel_projects_rt_4.1_patch-2D4.1.46-2Drt52.patch.xz=DwICaQ=I_0YwoKy7z5LMTVdyO6YCiE2uzI1jjZZuIPelcSjixA=cAXq_8W9Othb2h8ZcWv8Vw=a5KTpX9A0-pHwmQkOOLr5_xeWPavI-lgN7Z0Goes2So=Pm8ed_xvW1drpPxNkt035bj6ILvSImuN9vvgEAt8IiM=


You can also build from 4.1.46-rt51 by applying the incremental patch:


https://urldefense.proofpoint.com/v2/url?u=http-3A__www.kernel.org_pub_linux_kernel_projects_rt_4.1_incr_patch-2D4.1.46-2Drt51-2Drt52.patch.xz=DwICaQ=I_0YwoKy7z5LMTVdyO6YCiE2uzI1jjZZuIPelcSjixA=cAXq_8W9Othb2h8ZcWv8Vw=a5KTpX9A0-pHwmQkOOLr5_xeWPavI-lgN7Z0Goes2So=ZM64-JCd0WNFQgFlB3wbCfn8Jtf9rq5QyCO2-GJsC2Q=

Enjoy!
 Julia

Changes from v4.1.46-rt51:
---
Julia Cartwright (2):
workqueue: fixup rcu check for RT
Linux 4.1.46-rt52

Sebastian Andrzej Siewior (2):
PM / CPU: replace raw_notifier with atomic_notifier (fixup)

This change breaks the compile of 4.1 on ARM (and other things using
kernel/cpu_pm.c).

rcu_irq_enter_irqson() and friends doesn't exist in 4.1.  In fact,
rcu_irq_enter() doesn't even exist.

Sorry I didn't get to testing this earlier, I've been focused on other things.

Thanks!  My current builds apparently don't currently cover CPU_PM
builds, so I didn't see this :(.  Thanks for the report.

v4.1 does contain rcu_irq_enter(), and it looks like what we should be
using in 4.1 instead.


Ah, it does, I looked in the wrong place.


I'm thinking the below should be okay; I'll be standing up a proper cpu
hotplugging test to give it some runtime.


The below should work.  You'll need an ARM64, ARM or MIPS system with 
the proper

config settings to exercise this code, though.

-corey


Thanks,
Julia

-- 8< --
Subject: [PATCH] PM / CPU: replace usage of rcu_irq_{enter,exit}_irqson()

Commit c50243950f97e ("PM / CPU: replace raw_notifier with
atomic_notifier (fixup)") intended to fix up RCU usage in idle, by
telling RCU to pay attention.

However, the cherry-pick back from rt-devel included the invocation of
the _irqson() variants of rcu_irq_{enter,exit}() API which doesn't exist
in 4.1.

For 4.1, just simply drop the _irqson() variant, and instead use
rcu_irq_{enter,exit}() directly instead.  This is safe, because in 4.1,
these functions internally disable interrupts.

Reported-by: Corey Minyard 
Signed-off-by: Julia Cartwright 

Re: [ANNOUNCE] 4.1.46-rt52

[Corey Minyard]

On 12/11/2017 02:20 PM, Julia Cartwright wrote:

On Mon, Dec 11, 2017 at 01:04:58PM -0600, Corey Minyard wrote:

On 11/29/2017 05:13 PM, Julia Cartwright wrote:

Hello RT Folks!

I'm pleased to announce the 4.1.46-rt52 stable release.

You can get this release via the git tree at:

git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git

branch: v4.1-rt
Head SHA1: 6e737a91c1ce923d4e10db831e14cfef9a2459cc

Or to build 4.1.46-rt52 directly, the following patches should be applied:


https://urldefense.proofpoint.com/v2/url?u=http-3A__www.kernel.org_pub_linux_kernel_v4.x_linux-2D4.1.tar.xz=DwICaQ=I_0YwoKy7z5LMTVdyO6YCiE2uzI1jjZZuIPelcSjixA=cAXq_8W9Othb2h8ZcWv8Vw=a5KTpX9A0-pHwmQkOOLr5_xeWPavI-lgN7Z0Goes2So=tGmSNpZKEKvhTpeVu_ktmD4I5PIXfIJhZ79DxwbCjtQ=


https://urldefense.proofpoint.com/v2/url?u=http-3A__www.kernel.org_pub_linux_kernel_v4.x_patch-2D4.1.46.xz=DwICaQ=I_0YwoKy7z5LMTVdyO6YCiE2uzI1jjZZuIPelcSjixA=cAXq_8W9Othb2h8ZcWv8Vw=a5KTpX9A0-pHwmQkOOLr5_xeWPavI-lgN7Z0Goes2So=hx9pHgRy7tWhUeeiJhPOH0T8qdDrhtZvWxOWuIBZZgs=


https://urldefense.proofpoint.com/v2/url?u=http-3A__www.kernel.org_pub_linux_kernel_projects_rt_4.1_patch-2D4.1.46-2Drt52.patch.xz=DwICaQ=I_0YwoKy7z5LMTVdyO6YCiE2uzI1jjZZuIPelcSjixA=cAXq_8W9Othb2h8ZcWv8Vw=a5KTpX9A0-pHwmQkOOLr5_xeWPavI-lgN7Z0Goes2So=Pm8ed_xvW1drpPxNkt035bj6ILvSImuN9vvgEAt8IiM=


You can also build from 4.1.46-rt51 by applying the incremental patch:


https://urldefense.proofpoint.com/v2/url?u=http-3A__www.kernel.org_pub_linux_kernel_projects_rt_4.1_incr_patch-2D4.1.46-2Drt51-2Drt52.patch.xz=DwICaQ=I_0YwoKy7z5LMTVdyO6YCiE2uzI1jjZZuIPelcSjixA=cAXq_8W9Othb2h8ZcWv8Vw=a5KTpX9A0-pHwmQkOOLr5_xeWPavI-lgN7Z0Goes2So=ZM64-JCd0WNFQgFlB3wbCfn8Jtf9rq5QyCO2-GJsC2Q=

Enjoy!
 Julia

Changes from v4.1.46-rt51:
---
Julia Cartwright (2):
workqueue: fixup rcu check for RT
Linux 4.1.46-rt52

Sebastian Andrzej Siewior (2):
PM / CPU: replace raw_notifier with atomic_notifier (fixup)

This change breaks the compile of 4.1 on ARM (and other things using
kernel/cpu_pm.c).

rcu_irq_enter_irqson() and friends doesn't exist in 4.1.  In fact,
rcu_irq_enter() doesn't even exist.

Sorry I didn't get to testing this earlier, I've been focused on other things.

Thanks!  My current builds apparently don't currently cover CPU_PM
builds, so I didn't see this :(.  Thanks for the report.

v4.1 does contain rcu_irq_enter(), and it looks like what we should be
using in 4.1 instead.


Ah, it does, I looked in the wrong place.


I'm thinking the below should be okay; I'll be standing up a proper cpu
hotplugging test to give it some runtime.


The below should work.  You'll need an ARM64, ARM or MIPS system with 
the proper

config settings to exercise this code, though.

-corey


Thanks,
Julia

-- 8< --
Subject: [PATCH] PM / CPU: replace usage of rcu_irq_{enter,exit}_irqson()

Commit c50243950f97e ("PM / CPU: replace raw_notifier with
atomic_notifier (fixup)") intended to fix up RCU usage in idle, by
telling RCU to pay attention.

However, the cherry-pick back from rt-devel included the invocation of
the _irqson() variants of rcu_irq_{enter,exit}() API which doesn't exist
in 4.1.

For 4.1, just simply drop the _irqson() variant, and instead use
rcu_irq_{enter,exit}() directly instead.  This is safe, because in 4.1,
these functions internally disable interrupts.

Reported-by: Corey Minyard 
Signed-off-by: Julia Cartwright   
  	return notifier_to_errno(ret);

  }

Re: [PATCH 1/2] vmbus: unregister device_obj->channels_kset

[Greg KH]

On Mon, Dec 11, 2017 at 08:41:44PM +, Dexuan Cui wrote:
> > From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> > Behalf Of Stephen Hemminger
> > Sent: Tuesday, November 28, 2017 9:30 AM
> > To: Greg KH 
> > Cc: o...@aepfle.de; Stephen Hemminger ;
> > jasow...@redhat.com; linux-kernel@vger.kernel.org; sta...@vger.kernel.org;
> > a...@canonical.com; marcelo.ce...@canonical.com;
> > de...@linuxdriverproject.org; vkuzn...@redhat.com;
> > leann.ogasaw...@canonical.com
> > Subject: Re: [PATCH 1/2] vmbus: unregister device_obj->channels_kset
> > 
> > On Tue, 28 Nov 2017 16:56:05 +0100
> > Greg KH  wrote:
> > 
> > > On Tue, Nov 14, 2017 at 06:53:32AM -0700, k...@exchange.microsoft.com
> > wrote:
> > > > From: Dexuan Cui 
> > > >
> > > > Fixes: c2e5df616e1a ("vmbus: add per-channel sysfs info")
> > > >
> > > > Without the patch, a device can't be thoroughly destroyed, because
> > > > vmbus_device_register() -> kset_create_and_add() still holds a reference
> > > > to the hv_device's device.kobj.
> > > >
> > > > Signed-off-by: Dexuan Cui 
> > > > Cc: Stephen Hemminger 
> > > > Cc: sta...@vger.kernel.org
> > >
> > > Why is this marked for stable when the patch it "fixes" is in 4.15-rc1?
> > 
> > It doesn't need to go to stable.
> 
> Hi Greg,
> May I know the status of the patch?

It's still in my "to-apply" queue.  As it's only a 4.15-final thing,
don't worry, it will get there...

thanks,

greg k-h

Re: [PATCH 1/2] vmbus: unregister device_obj->channels_kset

[Greg KH]

On Mon, Dec 11, 2017 at 08:41:44PM +, Dexuan Cui wrote:
> > From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> > Behalf Of Stephen Hemminger
> > Sent: Tuesday, November 28, 2017 9:30 AM
> > To: Greg KH 
> > Cc: o...@aepfle.de; Stephen Hemminger ;
> > jasow...@redhat.com; linux-kernel@vger.kernel.org; sta...@vger.kernel.org;
> > a...@canonical.com; marcelo.ce...@canonical.com;
> > de...@linuxdriverproject.org; vkuzn...@redhat.com;
> > leann.ogasaw...@canonical.com
> > Subject: Re: [PATCH 1/2] vmbus: unregister device_obj->channels_kset
> > 
> > On Tue, 28 Nov 2017 16:56:05 +0100
> > Greg KH  wrote:
> > 
> > > On Tue, Nov 14, 2017 at 06:53:32AM -0700, k...@exchange.microsoft.com
> > wrote:
> > > > From: Dexuan Cui 
> > > >
> > > > Fixes: c2e5df616e1a ("vmbus: add per-channel sysfs info")
> > > >
> > > > Without the patch, a device can't be thoroughly destroyed, because
> > > > vmbus_device_register() -> kset_create_and_add() still holds a reference
> > > > to the hv_device's device.kobj.
> > > >
> > > > Signed-off-by: Dexuan Cui 
> > > > Cc: Stephen Hemminger 
> > > > Cc: sta...@vger.kernel.org
> > >
> > > Why is this marked for stable when the patch it "fixes" is in 4.15-rc1?
> > 
> > It doesn't need to go to stable.
> 
> Hi Greg,
> May I know the status of the patch?

It's still in my "to-apply" queue.  As it's only a 4.15-final thing,
don't worry, it will get there...

thanks,

greg k-h

Re: [PATCH] sched/fair: Prevent a division by 0 in scale_rt_capacity()

[Rohit Jain]

Hi Filippo,


On 12/09/2017 12:03 AM, Filippo Sironi wrote:

... since total = sched_avg_period() + delta can yield 0x1,
which results in a division by 0, given that div_u64() takes a u32
divisor.  Use div64_u64() instead.

divide error:  [#1] SMP
CPU: 7 PID: 0 Comm: swapper/7 Not tainted 4.9.58 #1
Hardware name: ...
task: 8800a24e2800 task.stack: c974c000
RIP: 0010:[] [] 
update_group_capacity+0x16e/0x1c0
RSP: 0018:8800a74e3c18 EFLAGS: 00010246
RAX: 00445ced RBX: 0007 RCX: 024d
RDX:  RSI:  RDI: 000160c0
RBP: 8800a74e3c38 R08: 8800a17d5ac0 R09: 8800a74e
R10:  R11:  R12: 8800a297e400
R13: 8800a17d5ac0 R14:  R15: 8800a17d5ac0
FS: () GS:8800a74e() knlGS:
CS: 0010 DS:  ES:  CR0: 80050033
CR2: 006f3580 CR3: 01607000 CR4: 007426e0
DR0:  DR1:  DR2: 
DR3:  DR6: fffe0ff0 DR7: 0400
PKRU: 5554
Stack:
8800a17d5180 8800a74e3e00 8800a17d5a01 8800a74e3c68
8800a74e3d90 810d37e6 fff8 002300010c40
0040 8800a17d5ad8  
Call Trace:
 [162553.008569] [] find_busiest_group+0xe6/0x950
[] load_balance+0x188/0xa70
[] ? update_rq_clock.part.88+0x13/0x30
[] rebalance_domains+0x210/0x290
[] run_rebalance_domains+0x1b0/0x1d0
[] __do_softirq+0x89/0x2b0
[] irq_exit+0xab/0xb0
[] smp_reschedule_interrupt+0x2e/0x30
[] reschedule_interrupt+0x84/0x90
 [162553.008603] [] ? cpuidle_enter_state+0x12f/0x2c0
[] cpuidle_enter+0x12/0x20
[] cpu_startup_entry+0x1a2/0x1f0
[] start_secondary+0x12d/0x140
Code: 0f 00 4c 8b 96 48 09 00 00 48 8b 86 40 09 00 00 48 8b b6 b0 08 00 00 48 d1 ea 
4c 29 d6 41 ba 00 00 00 00 49 0f 48 f2 01 d6 31 d2 <48> f7 f6 ba 00 04 00 00 48 
29 c2 48 3d ff 03 00 00 b8 01 00 00
RIP [] update_group_capacity+0x16e/0x1c0
RSP 

Cc: Ingo Molnar 
Cc: Peter Zijlstra 
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Filippo Sironi 


Reviewed by: Rohit Jain 


---
  kernel/sched/fair.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 4037e19bbca2..04b6f847a241 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7517,7 +7517,7 @@ static unsigned long scale_rt_capacity(int cpu)
  
  	total = sched_avg_period() + delta;
  
-	used = div_u64(avg, total);

+   used = div64_u64(avg, total);
  
  	if (likely(used < SCHED_CAPACITY_SCALE))

return SCHED_CAPACITY_SCALE - used;

Re: [PATCH] sched/fair: Prevent a division by 0 in scale_rt_capacity()

[Rohit Jain]

Hi Filippo,


On 12/09/2017 12:03 AM, Filippo Sironi wrote:

... since total = sched_avg_period() + delta can yield 0x1,
which results in a division by 0, given that div_u64() takes a u32
divisor.  Use div64_u64() instead.

divide error:  [#1] SMP
CPU: 7 PID: 0 Comm: swapper/7 Not tainted 4.9.58 #1
Hardware name: ...
task: 8800a24e2800 task.stack: c974c000
RIP: 0010:[] [] 
update_group_capacity+0x16e/0x1c0
RSP: 0018:8800a74e3c18 EFLAGS: 00010246
RAX: 00445ced RBX: 0007 RCX: 024d
RDX:  RSI:  RDI: 000160c0
RBP: 8800a74e3c38 R08: 8800a17d5ac0 R09: 8800a74e
R10:  R11:  R12: 8800a297e400
R13: 8800a17d5ac0 R14:  R15: 8800a17d5ac0
FS: () GS:8800a74e() knlGS:
CS: 0010 DS:  ES:  CR0: 80050033
CR2: 006f3580 CR3: 01607000 CR4: 007426e0
DR0:  DR1:  DR2: 
DR3:  DR6: fffe0ff0 DR7: 0400
PKRU: 5554
Stack:
8800a17d5180 8800a74e3e00 8800a17d5a01 8800a74e3c68
8800a74e3d90 810d37e6 fff8 002300010c40
0040 8800a17d5ad8  
Call Trace:
 [162553.008569] [] find_busiest_group+0xe6/0x950
[] load_balance+0x188/0xa70
[] ? update_rq_clock.part.88+0x13/0x30
[] rebalance_domains+0x210/0x290
[] run_rebalance_domains+0x1b0/0x1d0
[] __do_softirq+0x89/0x2b0
[] irq_exit+0xab/0xb0
[] smp_reschedule_interrupt+0x2e/0x30
[] reschedule_interrupt+0x84/0x90
 [162553.008603] [] ? cpuidle_enter_state+0x12f/0x2c0
[] cpuidle_enter+0x12/0x20
[] cpu_startup_entry+0x1a2/0x1f0
[] start_secondary+0x12d/0x140
Code: 0f 00 4c 8b 96 48 09 00 00 48 8b 86 40 09 00 00 48 8b b6 b0 08 00 00 48 d1 ea 
4c 29 d6 41 ba 00 00 00 00 49 0f 48 f2 01 d6 31 d2 <48> f7 f6 ba 00 04 00 00 48 
29 c2 48 3d ff 03 00 00 b8 01 00 00
RIP [] update_group_capacity+0x16e/0x1c0
RSP 

Cc: Ingo Molnar 
Cc: Peter Zijlstra 
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Filippo Sironi 


Reviewed by: Rohit Jain 


---
  kernel/sched/fair.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 4037e19bbca2..04b6f847a241 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7517,7 +7517,7 @@ static unsigned long scale_rt_capacity(int cpu)
  
  	total = sched_avg_period() + delta;
  
-	used = div_u64(avg, total);

+   used = div64_u64(avg, total);
  
  	if (likely(used < SCHED_CAPACITY_SCALE))

return SCHED_CAPACITY_SCALE - used;

[PATCH net,stable] net: qmi_wwan: add Sierra EM7565 1199:9091

[ssjoholm]

From: Sebastian Sjoholm 

Sierra Wireless EM7565 is an Qualcomm MDM9x50 based M.2 modem.
The USB id is added to qmi_wwan.c to allow QMI communication 
with the EM7565.

Signed-off-by: Sebastian Sjoholm 
Acked-by: Bjørn Mork 
---
[The corresponding qcserial patch will be submitted by Reinhard Speyerer.]
---
 drivers/net/usb/qmi_wwan.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 304ec6555cd8..3cebd6683938 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1204,6 +1204,7 @@ static const struct usb_device_id products[] = {
{QMI_FIXED_INTF(0x1199, 0x9079, 10)},   /* Sierra Wireless EM74xx */
{QMI_FIXED_INTF(0x1199, 0x907b, 8)},/* Sierra Wireless EM74xx */
{QMI_FIXED_INTF(0x1199, 0x907b, 10)},   /* Sierra Wireless EM74xx */
+   {QMI_FIXED_INTF(0x1199, 0x9091, 8)},/* Sierra Wireless EM7565 */
{QMI_FIXED_INTF(0x1bbb, 0x011e, 4)},/* Telekom Speedstick LTE II 
(Alcatel One Touch L100V LTE) */
{QMI_FIXED_INTF(0x1bbb, 0x0203, 2)},/* Alcatel L800MA */
{QMI_FIXED_INTF(0x2357, 0x0201, 4)},/* TP-LINK HSUPA Modem MA180 */
-- 
2.14.1

[PATCH net,stable] net: qmi_wwan: add Sierra EM7565 1199:9091

[ssjoholm]

From: Sebastian Sjoholm 

Sierra Wireless EM7565 is an Qualcomm MDM9x50 based M.2 modem.
The USB id is added to qmi_wwan.c to allow QMI communication 
with the EM7565.

Signed-off-by: Sebastian Sjoholm 
Acked-by: Bjørn Mork 
---
[The corresponding qcserial patch will be submitted by Reinhard Speyerer.]
---
 drivers/net/usb/qmi_wwan.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 304ec6555cd8..3cebd6683938 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1204,6 +1204,7 @@ static const struct usb_device_id products[] = {
{QMI_FIXED_INTF(0x1199, 0x9079, 10)},   /* Sierra Wireless EM74xx */
{QMI_FIXED_INTF(0x1199, 0x907b, 8)},/* Sierra Wireless EM74xx */
{QMI_FIXED_INTF(0x1199, 0x907b, 10)},   /* Sierra Wireless EM74xx */
+   {QMI_FIXED_INTF(0x1199, 0x9091, 8)},/* Sierra Wireless EM7565 */
{QMI_FIXED_INTF(0x1bbb, 0x011e, 4)},/* Telekom Speedstick LTE II 
(Alcatel One Touch L100V LTE) */
{QMI_FIXED_INTF(0x1bbb, 0x0203, 2)},/* Alcatel L800MA */
{QMI_FIXED_INTF(0x2357, 0x0201, 4)},/* TP-LINK HSUPA Modem MA180 */
-- 
2.14.1

linux-next: Signed-off-by missing for commit in the nand tree

[Stephen Rothwell]

Hi Boris,

Commit

  b8267c7ef4f1 ("mtd: nand: add ->exec_op() implementation")

is missing a Signed-off-by from its committer.

-- 
Cheers,
Stephen Rothwell

linux-next: Signed-off-by missing for commit in the nand tree

[Stephen Rothwell]

Hi Boris,

Commit

  b8267c7ef4f1 ("mtd: nand: add ->exec_op() implementation")

is missing a Signed-off-by from its committer.

-- 
Cheers,
Stephen Rothwell

Re: [PATCH] KVM: X86: Fix host dr6 miss restore

[David Hildenbrand]

On 10.12.2017 01:44, Wanpeng Li wrote:
> 2017-12-08 20:39 GMT+08:00 David Hildenbrand :
>> On 08.12.2017 10:12, Wanpeng Li wrote:
>>> From: Wanpeng Li 
>>>
>>> Reported by syzkaller:
>>>
>>>WARNING: CPU: 0 PID: 12927 at arch/x86/kernel/traps.c:780 
>>> do_debug+0x222/0x250
>>>CPU: 0 PID: 12927 Comm: syz-executor Tainted: G   OE
>>> 4.15.0-rc2+ #16
>>>RIP: 0010:do_debug+0x222/0x250
>>>Call Trace:
>>> <#DB>
>>> debug+0x3e/0x70
>>>RIP: 0010:copy_user_enhanced_fast_string+0x10/0x20
>>> 
>>> _copy_from_user+0x5b/0x90
>>> SyS_timer_create+0x33/0x80
>>> entry_SYSCALL_64_fastpath+0x23/0x9a
>>>
>>> The syzkaller will mmap a buffer which is also the struct sigevent 
>>> parameter of
>>> timer_create(), it will also call perf_event_open() to set a BP for the 
>>> buffer,
>>> so when the implementation of timer_create() in kernel tries to get the 
>>> struct
>>> sigevent parameter by copy_from_user(), rep movsb triggers the BP. The 
>>> syzkaller
>>> testcase also sets the debug registers for the guest, however, the kvm just
>>> restores host debug registers when we have active breakpoints. I can observe
>>> the dr6 single step bit is set and !hw_breakpoint_active() sporadically by 
>>> print
>>> when running the testcase heavy multithreading. The do_debug() which is 
>>> triggered
>>> by rep movsb will splash when (dr6 & DR_STEP && !user_mode(regs)).
>>>
>>> This patch fixes it by restoring host dr6 unconditionally before preempt/irq
>>> enable.
>>>
>>> Reported-by: Dmitry Vyukov 
>>> Cc: Paolo Bonzini 
>>> Cc: Radim Krčmář 
>>> Cc: David Hildenbrand 
>>> Cc: Dmitry Vyukov 
>>> Signed-off-by: Wanpeng Li 
>>> ---
>>>  arch/x86/kvm/x86.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>> index 0c5d55c..a6370fd 100644
>>> --- a/arch/x86/kvm/x86.c
>>> +++ b/arch/x86/kvm/x86.c
>>> @@ -7065,6 +7065,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>>>*/
>>>   if (hw_breakpoint_active())
>>>   hw_breakpoint_restore();
>>> + else
>>> + set_debugreg(current->thread.debugreg6, 6);
>>>
>>>   vcpu->arch.last_guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc());
>>>
>>>
>>
>> If you haven't seen it, I analyzed this in
>> https://lkml.org/lkml/2017/11/7/638 but nobody would respond for now to
>> my suggestion/question.
> 
> I think it's fine to restore dr6 before preempt/irq enable.

That make sense, as I assume this is the first time that a trap would be
delivered.

Reviewed-by: David Hildenbrand 

And certainly stable material?

> 
> Regards,
> Wanpeng Li
> 


-- 

Thanks,

David / dhildenb

Re: [PATCH] KVM: X86: Fix host dr6 miss restore

[David Hildenbrand]

On 10.12.2017 01:44, Wanpeng Li wrote:
> 2017-12-08 20:39 GMT+08:00 David Hildenbrand :
>> On 08.12.2017 10:12, Wanpeng Li wrote:
>>> From: Wanpeng Li 
>>>
>>> Reported by syzkaller:
>>>
>>>WARNING: CPU: 0 PID: 12927 at arch/x86/kernel/traps.c:780 
>>> do_debug+0x222/0x250
>>>CPU: 0 PID: 12927 Comm: syz-executor Tainted: G   OE
>>> 4.15.0-rc2+ #16
>>>RIP: 0010:do_debug+0x222/0x250
>>>Call Trace:
>>> <#DB>
>>> debug+0x3e/0x70
>>>RIP: 0010:copy_user_enhanced_fast_string+0x10/0x20
>>> 
>>> _copy_from_user+0x5b/0x90
>>> SyS_timer_create+0x33/0x80
>>> entry_SYSCALL_64_fastpath+0x23/0x9a
>>>
>>> The syzkaller will mmap a buffer which is also the struct sigevent 
>>> parameter of
>>> timer_create(), it will also call perf_event_open() to set a BP for the 
>>> buffer,
>>> so when the implementation of timer_create() in kernel tries to get the 
>>> struct
>>> sigevent parameter by copy_from_user(), rep movsb triggers the BP. The 
>>> syzkaller
>>> testcase also sets the debug registers for the guest, however, the kvm just
>>> restores host debug registers when we have active breakpoints. I can observe
>>> the dr6 single step bit is set and !hw_breakpoint_active() sporadically by 
>>> print
>>> when running the testcase heavy multithreading. The do_debug() which is 
>>> triggered
>>> by rep movsb will splash when (dr6 & DR_STEP && !user_mode(regs)).
>>>
>>> This patch fixes it by restoring host dr6 unconditionally before preempt/irq
>>> enable.
>>>
>>> Reported-by: Dmitry Vyukov 
>>> Cc: Paolo Bonzini 
>>> Cc: Radim Krčmář 
>>> Cc: David Hildenbrand 
>>> Cc: Dmitry Vyukov 
>>> Signed-off-by: Wanpeng Li 
>>> ---
>>>  arch/x86/kvm/x86.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>> index 0c5d55c..a6370fd 100644
>>> --- a/arch/x86/kvm/x86.c
>>> +++ b/arch/x86/kvm/x86.c
>>> @@ -7065,6 +7065,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>>>*/
>>>   if (hw_breakpoint_active())
>>>   hw_breakpoint_restore();
>>> + else
>>> + set_debugreg(current->thread.debugreg6, 6);
>>>
>>>   vcpu->arch.last_guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc());
>>>
>>>
>>
>> If you haven't seen it, I analyzed this in
>> https://lkml.org/lkml/2017/11/7/638 but nobody would respond for now to
>> my suggestion/question.
> 
> I think it's fine to restore dr6 before preempt/irq enable.

That make sense, as I assume this is the first time that a trap would be
delivered.

Reviewed-by: David Hildenbrand 

And certainly stable material?

> 
> Regards,
> Wanpeng Li
> 


-- 

Thanks,

David / dhildenb

Re: [PATCH RESEND] KVM: X86: Fix load bad host fpu state

[David Hildenbrand]

On 10.12.2017 22:44, Wanpeng Li wrote:
> From: Wanpeng Li 
> 
> [ cut here ]
>  Bad FPU state detected at kvm_put_guest_fpu+0xd8/0x2d0 [kvm], reinitializing 
> FPU registers.
>  WARNING: CPU: 1 PID: 4594 at arch/x86/mm/extable.c:103 
> ex_handler_fprestore+0x88/0x90
>  CPU: 1 PID: 4594 Comm: qemu-system-x86 Tainted: GB  OE
> 4.15.0-rc2+ #10
>  RIP: 0010:ex_handler_fprestore+0x88/0x90
>  Call Trace:
>   fixup_exception+0x4e/0x60
>   do_general_protection+0xff/0x270
>   general_protection+0x22/0x30
>  RIP: 0010:kvm_put_guest_fpu+0xd8/0x2d0 [kvm]
>  RSP: 0018:8803d5627810 EFLAGS: 00010246
>   kvm_vcpu_reset+0x3b4/0x3c0 [kvm]
>   kvm_apic_accept_events+0x1c0/0x240 [kvm]
>   kvm_arch_vcpu_ioctl_run+0x1658/0x2fb0 [kvm]
>   kvm_vcpu_ioctl+0x479/0x880 [kvm]
>   do_vfs_ioctl+0x142/0x9a0
>   SyS_ioctl+0x74/0x80
>   do_syscall_64+0x15f/0x600
> 
> This can be reproduced by running any testcase in kvm-unit-tests since 
> the qemu userspace FPU context is not initialized, which results in the 
> init path from kvm_apic_accept_events() will load/put qemu userspace 
> FPU context w/o initialized. In addition, w/o this splatting we still 
> should initialize vcpu->arch.user_fpu instead of current->thread.fpu. 
> This patch fixes it by initializing qemu user FPU context if it is 
> uninitialized before KVM_RUN.
> 
> Cc: Paolo Bonzini 
> Cc: Radim Krčmář 
> Cc: Rik van Riel 
> Cc: sta...@vger.kernel.org
> Fixes: f775b13eedee (x86,kvm: move qemu/guest FPU switching out to vcpu_run)
> Signed-off-by: Wanpeng Li 
> ---
>  arch/x86/kvm/x86.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index a92b22f..063a643 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7273,10 +7273,13 @@ static int complete_emulated_mmio(struct kvm_vcpu 
> *vcpu)
>  
>  int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>  {
> - struct fpu *fpu = >thread.fpu;
> + struct fpu *fpu = >arch.user_fpu;
>   int r;
>  
> - fpu__initialize(fpu);
> + if (!fpu->initialized) {
> + fpstate_init(>state);
> + fpu->initialized = 1;
> + }

Is there a chance of keeping using fpu__initialize() ? Duplicating the
code is ugly.

E.g. can't we simply initialize that in kvm_load_guest_fpu?


>  
>   kvm_sigset_activate(vcpu);
>  
> 


-- 

Thanks,

David / dhildenb

Re: [PATCH RESEND] KVM: X86: Fix load bad host fpu state

[David Hildenbrand]

On 10.12.2017 22:44, Wanpeng Li wrote:
> From: Wanpeng Li 
> 
> [ cut here ]
>  Bad FPU state detected at kvm_put_guest_fpu+0xd8/0x2d0 [kvm], reinitializing 
> FPU registers.
>  WARNING: CPU: 1 PID: 4594 at arch/x86/mm/extable.c:103 
> ex_handler_fprestore+0x88/0x90
>  CPU: 1 PID: 4594 Comm: qemu-system-x86 Tainted: GB  OE
> 4.15.0-rc2+ #10
>  RIP: 0010:ex_handler_fprestore+0x88/0x90
>  Call Trace:
>   fixup_exception+0x4e/0x60
>   do_general_protection+0xff/0x270
>   general_protection+0x22/0x30
>  RIP: 0010:kvm_put_guest_fpu+0xd8/0x2d0 [kvm]
>  RSP: 0018:8803d5627810 EFLAGS: 00010246
>   kvm_vcpu_reset+0x3b4/0x3c0 [kvm]
>   kvm_apic_accept_events+0x1c0/0x240 [kvm]
>   kvm_arch_vcpu_ioctl_run+0x1658/0x2fb0 [kvm]
>   kvm_vcpu_ioctl+0x479/0x880 [kvm]
>   do_vfs_ioctl+0x142/0x9a0
>   SyS_ioctl+0x74/0x80
>   do_syscall_64+0x15f/0x600
> 
> This can be reproduced by running any testcase in kvm-unit-tests since 
> the qemu userspace FPU context is not initialized, which results in the 
> init path from kvm_apic_accept_events() will load/put qemu userspace 
> FPU context w/o initialized. In addition, w/o this splatting we still 
> should initialize vcpu->arch.user_fpu instead of current->thread.fpu. 
> This patch fixes it by initializing qemu user FPU context if it is 
> uninitialized before KVM_RUN.
> 
> Cc: Paolo Bonzini 
> Cc: Radim Krčmář 
> Cc: Rik van Riel 
> Cc: sta...@vger.kernel.org
> Fixes: f775b13eedee (x86,kvm: move qemu/guest FPU switching out to vcpu_run)
> Signed-off-by: Wanpeng Li 
> ---
>  arch/x86/kvm/x86.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index a92b22f..063a643 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7273,10 +7273,13 @@ static int complete_emulated_mmio(struct kvm_vcpu 
> *vcpu)
>  
>  int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>  {
> - struct fpu *fpu = >thread.fpu;
> + struct fpu *fpu = >arch.user_fpu;
>   int r;
>  
> - fpu__initialize(fpu);
> + if (!fpu->initialized) {
> + fpstate_init(>state);
> + fpu->initialized = 1;
> + }

Is there a chance of keeping using fpu__initialize() ? Duplicating the
code is ugly.

E.g. can't we simply initialize that in kvm_load_guest_fpu?


>  
>   kvm_sigset_activate(vcpu);
>  
> 


-- 

Thanks,

David / dhildenb

[PATCH 41/45] arch/x86: remove duplicate includes

[Pravin Shedge]

These duplicate includes have been found with scripts/checkincludes.pl but
they have been removed manually to avoid removing false positives.

Signed-off-by: Pravin Shedge 
---
 arch/x86/kernel/itmt.c | 1 -
 arch/x86/kernel/process.c  | 1 -
 arch/x86/kernel/setup.c| 1 -
 arch/x86/kernel/smpboot.c  | 1 -
 arch/x86/platform/efi/efi_64.c | 1 -
 arch/x86/xen/spinlock.c| 2 --
 6 files changed, 7 deletions(-)

diff --git a/arch/x86/kernel/itmt.c b/arch/x86/kernel/itmt.c
index f73f475..d177940 100644
--- a/arch/x86/kernel/itmt.c
+++ b/arch/x86/kernel/itmt.c
@@ -24,7 +24,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 97fb3e5..9824553 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 8af2e8d..c8e0447 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -114,7 +114,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 3d01df7..8dd0f51 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -75,7 +75,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index 6a151ce..1e5184d 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -25,7 +25,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 02f3445..cd97a62 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -23,8 +23,6 @@ static DEFINE_PER_CPU(int, lock_kicker_irq) = -1;
 static DEFINE_PER_CPU(char *, irq_name);
 static bool xen_pvspin = true;
 
-#include 
-
 static void xen_qlock_kick(int cpu)
 {
int irq = per_cpu(lock_kicker_irq, cpu);
-- 
2.7.4

[PATCH 41/45] arch/x86: remove duplicate includes

[Pravin Shedge]

These duplicate includes have been found with scripts/checkincludes.pl but
they have been removed manually to avoid removing false positives.

Signed-off-by: Pravin Shedge 
---
 arch/x86/kernel/itmt.c | 1 -
 arch/x86/kernel/process.c  | 1 -
 arch/x86/kernel/setup.c| 1 -
 arch/x86/kernel/smpboot.c  | 1 -
 arch/x86/platform/efi/efi_64.c | 1 -
 arch/x86/xen/spinlock.c| 2 --
 6 files changed, 7 deletions(-)

diff --git a/arch/x86/kernel/itmt.c b/arch/x86/kernel/itmt.c
index f73f475..d177940 100644
--- a/arch/x86/kernel/itmt.c
+++ b/arch/x86/kernel/itmt.c
@@ -24,7 +24,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 97fb3e5..9824553 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 8af2e8d..c8e0447 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -114,7 +114,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 3d01df7..8dd0f51 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -75,7 +75,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index 6a151ce..1e5184d 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -25,7 +25,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 02f3445..cd97a62 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -23,8 +23,6 @@ static DEFINE_PER_CPU(int, lock_kicker_irq) = -1;
 static DEFINE_PER_CPU(char *, irq_name);
 static bool xen_pvspin = true;
 
-#include 
-
 static void xen_qlock_kick(int cpu)
 {
int irq = per_cpu(lock_kicker_irq, cpu);
-- 
2.7.4

RE: [PATCH 1/2] vmbus: unregister device_obj->channels_kset

[Dexuan Cui]

> From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> Behalf Of Stephen Hemminger
> Sent: Tuesday, November 28, 2017 9:30 AM
> To: Greg KH 
> Cc: o...@aepfle.de; Stephen Hemminger ;
> jasow...@redhat.com; linux-kernel@vger.kernel.org; sta...@vger.kernel.org;
> a...@canonical.com; marcelo.ce...@canonical.com;
> de...@linuxdriverproject.org; vkuzn...@redhat.com;
> leann.ogasaw...@canonical.com
> Subject: Re: [PATCH 1/2] vmbus: unregister device_obj->channels_kset
> 
> On Tue, 28 Nov 2017 16:56:05 +0100
> Greg KH  wrote:
> 
> > On Tue, Nov 14, 2017 at 06:53:32AM -0700, k...@exchange.microsoft.com
> wrote:
> > > From: Dexuan Cui 
> > >
> > > Fixes: c2e5df616e1a ("vmbus: add per-channel sysfs info")
> > >
> > > Without the patch, a device can't be thoroughly destroyed, because
> > > vmbus_device_register() -> kset_create_and_add() still holds a reference
> > > to the hv_device's device.kobj.
> > >
> > > Signed-off-by: Dexuan Cui 
> > > Cc: Stephen Hemminger 
> > > Cc: sta...@vger.kernel.org
> >
> > Why is this marked for stable when the patch it "fixes" is in 4.15-rc1?
> 
> It doesn't need to go to stable.

Hi Greg,
May I know the status of the patch?

Thanks,
-- Dexuan

RE: [PATCH 1/2] vmbus: unregister device_obj->channels_kset

[Dexuan Cui]

> From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> Behalf Of Stephen Hemminger
> Sent: Tuesday, November 28, 2017 9:30 AM
> To: Greg KH 
> Cc: o...@aepfle.de; Stephen Hemminger ;
> jasow...@redhat.com; linux-kernel@vger.kernel.org; sta...@vger.kernel.org;
> a...@canonical.com; marcelo.ce...@canonical.com;
> de...@linuxdriverproject.org; vkuzn...@redhat.com;
> leann.ogasaw...@canonical.com
> Subject: Re: [PATCH 1/2] vmbus: unregister device_obj->channels_kset
> 
> On Tue, 28 Nov 2017 16:56:05 +0100
> Greg KH  wrote:
> 
> > On Tue, Nov 14, 2017 at 06:53:32AM -0700, k...@exchange.microsoft.com
> wrote:
> > > From: Dexuan Cui 
> > >
> > > Fixes: c2e5df616e1a ("vmbus: add per-channel sysfs info")
> > >
> > > Without the patch, a device can't be thoroughly destroyed, because
> > > vmbus_device_register() -> kset_create_and_add() still holds a reference
> > > to the hv_device's device.kobj.
> > >
> > > Signed-off-by: Dexuan Cui 
> > > Cc: Stephen Hemminger 
> > > Cc: sta...@vger.kernel.org
> >
> > Why is this marked for stable when the patch it "fixes" is in 4.15-rc1?
> 
> It doesn't need to go to stable.

Hi Greg,
May I know the status of the patch?

Thanks,
-- Dexuan

Re: [PATCH 2/2] ntb_hw_switchtec: Check for alignment of the buffer in mw_set_trans()

[Logan Gunthorpe]

On 11/12/17 01:06 PM, Allen Hubbe wrote:

In switchtec_ntb_mw_get_align, for the lut case it seems to require alignment 
the same as Intel, aligned to mw size, but for the non-lut case you are saying 
that SZ_4K is not necessarily correct.  The SZ_4K is the minimum, but the 
actual alignment restriction depends on the size of the buffer actually 
translated.  Right?


Yes, that is correct.


Also, for the lut case, it looks like the size also has to be the same size as 
the mw.  So, a client can't allocate a smaller buffer, assume we can get one 
that is aligned, point the start of the mw at it, and limit the size of the mw?


The LUT case is much simpler. The size must be exactly 64K (as chosen by 
the driver... it may be a module parameter later) and therefore the 
alignment must also be exactly 64k.



For the non-lut case I wonder, with the restriction that addr needs to be 
aligned to the size of the buffer, does the size of the buffer also need to be 
some power of two?  That would make sense, if it determines the alignment.  If 
so, SZ_4K wouldn't be correct for size_align, either.


It would be weird not to make the size a power of two but this is not a 
requirement of the hardware. The alignment must be the next highest 
power of two after the size. For example, you could have a 768KB buffer 
but it would need to be aligned to 1M. This is how dma_alloc_coherent() 
behaves as well.


Think of it this way: in the hardware we program the number of 
translation bits (xlate_pos in the code). That number of low bits in the 
destination address are replaced with the same bits in the source 
address. So if any of the translated bits in the destination address 
were not zero, they will be after the replacement. Do you know if Intel 
hardware does something similar?



Do you need the intended buffer size passed in as another parameter to 
ntb_mw_get_align?  The point of ntb_mw_get_align is to figure out all the 
alignment restrictions before allocating memory.


We could, but I don't really see the point of doing that. There's really 
nothing the client would/could do differently if we added that. Remember 
this restriction is already (mostly) satisfied by dma_alloc_coherent and 
if that wasn't the case then all the tricks that the client currently 
does to try and obey ntb_mw_get_align would not work.


Actually, if we were going to change anything, I'd suggest creating an 
API that's something like:


void *ntb_mw_alloc(struct ntb_dev *ntb, int pidx, int widx,
size_t buf_size, dma_addr_t *dma_addr, int flags);

This would do the DMA allocation and adjust the size as necessary to 
satisfy the alignment requirements.


Then there would be a common place that enforces the alignment issues 
instead of expecting every client to get that right. Takes a lot of the 
boiler plate out of the clients as well.


Logan

Re: [PATCH 2/2] ntb_hw_switchtec: Check for alignment of the buffer in mw_set_trans()

[Logan Gunthorpe]

On 11/12/17 01:06 PM, Allen Hubbe wrote:

In switchtec_ntb_mw_get_align, for the lut case it seems to require alignment 
the same as Intel, aligned to mw size, but for the non-lut case you are saying 
that SZ_4K is not necessarily correct.  The SZ_4K is the minimum, but the 
actual alignment restriction depends on the size of the buffer actually 
translated.  Right?


Yes, that is correct.


Also, for the lut case, it looks like the size also has to be the same size as 
the mw.  So, a client can't allocate a smaller buffer, assume we can get one 
that is aligned, point the start of the mw at it, and limit the size of the mw?


The LUT case is much simpler. The size must be exactly 64K (as chosen by 
the driver... it may be a module parameter later) and therefore the 
alignment must also be exactly 64k.



For the non-lut case I wonder, with the restriction that addr needs to be 
aligned to the size of the buffer, does the size of the buffer also need to be 
some power of two?  That would make sense, if it determines the alignment.  If 
so, SZ_4K wouldn't be correct for size_align, either.


It would be weird not to make the size a power of two but this is not a 
requirement of the hardware. The alignment must be the next highest 
power of two after the size. For example, you could have a 768KB buffer 
but it would need to be aligned to 1M. This is how dma_alloc_coherent() 
behaves as well.


Think of it this way: in the hardware we program the number of 
translation bits (xlate_pos in the code). That number of low bits in the 
destination address are replaced with the same bits in the source 
address. So if any of the translated bits in the destination address 
were not zero, they will be after the replacement. Do you know if Intel 
hardware does something similar?



Do you need the intended buffer size passed in as another parameter to 
ntb_mw_get_align?  The point of ntb_mw_get_align is to figure out all the 
alignment restrictions before allocating memory.


We could, but I don't really see the point of doing that. There's really 
nothing the client would/could do differently if we added that. Remember 
this restriction is already (mostly) satisfied by dma_alloc_coherent and 
if that wasn't the case then all the tricks that the client currently 
does to try and obey ntb_mw_get_align would not work.


Actually, if we were going to change anything, I'd suggest creating an 
API that's something like:


void *ntb_mw_alloc(struct ntb_dev *ntb, int pidx, int widx,
size_t buf_size, dma_addr_t *dma_addr, int flags);

This would do the DMA allocation and adjust the size as necessary to 
satisfy the alignment requirements.


Then there would be a common place that enforces the alignment issues 
instead of expecting every client to get that right. Takes a lot of the 
boiler plate out of the clients as well.


Logan

[REGRESSION][4.13.y][4.14.y][v4.15.y] net: reduce skb_warn_bad_offload() noise

[Joseph Salisbury]

Hi Eric,

A kernel bug report was opened against Ubuntu [0].  It was found that
reverting the following commit resolved this bug:

commit b2504a5dbef3305ef41988ad270b0e8ec289331c
Author: Eric Dumazet 
Date:   Tue Jan 31 10:20:32 2017 -0800

    net: reduce skb_warn_bad_offload() noise
   

The regression was introduced as of v4.11-rc1 and still exists in
current mainline.
   
I was hoping to get your feedback, since you are the patch author.  Do
you think gathering any additional data will help diagnose this issue,
or would it be best to submit a revert request?
   
This commit did in fact resolve another bug[1], but in the process
introduced this regression.
  
Thanks,

Joe

[0] http://pad.lv/1715609
[1] http://pad.lv/1705447

Re: [PATCH net,stable] net: qmi_wwan: add Sierra EM7565 1199:9091

[Bjørn Mork]

ssjoh...@mac.com writes:

> From: Sebastian Sjoholm 
>
> From: Sebastian Sjoholm 
>
> Sierra Wireless EM7565 is an Qualcomm MDM9x50 based M.2 modem.
> The USB id is added to qmi_wwan.c to allow QMI communication with the EM7565.
>
> Signed-off-by: Sebastian Sjoholm 
> ---
> [The corresponding qcserial patch will be submitted by Reinhard Speyerer.]
>
> ---
>  drivers/net/usb/qmi_wwan.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index 304ec6555cd8..3cebd6683938 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -1204,6 +1204,7 @@ static const struct usb_device_id products[] = {
>   {QMI_FIXED_INTF(0x1199, 0x9079, 10)},   /* Sierra Wireless EM74xx */
>   {QMI_FIXED_INTF(0x1199, 0x907b, 8)},/* Sierra Wireless EM74xx */
>   {QMI_FIXED_INTF(0x1199, 0x907b, 10)},   /* Sierra Wireless EM74xx */
> + {QMI_FIXED_INTF(0x1199, 0x9091, 8)},/* Sierra Wireless EM7565 */
>   {QMI_FIXED_INTF(0x1bbb, 0x011e, 4)},/* Telekom Speedstick LTE II 
> (Alcatel One Touch L100V LTE) */
>   {QMI_FIXED_INTF(0x1bbb, 0x0203, 2)},/* Alcatel L800MA */
>   {QMI_FIXED_INTF(0x2357, 0x0201, 4)},/* TP-LINK HSUPA Modem MA180 */

Looks good except for the duplicate 'From' line.  Drop that and you can
add

Acked-by: Bjørn Mork 

[REGRESSION][4.13.y][4.14.y][v4.15.y] net: reduce skb_warn_bad_offload() noise

[Joseph Salisbury]

Hi Eric,

A kernel bug report was opened against Ubuntu [0].  It was found that
reverting the following commit resolved this bug:

commit b2504a5dbef3305ef41988ad270b0e8ec289331c
Author: Eric Dumazet 
Date:   Tue Jan 31 10:20:32 2017 -0800

    net: reduce skb_warn_bad_offload() noise
   

The regression was introduced as of v4.11-rc1 and still exists in
current mainline.
   
I was hoping to get your feedback, since you are the patch author.  Do
you think gathering any additional data will help diagnose this issue,
or would it be best to submit a revert request?
   
This commit did in fact resolve another bug[1], but in the process
introduced this regression.
  
Thanks,

Joe

[0] http://pad.lv/1715609
[1] http://pad.lv/1705447

Re: [PATCH net,stable] net: qmi_wwan: add Sierra EM7565 1199:9091

[Bjørn Mork]

ssjoh...@mac.com writes:

> From: Sebastian Sjoholm 
>
> From: Sebastian Sjoholm 
>
> Sierra Wireless EM7565 is an Qualcomm MDM9x50 based M.2 modem.
> The USB id is added to qmi_wwan.c to allow QMI communication with the EM7565.
>
> Signed-off-by: Sebastian Sjoholm 
> ---
> [The corresponding qcserial patch will be submitted by Reinhard Speyerer.]
>
> ---
>  drivers/net/usb/qmi_wwan.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index 304ec6555cd8..3cebd6683938 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -1204,6 +1204,7 @@ static const struct usb_device_id products[] = {
>   {QMI_FIXED_INTF(0x1199, 0x9079, 10)},   /* Sierra Wireless EM74xx */
>   {QMI_FIXED_INTF(0x1199, 0x907b, 8)},/* Sierra Wireless EM74xx */
>   {QMI_FIXED_INTF(0x1199, 0x907b, 10)},   /* Sierra Wireless EM74xx */
> + {QMI_FIXED_INTF(0x1199, 0x9091, 8)},/* Sierra Wireless EM7565 */
>   {QMI_FIXED_INTF(0x1bbb, 0x011e, 4)},/* Telekom Speedstick LTE II 
> (Alcatel One Touch L100V LTE) */
>   {QMI_FIXED_INTF(0x1bbb, 0x0203, 2)},/* Alcatel L800MA */
>   {QMI_FIXED_INTF(0x2357, 0x0201, 4)},/* TP-LINK HSUPA Modem MA180 */

Looks good except for the duplicate 'From' line.  Drop that and you can
add

Acked-by: Bjørn Mork