Re: WARNING in kill_block_super

[Tetsuo Handa]

Michal Hocko wrote:
> On Wed 04-04-18 19:53:07, Tetsuo Handa wrote:
> > Al and Michal, are you OK with this patch?
> 
> Maybe I've misunderstood, but hasn't Al explained [1] that the
> appropriate fix is in the fs code?
> 
> [1] http://lkml.kernel.org/r/20180402143415.gc30...@zeniv.linux.org.uk

Yes. But I wonder whether it worth complicating sget() only for handling
kmalloc() failure.


static struct file_system_type fuseblk_fs_type = {
  .owner  = THIS_MODULE,
  .name   = "fuseblk",
  .mount  = fuse_mount_blk,
  .kill_sb= fuse_kill_sb_blk,
  .fs_flags   = FS_REQUIRES_DEV | FS_HAS_SUBTYPE,
};

static struct dentry *fuse_mount_blk(struct file_system_type *fs_type, int 
flags, const char *dev_name, void *raw_data) {
  return mount_bdev(fs_type, flags, dev_name, raw_data, fuse_fill_super) {
fmode_t mode = FMODE_READ | FMODE_EXCL;
if (!(flags & MS_RDONLY)) mode |= FMODE_WRITE;
s = sget(fs_type, test_bdev_super, set_bdev_super, flags | MS_NOSEC, bdev) {
  return sget_userns(type, test, set, flags, user_ns, data) {
s = alloc_super(type, (flags & ~MS_SUBMOUNT), user_ns);
err = register_shrinker(>s_shrink);
if (err) {
  deactivate_locked_super(s) {
fs->kill_sb(s) = fuse_kill_sb_blk(s) {
  kill_block_super(sb) {
struct block_device *bdev = sb->s_bdev;
fmode_t mode = sb->s_mode;
WARN_ON_ONCE(!(mode & FMODE_EXCL)); // <= Unsafe because 
FMODE_EXCL is not yet set which will be set at
blkdev_put(bdev, mode | FMODE_EXCL);
  }
}
  }
  s = ERR_PTR(err);
}
  }
}
/* If sget() succeeds then ... */
s->s_mode = mode;   // <= this location.
error = fill_super(s, data, flags & MS_SILENT ? 1 : 0);
if (error) {
  deactivate_locked_super(s) {
fs->kill_sb(s) = fuse_kill_sb_blk(s) {
  kill_block_super(sb) {
struct block_device *bdev = sb->s_bdev;
fmode_t mode = sb->s_mode;
WARN_ON_ONCE(!(mode & FMODE_EXCL)); // <= Safe because 
FMODE_EXCL already set.
blkdev_put(bdev, mode | FMODE_EXCL);
  }
}
  }
  goto error;
}
/* If sget() fails then ... */
error = PTR_ERR(s);
blkdev_put(bdev, mode); // <= Calls blkdev_put() 
after deactivate_locked_super() already called blkdev_put().
  }
}


mount_bdev() is not ready to call blkdev_put() from sget().
Do we want to pass "s->s_mode" to sget() which allocates "s" ?

I feel it is preposterous that a function which allocates memory for an object
requires some of fields being already initialized in order to call a destroy
function.

By splitting register_shrinker() into prepare_shrinker() which might fail and
register_shrinker_prepared() which will not fail, we can allow shrinker users
to allocate memory at object creation time. I wrote a patch which adds
__must_check to register_shrinker() and we keep that patch in linux-next.git,
but what we got is a fake change which do not implement proper error handling
(e.g.

  Commit 6c4ca1e36cdc1a0a ("bcache: check return value of register_shrinker")

if (register_shrinker(>shrink))
pr_warn("bcache: %s: could not register shrinker",
__func__);

). It is not trivial to undo an error at register_shrinker().
Allocating memory for the shrinker at the time memory for an object which
contains the shrinker is allocated is much easier to undo.

Re: WARNING in kill_block_super

[Tetsuo Handa]

Michal Hocko wrote:
> On Wed 04-04-18 19:53:07, Tetsuo Handa wrote:
> > Al and Michal, are you OK with this patch?
> 
> Maybe I've misunderstood, but hasn't Al explained [1] that the
> appropriate fix is in the fs code?
> 
> [1] http://lkml.kernel.org/r/20180402143415.gc30...@zeniv.linux.org.uk

Yes. But I wonder whether it worth complicating sget() only for handling
kmalloc() failure.


static struct file_system_type fuseblk_fs_type = {
  .owner  = THIS_MODULE,
  .name   = "fuseblk",
  .mount  = fuse_mount_blk,
  .kill_sb= fuse_kill_sb_blk,
  .fs_flags   = FS_REQUIRES_DEV | FS_HAS_SUBTYPE,
};

static struct dentry *fuse_mount_blk(struct file_system_type *fs_type, int 
flags, const char *dev_name, void *raw_data) {
  return mount_bdev(fs_type, flags, dev_name, raw_data, fuse_fill_super) {
fmode_t mode = FMODE_READ | FMODE_EXCL;
if (!(flags & MS_RDONLY)) mode |= FMODE_WRITE;
s = sget(fs_type, test_bdev_super, set_bdev_super, flags | MS_NOSEC, bdev) {
  return sget_userns(type, test, set, flags, user_ns, data) {
s = alloc_super(type, (flags & ~MS_SUBMOUNT), user_ns);
err = register_shrinker(>s_shrink);
if (err) {
  deactivate_locked_super(s) {
fs->kill_sb(s) = fuse_kill_sb_blk(s) {
  kill_block_super(sb) {
struct block_device *bdev = sb->s_bdev;
fmode_t mode = sb->s_mode;
WARN_ON_ONCE(!(mode & FMODE_EXCL)); // <= Unsafe because 
FMODE_EXCL is not yet set which will be set at
blkdev_put(bdev, mode | FMODE_EXCL);
  }
}
  }
  s = ERR_PTR(err);
}
  }
}
/* If sget() succeeds then ... */
s->s_mode = mode;   // <= this location.
error = fill_super(s, data, flags & MS_SILENT ? 1 : 0);
if (error) {
  deactivate_locked_super(s) {
fs->kill_sb(s) = fuse_kill_sb_blk(s) {
  kill_block_super(sb) {
struct block_device *bdev = sb->s_bdev;
fmode_t mode = sb->s_mode;
WARN_ON_ONCE(!(mode & FMODE_EXCL)); // <= Safe because 
FMODE_EXCL already set.
blkdev_put(bdev, mode | FMODE_EXCL);
  }
}
  }
  goto error;
}
/* If sget() fails then ... */
error = PTR_ERR(s);
blkdev_put(bdev, mode); // <= Calls blkdev_put() 
after deactivate_locked_super() already called blkdev_put().
  }
}


mount_bdev() is not ready to call blkdev_put() from sget().
Do we want to pass "s->s_mode" to sget() which allocates "s" ?

I feel it is preposterous that a function which allocates memory for an object
requires some of fields being already initialized in order to call a destroy
function.

By splitting register_shrinker() into prepare_shrinker() which might fail and
register_shrinker_prepared() which will not fail, we can allow shrinker users
to allocate memory at object creation time. I wrote a patch which adds
__must_check to register_shrinker() and we keep that patch in linux-next.git,
but what we got is a fake change which do not implement proper error handling
(e.g.

  Commit 6c4ca1e36cdc1a0a ("bcache: check return value of register_shrinker")

if (register_shrinker(>shrink))
pr_warn("bcache: %s: could not register shrinker",
__func__);

). It is not trivial to undo an error at register_shrinker().
Allocating memory for the shrinker at the time memory for an object which
contains the shrinker is allocated is much easier to undo.

Re: [PATCH 10/10] locking/qspinlock: Elide back-to-back RELEASE operations with smp_wmb()

[Boqun Feng]

On Thu, Apr 05, 2018 at 05:59:07PM +0100, Will Deacon wrote:
> The qspinlock slowpath must ensure that the MCS node is fully initialised
> before it can be reached by another other CPU. This is currently enforced
> by using a RELEASE operation when updating the tail and also when linking
> the node into the waitqueue (since the control dependency off xchg_tail
> is insufficient to enforce sufficient ordering -- see 95bcade33a8a
> ("locking/qspinlock: Ensure node is initialised before updating prev->next")).
> 
> Back-to-back RELEASE operations may be expensive on some architectures,
> particularly those that implement them using fences under the hood. We
> can replace the two RELEASE operations with a single smp_wmb() fence and
> use RELAXED operations for the subsequent publishing of the node.
> 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Signed-off-by: Will Deacon 
> ---
>  kernel/locking/qspinlock.c | 32 +++-
>  1 file changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
> index 3ad8786a47e2..42c61f7b37c5 100644
> --- a/kernel/locking/qspinlock.c
> +++ b/kernel/locking/qspinlock.c
> @@ -141,10 +141,10 @@ static __always_inline void 
> clear_pending_set_locked(struct qspinlock *lock)
>  static __always_inline u32 xchg_tail(struct qspinlock *lock, u32 tail)
>  {
>   /*
> -  * Use release semantics to make sure that the MCS node is properly
> -  * initialized before changing the tail code.
> +  * We can use relaxed semantics since the caller ensures that the
> +  * MCS node is properly initialized before updating the tail.
>*/
> - return (u32)xchg_release(>tail,
> + return (u32)xchg_relaxed(>tail,
>tail >> _Q_TAIL_OFFSET) << _Q_TAIL_OFFSET;
>  }
>  
> @@ -178,10 +178,11 @@ static __always_inline u32 xchg_tail(struct qspinlock 
> *lock, u32 tail)
>   for (;;) {
>   new = (val & _Q_LOCKED_PENDING_MASK) | tail;
>   /*
> -  * Use release semantics to make sure that the MCS node is
> -  * properly initialized before changing the tail code.
> +  * We can use relaxed semantics since the caller ensures that
> +  * the MCS node is properly initialized before updating the
> +  * tail.
>*/
> - old = atomic_cmpxchg_release(>val, val, new);
> + old = atomic_cmpxchg_relaxed(>val, val, new);
>   if (old == val)
>   break;
>  
> @@ -340,12 +341,17 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, 
> u32 val)
>   goto release;
>  
>   /*
> +  * Ensure that the initialisation of @node is complete before we
> +  * publish the updated tail and potentially link @node into the

I think it might be better if we mention exactly where we "publish the
updated tail" and "link @node", how about:

* publish the update tail via xchg_tail() and potentially link
* @node into the waitqueue via WRITE_ONCE(->next,..) below.

and also add comments below like:

> +  * waitqueue.
> +  */
> + smp_wmb();
> +
> + /*
>* We have already touched the queueing cacheline; don't bother with
>* pending stuff.
>*
>* p,*,* -> n,*,*
> -  *
> -  * RELEASE, such that the stores to @node must be complete.

* publish the updated tail

>*/
>   old = xchg_tail(lock, tail);
>   next = NULL;
> @@ -356,15 +362,7 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, 
> u32 val)
>*/
>   if (old & _Q_TAIL_MASK) {
>   prev = decode_tail(old);
> -
> - /*
> -  * We must ensure that the stores to @node are observed before
> -  * the write to prev->next. The address dependency from
> -  * xchg_tail is not sufficient to ensure this because the read
> -  * component of xchg_tail is unordered with respect to the
> -  * initialisation of @node.
> -  */
> - smp_store_release(>next, node);

/* Eventually link @node to the wait queue */

Thoughts?

Regards,
Boqun

> + WRITE_ONCE(prev->next, node);
>  
>   pv_wait_node(node, prev);
>   arch_mcs_spin_lock_contended(>locked);
> -- 
> 2.1.4
> 


signature.asc
Description: PGP signature

Re: [PATCH 10/10] locking/qspinlock: Elide back-to-back RELEASE operations with smp_wmb()

[Boqun Feng]

On Thu, Apr 05, 2018 at 05:59:07PM +0100, Will Deacon wrote:
> The qspinlock slowpath must ensure that the MCS node is fully initialised
> before it can be reached by another other CPU. This is currently enforced
> by using a RELEASE operation when updating the tail and also when linking
> the node into the waitqueue (since the control dependency off xchg_tail
> is insufficient to enforce sufficient ordering -- see 95bcade33a8a
> ("locking/qspinlock: Ensure node is initialised before updating prev->next")).
> 
> Back-to-back RELEASE operations may be expensive on some architectures,
> particularly those that implement them using fences under the hood. We
> can replace the two RELEASE operations with a single smp_wmb() fence and
> use RELAXED operations for the subsequent publishing of the node.
> 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Signed-off-by: Will Deacon 
> ---
>  kernel/locking/qspinlock.c | 32 +++-
>  1 file changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
> index 3ad8786a47e2..42c61f7b37c5 100644
> --- a/kernel/locking/qspinlock.c
> +++ b/kernel/locking/qspinlock.c
> @@ -141,10 +141,10 @@ static __always_inline void 
> clear_pending_set_locked(struct qspinlock *lock)
>  static __always_inline u32 xchg_tail(struct qspinlock *lock, u32 tail)
>  {
>   /*
> -  * Use release semantics to make sure that the MCS node is properly
> -  * initialized before changing the tail code.
> +  * We can use relaxed semantics since the caller ensures that the
> +  * MCS node is properly initialized before updating the tail.
>*/
> - return (u32)xchg_release(>tail,
> + return (u32)xchg_relaxed(>tail,
>tail >> _Q_TAIL_OFFSET) << _Q_TAIL_OFFSET;
>  }
>  
> @@ -178,10 +178,11 @@ static __always_inline u32 xchg_tail(struct qspinlock 
> *lock, u32 tail)
>   for (;;) {
>   new = (val & _Q_LOCKED_PENDING_MASK) | tail;
>   /*
> -  * Use release semantics to make sure that the MCS node is
> -  * properly initialized before changing the tail code.
> +  * We can use relaxed semantics since the caller ensures that
> +  * the MCS node is properly initialized before updating the
> +  * tail.
>*/
> - old = atomic_cmpxchg_release(>val, val, new);
> + old = atomic_cmpxchg_relaxed(>val, val, new);
>   if (old == val)
>   break;
>  
> @@ -340,12 +341,17 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, 
> u32 val)
>   goto release;
>  
>   /*
> +  * Ensure that the initialisation of @node is complete before we
> +  * publish the updated tail and potentially link @node into the

I think it might be better if we mention exactly where we "publish the
updated tail" and "link @node", how about:

* publish the update tail via xchg_tail() and potentially link
* @node into the waitqueue via WRITE_ONCE(->next,..) below.

and also add comments below like:

> +  * waitqueue.
> +  */
> + smp_wmb();
> +
> + /*
>* We have already touched the queueing cacheline; don't bother with
>* pending stuff.
>*
>* p,*,* -> n,*,*
> -  *
> -  * RELEASE, such that the stores to @node must be complete.

* publish the updated tail

>*/
>   old = xchg_tail(lock, tail);
>   next = NULL;
> @@ -356,15 +362,7 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, 
> u32 val)
>*/
>   if (old & _Q_TAIL_MASK) {
>   prev = decode_tail(old);
> -
> - /*
> -  * We must ensure that the stores to @node are observed before
> -  * the write to prev->next. The address dependency from
> -  * xchg_tail is not sufficient to ensure this because the read
> -  * component of xchg_tail is unordered with respect to the
> -  * initialisation of @node.
> -  */
> - smp_store_release(>next, node);

/* Eventually link @node to the wait queue */

Thoughts?

Regards,
Boqun

> + WRITE_ONCE(prev->next, node);
>  
>   pv_wait_node(node, prev);
>   arch_mcs_spin_lock_contended(>locked);
> -- 
> 2.1.4
> 


signature.asc
Description: PGP signature

[PATCH] platform/x86: asus-wireless: Fix NULL pointer dereference

[João Paulo Rechi Vita]

When the module is removed the led workqueue is destroyed in the remove
callback, before the led device is unregistered from the led subsystem.

This leads to a NULL pointer derefence when the led device is
unregistered automatically later as part of the module removal cleanup.
Bellow is the backtrace showing the problem.

  BUG: unable to handle kernel NULL pointer dereference at   (null)
  IP: __queue_work+0x8c/0x410
  PGD 0 P4D 0
  Oops:  [#1] SMP NOPTI
  Modules linked in: ccm edac_mce_amd kvm_amd kvm irqbypass crct10dif_pclmul 
crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64 joydev crypto_simd 
asus_nb_wmi glue_helper uvcvideo snd_hda_codec_conexant snd_hda_codec_generic 
snd_hda_codec_hdmi snd_hda_intel asus_wmi snd_hda_codec cryptd snd_hda_core 
sparse_keymap videobuf2_vmalloc arc4 videobuf2_memops snd_hwdep input_leds 
videobuf2_v4l2 ath9k psmouse videobuf2_core videodev ath9k_common snd_pcm 
ath9k_hw media fam15h_power ath k10temp snd_timer mac80211 i2c_piix4 r8169 mii 
mac_hid cfg80211 asus_wireless(-) snd soundcore wmi shpchp 8250_dw ip_tables 
x_tables amdkfd amd_iommu_v2 amdgpu radeon chash i2c_algo_bit drm_kms_helper 
syscopyarea serio_raw sysfillrect sysimgblt fb_sys_fops ahci ttm libahci drm 
video
  CPU: 3 PID: 2177 Comm: rmmod Not tainted 4.15.0-5-generic 
#6+dev94.b4287e5bem1-Endless
  Hardware name: ASUSTeK COMPUTER INC. X555DG/X555DG, BIOS 5.011 05/05/2015
  RIP: 0010:__queue_work+0x8c/0x410
  RSP: 0018:be8cc249fcd8 EFLAGS: 00010086
  RAX: 992ac6810800 RBX:  RCX: 0008
  RDX:  RSI: 0008 RDI: 992ac6400e18
  RBP: be8cc249fd18 R08: 992ac6400db0 R09: 
  R10: 0040 R11: 992ac6400dd8 R12: 2000
  R13: 992abd762e00 R14: 992abd763e38 R15: 0001ebe0
  FS:  7f318203e700() GS:992aced8() knlGS:
  CS:  0010 DS:  ES:  CR0: 80050033
  CR2:  CR3: 0001c720e000 CR4: 001406e0
  Call Trace:
   queue_work_on+0x38/0x40
   led_state_set+0x2c/0x40 [asus_wireless]
   led_set_brightness_nopm+0x14/0x40
   led_set_brightness+0x37/0x60
   led_trigger_set+0xfc/0x1d0
   led_classdev_unregister+0x32/0xd0
   devm_led_classdev_release+0x11/0x20
   release_nodes+0x109/0x1f0
   devres_release_all+0x3c/0x50
   device_release_driver_internal+0x16d/0x220
   driver_detach+0x3f/0x80
   bus_remove_driver+0x55/0xd0
   driver_unregister+0x2c/0x40
   acpi_bus_unregister_driver+0x15/0x20
   asus_wireless_driver_exit+0x10/0xb7c [asus_wireless]
   SyS_delete_module+0x1da/0x2b0
   entry_SYSCALL_64_fastpath+0x24/0x87
  RIP: 0033:0x7f3181b65fd7
  RSP: 002b:7ffe74bcbe18 EFLAGS: 0206 ORIG_RAX: 00b0
  RAX: ffda RBX:  RCX: 7f3181b65fd7
  RDX: 000a RSI: 0800 RDI: 555ea2559258
  RBP: 555ea25591f0 R08: 7ffe74bcad91 R09: 000a
  R10:  R11: 0206 R12: 0003
  R13: 7ffe74bcae00 R14:  R15: 555ea25591f0
  Code: 01 00 00 02 0f 85 7d 01 00 00 48 63 45 d4 48 c7 c6 00 f4 fa 87 49 8b 9d 
08 01 00 00 48 03 1c c6 4c 89 f7 e8 87 fb ff ff 48 85 c0 <48> 8b 3b 0f 84 c5 01 
00 00 48 39 f8 0f 84 bc 01 00 00 48 89 c7
  RIP: __queue_work+0x8c/0x410 RSP: be8cc249fcd8
  CR2: 
  ---[ end trace 7aa4f4a232e9c39c ]---

Unregistering the led device on the remove callback before destroying the
workqueue avoids this problem.

https://bugzilla.kernel.org/show_bug.cgi?id=196097

Reported-by: Dun Hum 
Signed-off-by: João Paulo Rechi Vita 
---
 drivers/platform/x86/asus-wireless.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/asus-wireless.c 
b/drivers/platform/x86/asus-wireless.c
index 343e12547660..ecd715c82de5 100644
--- a/drivers/platform/x86/asus-wireless.c
+++ b/drivers/platform/x86/asus-wireless.c
@@ -181,6 +181,7 @@ static int asus_wireless_remove(struct acpi_device *adev)
 {
struct asus_wireless_data *data = acpi_driver_data(adev);
 
+   devm_led_classdev_unregister(>dev, >led);
if (data->wq)
destroy_workqueue(data->wq);
return 0;
-- 
2.16.3

[PATCH] platform/x86: asus-wireless: Fix NULL pointer dereference

[João Paulo Rechi Vita]

When the module is removed the led workqueue is destroyed in the remove
callback, before the led device is unregistered from the led subsystem.

This leads to a NULL pointer derefence when the led device is
unregistered automatically later as part of the module removal cleanup.
Bellow is the backtrace showing the problem.

  BUG: unable to handle kernel NULL pointer dereference at   (null)
  IP: __queue_work+0x8c/0x410
  PGD 0 P4D 0
  Oops:  [#1] SMP NOPTI
  Modules linked in: ccm edac_mce_amd kvm_amd kvm irqbypass crct10dif_pclmul 
crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64 joydev crypto_simd 
asus_nb_wmi glue_helper uvcvideo snd_hda_codec_conexant snd_hda_codec_generic 
snd_hda_codec_hdmi snd_hda_intel asus_wmi snd_hda_codec cryptd snd_hda_core 
sparse_keymap videobuf2_vmalloc arc4 videobuf2_memops snd_hwdep input_leds 
videobuf2_v4l2 ath9k psmouse videobuf2_core videodev ath9k_common snd_pcm 
ath9k_hw media fam15h_power ath k10temp snd_timer mac80211 i2c_piix4 r8169 mii 
mac_hid cfg80211 asus_wireless(-) snd soundcore wmi shpchp 8250_dw ip_tables 
x_tables amdkfd amd_iommu_v2 amdgpu radeon chash i2c_algo_bit drm_kms_helper 
syscopyarea serio_raw sysfillrect sysimgblt fb_sys_fops ahci ttm libahci drm 
video
  CPU: 3 PID: 2177 Comm: rmmod Not tainted 4.15.0-5-generic 
#6+dev94.b4287e5bem1-Endless
  Hardware name: ASUSTeK COMPUTER INC. X555DG/X555DG, BIOS 5.011 05/05/2015
  RIP: 0010:__queue_work+0x8c/0x410
  RSP: 0018:be8cc249fcd8 EFLAGS: 00010086
  RAX: 992ac6810800 RBX:  RCX: 0008
  RDX:  RSI: 0008 RDI: 992ac6400e18
  RBP: be8cc249fd18 R08: 992ac6400db0 R09: 
  R10: 0040 R11: 992ac6400dd8 R12: 2000
  R13: 992abd762e00 R14: 992abd763e38 R15: 0001ebe0
  FS:  7f318203e700() GS:992aced8() knlGS:
  CS:  0010 DS:  ES:  CR0: 80050033
  CR2:  CR3: 0001c720e000 CR4: 001406e0
  Call Trace:
   queue_work_on+0x38/0x40
   led_state_set+0x2c/0x40 [asus_wireless]
   led_set_brightness_nopm+0x14/0x40
   led_set_brightness+0x37/0x60
   led_trigger_set+0xfc/0x1d0
   led_classdev_unregister+0x32/0xd0
   devm_led_classdev_release+0x11/0x20
   release_nodes+0x109/0x1f0
   devres_release_all+0x3c/0x50
   device_release_driver_internal+0x16d/0x220
   driver_detach+0x3f/0x80
   bus_remove_driver+0x55/0xd0
   driver_unregister+0x2c/0x40
   acpi_bus_unregister_driver+0x15/0x20
   asus_wireless_driver_exit+0x10/0xb7c [asus_wireless]
   SyS_delete_module+0x1da/0x2b0
   entry_SYSCALL_64_fastpath+0x24/0x87
  RIP: 0033:0x7f3181b65fd7
  RSP: 002b:7ffe74bcbe18 EFLAGS: 0206 ORIG_RAX: 00b0
  RAX: ffda RBX:  RCX: 7f3181b65fd7
  RDX: 000a RSI: 0800 RDI: 555ea2559258
  RBP: 555ea25591f0 R08: 7ffe74bcad91 R09: 000a
  R10:  R11: 0206 R12: 0003
  R13: 7ffe74bcae00 R14:  R15: 555ea25591f0
  Code: 01 00 00 02 0f 85 7d 01 00 00 48 63 45 d4 48 c7 c6 00 f4 fa 87 49 8b 9d 
08 01 00 00 48 03 1c c6 4c 89 f7 e8 87 fb ff ff 48 85 c0 <48> 8b 3b 0f 84 c5 01 
00 00 48 39 f8 0f 84 bc 01 00 00 48 89 c7
  RIP: __queue_work+0x8c/0x410 RSP: be8cc249fcd8
  CR2: 
  ---[ end trace 7aa4f4a232e9c39c ]---

Unregistering the led device on the remove callback before destroying the
workqueue avoids this problem.

https://bugzilla.kernel.org/show_bug.cgi?id=196097

Reported-by: Dun Hum 
Signed-off-by: João Paulo Rechi Vita 
---
 drivers/platform/x86/asus-wireless.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/asus-wireless.c 
b/drivers/platform/x86/asus-wireless.c
index 343e12547660..ecd715c82de5 100644
--- a/drivers/platform/x86/asus-wireless.c
+++ b/drivers/platform/x86/asus-wireless.c
@@ -181,6 +181,7 @@ static int asus_wireless_remove(struct acpi_device *adev)
 {
struct asus_wireless_data *data = acpi_driver_data(adev);
 
+   devm_led_classdev_unregister(>dev, >led);
if (data->wq)
destroy_workqueue(data->wq);
return 0;
-- 
2.16.3

Re: [PATCH 08/10] locking/qspinlock: Merge struct __qspinlock into struct qspinlock

[Boqun Feng]

On Thu, Apr 05, 2018 at 05:59:05PM +0100, Will Deacon wrote:
> struct __qspinlock provides a handy union of fields so that
> subcomponents of the lockword can be accessed by name, without having to
> manage shifts and masks explicitly and take endianness into account.
> 
> This is useful in qspinlock.h and also potentially in arch headers, so
> move the struct __qspinlock into struct qspinlock and kill the extra
> definition.
> 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Signed-off-by: Will Deacon 

As I said in the IRC, it's glad to see such a merge ;-)

Acked-by: Boqun Feng 

Regards,
Boqun

> ---
>  arch/x86/include/asm/qspinlock.h  |  2 +-
>  arch/x86/include/asm/qspinlock_paravirt.h |  3 +-
>  include/asm-generic/qspinlock_types.h | 32 +++--
>  kernel/locking/qspinlock.c| 46 
> ++-
>  kernel/locking/qspinlock_paravirt.h   | 34 ---
>  5 files changed, 46 insertions(+), 71 deletions(-)
> 
> diff --git a/arch/x86/include/asm/qspinlock.h 
> b/arch/x86/include/asm/qspinlock.h
> index 5e16b5d40d32..90b0b0ed8161 100644
> --- a/arch/x86/include/asm/qspinlock.h
> +++ b/arch/x86/include/asm/qspinlock.h
> @@ -16,7 +16,7 @@
>   */
>  static inline void native_queued_spin_unlock(struct qspinlock *lock)
>  {
> - smp_store_release((u8 *)lock, 0);
> + smp_store_release(>locked, 0);
>  }
>  
>  #ifdef CONFIG_PARAVIRT_SPINLOCKS
> diff --git a/arch/x86/include/asm/qspinlock_paravirt.h 
> b/arch/x86/include/asm/qspinlock_paravirt.h
> index 923307ea11c7..9ef5ee03d2d7 100644
> --- a/arch/x86/include/asm/qspinlock_paravirt.h
> +++ b/arch/x86/include/asm/qspinlock_paravirt.h
> @@ -22,8 +22,7 @@ PV_CALLEE_SAVE_REGS_THUNK(__pv_queued_spin_unlock_slowpath);
>   *
>   * void __pv_queued_spin_unlock(struct qspinlock *lock)
>   * {
> - *   struct __qspinlock *l = (void *)lock;
> - *   u8 lockval = cmpxchg(>locked, _Q_LOCKED_VAL, 0);
> + *   u8 lockval = cmpxchg(>locked, _Q_LOCKED_VAL, 0);
>   *
>   *   if (likely(lockval == _Q_LOCKED_VAL))
>   *   return;
> diff --git a/include/asm-generic/qspinlock_types.h 
> b/include/asm-generic/qspinlock_types.h
> index 034acd0c4956..0763f065b975 100644
> --- a/include/asm-generic/qspinlock_types.h
> +++ b/include/asm-generic/qspinlock_types.h
> @@ -29,13 +29,41 @@
>  #endif
>  
>  typedef struct qspinlock {
> - atomic_tval;
> + union {
> + atomic_t val;
> +
> + /*
> +  * By using the whole 2nd least significant byte for the
> +  * pending bit, we can allow better optimization of the lock
> +  * acquisition for the pending bit holder.
> +  */
> +#ifdef __LITTLE_ENDIAN
> + struct {
> + u8  locked;
> + u8  pending;
> + };
> + struct {
> + u16 locked_pending;
> + u16 tail;
> + };
> +#else
> + struct {
> + u16 tail;
> + u16 locked_pending;
> + };
> + struct {
> + u8  reserved[2];
> + u8  pending;
> + u8  locked;
> + };
> +#endif
> + };
>  } arch_spinlock_t;
>  
>  /*
>   * Initializier
>   */
> -#define  __ARCH_SPIN_LOCK_UNLOCKED   { ATOMIC_INIT(0) }
> +#define  __ARCH_SPIN_LOCK_UNLOCKED   { .val = ATOMIC_INIT(0) }
>  
>  /*
>   * Bitfields in the atomic value:
> diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
> index c8b57d375b49..3ad8786a47e2 100644
> --- a/kernel/locking/qspinlock.c
> +++ b/kernel/locking/qspinlock.c
> @@ -114,40 +114,6 @@ static inline __pure struct mcs_spinlock 
> *decode_tail(u32 tail)
>  
>  #define _Q_LOCKED_PENDING_MASK (_Q_LOCKED_MASK | _Q_PENDING_MASK)
>  
> -/*
> - * By using the whole 2nd least significant byte for the pending bit, we
> - * can allow better optimization of the lock acquisition for the pending
> - * bit holder.
> - *
> - * This internal structure is also used by the set_locked function which
> - * is not restricted to _Q_PENDING_BITS == 8.
> - */
> -struct __qspinlock {
> - union {
> - atomic_t val;
> -#ifdef __LITTLE_ENDIAN
> - struct {
> - u8  locked;
> - u8  pending;
> - };
> - struct {
> - u16 locked_pending;
> - u16 tail;
> - };
> -#else
> - struct {
> - u16 tail;
> - u16 locked_pending;
> - };
> - struct {
> - u8  reserved[2];
> - u8  pending;
> - u8  locked;
> - };
> -#endif
> - };
> -};
> -
>  

Re: [PATCH 08/10] locking/qspinlock: Merge struct __qspinlock into struct qspinlock

[Boqun Feng]

On Thu, Apr 05, 2018 at 05:59:05PM +0100, Will Deacon wrote:
> struct __qspinlock provides a handy union of fields so that
> subcomponents of the lockword can be accessed by name, without having to
> manage shifts and masks explicitly and take endianness into account.
> 
> This is useful in qspinlock.h and also potentially in arch headers, so
> move the struct __qspinlock into struct qspinlock and kill the extra
> definition.
> 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Signed-off-by: Will Deacon 

As I said in the IRC, it's glad to see such a merge ;-)

Acked-by: Boqun Feng 

Regards,
Boqun

> ---
>  arch/x86/include/asm/qspinlock.h  |  2 +-
>  arch/x86/include/asm/qspinlock_paravirt.h |  3 +-
>  include/asm-generic/qspinlock_types.h | 32 +++--
>  kernel/locking/qspinlock.c| 46 
> ++-
>  kernel/locking/qspinlock_paravirt.h   | 34 ---
>  5 files changed, 46 insertions(+), 71 deletions(-)
> 
> diff --git a/arch/x86/include/asm/qspinlock.h 
> b/arch/x86/include/asm/qspinlock.h
> index 5e16b5d40d32..90b0b0ed8161 100644
> --- a/arch/x86/include/asm/qspinlock.h
> +++ b/arch/x86/include/asm/qspinlock.h
> @@ -16,7 +16,7 @@
>   */
>  static inline void native_queued_spin_unlock(struct qspinlock *lock)
>  {
> - smp_store_release((u8 *)lock, 0);
> + smp_store_release(>locked, 0);
>  }
>  
>  #ifdef CONFIG_PARAVIRT_SPINLOCKS
> diff --git a/arch/x86/include/asm/qspinlock_paravirt.h 
> b/arch/x86/include/asm/qspinlock_paravirt.h
> index 923307ea11c7..9ef5ee03d2d7 100644
> --- a/arch/x86/include/asm/qspinlock_paravirt.h
> +++ b/arch/x86/include/asm/qspinlock_paravirt.h
> @@ -22,8 +22,7 @@ PV_CALLEE_SAVE_REGS_THUNK(__pv_queued_spin_unlock_slowpath);
>   *
>   * void __pv_queued_spin_unlock(struct qspinlock *lock)
>   * {
> - *   struct __qspinlock *l = (void *)lock;
> - *   u8 lockval = cmpxchg(>locked, _Q_LOCKED_VAL, 0);
> + *   u8 lockval = cmpxchg(>locked, _Q_LOCKED_VAL, 0);
>   *
>   *   if (likely(lockval == _Q_LOCKED_VAL))
>   *   return;
> diff --git a/include/asm-generic/qspinlock_types.h 
> b/include/asm-generic/qspinlock_types.h
> index 034acd0c4956..0763f065b975 100644
> --- a/include/asm-generic/qspinlock_types.h
> +++ b/include/asm-generic/qspinlock_types.h
> @@ -29,13 +29,41 @@
>  #endif
>  
>  typedef struct qspinlock {
> - atomic_tval;
> + union {
> + atomic_t val;
> +
> + /*
> +  * By using the whole 2nd least significant byte for the
> +  * pending bit, we can allow better optimization of the lock
> +  * acquisition for the pending bit holder.
> +  */
> +#ifdef __LITTLE_ENDIAN
> + struct {
> + u8  locked;
> + u8  pending;
> + };
> + struct {
> + u16 locked_pending;
> + u16 tail;
> + };
> +#else
> + struct {
> + u16 tail;
> + u16 locked_pending;
> + };
> + struct {
> + u8  reserved[2];
> + u8  pending;
> + u8  locked;
> + };
> +#endif
> + };
>  } arch_spinlock_t;
>  
>  /*
>   * Initializier
>   */
> -#define  __ARCH_SPIN_LOCK_UNLOCKED   { ATOMIC_INIT(0) }
> +#define  __ARCH_SPIN_LOCK_UNLOCKED   { .val = ATOMIC_INIT(0) }
>  
>  /*
>   * Bitfields in the atomic value:
> diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
> index c8b57d375b49..3ad8786a47e2 100644
> --- a/kernel/locking/qspinlock.c
> +++ b/kernel/locking/qspinlock.c
> @@ -114,40 +114,6 @@ static inline __pure struct mcs_spinlock 
> *decode_tail(u32 tail)
>  
>  #define _Q_LOCKED_PENDING_MASK (_Q_LOCKED_MASK | _Q_PENDING_MASK)
>  
> -/*
> - * By using the whole 2nd least significant byte for the pending bit, we
> - * can allow better optimization of the lock acquisition for the pending
> - * bit holder.
> - *
> - * This internal structure is also used by the set_locked function which
> - * is not restricted to _Q_PENDING_BITS == 8.
> - */
> -struct __qspinlock {
> - union {
> - atomic_t val;
> -#ifdef __LITTLE_ENDIAN
> - struct {
> - u8  locked;
> - u8  pending;
> - };
> - struct {
> - u16 locked_pending;
> - u16 tail;
> - };
> -#else
> - struct {
> - u16 tail;
> - u16 locked_pending;
> - };
> - struct {
> - u8  reserved[2];
> - u8  pending;
> - u8  locked;
> - };
> -#endif
> - };
> -};
> -
>  #if _Q_PENDING_BITS == 8
>  /**
>   * clear_pending_set_locked - take ownership and 

Re: [PATCH v7 01/11] arm64: dts: actions: Add S900 clock management unit nodes

[Manivannan Sadhasivam]

Hi Andreas,

Since the clk driver got applied, can you please take the dts changes
through linux-actions tree?

Thanks,
Mani

On Mon, Mar 26, 2018 at 11:08:55PM +0530, Manivannan Sadhasivam wrote:
> Add Actions Semi S900 Clock Management Unit (CMU) nodes
> 
> Signed-off-by: Manivannan Sadhasivam 
> ---
>  arch/arm64/boot/dts/actions/s900.dtsi | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/actions/s900.dtsi 
> b/arch/arm64/boot/dts/actions/s900.dtsi
> index 11406f6d3a6d..fee0c9557656 100644
> --- a/arch/arm64/boot/dts/actions/s900.dtsi
> +++ b/arch/arm64/boot/dts/actions/s900.dtsi
> @@ -4,6 +4,7 @@
>   * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>   */
>  
> +#include 
>  #include 
>  
>  / {
> @@ -88,6 +89,18 @@
>   #clock-cells = <0>;
>   };
>  
> + losc: losc {
> + compatible = "fixed-clock";
> + clock-frequency = <32768>;
> + #clock-cells = <0>;
> + };
> +
> + diff24M: diff24M {
> + compatible = "fixed-clock";
> + clock-frequency = <2400>;
> + #clock-cells = <0>;
> + };
> +
>   soc {
>   compatible = "simple-bus";
>   #address-cells = <2>;
> @@ -154,6 +167,13 @@
>   status = "disabled";
>   };
>  
> + cmu: clock-controller@e016 {
> + compatible = "actions,s900-cmu";
> + reg = <0x0 0xe016 0x0 0x1000>;
> + clocks = <>, <>;
> + #clock-cells = <1>;
> + };
> +
>   timer: timer@e0228000 {
>   compatible = "actions,s900-timer";
>   reg = <0x0 0xe0228000 0x0 0x8000>;
> -- 
> 2.14.1
> 

Re: [PATCH v7 01/11] arm64: dts: actions: Add S900 clock management unit nodes

[Manivannan Sadhasivam]

Hi Andreas,

Since the clk driver got applied, can you please take the dts changes
through linux-actions tree?

Thanks,
Mani

On Mon, Mar 26, 2018 at 11:08:55PM +0530, Manivannan Sadhasivam wrote:
> Add Actions Semi S900 Clock Management Unit (CMU) nodes
> 
> Signed-off-by: Manivannan Sadhasivam 
> ---
>  arch/arm64/boot/dts/actions/s900.dtsi | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/actions/s900.dtsi 
> b/arch/arm64/boot/dts/actions/s900.dtsi
> index 11406f6d3a6d..fee0c9557656 100644
> --- a/arch/arm64/boot/dts/actions/s900.dtsi
> +++ b/arch/arm64/boot/dts/actions/s900.dtsi
> @@ -4,6 +4,7 @@
>   * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>   */
>  
> +#include 
>  #include 
>  
>  / {
> @@ -88,6 +89,18 @@
>   #clock-cells = <0>;
>   };
>  
> + losc: losc {
> + compatible = "fixed-clock";
> + clock-frequency = <32768>;
> + #clock-cells = <0>;
> + };
> +
> + diff24M: diff24M {
> + compatible = "fixed-clock";
> + clock-frequency = <2400>;
> + #clock-cells = <0>;
> + };
> +
>   soc {
>   compatible = "simple-bus";
>   #address-cells = <2>;
> @@ -154,6 +167,13 @@
>   status = "disabled";
>   };
>  
> + cmu: clock-controller@e016 {
> + compatible = "actions,s900-cmu";
> + reg = <0x0 0xe016 0x0 0x1000>;
> + clocks = <>, <>;
> + #clock-cells = <1>;
> + };
> +
>   timer: timer@e0228000 {
>   compatible = "actions,s900-timer";
>   reg = <0x0 0xe0228000 0x0 0x8000>;
> -- 
> 2.14.1
> 

Re: [PATCH 1/1] arm64: To remove initrd reserved area entry from memblock

[Chandan Vn]

On Fri, Apr 6, 2018 at 9:47 PM, Laura Abbott  wrote:
> Does this have an impact on anything besides accounting
> in memblock?

Yes, the impact is only on accounting or debugging.

We were trying to reduce the reserved memory by removing initrd reserved area.
After disabling "keepinitrd", only way to check if it was removed or
not was to check
/sys/kernel/debug/memblock/reserved. We found the entry to be present
irrespective of
"keepinitrd" being enabled/disabled.
I hope that with the fix others wont face similar issue. Also we did
not find any such problem
with ARM32 ARCHITECTURE.

On Fri, Apr 6, 2018 at 9:47 PM, Laura Abbott  wrote:
> On 04/05/2018 09:53 PM, CHANDAN VN wrote:
>>
>> INITRD reserved area entry is not removed from memblock
>> even though initrd reserved area is freed. After freeing
>> the memory it is released from memblock. The same can be
>> checked from /sys/kernel/debug/memblock/reserved.
>>
>> The patch makes sure that the initrd entry is removed from
>> memblock when keepinitrd is not enabled.
>>
>
> Does this have an impact on anything besides accounting
> in memblock?
>
>
>> Signed-off-by: CHANDAN VN 
>> ---
>>   arch/arm64/mm/init.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 9f3c47a..1b18b47 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -646,8 +646,10 @@ void free_initmem(void)
>> void __init free_initrd_mem(unsigned long start, unsigned long end)
>>   {
>> -   if (!keep_initrd)
>> +   if (!keep_initrd) {
>> free_reserved_area((void *)start, (void *)end, 0,
>> "initrd");
>> +   memblock_free(__virt_to_phys(start), end - start);
>> +   }
>>   }
>> static int __init keepinitrd_setup(char *__unused)
>>
>

Re: [PATCH 1/1] arm64: To remove initrd reserved area entry from memblock

[Chandan Vn]

On Fri, Apr 6, 2018 at 9:47 PM, Laura Abbott  wrote:
> Does this have an impact on anything besides accounting
> in memblock?

Yes, the impact is only on accounting or debugging.

We were trying to reduce the reserved memory by removing initrd reserved area.
After disabling "keepinitrd", only way to check if it was removed or
not was to check
/sys/kernel/debug/memblock/reserved. We found the entry to be present
irrespective of
"keepinitrd" being enabled/disabled.
I hope that with the fix others wont face similar issue. Also we did
not find any such problem
with ARM32 ARCHITECTURE.

On Fri, Apr 6, 2018 at 9:47 PM, Laura Abbott  wrote:
> On 04/05/2018 09:53 PM, CHANDAN VN wrote:
>>
>> INITRD reserved area entry is not removed from memblock
>> even though initrd reserved area is freed. After freeing
>> the memory it is released from memblock. The same can be
>> checked from /sys/kernel/debug/memblock/reserved.
>>
>> The patch makes sure that the initrd entry is removed from
>> memblock when keepinitrd is not enabled.
>>
>
> Does this have an impact on anything besides accounting
> in memblock?
>
>
>> Signed-off-by: CHANDAN VN 
>> ---
>>   arch/arm64/mm/init.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 9f3c47a..1b18b47 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -646,8 +646,10 @@ void free_initmem(void)
>> void __init free_initrd_mem(unsigned long start, unsigned long end)
>>   {
>> -   if (!keep_initrd)
>> +   if (!keep_initrd) {
>> free_reserved_area((void *)start, (void *)end, 0,
>> "initrd");
>> +   memblock_free(__virt_to_phys(start), end - start);
>> +   }
>>   }
>> static int __init keepinitrd_setup(char *__unused)
>>
>

Re: [PATCH] test/nvme/003: add test case for patch "nvme: don't send keep-alives to the discovery controller"

[Omar Sandoval]

On Tue, Mar 27, 2018 at 11:31:47AM +0200, Johannes Thumshirn wrote:
> Add a regression test for the patch titled "nvme: don't send
> keep-alives to the discovery controller".
> 
> This patch creates a local loopback nvme target and then connects to
> it. If the patch is not applied we see two error messages in dmesg:
> "failed nvme_keep_alive_end_io error=" from the nvme host and "nvmet:
> unsupported cmd 24" from the nvme target. If the patch is applied
> dmesg is quiet.
> 
> Signed-off-by: Johannes Thumshirn 

Applied, thanks, Johannes!

Re: [PATCH] test/nvme/003: add test case for patch "nvme: don't send keep-alives to the discovery controller"

[Omar Sandoval]

On Tue, Mar 27, 2018 at 11:31:47AM +0200, Johannes Thumshirn wrote:
> Add a regression test for the patch titled "nvme: don't send
> keep-alives to the discovery controller".
> 
> This patch creates a local loopback nvme target and then connects to
> it. If the patch is not applied we see two error messages in dmesg:
> "failed nvme_keep_alive_end_io error=" from the nvme host and "nvmet:
> unsupported cmd 24" from the nvme target. If the patch is applied
> dmesg is quiet.
> 
> Signed-off-by: Johannes Thumshirn 

Applied, thanks, Johannes!

Re: [PATCH] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls

[Keith Packard]

Jason Ekstrand  writes:

> Is the given sequence number guaranteed to be hit in finite time?

Certainly, it's a finite value...

However, realistically, it's just like all of the other vblank
interfaces where you can specify a crazy sequence and block for a very
long time. So, no different from the current situation.

Of course, the only vulkan API available today only lets you wait for
the next vblank, so we'll be asking for a sequence which is one more
than the current sequence.

> Just to make sure I've got this straight, the client queues a syncobj with
> queue_syncobj to fire at a given sequence number.  Once the syncobj has
> fired (which it can find out by waiting on it), it then calls get_syncobj
> to figure out when it was fired?

If it cares, it can ask. It might not care at all, in which case it
wouldn't have to ask. The syncobj API doesn't provide any direct
information about the state of the object in the wait call.

> If so, what happens if a syncobj is re-used?  Do you just loose the
> information?

You can't reuse one of these -- the 'queue_syncobj' API creates a new
one each time.

> If we have a finite time guarantee, I'm kind-of thinking a sync_file might
> be better as it's a one-shot and doesn't have the information loss
> problem.  I'm not actually sure though.

It's a one-shot, once signaled, you're done with it.

> This whole "wait for a syncobj and then ask when it fired" thing is a bit
> odd.  I'll have to think on it.

Yeah, the event mechanism has the nice feature that you get data with
each event. I guess the alternative would be to have a way to get an
event when a sync object was ready; perhaps a call which provided a set
of syncobjs and delivered a DRM event when any of them was ready?

That has a lot of appeal; it turns the poll-like nature of the current
API into an epoll-like system. Hrm.

-- 
-keith


signature.asc
Description: PGP signature

Re: [PATCH] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls

[Keith Packard]

Jason Ekstrand  writes:

> Is the given sequence number guaranteed to be hit in finite time?

Certainly, it's a finite value...

However, realistically, it's just like all of the other vblank
interfaces where you can specify a crazy sequence and block for a very
long time. So, no different from the current situation.

Of course, the only vulkan API available today only lets you wait for
the next vblank, so we'll be asking for a sequence which is one more
than the current sequence.

> Just to make sure I've got this straight, the client queues a syncobj with
> queue_syncobj to fire at a given sequence number.  Once the syncobj has
> fired (which it can find out by waiting on it), it then calls get_syncobj
> to figure out when it was fired?

If it cares, it can ask. It might not care at all, in which case it
wouldn't have to ask. The syncobj API doesn't provide any direct
information about the state of the object in the wait call.

> If so, what happens if a syncobj is re-used?  Do you just loose the
> information?

You can't reuse one of these -- the 'queue_syncobj' API creates a new
one each time.

> If we have a finite time guarantee, I'm kind-of thinking a sync_file might
> be better as it's a one-shot and doesn't have the information loss
> problem.  I'm not actually sure though.

It's a one-shot, once signaled, you're done with it.

> This whole "wait for a syncobj and then ask when it fired" thing is a bit
> odd.  I'll have to think on it.

Yeah, the event mechanism has the nice feature that you get data with
each event. I guess the alternative would be to have a way to get an
event when a sync object was ready; perhaps a call which provided a set
of syncobjs and delivered a DRM event when any of them was ready?

That has a lot of appeal; it turns the poll-like nature of the current
API into an epoll-like system. Hrm.

-- 
-keith


signature.asc
Description: PGP signature

Re: [PATCH] drivers/memory: can't open emif-asm-offsets.s for writing

[Masahiro Yamada]

2018-04-06 22:00 GMT+09:00 Anders Roxell :
> Build failes due to that the directory isn't created before we execute
> the build rule.
> cc1: fatal error: can’t open ‘drivers/memory/emif-asm-offsets.s’ for
>   writing: No such file or directory compilation terminated.
> drivers/memory/Makefile.asm-offsets:2: recipe for target 
> 'drivers/memory/emif-asm-offsets.s' failed
> make[2]: *** [drivers/memory/emif-asm-offsets.s] Error 1
> Makefile:1060: recipe for target
> 'arch/arm/mach-omap2' failed
> make[1]: *** [arch/arm/mach-omap2] Error 2
> make[1]: *** Waiting for unfinished jobs
>
> Current code adds a file check before checking the dependency, in
> filechk it does mkdir -p $(dir $@).
>
> Fixes: 41d9d44d7258 ("ARM: OMAP2+: pm33xx-core: Add platform code needed for 
> PM")
> Signed-off-by: Anders Roxell 
> ---
>  drivers/memory/Makefile.asm-offsets | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/memory/Makefile.asm-offsets 
> b/drivers/memory/Makefile.asm-offsets
> index 843ff60ccb5a..70d44a9ed32a 100644
> --- a/drivers/memory/Makefile.asm-offsets
> +++ b/drivers/memory/Makefile.asm-offsets
> @@ -1,4 +1,5 @@
>  drivers/memory/emif-asm-offsets.s: drivers/memory/emif-asm-offsets.c
> +   $(call filechk,$@)
> $(call if_changed_dep,cc_s_c)
>
>  include/generated/ti-emif-asm-offsets.h: drivers/memory/emif-asm-offsets.s 
> FORCE
> --
> 2.16.3


Why filechk just for creating a directory?


$(Q)mkdir $(dir $@)

is enough, but still this is a bad fix.


Generating the same object from different directories
is fragile in parallel building.




-- 
Best Regards
Masahiro Yamada

Re: [PATCH] drivers/memory: can't open emif-asm-offsets.s for writing

[Masahiro Yamada]

2018-04-06 22:00 GMT+09:00 Anders Roxell :
> Build failes due to that the directory isn't created before we execute
> the build rule.
> cc1: fatal error: can’t open ‘drivers/memory/emif-asm-offsets.s’ for
>   writing: No such file or directory compilation terminated.
> drivers/memory/Makefile.asm-offsets:2: recipe for target 
> 'drivers/memory/emif-asm-offsets.s' failed
> make[2]: *** [drivers/memory/emif-asm-offsets.s] Error 1
> Makefile:1060: recipe for target
> 'arch/arm/mach-omap2' failed
> make[1]: *** [arch/arm/mach-omap2] Error 2
> make[1]: *** Waiting for unfinished jobs
>
> Current code adds a file check before checking the dependency, in
> filechk it does mkdir -p $(dir $@).
>
> Fixes: 41d9d44d7258 ("ARM: OMAP2+: pm33xx-core: Add platform code needed for 
> PM")
> Signed-off-by: Anders Roxell 
> ---
>  drivers/memory/Makefile.asm-offsets | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/memory/Makefile.asm-offsets 
> b/drivers/memory/Makefile.asm-offsets
> index 843ff60ccb5a..70d44a9ed32a 100644
> --- a/drivers/memory/Makefile.asm-offsets
> +++ b/drivers/memory/Makefile.asm-offsets
> @@ -1,4 +1,5 @@
>  drivers/memory/emif-asm-offsets.s: drivers/memory/emif-asm-offsets.c
> +   $(call filechk,$@)
> $(call if_changed_dep,cc_s_c)
>
>  include/generated/ti-emif-asm-offsets.h: drivers/memory/emif-asm-offsets.s 
> FORCE
> --
> 2.16.3


Why filechk just for creating a directory?


$(Q)mkdir $(dir $@)

is enough, but still this is a bad fix.


Generating the same object from different directories
is fragile in parallel building.




-- 
Best Regards
Masahiro Yamada

Re: [PATCH ipmi/kcs_bmc v1] ipmi: kcs_bmc: optimize the data buffers allocation

[Wang, Haiyue]

On 2018-04-07 05:47, Corey Minyard wrote:

On 03/15/2018 07:20 AM, Haiyue Wang wrote:

Allocate a continuous memory block for the three KCS data buffers with
related index assignment.


I'm finally getting to this.

Is there a reason you want to do this?  In general, it's better to not 
try to

outsmart your base system.  Depending on the memory allocator, in this
case, you might actually use more memory.  You probably won't use any
less.


I got this idea from another code review, but that patch allocates 30 more
the same size memory block, reducing the devm_kmalloc call will be better.
For KCS only have 3, may be the key point is memory waste.

In the original case, you allocate three 1000 byte buffers, resulting 
in 3

1024 byte slab allocated.

In the changed case, you will allocate a 3000 byte buffer, resulting in
a single 4096 byte slab allocation, wasting 1024 more bytes of memory.


As the kcs has memory copy between in/out/kbuffer, put them in the same
page will be better ? Such as the same TLB ? (Well, I just got this from 
book,

no real experience of memory accessing performance. And also, I was told
that using space to save the time. :-)).

Just my stupid thinking. I'm OK to drop this patch if it doesn't help with
performance, or something else.

BR.
Haiyue


-corey


Signed-off-by: Haiyue Wang 
---
  drivers/char/ipmi/kcs_bmc.c | 10 ++
  1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c
index fbfc05e..dc19c0d 100644
--- a/drivers/char/ipmi/kcs_bmc.c
+++ b/drivers/char/ipmi/kcs_bmc.c
@@ -435,6 +435,7 @@ static const struct file_operations kcs_bmc_fops = {
  struct kcs_bmc *kcs_bmc_alloc(struct device *dev, int sizeof_priv, 
u32 channel)

  {
  struct kcs_bmc *kcs_bmc;
+    void *buf;
    kcs_bmc = devm_kzalloc(dev, sizeof(*kcs_bmc) + sizeof_priv, 
GFP_KERNEL);

  if (!kcs_bmc)
@@ -448,11 +449,12 @@ struct kcs_bmc *kcs_bmc_alloc(struct device 
*dev, int sizeof_priv, u32 channel)

  mutex_init(_bmc->mutex);
  init_waitqueue_head(_bmc->queue);
  -    kcs_bmc->data_in = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
-    kcs_bmc->data_out = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
-    kcs_bmc->kbuffer = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
-    if (!kcs_bmc->data_in || !kcs_bmc->data_out || !kcs_bmc->kbuffer)
+    buf = devm_kmalloc_array(dev, 3, KCS_MSG_BUFSIZ, GFP_KERNEL);
+    if (!buf)
  return NULL;
+    kcs_bmc->data_in  = buf;
+    kcs_bmc->data_out = buf + KCS_MSG_BUFSIZ;
+    kcs_bmc->kbuffer  = buf + KCS_MSG_BUFSIZ * 2;
    kcs_bmc->miscdev.minor = MISC_DYNAMIC_MINOR;
  kcs_bmc->miscdev.name = dev_name(dev);

Re: [PATCH ipmi/kcs_bmc v1] ipmi: kcs_bmc: optimize the data buffers allocation

[Wang, Haiyue]

On 2018-04-07 05:47, Corey Minyard wrote:

On 03/15/2018 07:20 AM, Haiyue Wang wrote:

Allocate a continuous memory block for the three KCS data buffers with
related index assignment.


I'm finally getting to this.

Is there a reason you want to do this?  In general, it's better to not 
try to

outsmart your base system.  Depending on the memory allocator, in this
case, you might actually use more memory.  You probably won't use any
less.


I got this idea from another code review, but that patch allocates 30 more
the same size memory block, reducing the devm_kmalloc call will be better.
For KCS only have 3, may be the key point is memory waste.

In the original case, you allocate three 1000 byte buffers, resulting 
in 3

1024 byte slab allocated.

In the changed case, you will allocate a 3000 byte buffer, resulting in
a single 4096 byte slab allocation, wasting 1024 more bytes of memory.


As the kcs has memory copy between in/out/kbuffer, put them in the same
page will be better ? Such as the same TLB ? (Well, I just got this from 
book,

no real experience of memory accessing performance. And also, I was told
that using space to save the time. :-)).

Just my stupid thinking. I'm OK to drop this patch if it doesn't help with
performance, or something else.

BR.
Haiyue


-corey


Signed-off-by: Haiyue Wang 
---
  drivers/char/ipmi/kcs_bmc.c | 10 ++
  1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c
index fbfc05e..dc19c0d 100644
--- a/drivers/char/ipmi/kcs_bmc.c
+++ b/drivers/char/ipmi/kcs_bmc.c
@@ -435,6 +435,7 @@ static const struct file_operations kcs_bmc_fops = {
  struct kcs_bmc *kcs_bmc_alloc(struct device *dev, int sizeof_priv, 
u32 channel)

  {
  struct kcs_bmc *kcs_bmc;
+    void *buf;
    kcs_bmc = devm_kzalloc(dev, sizeof(*kcs_bmc) + sizeof_priv, 
GFP_KERNEL);

  if (!kcs_bmc)
@@ -448,11 +449,12 @@ struct kcs_bmc *kcs_bmc_alloc(struct device 
*dev, int sizeof_priv, u32 channel)

  mutex_init(_bmc->mutex);
  init_waitqueue_head(_bmc->queue);
  -    kcs_bmc->data_in = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
-    kcs_bmc->data_out = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
-    kcs_bmc->kbuffer = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
-    if (!kcs_bmc->data_in || !kcs_bmc->data_out || !kcs_bmc->kbuffer)
+    buf = devm_kmalloc_array(dev, 3, KCS_MSG_BUFSIZ, GFP_KERNEL);
+    if (!buf)
  return NULL;
+    kcs_bmc->data_in  = buf;
+    kcs_bmc->data_out = buf + KCS_MSG_BUFSIZ;
+    kcs_bmc->kbuffer  = buf + KCS_MSG_BUFSIZ * 2;
    kcs_bmc->miscdev.minor = MISC_DYNAMIC_MINOR;
  kcs_bmc->miscdev.name = dev_name(dev);

Re: [PATCH] selftests/livepatch: introduce tests

[Josh Poimboeuf]

On Wed, Mar 28, 2018 at 03:49:48PM -0400, Joe Lawrence wrote:
> Add a few livepatch modules and simple target modules that the included
> regression suite can run tests against.
> 
> Signed-off-by: Joe Lawrence 
> ---
>  lib/Kconfig.debug  |  12 +
>  lib/Makefile   |   2 +
>  lib/livepatch/Makefile |  18 +
>  lib/livepatch/test_klp_atomic_replace.c|  69 +++
>  lib/livepatch/test_klp_callbacks_busy.c|  43 ++
>  lib/livepatch/test_klp_callbacks_demo.c| 132 ++
>  lib/livepatch/test_klp_callbacks_demo2.c   | 104 
>  lib/livepatch/test_klp_callbacks_mod.c |  24 +
>  lib/livepatch/test_klp_livepatch.c |  62 +++
>  tools/testing/selftests/Makefile   |   1 +
>  tools/testing/selftests/livepatch/Makefile |   8 +
>  tools/testing/selftests/livepatch/config   |   1 +
>  tools/testing/selftests/livepatch/functions.sh | 202 
>  .../testing/selftests/livepatch/test-callbacks.sh  | 526 
> +
>  .../testing/selftests/livepatch/test-livepatch.sh  | 177 +++
>  .../selftests/livepatch/test-shadow-vars.sh|  13 +
>  16 files changed, 1394 insertions(+)
>  create mode 100644 lib/livepatch/Makefile
>  create mode 100644 lib/livepatch/test_klp_atomic_replace.c
>  create mode 100644 lib/livepatch/test_klp_callbacks_busy.c
>  create mode 100644 lib/livepatch/test_klp_callbacks_demo.c
>  create mode 100644 lib/livepatch/test_klp_callbacks_demo2.c
>  create mode 100644 lib/livepatch/test_klp_callbacks_mod.c
>  create mode 100644 lib/livepatch/test_klp_livepatch.c
>  create mode 100644 tools/testing/selftests/livepatch/Makefile
>  create mode 100644 tools/testing/selftests/livepatch/config
>  create mode 100644 tools/testing/selftests/livepatch/functions.sh
>  create mode 100755 tools/testing/selftests/livepatch/test-callbacks.sh
>  create mode 100755 tools/testing/selftests/livepatch/test-livepatch.sh
>  create mode 100755 tools/testing/selftests/livepatch/test-shadow-vars.sh

I love this.  Nice work!

As you and Petr discussed, it would be nice to get rid of some of the
delays, and also the callback tests will be very important.

-- 
Josh

Re: [PATCH v9 06/10] time: tick-sched: Split tick_nohz_stop_sched_tick()

[Frederic Weisbecker]

On Wed, Apr 04, 2018 at 10:41:13AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> In order to address the issue with short idle duration predictions
> by the idle governor after the scheduler tick has been stopped, split
> tick_nohz_stop_sched_tick() into two separate routines, one computing
> the time to the next timer event and the other simply stopping the
> tick when the time to the next timer event is known.
> 
> Prepare these two routines to be called separately, as one of them
> will be called by the idle governor in the cpuidle_select() code
> path after subsequent changes.
> 
> Update the former callers of tick_nohz_stop_sched_tick() to use
> the new routines, tick_nohz_next_event() and tick_nohz_stop_tick(),
> instead of it and move the updates of the sleep_length field in
> struct tick_sched into __tick_nohz_idle_stop_tick() as it doesn't
> need to be updated anywhere else.
> 
> There should be no intentional visible changes in functionality
> resulting from this change.
> 
> Signed-off-by: Rafael J. Wysocki 

Reviewed-by: Frederic Weisbecker 

Thanks! And sorry for the slow reviews, the changes are sensitive
and I want to make sure we are not breaking some subtlety.

Re: [PATCH] selftests/livepatch: introduce tests

[Josh Poimboeuf]

On Wed, Mar 28, 2018 at 03:49:48PM -0400, Joe Lawrence wrote:
> Add a few livepatch modules and simple target modules that the included
> regression suite can run tests against.
> 
> Signed-off-by: Joe Lawrence 
> ---
>  lib/Kconfig.debug  |  12 +
>  lib/Makefile   |   2 +
>  lib/livepatch/Makefile |  18 +
>  lib/livepatch/test_klp_atomic_replace.c|  69 +++
>  lib/livepatch/test_klp_callbacks_busy.c|  43 ++
>  lib/livepatch/test_klp_callbacks_demo.c| 132 ++
>  lib/livepatch/test_klp_callbacks_demo2.c   | 104 
>  lib/livepatch/test_klp_callbacks_mod.c |  24 +
>  lib/livepatch/test_klp_livepatch.c |  62 +++
>  tools/testing/selftests/Makefile   |   1 +
>  tools/testing/selftests/livepatch/Makefile |   8 +
>  tools/testing/selftests/livepatch/config   |   1 +
>  tools/testing/selftests/livepatch/functions.sh | 202 
>  .../testing/selftests/livepatch/test-callbacks.sh  | 526 
> +
>  .../testing/selftests/livepatch/test-livepatch.sh  | 177 +++
>  .../selftests/livepatch/test-shadow-vars.sh|  13 +
>  16 files changed, 1394 insertions(+)
>  create mode 100644 lib/livepatch/Makefile
>  create mode 100644 lib/livepatch/test_klp_atomic_replace.c
>  create mode 100644 lib/livepatch/test_klp_callbacks_busy.c
>  create mode 100644 lib/livepatch/test_klp_callbacks_demo.c
>  create mode 100644 lib/livepatch/test_klp_callbacks_demo2.c
>  create mode 100644 lib/livepatch/test_klp_callbacks_mod.c
>  create mode 100644 lib/livepatch/test_klp_livepatch.c
>  create mode 100644 tools/testing/selftests/livepatch/Makefile
>  create mode 100644 tools/testing/selftests/livepatch/config
>  create mode 100644 tools/testing/selftests/livepatch/functions.sh
>  create mode 100755 tools/testing/selftests/livepatch/test-callbacks.sh
>  create mode 100755 tools/testing/selftests/livepatch/test-livepatch.sh
>  create mode 100755 tools/testing/selftests/livepatch/test-shadow-vars.sh

I love this.  Nice work!

As you and Petr discussed, it would be nice to get rid of some of the
delays, and also the callback tests will be very important.

-- 
Josh

Re: [PATCH v9 06/10] time: tick-sched: Split tick_nohz_stop_sched_tick()

[Frederic Weisbecker]

On Wed, Apr 04, 2018 at 10:41:13AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> In order to address the issue with short idle duration predictions
> by the idle governor after the scheduler tick has been stopped, split
> tick_nohz_stop_sched_tick() into two separate routines, one computing
> the time to the next timer event and the other simply stopping the
> tick when the time to the next timer event is known.
> 
> Prepare these two routines to be called separately, as one of them
> will be called by the idle governor in the cpuidle_select() code
> path after subsequent changes.
> 
> Update the former callers of tick_nohz_stop_sched_tick() to use
> the new routines, tick_nohz_next_event() and tick_nohz_stop_tick(),
> instead of it and move the updates of the sleep_length field in
> struct tick_sched into __tick_nohz_idle_stop_tick() as it doesn't
> need to be updated anywhere else.
> 
> There should be no intentional visible changes in functionality
> resulting from this change.
> 
> Signed-off-by: Rafael J. Wysocki 

Reviewed-by: Frederic Weisbecker 

Thanks! And sorry for the slow reviews, the changes are sensitive
and I want to make sure we are not breaking some subtlety.

Re: 4.15.14 crash with iscsi target and dvd

[Bart Van Assche]

On Fri, 2018-04-06 at 21:03 -0400, Wakko Warner wrote:
> Bart Van Assche wrote:
> > On Thu, 2018-04-05 at 22:06 -0400, Wakko Warner wrote:
> > > I know now why scsi_print_command isn't doing anything.  cmd->cmnd is 
> > > null.
> > > I added a dev_printk in scsi_print_command where the 2 if statements 
> > > return.
> > > Logs:
> > > [  29.866415] sr 3:0:0:0: cmd->cmnd is NULL
> > 
> > That's something that should never happen. As one can see in
> > scsi_setup_scsi_cmnd() and scsi_setup_fs_cmnd() both functions initialize
> > that pointer. Since I have not yet been able to reproduce myself what you
> > reported, would it be possible for you to bisect this issue? You will need
> > to follow something like the following procedure (see also
> > https://git-scm.com/docs/git-bisect):
> > 
> > git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > git bisect start
> > git bisect bad v4.10
> > git bisect good v4.9
> > 
> > and then build the kernel, install it, boot the kernel and test it.
> > Depending on the result, run either git bisect bad or git bisect good and
> > keep going until git bisect comes to a conclusion. This can take an hour or
> > more.
> 
> I have 1 question.  Should make clean be done between tests?  My box
> compiles the whole kernel in 2 minutes.

If you trust that the build system will figure out all dependencies then
running make clean is not necessary. Personally I always run make clean
during a bisect before rebuilding the kernel because if a header file has
changed in e.g. the block layer a huge number of files has to be rebuilt
anyway.

Bart.

Re: 4.15.14 crash with iscsi target and dvd

[Bart Van Assche]

On Fri, 2018-04-06 at 21:03 -0400, Wakko Warner wrote:
> Bart Van Assche wrote:
> > On Thu, 2018-04-05 at 22:06 -0400, Wakko Warner wrote:
> > > I know now why scsi_print_command isn't doing anything.  cmd->cmnd is 
> > > null.
> > > I added a dev_printk in scsi_print_command where the 2 if statements 
> > > return.
> > > Logs:
> > > [  29.866415] sr 3:0:0:0: cmd->cmnd is NULL
> > 
> > That's something that should never happen. As one can see in
> > scsi_setup_scsi_cmnd() and scsi_setup_fs_cmnd() both functions initialize
> > that pointer. Since I have not yet been able to reproduce myself what you
> > reported, would it be possible for you to bisect this issue? You will need
> > to follow something like the following procedure (see also
> > https://git-scm.com/docs/git-bisect):
> > 
> > git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > git bisect start
> > git bisect bad v4.10
> > git bisect good v4.9
> > 
> > and then build the kernel, install it, boot the kernel and test it.
> > Depending on the result, run either git bisect bad or git bisect good and
> > keep going until git bisect comes to a conclusion. This can take an hour or
> > more.
> 
> I have 1 question.  Should make clean be done between tests?  My box
> compiles the whole kernel in 2 minutes.

If you trust that the build system will figure out all dependencies then
running make clean is not necessary. Personally I always run make clean
during a bisect before rebuilding the kernel because if a header file has
changed in e.g. the block layer a huge number of files has to be rebuilt
anyway.

Bart.

[RFC linux v2] init: make all setup_arch() output string to boot_command_line[]

[yuan linyu]

From: yuan linyu 

then all arch boot parameter handled in the same way in start_kernel()

Signed-off-by: yuan linyu 
---
v2:
fix kbuild issue of parisc

 arch/alpha/kernel/setup.c  |  4 +---
 arch/arc/kernel/setup.c|  5 +
 arch/arm/kernel/setup.c|  7 +--
 arch/arm64/kernel/setup.c  |  4 +---
 arch/c6x/kernel/setup.c|  5 +
 arch/h8300/kernel/setup.c  |  3 +--
 arch/hexagon/kernel/setup.c| 16 +---
 arch/ia64/kernel/setup.c   | 15 ---
 arch/m68k/kernel/setup_mm.c|  5 ++---
 arch/m68k/kernel/setup_no.c|  3 +--
 arch/microblaze/kernel/setup.c |  4 +---
 arch/mips/kernel/setup.c   | 11 +++
 arch/nds32/kernel/setup.c  |  3 +--
 arch/nios2/kernel/setup.c  |  5 +
 arch/openrisc/kernel/setup.c   |  4 +---
 arch/parisc/kernel/setup.c | 11 +++
 arch/powerpc/kernel/setup-common.c |  4 +---
 arch/riscv/kernel/setup.c  |  4 +---
 arch/s390/kernel/setup.c   |  6 +-
 arch/sh/kernel/setup.c |  7 ---
 arch/sparc/kernel/setup_32.c   |  9 +
 arch/sparc/kernel/setup_64.c   |  8 
 arch/um/kernel/um_arch.c   |  3 +--
 arch/unicore32/kernel/setup.c  |  8 +---
 arch/x86/kernel/setup.c|  6 +-
 arch/xtensa/kernel/setup.c |  9 +
 include/linux/init.h   |  2 +-
 init/main.c| 14 +++---
 28 files changed, 60 insertions(+), 125 deletions(-)

diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
index 5576f7646fb6..c74675cf7129 100644
--- a/arch/alpha/kernel/setup.c
+++ b/arch/alpha/kernel/setup.c
@@ -505,8 +505,7 @@ register_cpus(void)
 
 arch_initcall(register_cpus);
 
-void __init
-setup_arch(char **cmdline_p)
+void __init setup_arch(void)
 {
extern char _end[];
 
@@ -566,7 +565,6 @@ setup_arch(char **cmdline_p)
strlcpy(command_line, COMMAND_LINE, sizeof command_line);
}
strcpy(boot_command_line, command_line);
-   *cmdline_p = command_line;
 
/* 
 * Process command-line arguments.
diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
index b2cae79a25d7..9cfdcf42bf28 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -456,7 +456,7 @@ static inline int is_kernel(unsigned long addr)
return 0;
 }
 
-void __init setup_arch(char **cmdline_p)
+void __init setup_arch(void)
 {
 #ifdef CONFIG_ARC_UBOOT_SUPPORT
/* make sure that uboot passed pointer to cmdline/dtb is valid */
@@ -487,9 +487,6 @@ void __init setup_arch(char **cmdline_p)
}
}
 
-   /* Save unparsed command line copy for /proc/cmdline */
-   *cmdline_p = boot_command_line;
-
/* To force early parsing of things like mem=xxx */
parse_early_param();
 
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index fc40a2b40595..1025e3a37689 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -153,7 +153,6 @@ EXPORT_SYMBOL(elf_platform);
 
 static const char *cpu_name;
 static const char *machine_name;
-static char __initdata cmd_line[COMMAND_LINE_SIZE];
 const struct machine_desc *machine_desc __initdata;
 
 static union { char c[4]; unsigned long l; } endian_test __initdata = { { 'l', 
'?', '?', 'b' } };
@@ -1061,7 +1060,7 @@ void __init hyp_mode_check(void)
 #endif
 }
 
-void __init setup_arch(char **cmdline_p)
+void __init setup_arch(void)
 {
const struct machine_desc *mdesc;
 
@@ -1091,10 +1090,6 @@ void __init setup_arch(char **cmdline_p)
init_mm.end_data   = (unsigned long) _edata;
init_mm.brk= (unsigned long) _end;
 
-   /* populate cmd_line too for later use, preserving boot_command_line */
-   strlcpy(cmd_line, boot_command_line, COMMAND_LINE_SIZE);
-   *cmdline_p = cmd_line;
-
early_fixmap_init();
early_ioremap_init();
 
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 30ad2f085d1f..c7ba4d32e7f7 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -243,15 +243,13 @@ static void __init request_standard_resources(void)
 
 u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID };
 
-void __init setup_arch(char **cmdline_p)
+void __init setup_arch(void)
 {
init_mm.start_code = (unsigned long) _text;
init_mm.end_code   = (unsigned long) _etext;
init_mm.end_data   = (unsigned long) _edata;
init_mm.brk= (unsigned long) _end;
 
-   *cmdline_p = boot_command_line;
-
early_fixmap_init();
early_ioremap_init();
 
diff --git a/arch/c6x/kernel/setup.c b/arch/c6x/kernel/setup.c
index 786e36e2f61d..012c8e746889 100644
--- a/arch/c6x/kernel/setup.c
+++ b/arch/c6x/kernel/setup.c
@@ -294,16 +294,13 @@ notrace void __init 

[RFC linux v2] init: make all setup_arch() output string to boot_command_line[]

[yuan linyu]

From: yuan linyu 

then all arch boot parameter handled in the same way in start_kernel()

Signed-off-by: yuan linyu 
---
v2:
fix kbuild issue of parisc

 arch/alpha/kernel/setup.c  |  4 +---
 arch/arc/kernel/setup.c|  5 +
 arch/arm/kernel/setup.c|  7 +--
 arch/arm64/kernel/setup.c  |  4 +---
 arch/c6x/kernel/setup.c|  5 +
 arch/h8300/kernel/setup.c  |  3 +--
 arch/hexagon/kernel/setup.c| 16 +---
 arch/ia64/kernel/setup.c   | 15 ---
 arch/m68k/kernel/setup_mm.c|  5 ++---
 arch/m68k/kernel/setup_no.c|  3 +--
 arch/microblaze/kernel/setup.c |  4 +---
 arch/mips/kernel/setup.c   | 11 +++
 arch/nds32/kernel/setup.c  |  3 +--
 arch/nios2/kernel/setup.c  |  5 +
 arch/openrisc/kernel/setup.c   |  4 +---
 arch/parisc/kernel/setup.c | 11 +++
 arch/powerpc/kernel/setup-common.c |  4 +---
 arch/riscv/kernel/setup.c  |  4 +---
 arch/s390/kernel/setup.c   |  6 +-
 arch/sh/kernel/setup.c |  7 ---
 arch/sparc/kernel/setup_32.c   |  9 +
 arch/sparc/kernel/setup_64.c   |  8 
 arch/um/kernel/um_arch.c   |  3 +--
 arch/unicore32/kernel/setup.c  |  8 +---
 arch/x86/kernel/setup.c|  6 +-
 arch/xtensa/kernel/setup.c |  9 +
 include/linux/init.h   |  2 +-
 init/main.c| 14 +++---
 28 files changed, 60 insertions(+), 125 deletions(-)

diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
index 5576f7646fb6..c74675cf7129 100644
--- a/arch/alpha/kernel/setup.c
+++ b/arch/alpha/kernel/setup.c
@@ -505,8 +505,7 @@ register_cpus(void)
 
 arch_initcall(register_cpus);
 
-void __init
-setup_arch(char **cmdline_p)
+void __init setup_arch(void)
 {
extern char _end[];
 
@@ -566,7 +565,6 @@ setup_arch(char **cmdline_p)
strlcpy(command_line, COMMAND_LINE, sizeof command_line);
}
strcpy(boot_command_line, command_line);
-   *cmdline_p = command_line;
 
/* 
 * Process command-line arguments.
diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
index b2cae79a25d7..9cfdcf42bf28 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -456,7 +456,7 @@ static inline int is_kernel(unsigned long addr)
return 0;
 }
 
-void __init setup_arch(char **cmdline_p)
+void __init setup_arch(void)
 {
 #ifdef CONFIG_ARC_UBOOT_SUPPORT
/* make sure that uboot passed pointer to cmdline/dtb is valid */
@@ -487,9 +487,6 @@ void __init setup_arch(char **cmdline_p)
}
}
 
-   /* Save unparsed command line copy for /proc/cmdline */
-   *cmdline_p = boot_command_line;
-
/* To force early parsing of things like mem=xxx */
parse_early_param();
 
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index fc40a2b40595..1025e3a37689 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -153,7 +153,6 @@ EXPORT_SYMBOL(elf_platform);
 
 static const char *cpu_name;
 static const char *machine_name;
-static char __initdata cmd_line[COMMAND_LINE_SIZE];
 const struct machine_desc *machine_desc __initdata;
 
 static union { char c[4]; unsigned long l; } endian_test __initdata = { { 'l', 
'?', '?', 'b' } };
@@ -1061,7 +1060,7 @@ void __init hyp_mode_check(void)
 #endif
 }
 
-void __init setup_arch(char **cmdline_p)
+void __init setup_arch(void)
 {
const struct machine_desc *mdesc;
 
@@ -1091,10 +1090,6 @@ void __init setup_arch(char **cmdline_p)
init_mm.end_data   = (unsigned long) _edata;
init_mm.brk= (unsigned long) _end;
 
-   /* populate cmd_line too for later use, preserving boot_command_line */
-   strlcpy(cmd_line, boot_command_line, COMMAND_LINE_SIZE);
-   *cmdline_p = cmd_line;
-
early_fixmap_init();
early_ioremap_init();
 
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 30ad2f085d1f..c7ba4d32e7f7 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -243,15 +243,13 @@ static void __init request_standard_resources(void)
 
 u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID };
 
-void __init setup_arch(char **cmdline_p)
+void __init setup_arch(void)
 {
init_mm.start_code = (unsigned long) _text;
init_mm.end_code   = (unsigned long) _etext;
init_mm.end_data   = (unsigned long) _edata;
init_mm.brk= (unsigned long) _end;
 
-   *cmdline_p = boot_command_line;
-
early_fixmap_init();
early_ioremap_init();
 
diff --git a/arch/c6x/kernel/setup.c b/arch/c6x/kernel/setup.c
index 786e36e2f61d..012c8e746889 100644
--- a/arch/c6x/kernel/setup.c
+++ b/arch/c6x/kernel/setup.c
@@ -294,16 +294,13 @@ notrace void __init machine_init(unsigned long dt_ptr)
parse_early_param();
 }
 

Re: [PATCH v5 04/10] drivers: qcom: rpmh: add RPMH helper functions

[Stephen Boyd]

Quoting Lina Iyer (2018-04-05 09:18:28)
> diff --git a/include/soc/qcom/rpmh.h b/include/soc/qcom/rpmh.h
> new file mode 100644
> index ..95334d4c1ede
> --- /dev/null
> +++ b/include/soc/qcom/rpmh.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef __SOC_QCOM_RPMH_H__
> +#define __SOC_QCOM_RPMH_H__
> +
> +#include 
> +#include 
> +
> +struct rpmh_client;
> +
> +#if IS_ENABLED(CONFIG_QCOM_RPMH)
> +int rpmh_write(struct rpmh_client *rc, enum rpmh_state state,
> +  const struct tcs_cmd *cmd, u32 n);
> +
> +struct rpmh_client *rpmh_get_client(struct platform_device *pdev);
> +
> +void rpmh_release(struct rpmh_client *rc);

Please get rid of this 'client' layer and fold it into the rpmh driver.
Everything that uses the rpmh_client is a child device of the rpmh
device so they should be able to just pass in their device pointer as
their 'handle' and have the rpmh driver take that, get the parent device
pointer, and pull an rpmh_drv structure out of there. The 'common' code
can go into the base rpmh driver and get used from there and then we
don't have to hop between two files to see how rpmh is used by the
consumers. Code complexity goes down this way.

Re: [PATCH v5 04/10] drivers: qcom: rpmh: add RPMH helper functions

[Stephen Boyd]

Quoting Lina Iyer (2018-04-05 09:18:28)
> diff --git a/include/soc/qcom/rpmh.h b/include/soc/qcom/rpmh.h
> new file mode 100644
> index ..95334d4c1ede
> --- /dev/null
> +++ b/include/soc/qcom/rpmh.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef __SOC_QCOM_RPMH_H__
> +#define __SOC_QCOM_RPMH_H__
> +
> +#include 
> +#include 
> +
> +struct rpmh_client;
> +
> +#if IS_ENABLED(CONFIG_QCOM_RPMH)
> +int rpmh_write(struct rpmh_client *rc, enum rpmh_state state,
> +  const struct tcs_cmd *cmd, u32 n);
> +
> +struct rpmh_client *rpmh_get_client(struct platform_device *pdev);
> +
> +void rpmh_release(struct rpmh_client *rc);

Please get rid of this 'client' layer and fold it into the rpmh driver.
Everything that uses the rpmh_client is a child device of the rpmh
device so they should be able to just pass in their device pointer as
their 'handle' and have the rpmh driver take that, get the parent device
pointer, and pull an rpmh_drv structure out of there. The 'common' code
can go into the base rpmh driver and get used from there and then we
don't have to hop between two files to see how rpmh is used by the
consumers. Code complexity goes down this way.

Re: [PATCH v4 4/9] vsprintf: Consolidate handling of unknown pointer specifiers

[Sergey Senozhatsky]

On (04/06/18 18:00), Joe Perches wrote:
[..]
> This finds the current two bad uses in addition to
> the existing similar message for string concatenation
> without a space char between concatenated fragments.
> 
> For example:
> 
> WARNING: break quoted strings at a space character
> #3550: FILE: drivers/scsi/megaraid/megaraid_sas_base.c:3550:
> + dev_notice(>pdev->dev, "moving 
> cmd[%d]:%p:%d:%p"
> + "on the defer queue as internal\n",
> 
> WARNING: vsprintf %p string concatenation
> #3550: FILE: drivers/scsi/megaraid/megaraid_sas_base.c:3550:
> + dev_notice(>pdev->dev, "moving 
> cmd[%d]:%p:%d:%p"
> + "on the defer queue as internal\n",
> 
> I think the new message is not that useful really as the
> existing warning is probably enough.

Oh, so we already have it... Didn't know that. Yes, I think the existing
one is good enough. Thanks for the pointers.

-ss

Re: [PATCH v4 4/9] vsprintf: Consolidate handling of unknown pointer specifiers

[Sergey Senozhatsky]

On (04/06/18 18:00), Joe Perches wrote:
[..]
> This finds the current two bad uses in addition to
> the existing similar message for string concatenation
> without a space char between concatenated fragments.
> 
> For example:
> 
> WARNING: break quoted strings at a space character
> #3550: FILE: drivers/scsi/megaraid/megaraid_sas_base.c:3550:
> + dev_notice(>pdev->dev, "moving 
> cmd[%d]:%p:%d:%p"
> + "on the defer queue as internal\n",
> 
> WARNING: vsprintf %p string concatenation
> #3550: FILE: drivers/scsi/megaraid/megaraid_sas_base.c:3550:
> + dev_notice(>pdev->dev, "moving 
> cmd[%d]:%p:%d:%p"
> + "on the defer queue as internal\n",
> 
> I think the new message is not that useful really as the
> existing warning is probably enough.

Oh, so we already have it... Didn't know that. Yes, I think the existing
one is good enough. Thanks for the pointers.

-ss

Re: [PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs

[Stephen Boyd]

Quoting Lina Iyer (2018-04-05 09:18:26)
> diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt 
> b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> new file mode 100644
> index ..dcf71a5b302f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> @@ -0,0 +1,127 @@
> +RPMH RSC:
> +
> +
> +Resource Power Manager Hardened (RPMH) is the mechanism for communicating 
> with
> +the hardened resource accelerators on Qualcomm SoCs. Requests to the 
> resources
> +can be written to the Trigger Command Set (TCS)  registers and using a (addr,
> +val) pair and triggered. Messages in the TCS are then sent in sequence over 
> an
> +internal bus.
> +
> +The hardware block (Direct Resource Voter or DRV) is a part of the h/w entity
> +(Resource State Coordinator a.k.a RSC) that can handle a multiple sleep and

s/ a / /

> +active/wake resource requests. Multiple such DRVs can exist in a SoC and can
> +be written to from Linux. The structure of each DRV follows the same template
> +with a few variations that are captured by the properties here.
> +
> +A TCS may be triggered from Linux or triggered by the F/W after all the CPUs
> +have powered off to facilitate idle power saving. TCS could be classified as 
> -

s/ -/:/

> +
> +   SLEEP,  /* Triggered by F/W */
> +   WAKE,   /* Triggered by F/W */
> +   ACTIVE, /* Triggered by Linux */
> +   CONTROL /* Triggered by F/W */

Drop the commas?

> +
> +The order in which they are described in the DT, should match the hardware
> +configuration.
> +
> +Requests can be made for the state of a resource, when the subsystem is 
> active
> +or idle. When all subsystems like Modem, GPU, CPU are idle, the resource 
> state
> +will be an aggregate of the sleep votes from each of those subsystems. 
> Clients
> +may request a sleep value for their shared resources in addition to the 
> active
> +mode requests.
> +
> +Properties:
> +
> +- compatible:
> +   Usage: required
> +   Value type: 
> +   Definition: Should be "qcom,rpmh-rsc".
> +
> +- reg:
> +   Usage: required
> +   Value type: 
> +   Definition: The first register specifies the base address of the DRV.
> +   The second register specifies the start address of the
> +   TCS.
> +
> +- reg-names:
> +   Usage: required
> +   Value type: 
> +   Definition: Maps the register specified in the reg property. Must be
> +   "drv" and "tcs".
> +
> +- interrupts:
> +   Usage: required
> +   Value type: 
> +   Definition: The interrupt that trips when a message complete/response
> +  is received for this DRV from the accelerators.
> +
> +- qcom,drv-id:
> +   Usage: required
> +   Value type: 
> +   Definition: the id of the DRV in the RSC block.
> +
> +- qcom,tcs-config:
> +   Usage: required
> +   Value type: 
> +   Definition: the tuple defining the configuration of TCS.
> +   Must have 2 cells which describe each TCS type.
> +   .
> +   The order of the TCS must match the hardware
> +   configuration.
> +   - Cell #1 (TCS Type): TCS types to be specified -
> +   SLEEP_TCS
> +   WAKE_TCS
> +   ACTIVE_TCS
> +   CONTROL_TCS
> +   - Cell #2 (Number of TCS): 
> +
> +- label:
> +   Usage: optional
> +   Value type: 
> +   Definition: Name for the RSC. The name would be used in trace logs.
> +
> +Drivers that want to use the RSC to communicate with RPMH must specify their
> +bindings as child of the RSC controllers they wish to communicate with.

s/child/child nodes/


> +
> +Example 1:
> +
> +For a TCS whose RSC base address is is 0x179C and is at a DRV id of 2, 
> the
> +register offsets for DRV2 start at 0D00, the register calculations are like
> +this -
> +First tuple: 0x179C + 0x1 * 2 = 0x179E
> +Second tuple: 0x179E + 0xD00 = 0x179E0D00
> +
> +   apps_rsc: rsc@179e000 {
> +   label = "apps_rsc";
> +   compatible = "qcom,rpmh-rsc";
> +   reg = <0x179e 0x1>, <0x179e0d00 0x3000>;

The first reg property overlaps the second one. Does this second one
ever move around? I would hardcode it in the driver to be 0xd00 away
from the drv base instead of specifying it in DT if it's the same all
the time.

Also, the example shows 0x179c which I guess is the actual beginning
of the RSC block. So the binding seems to be for one DRV inside of an
RSC. Can we get the full description of the RSC in the binding instead?
I imagine that means there's a DRV0,1,2 and those probably have an
interrupt per each DRV and then a different TCS config per each one too?
If the binding can describe all of the RSC then we can use different
DRVs by changing the qcom,drv-id property.

rsc@179c {
compatible = 

Re: [PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs

[Stephen Boyd]

Quoting Lina Iyer (2018-04-05 09:18:26)
> diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt 
> b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> new file mode 100644
> index ..dcf71a5b302f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> @@ -0,0 +1,127 @@
> +RPMH RSC:
> +
> +
> +Resource Power Manager Hardened (RPMH) is the mechanism for communicating 
> with
> +the hardened resource accelerators on Qualcomm SoCs. Requests to the 
> resources
> +can be written to the Trigger Command Set (TCS)  registers and using a (addr,
> +val) pair and triggered. Messages in the TCS are then sent in sequence over 
> an
> +internal bus.
> +
> +The hardware block (Direct Resource Voter or DRV) is a part of the h/w entity
> +(Resource State Coordinator a.k.a RSC) that can handle a multiple sleep and

s/ a / /

> +active/wake resource requests. Multiple such DRVs can exist in a SoC and can
> +be written to from Linux. The structure of each DRV follows the same template
> +with a few variations that are captured by the properties here.
> +
> +A TCS may be triggered from Linux or triggered by the F/W after all the CPUs
> +have powered off to facilitate idle power saving. TCS could be classified as 
> -

s/ -/:/

> +
> +   SLEEP,  /* Triggered by F/W */
> +   WAKE,   /* Triggered by F/W */
> +   ACTIVE, /* Triggered by Linux */
> +   CONTROL /* Triggered by F/W */

Drop the commas?

> +
> +The order in which they are described in the DT, should match the hardware
> +configuration.
> +
> +Requests can be made for the state of a resource, when the subsystem is 
> active
> +or idle. When all subsystems like Modem, GPU, CPU are idle, the resource 
> state
> +will be an aggregate of the sleep votes from each of those subsystems. 
> Clients
> +may request a sleep value for their shared resources in addition to the 
> active
> +mode requests.
> +
> +Properties:
> +
> +- compatible:
> +   Usage: required
> +   Value type: 
> +   Definition: Should be "qcom,rpmh-rsc".
> +
> +- reg:
> +   Usage: required
> +   Value type: 
> +   Definition: The first register specifies the base address of the DRV.
> +   The second register specifies the start address of the
> +   TCS.
> +
> +- reg-names:
> +   Usage: required
> +   Value type: 
> +   Definition: Maps the register specified in the reg property. Must be
> +   "drv" and "tcs".
> +
> +- interrupts:
> +   Usage: required
> +   Value type: 
> +   Definition: The interrupt that trips when a message complete/response
> +  is received for this DRV from the accelerators.
> +
> +- qcom,drv-id:
> +   Usage: required
> +   Value type: 
> +   Definition: the id of the DRV in the RSC block.
> +
> +- qcom,tcs-config:
> +   Usage: required
> +   Value type: 
> +   Definition: the tuple defining the configuration of TCS.
> +   Must have 2 cells which describe each TCS type.
> +   .
> +   The order of the TCS must match the hardware
> +   configuration.
> +   - Cell #1 (TCS Type): TCS types to be specified -
> +   SLEEP_TCS
> +   WAKE_TCS
> +   ACTIVE_TCS
> +   CONTROL_TCS
> +   - Cell #2 (Number of TCS): 
> +
> +- label:
> +   Usage: optional
> +   Value type: 
> +   Definition: Name for the RSC. The name would be used in trace logs.
> +
> +Drivers that want to use the RSC to communicate with RPMH must specify their
> +bindings as child of the RSC controllers they wish to communicate with.

s/child/child nodes/


> +
> +Example 1:
> +
> +For a TCS whose RSC base address is is 0x179C and is at a DRV id of 2, 
> the
> +register offsets for DRV2 start at 0D00, the register calculations are like
> +this -
> +First tuple: 0x179C + 0x1 * 2 = 0x179E
> +Second tuple: 0x179E + 0xD00 = 0x179E0D00
> +
> +   apps_rsc: rsc@179e000 {
> +   label = "apps_rsc";
> +   compatible = "qcom,rpmh-rsc";
> +   reg = <0x179e 0x1>, <0x179e0d00 0x3000>;

The first reg property overlaps the second one. Does this second one
ever move around? I would hardcode it in the driver to be 0xd00 away
from the drv base instead of specifying it in DT if it's the same all
the time.

Also, the example shows 0x179c which I guess is the actual beginning
of the RSC block. So the binding seems to be for one DRV inside of an
RSC. Can we get the full description of the RSC in the binding instead?
I imagine that means there's a DRV0,1,2 and those probably have an
interrupt per each DRV and then a different TCS config per each one too?
If the binding can describe all of the RSC then we can use different
DRVs by changing the qcom,drv-id property.

rsc@179c {
compatible = 

[PATCH v5] drm/i915/dp: Send DPCD ON for MST before phy_up

[Lyude Paul]

When doing a modeset where the sink is transitioning from D3 to D0 , it
would sometimes be possible for the initial power_up_phy() to start
timing out. This would only be observed in the last action before the
sink went into D3 mode was intel_dp_sink_dpms(DRM_MODE_DPMS_OFF). We
originally thought this might be an issue with us accidentally shutting
off the aux block when putting the sink into D3, but since the DP spec
mandates that sinks must wake up within 1ms while we have 100ms to
respond to an ESI irq, this didn't really add up. Turns out that the
problem is more subtle then that:

It turns out that the timeout is from us not enabling DPMS on the MST
hub before actually trying to initiate sideband communications. This
would cause the first sideband communication (power_up_phy()), to start
timing out because the sink wasn't ready to respond. Afterwards, we
would call intel_dp_sink_dpms(DRM_MODE_DPMS_ON) in
intel_ddi_pre_enable_dp(), which would actually result in waking up the
sink so that sideband requests would work again.

Since DPMS is what lets us actually bring the hub up into a state where
sideband communications become functional again, we just need to make
sure to enable DPMS on the display before attempting to perform sideband
communications.

Changes since v1:
- Remove comment above if (!intel_dp->is_mst) - vsryjala
- Move intel_dp_sink_dpms() for MST into intel_dp_post_disable_mst() to
  keep enable/disable paths symmetrical
- Improve commit message - dhnkrn
Changes since v2:
- Only send DPMS off when we're disabling the last sink, and only send
  DPMS on when we're enabling the first sink - dhnkrn
Changes since v3:
- Check against is_mst, not intel_dp->is_mst - dhnkrn/vsyrjala

Signed-off-by: Lyude Paul 
Reviewed-by: Dhinakaran Pandiyan 
Reviewed-by: Ville Syrjälä 
Tested-by: Laura Abbott 
Cc: sta...@vger.kernel.org
Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.")
---
No actual changes other than t-b and r-bs. Resending because I don't
have access to the "test latest revision again" button and I'm very much
sure these CI results are bogus.

 drivers/gpu/drm/i915/intel_ddi.c| 8 ++--
 drivers/gpu/drm/i915/intel_dp_mst.c | 8 +++-
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index a6672a9abd85..92cb26b18a9b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2324,7 +2324,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder 
*encoder,
intel_prepare_dp_ddi_buffers(encoder, crtc_state);
 
intel_ddi_init_dp_buf_reg(encoder);
-   intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
+   if (!is_mst)
+   intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
intel_dp_start_link_train(intel_dp);
if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
intel_dp_stop_link_train(intel_dp);
@@ -2422,12 +2423,15 @@ static void intel_ddi_post_disable_dp(struct 
intel_encoder *encoder,
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
struct intel_digital_port *dig_port = enc_to_dig_port(>base);
struct intel_dp *intel_dp = _port->dp;
+   bool is_mst = intel_crtc_has_type(old_crtc_state,
+ INTEL_OUTPUT_DP_MST);
 
/*
 * Power down sink before disabling the port, otherwise we end
 * up getting interrupts from the sink on detecting link loss.
 */
-   intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
+   if (!is_mst)
+   intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 
intel_disable_ddi_buf(encoder);
 
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
b/drivers/gpu/drm/i915/intel_dp_mst.c
index c3de0918ee13..9e6956c08688 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -180,9 +180,11 @@ static void intel_mst_post_disable_dp(struct intel_encoder 
*encoder,
intel_dp->active_mst_links--;
 
intel_mst->connector = NULL;
-   if (intel_dp->active_mst_links == 0)
+   if (intel_dp->active_mst_links == 0) {
+   intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
intel_dig_port->base.post_disable(_dig_port->base,
  old_crtc_state, NULL);
+   }
 
DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
 }
@@ -223,7 +225,11 @@ static void intel_mst_pre_enable_dp(struct intel_encoder 
*encoder,
 
DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
 
+   if (intel_dp->active_mst_links == 0)
+   intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
+
drm_dp_send_power_updown_phy(_dp->mst_mgr, connector->port, true);
+
if (intel_dp->active_mst_links == 0)
 

[PATCH v5] drm/i915/dp: Send DPCD ON for MST before phy_up

[Lyude Paul]

When doing a modeset where the sink is transitioning from D3 to D0 , it
would sometimes be possible for the initial power_up_phy() to start
timing out. This would only be observed in the last action before the
sink went into D3 mode was intel_dp_sink_dpms(DRM_MODE_DPMS_OFF). We
originally thought this might be an issue with us accidentally shutting
off the aux block when putting the sink into D3, but since the DP spec
mandates that sinks must wake up within 1ms while we have 100ms to
respond to an ESI irq, this didn't really add up. Turns out that the
problem is more subtle then that:

It turns out that the timeout is from us not enabling DPMS on the MST
hub before actually trying to initiate sideband communications. This
would cause the first sideband communication (power_up_phy()), to start
timing out because the sink wasn't ready to respond. Afterwards, we
would call intel_dp_sink_dpms(DRM_MODE_DPMS_ON) in
intel_ddi_pre_enable_dp(), which would actually result in waking up the
sink so that sideband requests would work again.

Since DPMS is what lets us actually bring the hub up into a state where
sideband communications become functional again, we just need to make
sure to enable DPMS on the display before attempting to perform sideband
communications.

Changes since v1:
- Remove comment above if (!intel_dp->is_mst) - vsryjala
- Move intel_dp_sink_dpms() for MST into intel_dp_post_disable_mst() to
  keep enable/disable paths symmetrical
- Improve commit message - dhnkrn
Changes since v2:
- Only send DPMS off when we're disabling the last sink, and only send
  DPMS on when we're enabling the first sink - dhnkrn
Changes since v3:
- Check against is_mst, not intel_dp->is_mst - dhnkrn/vsyrjala

Signed-off-by: Lyude Paul 
Reviewed-by: Dhinakaran Pandiyan 
Reviewed-by: Ville Syrjälä 
Tested-by: Laura Abbott 
Cc: sta...@vger.kernel.org
Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.")
---
No actual changes other than t-b and r-bs. Resending because I don't
have access to the "test latest revision again" button and I'm very much
sure these CI results are bogus.

 drivers/gpu/drm/i915/intel_ddi.c| 8 ++--
 drivers/gpu/drm/i915/intel_dp_mst.c | 8 +++-
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index a6672a9abd85..92cb26b18a9b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2324,7 +2324,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder 
*encoder,
intel_prepare_dp_ddi_buffers(encoder, crtc_state);
 
intel_ddi_init_dp_buf_reg(encoder);
-   intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
+   if (!is_mst)
+   intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
intel_dp_start_link_train(intel_dp);
if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
intel_dp_stop_link_train(intel_dp);
@@ -2422,12 +2423,15 @@ static void intel_ddi_post_disable_dp(struct 
intel_encoder *encoder,
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
struct intel_digital_port *dig_port = enc_to_dig_port(>base);
struct intel_dp *intel_dp = _port->dp;
+   bool is_mst = intel_crtc_has_type(old_crtc_state,
+ INTEL_OUTPUT_DP_MST);
 
/*
 * Power down sink before disabling the port, otherwise we end
 * up getting interrupts from the sink on detecting link loss.
 */
-   intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
+   if (!is_mst)
+   intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 
intel_disable_ddi_buf(encoder);
 
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
b/drivers/gpu/drm/i915/intel_dp_mst.c
index c3de0918ee13..9e6956c08688 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -180,9 +180,11 @@ static void intel_mst_post_disable_dp(struct intel_encoder 
*encoder,
intel_dp->active_mst_links--;
 
intel_mst->connector = NULL;
-   if (intel_dp->active_mst_links == 0)
+   if (intel_dp->active_mst_links == 0) {
+   intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
intel_dig_port->base.post_disable(_dig_port->base,
  old_crtc_state, NULL);
+   }
 
DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
 }
@@ -223,7 +225,11 @@ static void intel_mst_pre_enable_dp(struct intel_encoder 
*encoder,
 
DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
 
+   if (intel_dp->active_mst_links == 0)
+   intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
+
drm_dp_send_power_updown_phy(_dp->mst_mgr, connector->port, true);
+
if (intel_dp->active_mst_links == 0)
intel_dig_port->base.pre_enable(_dig_port->base,

Re: [PATCH 4/9] x86, pkeys: override pkey when moving away from PROT_EXEC

[Ram Pai]

On Fri, Apr 06, 2018 at 05:47:29PM -0700, Dave Hansen wrote:
> On 04/06/2018 05:09 PM, Ram Pai wrote:
> >> -  /*
> >> -   * Look for a protection-key-drive execute-only mapping
> >> -   * which is now being given permissions that are not
> >> -   * execute-only.  Move it back to the default pkey.
> >> -   */
> >> -  if (vma_is_pkey_exec_only(vma) &&
> >> -  (prot & (PROT_READ|PROT_WRITE))) {
> >> -  return 0;
> >> -  }
> >> +
> > Dave,
> > this can be simply:
> > 
> > if ((vma_is_pkey_exec_only(vma) && (prot != PROT_EXEC))
> > return ARCH_DEFAULT_PKEY;
> 
> Yes, but we're removing that code entirely. :)

Well :). my point is add this code and delete the other
code that you add later in that function.

RP



-- 
Ram Pai

Re: [PATCH 4/9] x86, pkeys: override pkey when moving away from PROT_EXEC

[Ram Pai]

On Fri, Apr 06, 2018 at 05:47:29PM -0700, Dave Hansen wrote:
> On 04/06/2018 05:09 PM, Ram Pai wrote:
> >> -  /*
> >> -   * Look for a protection-key-drive execute-only mapping
> >> -   * which is now being given permissions that are not
> >> -   * execute-only.  Move it back to the default pkey.
> >> -   */
> >> -  if (vma_is_pkey_exec_only(vma) &&
> >> -  (prot & (PROT_READ|PROT_WRITE))) {
> >> -  return 0;
> >> -  }
> >> +
> > Dave,
> > this can be simply:
> > 
> > if ((vma_is_pkey_exec_only(vma) && (prot != PROT_EXEC))
> > return ARCH_DEFAULT_PKEY;
> 
> Yes, but we're removing that code entirely. :)

Well :). my point is add this code and delete the other
code that you add later in that function.

RP



-- 
Ram Pai

Re: 4.15.14 crash with iscsi target and dvd

[Wakko Warner]

Bart Van Assche wrote:
> On Thu, 2018-04-05 at 22:06 -0400, Wakko Warner wrote:
> > I know now why scsi_print_command isn't doing anything.  cmd->cmnd is null.
> > I added a dev_printk in scsi_print_command where the 2 if statements return.
> > Logs:
> > [  29.866415] sr 3:0:0:0: cmd->cmnd is NULL
> 
> That's something that should never happen. As one can see in
> scsi_setup_scsi_cmnd() and scsi_setup_fs_cmnd() both functions initialize
> that pointer. Since I have not yet been able to reproduce myself what you
> reported, would it be possible for you to bisect this issue? You will need
> to follow something like the following procedure (see also
> https://git-scm.com/docs/git-bisect):
> 
> git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git bisect start
> git bisect bad v4.10
> git bisect good v4.9
> 
> and then build the kernel, install it, boot the kernel and test it.
> Depending on the result, run either git bisect bad or git bisect good and
> keep going until git bisect comes to a conclusion. This can take an hour or
> more.

I have 1 question.  Should make clean be done between tests?  My box
compiles the whole kernel in 2 minutes.

-- 
 Microsoft has beaten Volkswagen's world record.  Volkswagen only created 22
 million bugs.

Re: 4.15.14 crash with iscsi target and dvd

[Wakko Warner]

Bart Van Assche wrote:
> On Thu, 2018-04-05 at 22:06 -0400, Wakko Warner wrote:
> > I know now why scsi_print_command isn't doing anything.  cmd->cmnd is null.
> > I added a dev_printk in scsi_print_command where the 2 if statements return.
> > Logs:
> > [  29.866415] sr 3:0:0:0: cmd->cmnd is NULL
> 
> That's something that should never happen. As one can see in
> scsi_setup_scsi_cmnd() and scsi_setup_fs_cmnd() both functions initialize
> that pointer. Since I have not yet been able to reproduce myself what you
> reported, would it be possible for you to bisect this issue? You will need
> to follow something like the following procedure (see also
> https://git-scm.com/docs/git-bisect):
> 
> git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git bisect start
> git bisect bad v4.10
> git bisect good v4.9
> 
> and then build the kernel, install it, boot the kernel and test it.
> Depending on the result, run either git bisect bad or git bisect good and
> keep going until git bisect comes to a conclusion. This can take an hour or
> more.

I have 1 question.  Should make clean be done between tests?  My box
compiles the whole kernel in 2 minutes.

-- 
 Microsoft has beaten Volkswagen's world record.  Volkswagen only created 22
 million bugs.

Re: [PATCH v4 4/9] vsprintf: Consolidate handling of unknown pointer specifiers

[Joe Perches]

On Sat, 2018-04-07 at 09:33 +0900, Sergey Senozhatsky wrote:
> Hi Joe,
> 
> On (04/06/18 16:59), Joe Perches wrote:
> > > 
> > > Can we tweak checkpatch to catch such things?
> > 
> > Not really, no.
> > 
> > Adding regex logic for this is tricky at best
> > and probably not worth the effort because of
> > the various bits of patch contexts aren't
> > necessarily visible.
> 
> Agreed. I was more thinking about catching "... %p" and saying
> that we'd rather prefer either "... %p," or "... %p " or "... %p\n".
> Doesn't sound so complex, can probably catch something fishy one day
> (or may be not), and more or less is visible to checkpatch. Well,
> more or less...

This finds the current two bad uses in addition to
the existing similar message for string concatenation
without a space char between concatenated fragments.

For example:

WARNING: break quoted strings at a space character
#3550: FILE: drivers/scsi/megaraid/megaraid_sas_base.c:3550:
+   dev_notice(>pdev->dev, "moving 
cmd[%d]:%p:%d:%p"
+   "on the defer queue as internal\n",

WARNING: vsprintf %p string concatenation
#3550: FILE: drivers/scsi/megaraid/megaraid_sas_base.c:3550:
+   dev_notice(>pdev->dev, "moving 
cmd[%d]:%p:%d:%p"
+   "on the defer queue as internal\n",

I think the new message is not that useful really as the
existing warning is probably enough.

---
 scripts/checkpatch.pl | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index eb534d48140e..a0e43232431e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5313,6 +5313,12 @@ sub process {
 "break quoted strings at a space character\n" . 
$hereprev);
}
 
+# check for vsprintf pointer extension concatenation
+   if ($prevrawline =~ /\%p"\s*$/ && $rawline =~ /^\+\s*"\w/) {
+   WARN('POINTER_CONCATENATION',
+"vsprintf %p string concatenation\n" . 
$hereprev);
+   }
+
 # check for an embedded function name in a string when the function is known
 # This does not work very well for -f --file checking as it depends on patch
 # context providing the function name or a single line form for in-file

Re: [PATCH v4 4/9] vsprintf: Consolidate handling of unknown pointer specifiers

[Joe Perches]

On Sat, 2018-04-07 at 09:33 +0900, Sergey Senozhatsky wrote:
> Hi Joe,
> 
> On (04/06/18 16:59), Joe Perches wrote:
> > > 
> > > Can we tweak checkpatch to catch such things?
> > 
> > Not really, no.
> > 
> > Adding regex logic for this is tricky at best
> > and probably not worth the effort because of
> > the various bits of patch contexts aren't
> > necessarily visible.
> 
> Agreed. I was more thinking about catching "... %p" and saying
> that we'd rather prefer either "... %p," or "... %p " or "... %p\n".
> Doesn't sound so complex, can probably catch something fishy one day
> (or may be not), and more or less is visible to checkpatch. Well,
> more or less...

This finds the current two bad uses in addition to
the existing similar message for string concatenation
without a space char between concatenated fragments.

For example:

WARNING: break quoted strings at a space character
#3550: FILE: drivers/scsi/megaraid/megaraid_sas_base.c:3550:
+   dev_notice(>pdev->dev, "moving 
cmd[%d]:%p:%d:%p"
+   "on the defer queue as internal\n",

WARNING: vsprintf %p string concatenation
#3550: FILE: drivers/scsi/megaraid/megaraid_sas_base.c:3550:
+   dev_notice(>pdev->dev, "moving 
cmd[%d]:%p:%d:%p"
+   "on the defer queue as internal\n",

I think the new message is not that useful really as the
existing warning is probably enough.

---
 scripts/checkpatch.pl | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index eb534d48140e..a0e43232431e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5313,6 +5313,12 @@ sub process {
 "break quoted strings at a space character\n" . 
$hereprev);
}
 
+# check for vsprintf pointer extension concatenation
+   if ($prevrawline =~ /\%p"\s*$/ && $rawline =~ /^\+\s*"\w/) {
+   WARN('POINTER_CONCATENATION',
+"vsprintf %p string concatenation\n" . 
$hereprev);
+   }
+
 # check for an embedded function name in a string when the function is known
 # This does not work very well for -f --file checking as it depends on patch
 # context providing the function name or a single line form for in-file

Re: Differences between builtins and modules

[Randy Dunlap]

On 02/24/2015 05:02 PM, Lucas De Marchi wrote:
> On Mon, Feb 23, 2015 at 12:51 PM, Michal Marek  wrote:
>> On 2015-02-23 15:30, Lucas De Marchi wrote:
>>> This could be particularly bad if in a kernel version an option was
>>> tristate and in a new version it changed to boolean. I'm not sure if
>>> this is common to happen in kernel. Any code that did "modprobe
>>> " would start to fail.
>>
>> I think it's quite uncommon (*) and also the use case for loading
>> builtin modules is not that common. I can think of:
>> 1) building the initramfs, to determine which *.ko files need to be
>>copied to it. Since such tools are often updated for other reasons,
>>it's not a big deal.
>> 2) Hardcoded module names in things like softdep -- hopefully not that
>>common either, plus the kernel-provided soft dependencies can be
>>fixed together with the change.
>>
>> Until not so long ago, the kernel would return EINVAL if passed a
>> non-existent (renamed, removed) module option to init_module, yet there
>> were no attempts at preserving the module options for compatibility reasons.
>>
>> (*) I now did a quick search:
>> $ git log -p origin/master --no-merges -- '*/Kconfig*' | grep -C3 '^-
>> *tristate' | grep '^+ *bool'
>> +   bool "Intel P state control"
>> +   bool "Intel microcode patch loading support"
>> +   bool "AMD microcode patch loading support"
>> +bool "STI text console"
>> +   bool "Enable DDC2 Support"
>> +   bool "Enable Console Acceleration"
>>
>> That's only 6 cases in the whole git history. Maybe there are a few more
>> hidden outside the three-line context as part of larger edits, but I'm
>> sure more modules have been *removed* entirely from the kernel over this
>> period.
> 
> thanks for looking in detail into this.
> 
>>
>>
>>> My questions are:
>>> 1) should we put *all* the "modules" in the builtin index?
>>
>> You mean all *.o files that do not end up in some *.ko? That won't work,
>> because unlike module names, the names of object files are not global.
> 
> I was actually meaning anything that can have a directory under
> /sys/module/. I figure we can't easily know this.
> 
>> Plus, there was IIRC an idea to teach lsmod to print builtin modules --
>> listing all *.o would make it rather useless.
> 
> This was one of my ideas... to traverse /sys/module and give more
> information than we actually output right now, including builtin
> modules. However, given the fact that builtin modules only have an
> entry in /sys/module if they have params and now that I'm aware of the
> race between the creation of the directory and the initstate file, I'm
> giving up on this idea for now.
> 
>>> 2) should we actually check /sys/module/ to report a
>>> module as builtin or just stop doing that and rely solely in the
>>> index? Initially I'd like to do the opposite, but given the race in
>>> deciding this I'm favoring the index.
>>
>> If the race between the creation of /sys/module/ and
>> /sys/module//initstate is inevitable, then I'm afraid we
>> have to rely on the index.
> 
> So my current plan is to rely solely on modules.builtin to output to
> modprobe that a module is builtin. So things like "modprobe vt" will
> start to fail saying there's no vt module. Any objections here?
> 

Hi,
[sorry to resurrect such as old thread]


Would someone please answer/reply to this (related) kernel bugzilla entry:
  https://bugzilla.kernel.org/show_bug.cgi?id=118661

or I could just close it?

thanks,
-- 
~Randy

Re: Differences between builtins and modules

[Randy Dunlap]

On 02/24/2015 05:02 PM, Lucas De Marchi wrote:
> On Mon, Feb 23, 2015 at 12:51 PM, Michal Marek  wrote:
>> On 2015-02-23 15:30, Lucas De Marchi wrote:
>>> This could be particularly bad if in a kernel version an option was
>>> tristate and in a new version it changed to boolean. I'm not sure if
>>> this is common to happen in kernel. Any code that did "modprobe
>>> " would start to fail.
>>
>> I think it's quite uncommon (*) and also the use case for loading
>> builtin modules is not that common. I can think of:
>> 1) building the initramfs, to determine which *.ko files need to be
>>copied to it. Since such tools are often updated for other reasons,
>>it's not a big deal.
>> 2) Hardcoded module names in things like softdep -- hopefully not that
>>common either, plus the kernel-provided soft dependencies can be
>>fixed together with the change.
>>
>> Until not so long ago, the kernel would return EINVAL if passed a
>> non-existent (renamed, removed) module option to init_module, yet there
>> were no attempts at preserving the module options for compatibility reasons.
>>
>> (*) I now did a quick search:
>> $ git log -p origin/master --no-merges -- '*/Kconfig*' | grep -C3 '^-
>> *tristate' | grep '^+ *bool'
>> +   bool "Intel P state control"
>> +   bool "Intel microcode patch loading support"
>> +   bool "AMD microcode patch loading support"
>> +bool "STI text console"
>> +   bool "Enable DDC2 Support"
>> +   bool "Enable Console Acceleration"
>>
>> That's only 6 cases in the whole git history. Maybe there are a few more
>> hidden outside the three-line context as part of larger edits, but I'm
>> sure more modules have been *removed* entirely from the kernel over this
>> period.
> 
> thanks for looking in detail into this.
> 
>>
>>
>>> My questions are:
>>> 1) should we put *all* the "modules" in the builtin index?
>>
>> You mean all *.o files that do not end up in some *.ko? That won't work,
>> because unlike module names, the names of object files are not global.
> 
> I was actually meaning anything that can have a directory under
> /sys/module/. I figure we can't easily know this.
> 
>> Plus, there was IIRC an idea to teach lsmod to print builtin modules --
>> listing all *.o would make it rather useless.
> 
> This was one of my ideas... to traverse /sys/module and give more
> information than we actually output right now, including builtin
> modules. However, given the fact that builtin modules only have an
> entry in /sys/module if they have params and now that I'm aware of the
> race between the creation of the directory and the initstate file, I'm
> giving up on this idea for now.
> 
>>> 2) should we actually check /sys/module/ to report a
>>> module as builtin or just stop doing that and rely solely in the
>>> index? Initially I'd like to do the opposite, but given the race in
>>> deciding this I'm favoring the index.
>>
>> If the race between the creation of /sys/module/ and
>> /sys/module//initstate is inevitable, then I'm afraid we
>> have to rely on the index.
> 
> So my current plan is to rely solely on modules.builtin to output to
> modprobe that a module is builtin. So things like "modprobe vt" will
> start to fail saying there's no vt module. Any objections here?
> 

Hi,
[sorry to resurrect such as old thread]


Would someone please answer/reply to this (related) kernel bugzilla entry:
  https://bugzilla.kernel.org/show_bug.cgi?id=118661

or I could just close it?

thanks,
-- 
~Randy

Re: [PATCH 4/9] x86, pkeys: override pkey when moving away from PROT_EXEC

[Dave Hansen]

On 04/06/2018 05:09 PM, Ram Pai wrote:
>> -/*
>> - * Look for a protection-key-drive execute-only mapping
>> - * which is now being given permissions that are not
>> - * execute-only.  Move it back to the default pkey.
>> - */
>> -if (vma_is_pkey_exec_only(vma) &&
>> -(prot & (PROT_READ|PROT_WRITE))) {
>> -return 0;
>> -}
>> +
> Dave,
>   this can be simply:
> 
>   if ((vma_is_pkey_exec_only(vma) && (prot != PROT_EXEC))
>   return ARCH_DEFAULT_PKEY;

Yes, but we're removing that code entirely. :)

Re: [PATCH 4/9] x86, pkeys: override pkey when moving away from PROT_EXEC

[Dave Hansen]

On 04/06/2018 05:09 PM, Ram Pai wrote:
>> -/*
>> - * Look for a protection-key-drive execute-only mapping
>> - * which is now being given permissions that are not
>> - * execute-only.  Move it back to the default pkey.
>> - */
>> -if (vma_is_pkey_exec_only(vma) &&
>> -(prot & (PROT_READ|PROT_WRITE))) {
>> -return 0;
>> -}
>> +
> Dave,
>   this can be simply:
> 
>   if ((vma_is_pkey_exec_only(vma) && (prot != PROT_EXEC))
>   return ARCH_DEFAULT_PKEY;

Yes, but we're removing that code entirely. :)

Re: [PATCH v4 4/9] vsprintf: Consolidate handling of unknown pointer specifiers

[Sergey Senozhatsky]

Hi Joe,

On (04/06/18 16:59), Joe Perches wrote:
> > 
> > Can we tweak checkpatch to catch such things?
> 
> Not really, no.
> 
> Adding regex logic for this is tricky at best
> and probably not worth the effort because of
> the various bits of patch contexts aren't
> necessarily visible.

Agreed. I was more thinking about catching "... %p" and saying
that we'd rather prefer either "... %p," or "... %p " or "... %p\n".
Doesn't sound so complex, can probably catch something fishy one day
(or may be not), and more or less is visible to checkpatch. Well,
more or less...

> There are also concatenations like
>   "foo" DEFINE "bar"
> where DEFINE may not be visible in the patch
> context and checkpatch is and likely will
> remain just a limited regex checker.

Right. One example might be XFS

alert("%s: Bad regular inode %Lu, ptr "PTR_FMT,
__func__, ip->i_ino, ip);

where PTR_FMT is

#ifdef DEBUG
# define PTR_FMT "%px"
#else
# define PTR_FMT "%p"
#endif

-ss

Re: [PATCH v4 4/9] vsprintf: Consolidate handling of unknown pointer specifiers

[Sergey Senozhatsky]

Hi Joe,

On (04/06/18 16:59), Joe Perches wrote:
> > 
> > Can we tweak checkpatch to catch such things?
> 
> Not really, no.
> 
> Adding regex logic for this is tricky at best
> and probably not worth the effort because of
> the various bits of patch contexts aren't
> necessarily visible.

Agreed. I was more thinking about catching "... %p" and saying
that we'd rather prefer either "... %p," or "... %p " or "... %p\n".
Doesn't sound so complex, can probably catch something fishy one day
(or may be not), and more or less is visible to checkpatch. Well,
more or less...

> There are also concatenations like
>   "foo" DEFINE "bar"
> where DEFINE may not be visible in the patch
> context and checkpatch is and likely will
> remain just a limited regex checker.

Right. One example might be XFS

alert("%s: Bad regular inode %Lu, ptr "PTR_FMT,
__func__, ip->i_ino, ip);

where PTR_FMT is

#ifdef DEBUG
# define PTR_FMT "%px"
#else
# define PTR_FMT "%p"
#endif

-ss

[PATCH v5] x86,sched: allow topologies where NUMA nodes share an LLC

[Alison Schofield]

From: Alison Schofield 

Intel's Skylake Server CPUs have a different LLC topology than previous
generations. When in Sub-NUMA-Clustering (SNC) mode, the package is
divided into two "slices", each containing half the cores, half the LLC,
and one memory controller and each slice is enumerated to Linux as a
NUMA node. This is similar to how the cores and LLC were arranged
for the Cluster-On-Die (CoD) feature.

CoD allowed the same cache line to be present in each half of the LLC.
But, with SNC, each line is only ever present in *one* slice. This
means that the portion of the LLC *available* to a CPU depends on the
data being accessed:

Remote socket: entire package LLC is shared
Local socket->local slice: data goes into local slice LLC
Local socket->remote slice: data goes into remote-slice LLC. Slightly
higher latency than local slice LLC.

The biggest implication from this is that a process accessing all
NUMA-local memory only sees half the LLC capacity.

The CPU describes its cache hierarchy with the CPUID instruction. One
of the CPUID leaves enumerates the "logical processors sharing this
cache". This information is used for scheduling decisions so that tasks
move more freely between CPUs sharing the cache.

But, the CPUID for the SNC configuration discussed above enumerates
the LLC as being shared by the entire package. This is not 100%
precise because the entire cache is not usable by all accesses. But,
it *is* the way the hardware enumerates itself, and this is not likely
to change.

The userspace visible impact of all the above is that the sysfs info
reports the entire LLC as being available to the entire package. As
noted above, this is not true for local socket accesses. This patch
does not correct the sysfs info. It is the same, pre and post patch.

This patch continues to allow this SNC topology and it does so without
complaint. It eliminates a warning that looks like this:

sched: CPU #3's llc-sibling CPU #0 is not on the same node! [node: 1 != 
0]. Ignoring dependency.

The warning is coming from the sane_topology check() in smpboot.c.
To fix this, add a vendor and model specific check to never call
topology_sane() for these systems. Also, just like "Cluster-on-Die"
we throw out the "coregroup" sched_domain_topology_level and use
NUMA information from the SRAT alone.

This is OK at least on the hardware we are immediately concerned about
because the LLC sharing happens at both the slice and at the package
level, which are also NUMA boundaries.

Signed-off-by: Alison Schofield 
Cc: Dave Hansen 
Cc: Tony Luck 
Cc: Tim Chen 
Cc: "H. Peter Anvin" 
Cc: Borislav Petkov 
Cc: Peter Zijlstra (Intel) 
Cc: David Rientjes 
Cc: Igor Mammedov 
Cc: Prarit Bhargava 
Cc: brice.gog...@gmail.com
Cc: Ingo Molnar 
---

Changes in v5:

 * Removed the redundant setting of boolean x86_has_numa_in_package.
   Related comments were put back in their pre-patch locations.

Changes in v4:

 * Added this to the patch description above:

   The userspace visible impact of all the above is that the sysfs info
   reports the entire LLC as being available to the entire package. As
   noted above, this is not true for local socket accesses. This patch
   does not correct the sysfs info. It is the same, pre and post patch.

   This patch continues to allow this SNC topology and it does so without
   complaint. It eliminates a warning that looks like this:
  
 * Changed the code comment per PeterZ/DaveH discussion wrt bypassing
   that topology_sane() check in match_llc()
/*
 * false means 'c' does not share the LLC of 'o'.
 * Note: this decision gets reflected all the way
 * out to userspace
 */
   This message hopes to clarify what happens when we return false.
   Note that returning false is *not* new to this patch. Without
   this patch we always returned false - with a warning. This avoids
   that warning and returns false directly.

 * Remove __initconst from snc_cpu[] declaration that I had added in
   v3. This is not an init time only path. 

 * Do not deal with the wrong sysfs info. It was wrong before this
   patch and it will be the exact same 'wrongness' after this patch.

   We can address the sysfs reporting separately. Here are some options:
   1) Change the way the LLC-size is reported.  Enumerate two separate,
  half-sized LLCs shared only by the slice when SNC mode is on.
   2) Do not export the sysfs info that is wrong. Prevents userspace
  from making bad decisions based on inaccurate info.


Changes in v3:

 * Use x86_match_cpu() for vendor & model check and moved related
   comments to the array define. (Still just one system model)

 * 

[PATCH v5] x86,sched: allow topologies where NUMA nodes share an LLC

[Alison Schofield]

From: Alison Schofield 

Intel's Skylake Server CPUs have a different LLC topology than previous
generations. When in Sub-NUMA-Clustering (SNC) mode, the package is
divided into two "slices", each containing half the cores, half the LLC,
and one memory controller and each slice is enumerated to Linux as a
NUMA node. This is similar to how the cores and LLC were arranged
for the Cluster-On-Die (CoD) feature.

CoD allowed the same cache line to be present in each half of the LLC.
But, with SNC, each line is only ever present in *one* slice. This
means that the portion of the LLC *available* to a CPU depends on the
data being accessed:

Remote socket: entire package LLC is shared
Local socket->local slice: data goes into local slice LLC
Local socket->remote slice: data goes into remote-slice LLC. Slightly
higher latency than local slice LLC.

The biggest implication from this is that a process accessing all
NUMA-local memory only sees half the LLC capacity.

The CPU describes its cache hierarchy with the CPUID instruction. One
of the CPUID leaves enumerates the "logical processors sharing this
cache". This information is used for scheduling decisions so that tasks
move more freely between CPUs sharing the cache.

But, the CPUID for the SNC configuration discussed above enumerates
the LLC as being shared by the entire package. This is not 100%
precise because the entire cache is not usable by all accesses. But,
it *is* the way the hardware enumerates itself, and this is not likely
to change.

The userspace visible impact of all the above is that the sysfs info
reports the entire LLC as being available to the entire package. As
noted above, this is not true for local socket accesses. This patch
does not correct the sysfs info. It is the same, pre and post patch.

This patch continues to allow this SNC topology and it does so without
complaint. It eliminates a warning that looks like this:

sched: CPU #3's llc-sibling CPU #0 is not on the same node! [node: 1 != 
0]. Ignoring dependency.

The warning is coming from the sane_topology check() in smpboot.c.
To fix this, add a vendor and model specific check to never call
topology_sane() for these systems. Also, just like "Cluster-on-Die"
we throw out the "coregroup" sched_domain_topology_level and use
NUMA information from the SRAT alone.

This is OK at least on the hardware we are immediately concerned about
because the LLC sharing happens at both the slice and at the package
level, which are also NUMA boundaries.

Signed-off-by: Alison Schofield 
Cc: Dave Hansen 
Cc: Tony Luck 
Cc: Tim Chen 
Cc: "H. Peter Anvin" 
Cc: Borislav Petkov 
Cc: Peter Zijlstra (Intel) 
Cc: David Rientjes 
Cc: Igor Mammedov 
Cc: Prarit Bhargava 
Cc: brice.gog...@gmail.com
Cc: Ingo Molnar 
---

Changes in v5:

 * Removed the redundant setting of boolean x86_has_numa_in_package.
   Related comments were put back in their pre-patch locations.

Changes in v4:

 * Added this to the patch description above:

   The userspace visible impact of all the above is that the sysfs info
   reports the entire LLC as being available to the entire package. As
   noted above, this is not true for local socket accesses. This patch
   does not correct the sysfs info. It is the same, pre and post patch.

   This patch continues to allow this SNC topology and it does so without
   complaint. It eliminates a warning that looks like this:
  
 * Changed the code comment per PeterZ/DaveH discussion wrt bypassing
   that topology_sane() check in match_llc()
/*
 * false means 'c' does not share the LLC of 'o'.
 * Note: this decision gets reflected all the way
 * out to userspace
 */
   This message hopes to clarify what happens when we return false.
   Note that returning false is *not* new to this patch. Without
   this patch we always returned false - with a warning. This avoids
   that warning and returns false directly.

 * Remove __initconst from snc_cpu[] declaration that I had added in
   v3. This is not an init time only path. 

 * Do not deal with the wrong sysfs info. It was wrong before this
   patch and it will be the exact same 'wrongness' after this patch.

   We can address the sysfs reporting separately. Here are some options:
   1) Change the way the LLC-size is reported.  Enumerate two separate,
  half-sized LLCs shared only by the slice when SNC mode is on.
   2) Do not export the sysfs info that is wrong. Prevents userspace
  from making bad decisions based on inaccurate info.


Changes in v3:

 * Use x86_match_cpu() for vendor & model check and moved related
   comments to the array define. (Still just one system model)

 * Updated the comments surrounding the topology_sane() check.


Changes in v2:

 * Add vendor check (Intel) where we only had a model check (Skylake_X).
   Considered the suggestion of adding a new flag here but thought that
   to be overkill for this usage.

 * Return false, 

Re: [PATCH v4] x86,sched: allow topologies where NUMA nodes share an LLC

[Alison Schofield]

On Wed, Apr 04, 2018 at 12:00:45PM -0700, Alison Schofield wrote:
> On Wed, Apr 04, 2018 at 11:42:11AM -0700, Tim Chen wrote:
> > On 04/04/2018 10:38 AM, Alison Schofield wrote:
> > > On Wed, Apr 04, 2018 at 10:24:49AM -0700, Tim Chen wrote:
> > >> On 04/03/2018 02:12 PM, Alison Schofield wrote:
> > >>
> > >>> +
> > >>> +   /*
> > >>> +* topology_sane() considers LLCs that span NUMA nodes to be
> > >>> +* insane and will display a warning message. Bypass the call
> > >>> +* to topology_sane() for snc_cpu's to avoid that warning.
> > >>> +*/
> > >>> +
> > >>> +   if (!topology_same_node(c, o) && x86_match_cpu(snc_cpu)) {
> > >>> +   /* Indicate that package has NUMA nodes inside: */
> > >>> +   x86_has_numa_in_package = true;
> > >>
> > >> Why does the x86_has_numa_in_package has to be set here when it would 
> > >> have
> > >> been done later in set_cpu_sibling_map?
> > > 
> > > Tim,
> > > I had that same thought when you commented on it previously. After 
> > > discussing w DaveH, decided that match_llc() and match_die(c,0)
> > > could be different and chose to be (cautiously) redundant.
> > > alisons
> > 
> > If it is redundant, I suggest it be removed, and only added if
> > there is truly a case where the current logic 
> > 
> > if (match_die(c, o) && !topology_same_node(c, o))
> > x86_has_numa_in_package = true;
> > 
> > fails.  And also the modification of this logic should be at the
> > same place for easy code maintenance. 
> 
> That makes good sense. I'll look to define the difference or remove
> the redundancy.
> 
> alisons
I found not reason for the redundancy via experimentation w my Skylake,
nor through code examination. I've removed it in v5. I'll see if
anyone claims theoretical case.
alisons
> 
> > 
> > Tim  
> > 
> > > 
> > > 
> > > 
> > >>
> > >>> +
> > >>> +   /*
> > >>> +* false means 'c' does not share the LLC of 'o'.
> > >>> +* Note: this decision gets reflected all the way
> > >>> +* out to userspace.
> > >>> +*/
> > >>> +
> > >>> +   return false;
> > >>
> > >> Thanks.
> > >>
> > >> Tim
> > 

Re: [PATCH v4] x86,sched: allow topologies where NUMA nodes share an LLC

[Alison Schofield]

On Wed, Apr 04, 2018 at 12:00:45PM -0700, Alison Schofield wrote:
> On Wed, Apr 04, 2018 at 11:42:11AM -0700, Tim Chen wrote:
> > On 04/04/2018 10:38 AM, Alison Schofield wrote:
> > > On Wed, Apr 04, 2018 at 10:24:49AM -0700, Tim Chen wrote:
> > >> On 04/03/2018 02:12 PM, Alison Schofield wrote:
> > >>
> > >>> +
> > >>> +   /*
> > >>> +* topology_sane() considers LLCs that span NUMA nodes to be
> > >>> +* insane and will display a warning message. Bypass the call
> > >>> +* to topology_sane() for snc_cpu's to avoid that warning.
> > >>> +*/
> > >>> +
> > >>> +   if (!topology_same_node(c, o) && x86_match_cpu(snc_cpu)) {
> > >>> +   /* Indicate that package has NUMA nodes inside: */
> > >>> +   x86_has_numa_in_package = true;
> > >>
> > >> Why does the x86_has_numa_in_package has to be set here when it would 
> > >> have
> > >> been done later in set_cpu_sibling_map?
> > > 
> > > Tim,
> > > I had that same thought when you commented on it previously. After 
> > > discussing w DaveH, decided that match_llc() and match_die(c,0)
> > > could be different and chose to be (cautiously) redundant.
> > > alisons
> > 
> > If it is redundant, I suggest it be removed, and only added if
> > there is truly a case where the current logic 
> > 
> > if (match_die(c, o) && !topology_same_node(c, o))
> > x86_has_numa_in_package = true;
> > 
> > fails.  And also the modification of this logic should be at the
> > same place for easy code maintenance. 
> 
> That makes good sense. I'll look to define the difference or remove
> the redundancy.
> 
> alisons
I found not reason for the redundancy via experimentation w my Skylake,
nor through code examination. I've removed it in v5. I'll see if
anyone claims theoretical case.
alisons
> 
> > 
> > Tim  
> > 
> > > 
> > > 
> > > 
> > >>
> > >>> +
> > >>> +   /*
> > >>> +* false means 'c' does not share the LLC of 'o'.
> > >>> +* Note: this decision gets reflected all the way
> > >>> +* out to userspace.
> > >>> +*/
> > >>> +
> > >>> +   return false;
> > >>
> > >> Thanks.
> > >>
> > >> Tim
> > 

Re: [PATCH 4/9] x86, pkeys: override pkey when moving away from PROT_EXEC

[Ram Pai]

On Mon, Mar 26, 2018 at 10:27:27AM -0700, Dave Hansen wrote:
> 
> From: Dave Hansen 
> 
> I got a bug report that the following code (roughly) was
> causing a SIGSEGV:
> 
>   mprotect(ptr, size, PROT_EXEC);
>   mprotect(ptr, size, PROT_NONE);
>   mprotect(ptr, size, PROT_READ);
>   *ptr = 100;
> 
> The problem is hit when the mprotect(PROT_EXEC)
> is implicitly assigned a protection key to the VMA, and made
> that key ACCESS_DENY|WRITE_DENY.  The PROT_NONE mprotect()
> failed to remove the protection key, and the PROT_NONE->
> PROT_READ left the PTE usable, but the pkey still in place
> and left the memory inaccessible.
> 
> To fix this, we ensure that we always "override" the pkee
> at mprotect() if the VMA does not have execute-only
> permissions, but the VMA has the execute-only pkey.
> 
> We had a check for PROT_READ/WRITE, but it did not work
> for PROT_NONE.  This entirely removes the PROT_* checks,
> which ensures that PROT_NONE now works.
> 
> Reported-by: Shakeel Butt 
> 
> Signed-off-by: Dave Hansen 
> Fixes: 62b5f7d013f ("mm/core, x86/mm/pkeys: Add execute-only protection keys 
> support")
> Cc: sta...@kernel.org
> Cc: Ram Pai 
> Cc: Thomas Gleixner 
> Cc: Dave Hansen 
> Cc: Michael Ellermen 
> Cc: Ingo Molnar 
> Cc: Andrew Morton 
> Cc: Shuah Khan 
> ---
> 
>  b/arch/x86/include/asm/pkeys.h |   12 +++-
>  b/arch/x86/mm/pkeys.c  |   19 ++-
>  2 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff -puN 
> arch/x86/include/asm/pkeys.h~pkeys-abandon-exec-only-pkey-more-aggressively 
> arch/x86/include/asm/pkeys.h
> --- 
> a/arch/x86/include/asm/pkeys.h~pkeys-abandon-exec-only-pkey-more-aggressively 
> 2018-03-26 10:22:35.380170193 -0700
> +++ b/arch/x86/include/asm/pkeys.h2018-03-26 10:22:35.385170193 -0700
> @@ -2,6 +2,8 @@
>  #ifndef _ASM_X86_PKEYS_H
>  #define _ASM_X86_PKEYS_H
> 
> +#define ARCH_DEFAULT_PKEY0
> +
>  #define arch_max_pkey() (boot_cpu_has(X86_FEATURE_OSPKE) ? 16 : 1)
> 
>  extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> @@ -15,7 +17,7 @@ extern int __execute_only_pkey(struct mm
>  static inline int execute_only_pkey(struct mm_struct *mm)
>  {
>   if (!boot_cpu_has(X86_FEATURE_OSPKE))
> - return 0;
> + return ARCH_DEFAULT_PKEY;
> 
>   return __execute_only_pkey(mm);
>  }
> @@ -56,6 +58,14 @@ bool mm_pkey_is_allocated(struct mm_stru
>   return false;
>   if (pkey >= arch_max_pkey())
>   return false;
> + /*
> +  * The exec-only pkey is set in the allocation map, but
> +  * is not available to any of the user interfaces like
> +  * mprotect_pkey().
> +  */
> + if (pkey == mm->context.execute_only_pkey)
> + return false;
> +
>   return mm_pkey_allocation_map(mm) & (1U << pkey);
>  }
> 
> diff -puN arch/x86/mm/pkeys.c~pkeys-abandon-exec-only-pkey-more-aggressively 
> arch/x86/mm/pkeys.c
> --- a/arch/x86/mm/pkeys.c~pkeys-abandon-exec-only-pkey-more-aggressively  
> 2018-03-26 10:22:35.381170193 -0700
> +++ b/arch/x86/mm/pkeys.c 2018-03-26 10:22:35.385170193 -0700
> @@ -94,15 +94,7 @@ int __arch_override_mprotect_pkey(struct
>*/
>   if (pkey != -1)
>   return pkey;
> - /*
> -  * Look for a protection-key-drive execute-only mapping
> -  * which is now being given permissions that are not
> -  * execute-only.  Move it back to the default pkey.
> -  */
> - if (vma_is_pkey_exec_only(vma) &&
> - (prot & (PROT_READ|PROT_WRITE))) {
> - return 0;
> - }
> +

Dave,
this can be simply:

if ((vma_is_pkey_exec_only(vma) && (prot != PROT_EXEC))
return ARCH_DEFAULT_PKEY;

No?
RP

Re: [PATCH 4/9] x86, pkeys: override pkey when moving away from PROT_EXEC

[Ram Pai]

On Mon, Mar 26, 2018 at 10:27:27AM -0700, Dave Hansen wrote:
> 
> From: Dave Hansen 
> 
> I got a bug report that the following code (roughly) was
> causing a SIGSEGV:
> 
>   mprotect(ptr, size, PROT_EXEC);
>   mprotect(ptr, size, PROT_NONE);
>   mprotect(ptr, size, PROT_READ);
>   *ptr = 100;
> 
> The problem is hit when the mprotect(PROT_EXEC)
> is implicitly assigned a protection key to the VMA, and made
> that key ACCESS_DENY|WRITE_DENY.  The PROT_NONE mprotect()
> failed to remove the protection key, and the PROT_NONE->
> PROT_READ left the PTE usable, but the pkey still in place
> and left the memory inaccessible.
> 
> To fix this, we ensure that we always "override" the pkee
> at mprotect() if the VMA does not have execute-only
> permissions, but the VMA has the execute-only pkey.
> 
> We had a check for PROT_READ/WRITE, but it did not work
> for PROT_NONE.  This entirely removes the PROT_* checks,
> which ensures that PROT_NONE now works.
> 
> Reported-by: Shakeel Butt 
> 
> Signed-off-by: Dave Hansen 
> Fixes: 62b5f7d013f ("mm/core, x86/mm/pkeys: Add execute-only protection keys 
> support")
> Cc: sta...@kernel.org
> Cc: Ram Pai 
> Cc: Thomas Gleixner 
> Cc: Dave Hansen 
> Cc: Michael Ellermen 
> Cc: Ingo Molnar 
> Cc: Andrew Morton 
> Cc: Shuah Khan 
> ---
> 
>  b/arch/x86/include/asm/pkeys.h |   12 +++-
>  b/arch/x86/mm/pkeys.c  |   19 ++-
>  2 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff -puN 
> arch/x86/include/asm/pkeys.h~pkeys-abandon-exec-only-pkey-more-aggressively 
> arch/x86/include/asm/pkeys.h
> --- 
> a/arch/x86/include/asm/pkeys.h~pkeys-abandon-exec-only-pkey-more-aggressively 
> 2018-03-26 10:22:35.380170193 -0700
> +++ b/arch/x86/include/asm/pkeys.h2018-03-26 10:22:35.385170193 -0700
> @@ -2,6 +2,8 @@
>  #ifndef _ASM_X86_PKEYS_H
>  #define _ASM_X86_PKEYS_H
> 
> +#define ARCH_DEFAULT_PKEY0
> +
>  #define arch_max_pkey() (boot_cpu_has(X86_FEATURE_OSPKE) ? 16 : 1)
> 
>  extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> @@ -15,7 +17,7 @@ extern int __execute_only_pkey(struct mm
>  static inline int execute_only_pkey(struct mm_struct *mm)
>  {
>   if (!boot_cpu_has(X86_FEATURE_OSPKE))
> - return 0;
> + return ARCH_DEFAULT_PKEY;
> 
>   return __execute_only_pkey(mm);
>  }
> @@ -56,6 +58,14 @@ bool mm_pkey_is_allocated(struct mm_stru
>   return false;
>   if (pkey >= arch_max_pkey())
>   return false;
> + /*
> +  * The exec-only pkey is set in the allocation map, but
> +  * is not available to any of the user interfaces like
> +  * mprotect_pkey().
> +  */
> + if (pkey == mm->context.execute_only_pkey)
> + return false;
> +
>   return mm_pkey_allocation_map(mm) & (1U << pkey);
>  }
> 
> diff -puN arch/x86/mm/pkeys.c~pkeys-abandon-exec-only-pkey-more-aggressively 
> arch/x86/mm/pkeys.c
> --- a/arch/x86/mm/pkeys.c~pkeys-abandon-exec-only-pkey-more-aggressively  
> 2018-03-26 10:22:35.381170193 -0700
> +++ b/arch/x86/mm/pkeys.c 2018-03-26 10:22:35.385170193 -0700
> @@ -94,15 +94,7 @@ int __arch_override_mprotect_pkey(struct
>*/
>   if (pkey != -1)
>   return pkey;
> - /*
> -  * Look for a protection-key-drive execute-only mapping
> -  * which is now being given permissions that are not
> -  * execute-only.  Move it back to the default pkey.
> -  */
> - if (vma_is_pkey_exec_only(vma) &&
> - (prot & (PROT_READ|PROT_WRITE))) {
> - return 0;
> - }
> +

Dave,
this can be simply:

if ((vma_is_pkey_exec_only(vma) && (prot != PROT_EXEC))
return ARCH_DEFAULT_PKEY;

No?
RP

[PATCH 0/1] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls

[Keith Packard]

(This is an RFC on whether this pair of ioctls seems reasonable. The
code compiles, but I haven't tested it as I'm away from home this
weekend.)

I'm rewriting my implementation of the Vulkan EXT_display_control
extension, which provides a way to signal a Vulkan fence at vblank
time. I had implemented it using events, but that isn't great as the
Vulkan API includes the ability to wait for any of a set of fences to
be signaled. As the other Vulkan fences are implemented using
dma_fences in the kernel, and (eventually) using syncobj up in user
space, it seems reasonable to use syncobjs for everything and hook
vblank to those.

In any case, I'm proposing two new syncobj/vblank ioctls (the names
aren't great; suggestions welcome, as usual):

DRM_IOCTL_CRTC_QUEUE_SYNCOBJ

Create a new syncobj that will be signaled at (or after) the
specified vblank sequence value. This uses the same parameters
to specify the target sequence as
DRM_IOCTL_CRTC_QUEUE_SEQUENCE.

DRM_IOCTL_CRTC_GET_SYNCOBJ

Once the above syncobj has been signaled, this ioctl allows
the application to find out when that happened, returning both
the vblank sequence count and time (in ns).

I'd like to hear comments on whether this seems reasonable, or whether
I should go in some other direction.

[PATCH] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls

[Keith Packard]

crtc_queue_syncobj creates a new syncobj that will get signaled at a
specified vblank sequence count.

crtc_get_syncobj returns the time and vblank sequence count when the
syncobj was signaled.

The pair of these allows use of syncobjs instead of events for
monitoring vblank activity.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/drm_file.c |   5 +
 drivers/gpu/drm/drm_internal.h |   4 +
 drivers/gpu/drm/drm_ioctl.c|   2 +
 drivers/gpu/drm/drm_syncobj.c  |  13 ++-
 drivers/gpu/drm/drm_vblank.c   | 212 +
 include/drm/drm_file.h |  11 ++-
 include/drm/drm_syncobj.h  |  13 +++
 include/uapi/drm/drm.h |  17 
 8 files changed, 253 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index b3c6e997ccdb..c1ada3fe70b0 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -37,6 +37,7 @@
 
 #include 
 #include 
+#include 
 
 #include "drm_legacy.h"
 #include "drm_internal.h"
@@ -711,6 +712,10 @@ void drm_send_event_locked(struct drm_device *dev, struct 
drm_pending_event *e)
dma_fence_put(e->fence);
}
 
+   if (e->syncobj) {
+   drm_syncobj_put(e->syncobj);
+   }
+
if (!e->file_priv) {
kfree(e);
return;
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index c9d5a6cd4d41..71b9435b5b37 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -75,6 +75,10 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void 
*data,
 
 int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,
  struct drm_file *filp);
+int drm_crtc_queue_syncobj_ioctl(struct drm_device *dev, void *data,
+struct drm_file *filp);
+int drm_crtc_get_syncobj_ioctl(struct drm_device *dev, void *data,
+struct drm_file *filp);
 
 /* drm_auth.c */
 int drm_getmagic(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 4aafe4802099..309611fb5d0d 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -665,6 +665,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, 
DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, 
drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SYNCOBJ, 
drm_crtc_queue_syncobj_ioctl, DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SYNCOBJ, drm_crtc_get_syncobj_ioctl, 
DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index cb4d09c70fd4..e197b007079d 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -208,7 +208,7 @@ static const struct dma_fence_ops 
drm_syncobj_null_fence_ops = {
.release = NULL,
 };
 
-static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj)
+int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj, bool signal)
 {
struct drm_syncobj_null_fence *fence;
fence = kzalloc(sizeof(*fence), GFP_KERNEL);
@@ -218,7 +218,8 @@ static int drm_syncobj_assign_null_handle(struct 
drm_syncobj *syncobj)
spin_lock_init(>lock);
dma_fence_init(>base, _syncobj_null_fence_ops,
   >lock, 0, 0);
-   dma_fence_signal(>base);
+   if (signal)
+   dma_fence_signal(>base);
 
drm_syncobj_replace_fence(syncobj, >base);
 
@@ -226,6 +227,7 @@ static int drm_syncobj_assign_null_handle(struct 
drm_syncobj *syncobj)
 
return 0;
 }
+EXPORT_SYMBOL(drm_syncobj_assign_null_handle);
 
 int drm_syncobj_find_fence(struct drm_file *file_private,
   u32 handle,
@@ -283,7 +285,7 @@ int drm_syncobj_create(struct drm_syncobj **out_syncobj, 
uint32_t flags,
spin_lock_init(>lock);
 
if (flags & DRM_SYNCOBJ_CREATE_SIGNALED) {
-   ret = drm_syncobj_assign_null_handle(syncobj);
+   ret = drm_syncobj_assign_null_handle(syncobj, true);
if (ret < 0) {
drm_syncobj_put(syncobj);
return ret;
@@ -341,7 +343,7 @@ static int drm_syncobj_create_as_handle(struct drm_file 
*file_private,
return ret;
 }
 
-static int drm_syncobj_destroy(struct drm_file *file_private,
+int drm_syncobj_destroy(struct drm_file 

[PATCH 0/1] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls

[Keith Packard]

(This is an RFC on whether this pair of ioctls seems reasonable. The
code compiles, but I haven't tested it as I'm away from home this
weekend.)

I'm rewriting my implementation of the Vulkan EXT_display_control
extension, which provides a way to signal a Vulkan fence at vblank
time. I had implemented it using events, but that isn't great as the
Vulkan API includes the ability to wait for any of a set of fences to
be signaled. As the other Vulkan fences are implemented using
dma_fences in the kernel, and (eventually) using syncobj up in user
space, it seems reasonable to use syncobjs for everything and hook
vblank to those.

In any case, I'm proposing two new syncobj/vblank ioctls (the names
aren't great; suggestions welcome, as usual):

DRM_IOCTL_CRTC_QUEUE_SYNCOBJ

Create a new syncobj that will be signaled at (or after) the
specified vblank sequence value. This uses the same parameters
to specify the target sequence as
DRM_IOCTL_CRTC_QUEUE_SEQUENCE.

DRM_IOCTL_CRTC_GET_SYNCOBJ

Once the above syncobj has been signaled, this ioctl allows
the application to find out when that happened, returning both
the vblank sequence count and time (in ns).

I'd like to hear comments on whether this seems reasonable, or whether
I should go in some other direction.

[PATCH] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls

[Keith Packard]

crtc_queue_syncobj creates a new syncobj that will get signaled at a
specified vblank sequence count.

crtc_get_syncobj returns the time and vblank sequence count when the
syncobj was signaled.

The pair of these allows use of syncobjs instead of events for
monitoring vblank activity.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/drm_file.c |   5 +
 drivers/gpu/drm/drm_internal.h |   4 +
 drivers/gpu/drm/drm_ioctl.c|   2 +
 drivers/gpu/drm/drm_syncobj.c  |  13 ++-
 drivers/gpu/drm/drm_vblank.c   | 212 +
 include/drm/drm_file.h |  11 ++-
 include/drm/drm_syncobj.h  |  13 +++
 include/uapi/drm/drm.h |  17 
 8 files changed, 253 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index b3c6e997ccdb..c1ada3fe70b0 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -37,6 +37,7 @@
 
 #include 
 #include 
+#include 
 
 #include "drm_legacy.h"
 #include "drm_internal.h"
@@ -711,6 +712,10 @@ void drm_send_event_locked(struct drm_device *dev, struct 
drm_pending_event *e)
dma_fence_put(e->fence);
}
 
+   if (e->syncobj) {
+   drm_syncobj_put(e->syncobj);
+   }
+
if (!e->file_priv) {
kfree(e);
return;
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index c9d5a6cd4d41..71b9435b5b37 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -75,6 +75,10 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void 
*data,
 
 int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,
  struct drm_file *filp);
+int drm_crtc_queue_syncobj_ioctl(struct drm_device *dev, void *data,
+struct drm_file *filp);
+int drm_crtc_get_syncobj_ioctl(struct drm_device *dev, void *data,
+struct drm_file *filp);
 
 /* drm_auth.c */
 int drm_getmagic(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 4aafe4802099..309611fb5d0d 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -665,6 +665,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, 
DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, 
drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SYNCOBJ, 
drm_crtc_queue_syncobj_ioctl, DRM_UNLOCKED),
+   DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SYNCOBJ, drm_crtc_get_syncobj_ioctl, 
DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, 
DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index cb4d09c70fd4..e197b007079d 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -208,7 +208,7 @@ static const struct dma_fence_ops 
drm_syncobj_null_fence_ops = {
.release = NULL,
 };
 
-static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj)
+int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj, bool signal)
 {
struct drm_syncobj_null_fence *fence;
fence = kzalloc(sizeof(*fence), GFP_KERNEL);
@@ -218,7 +218,8 @@ static int drm_syncobj_assign_null_handle(struct 
drm_syncobj *syncobj)
spin_lock_init(>lock);
dma_fence_init(>base, _syncobj_null_fence_ops,
   >lock, 0, 0);
-   dma_fence_signal(>base);
+   if (signal)
+   dma_fence_signal(>base);
 
drm_syncobj_replace_fence(syncobj, >base);
 
@@ -226,6 +227,7 @@ static int drm_syncobj_assign_null_handle(struct 
drm_syncobj *syncobj)
 
return 0;
 }
+EXPORT_SYMBOL(drm_syncobj_assign_null_handle);
 
 int drm_syncobj_find_fence(struct drm_file *file_private,
   u32 handle,
@@ -283,7 +285,7 @@ int drm_syncobj_create(struct drm_syncobj **out_syncobj, 
uint32_t flags,
spin_lock_init(>lock);
 
if (flags & DRM_SYNCOBJ_CREATE_SIGNALED) {
-   ret = drm_syncobj_assign_null_handle(syncobj);
+   ret = drm_syncobj_assign_null_handle(syncobj, true);
if (ret < 0) {
drm_syncobj_put(syncobj);
return ret;
@@ -341,7 +343,7 @@ static int drm_syncobj_create_as_handle(struct drm_file 
*file_private,
return ret;
 }
 
-static int drm_syncobj_destroy(struct drm_file *file_private,
+int drm_syncobj_destroy(struct drm_file *file_private,

Re: [PATCH v4 4/9] vsprintf: Consolidate handling of unknown pointer specifiers

[Joe Perches]

On Sat, 2018-04-07 at 08:52 +0900, Sergey Senozhatsky wrote:
> On (04/05/18 16:55), Joe Perches wrote:
> > On Thu, 2018-04-05 at 16:45 -0700, Joe Perches wrote:
> > > On Thu, 2018-04-05 at 16:25 +0200, Rasmus Villemoes wrote:
> > > > Even just git grep -1 -E '%p"$' finds %pt and %po
> > > > which should get fixed before somebody claims those extensions.
> > > 
> > > Neither %pt nor %po is used in a vsprintf
> > > in the kernel.
> > 
> > Nope, you are right, both are defectively used in the
> > kernel via string concatenation.
> > 
> > Also there's a missing space in a concatenation adjacent.
> 
> Can we tweak checkpatch to catch such things?

Not really, no.

Adding regex logic for this is tricky at best
and probably not worth the effort because of
the various bits of patch contexts aren't
necessarily visible.

There are also concatenations like
"foo" DEFINE "bar"
where DEFINE may not be visible in the patch
context and checkpatch is and likely will
remain just a limited regex checker.

Re: [PATCH v4 4/9] vsprintf: Consolidate handling of unknown pointer specifiers

[Joe Perches]

On Sat, 2018-04-07 at 08:52 +0900, Sergey Senozhatsky wrote:
> On (04/05/18 16:55), Joe Perches wrote:
> > On Thu, 2018-04-05 at 16:45 -0700, Joe Perches wrote:
> > > On Thu, 2018-04-05 at 16:25 +0200, Rasmus Villemoes wrote:
> > > > Even just git grep -1 -E '%p"$' finds %pt and %po
> > > > which should get fixed before somebody claims those extensions.
> > > 
> > > Neither %pt nor %po is used in a vsprintf
> > > in the kernel.
> > 
> > Nope, you are right, both are defectively used in the
> > kernel via string concatenation.
> > 
> > Also there's a missing space in a concatenation adjacent.
> 
> Can we tweak checkpatch to catch such things?

Not really, no.

Adding regex logic for this is tricky at best
and probably not worth the effort because of
the various bits of patch contexts aren't
necessarily visible.

There are also concatenations like
"foo" DEFINE "bar"
where DEFINE may not be visible in the patch
context and checkpatch is and likely will
remain just a limited regex checker.

[GIT PULL] leaking_addresses: changes for 14.17-rc1

[Tobin C. Harding]

Hi Linus,

This is your favourite noob maintainer requesting that you pretty please
with sugar on top pull the leaking_addresses patch set.

I have hopefully lifted my game after the abysmal effort last merge
window.  The script now actually runs in an acceptable time, the tree is
on kernel.org and I have convinced another (more experienced) kernel
hacker to cast an eye over things.

All patches have been in linux-next since about half way through last
dev cycle and testing was done on x86_64, ARM (32 bit), and PowerPC.

thanks,
Tobin.


The following changes since commit 0adb32858b0bddf4ada5f364a84ed60b196dbcda:

  Linux 4.16 (2018-04-01 14:20:27 -0700)

are available in the git repository at:

  ssh://g...@gitolite.kernel.org/pub/scm/linux/kernel/git/tobin/leaks.git 
tags/leaks-4.17-rc1

for you to fetch changes up to e875d33d7f06d1107c057d12bb5aaba84738e418:

  MAINTAINERS: Update LEAKING_ADDRESSES (2018-04-07 09:35:37 +1000)


Leaking-addresses patches for 4.17-rc1

Here is the patch set for the 4.17-rc1 merge window.  This set
represents improvements to the scripts/leaking_addresses.pl script.  The
major improvement is that with this set applied the script actually runs
in a reasonable amount of time (less than a minute on a standard stock
Ubuntu user desktop).  Also, we have a second maintainer now and a tree
hosted on kernel.org

We do a few code clean ups.  We fix the command help output.  Handling
of the vsyscall address range is fixed to check the whole range instead
of just the start/end addresses.  We add support for 5 page table levels
(suggested on LKML).  We use a system command to get the machine
architecture instead of using Perl.  Calling this command for every
regex comparison is what previously choked the script, caching the
result of this call gave the major speed improvement.  We add support
for scanning 32-bit kernels using the user/kernel memory split.  Path
skipping code refactored and simplified (meaning easier script
configuration).  We remove version numbering.  We add a variable name to
improve readability of a regex and finally we check filenames for
leaking addresses.

Currently script scans /proc/PID for all PID.  With this set applied we
only scan for PID==1. It was observed that on an idle system files under
/proc/PID are predominantly the same for all processes.  Also it was
noted that the script does not scan _all_ the kernel since it only scans
active processes.  Scanning only for PID==1 makes explicit the inherent
flaw in the script that the scan is only partial and also speeds things up.

Signed-off-by: Tobin C. Harding 


Tobin C. Harding (19):
  leaking_addresses: fix typo function not called
  leaking_addresses: remove mention of kptr_restrict
  leaking_addresses: remove command examples
  leaking_addresses: indent dependant options
  leaking_addresses: add range check for vsyscall memory
  leaking_addresses: add support for kernel config file
  leaking_addresses: add support for 5 page table levels
  leaking_addresses: use system command to get arch
  leaking_addresses: add is_arch() wrapper subroutine
  leaking_addresses: add 32-bit support
  leaking_addresses: do not parse binary files
  leaking_addresses: simplify path skipping
  leaking_addresses: cache architecture name
  leaking_addresses: skip all /proc/PID except /proc/1
  leaking_addresses: skip '/proc/1/syscall'
  leaking_addresses: remove version number
  leaking_addresses: explicitly name variable used in regex
  leaking_addresses: check if file name contains address
  MAINTAINERS: Update LEAKING_ADDRESSES

 MAINTAINERS  |   3 +
 scripts/leaking_addresses.pl | 372 ++-
 2 files changed, 262 insertions(+), 113 deletions(-)

[GIT PULL] leaking_addresses: changes for 14.17-rc1

[Tobin C. Harding]

Hi Linus,

This is your favourite noob maintainer requesting that you pretty please
with sugar on top pull the leaking_addresses patch set.

I have hopefully lifted my game after the abysmal effort last merge
window.  The script now actually runs in an acceptable time, the tree is
on kernel.org and I have convinced another (more experienced) kernel
hacker to cast an eye over things.

All patches have been in linux-next since about half way through last
dev cycle and testing was done on x86_64, ARM (32 bit), and PowerPC.

thanks,
Tobin.


The following changes since commit 0adb32858b0bddf4ada5f364a84ed60b196dbcda:

  Linux 4.16 (2018-04-01 14:20:27 -0700)

are available in the git repository at:

  ssh://g...@gitolite.kernel.org/pub/scm/linux/kernel/git/tobin/leaks.git 
tags/leaks-4.17-rc1

for you to fetch changes up to e875d33d7f06d1107c057d12bb5aaba84738e418:

  MAINTAINERS: Update LEAKING_ADDRESSES (2018-04-07 09:35:37 +1000)


Leaking-addresses patches for 4.17-rc1

Here is the patch set for the 4.17-rc1 merge window.  This set
represents improvements to the scripts/leaking_addresses.pl script.  The
major improvement is that with this set applied the script actually runs
in a reasonable amount of time (less than a minute on a standard stock
Ubuntu user desktop).  Also, we have a second maintainer now and a tree
hosted on kernel.org

We do a few code clean ups.  We fix the command help output.  Handling
of the vsyscall address range is fixed to check the whole range instead
of just the start/end addresses.  We add support for 5 page table levels
(suggested on LKML).  We use a system command to get the machine
architecture instead of using Perl.  Calling this command for every
regex comparison is what previously choked the script, caching the
result of this call gave the major speed improvement.  We add support
for scanning 32-bit kernels using the user/kernel memory split.  Path
skipping code refactored and simplified (meaning easier script
configuration).  We remove version numbering.  We add a variable name to
improve readability of a regex and finally we check filenames for
leaking addresses.

Currently script scans /proc/PID for all PID.  With this set applied we
only scan for PID==1. It was observed that on an idle system files under
/proc/PID are predominantly the same for all processes.  Also it was
noted that the script does not scan _all_ the kernel since it only scans
active processes.  Scanning only for PID==1 makes explicit the inherent
flaw in the script that the scan is only partial and also speeds things up.

Signed-off-by: Tobin C. Harding 


Tobin C. Harding (19):
  leaking_addresses: fix typo function not called
  leaking_addresses: remove mention of kptr_restrict
  leaking_addresses: remove command examples
  leaking_addresses: indent dependant options
  leaking_addresses: add range check for vsyscall memory
  leaking_addresses: add support for kernel config file
  leaking_addresses: add support for 5 page table levels
  leaking_addresses: use system command to get arch
  leaking_addresses: add is_arch() wrapper subroutine
  leaking_addresses: add 32-bit support
  leaking_addresses: do not parse binary files
  leaking_addresses: simplify path skipping
  leaking_addresses: cache architecture name
  leaking_addresses: skip all /proc/PID except /proc/1
  leaking_addresses: skip '/proc/1/syscall'
  leaking_addresses: remove version number
  leaking_addresses: explicitly name variable used in regex
  leaking_addresses: check if file name contains address
  MAINTAINERS: Update LEAKING_ADDRESSES

 MAINTAINERS  |   3 +
 scripts/leaking_addresses.pl | 372 ++-
 2 files changed, 262 insertions(+), 113 deletions(-)

Re: [PATCH v4 4/9] vsprintf: Consolidate handling of unknown pointer specifiers

[Sergey Senozhatsky]

On (04/05/18 16:55), Joe Perches wrote:
> On Thu, 2018-04-05 at 16:45 -0700, Joe Perches wrote:
> > On Thu, 2018-04-05 at 16:25 +0200, Rasmus Villemoes wrote:
> > > Even just git grep -1 -E '%p"$' finds %pt and %po
> > > which should get fixed before somebody claims those extensions.
> > 
> > Neither %pt nor %po is used in a vsprintf
> > in the kernel.
> 
> Nope, you are right, both are defectively used in the
> kernel via string concatenation.
> 
> Also there's a missing space in a concatenation adjacent.

Can we tweak checkpatch to catch such things?


Hm... *Probably* also wouldn't hurt if checkpatch can require at least
one character after pointer format specifiers:

printk("string %p"  vs   printk("string %p "
   " Object\n", ptr);   "Object\n", ptr);

Especially if we can have a "potential" %px, like here

   dev_vdbg(>input->dev,
   "%s: *axis=%02X(%d) size=%d max=%08X xy_data=%p"
   " xy_data[%d]=%02X(%d) bofs=%d\n",

or here

   dev_vdbg(>input->dev,
   "%s: *axis=%02X(%d) size=%d max=%08X xy_data=%p"
   " xy_data[%d]=%02X(%d)\n",

Opinions?

-ss

Re: [PATCH v4 4/9] vsprintf: Consolidate handling of unknown pointer specifiers

[Sergey Senozhatsky]

On (04/05/18 16:55), Joe Perches wrote:
> On Thu, 2018-04-05 at 16:45 -0700, Joe Perches wrote:
> > On Thu, 2018-04-05 at 16:25 +0200, Rasmus Villemoes wrote:
> > > Even just git grep -1 -E '%p"$' finds %pt and %po
> > > which should get fixed before somebody claims those extensions.
> > 
> > Neither %pt nor %po is used in a vsprintf
> > in the kernel.
> 
> Nope, you are right, both are defectively used in the
> kernel via string concatenation.
> 
> Also there's a missing space in a concatenation adjacent.

Can we tweak checkpatch to catch such things?


Hm... *Probably* also wouldn't hurt if checkpatch can require at least
one character after pointer format specifiers:

printk("string %p"  vs   printk("string %p "
   " Object\n", ptr);   "Object\n", ptr);

Especially if we can have a "potential" %px, like here

   dev_vdbg(>input->dev,
   "%s: *axis=%02X(%d) size=%d max=%08X xy_data=%p"
   " xy_data[%d]=%02X(%d) bofs=%d\n",

or here

   dev_vdbg(>input->dev,
   "%s: *axis=%02X(%d) size=%d max=%08X xy_data=%p"
   " xy_data[%d]=%02X(%d)\n",

Opinions?

-ss

Re: [PATCH] sched/fair: schedutil: update only with all info available

[Joel Fernandes]

On Fri, Apr 6, 2018 at 10:28 AM, Patrick Bellasi
 wrote:
> Schedutil is not properly updated when the first FAIR task wakes up on a
> CPU and when a RQ is (un)throttled. This is mainly due to the current
> integration strategy, which relies on updates being triggered implicitly
> each time a cfs_rq's utilization is updated.

To me that's not implicit, but is explicitly done when the util is
updated. It seems that's the logical place..

>
> Those updates are currently provided (mainly) via
>cfs_rq_util_change()
> which is used in:
>  - update_cfs_rq_load_avg()
>when the utilization of a cfs_rq is updated
>  - {attach,detach}_entity_load_avg()
> This is done based on the idea that "we should callback schedutil
> frequently enough" to properly update the CPU frequency at every

This also I didn't get, its not called "frequently enough" but at the
right time (when the util is updated).

> utilization change.
>
> Since this recent schedutil update:
>
>   commit 8f111bc357aa ("cpufreq/schedutil: Rewrite CPUFREQ_RT support")
>
> we use additional RQ information to properly account for FAIR tasks
> utilization. Specifically, cfs_rq::h_nr_running has to be non-zero
> in sugov_aggregate_util() to sum up the cfs_rq's utilization.
>
> However, cfs_rq::h_nr_running is usually updated as:
>
> enqueue_entity()
>...
>update_load_avg()
>   ...
>   cfs_rq_util_change ==> trigger schedutil update
> ...
> cfs_rq->h_nr_running += number_of_tasks
>
> both in enqueue_task_fair() as well as in unthrottle_cfs_rq().
> A similar pattern is used also in dequeue_task_fair() and
> throttle_cfs_rq() to remove tasks.

Maybe this should be fixed then? It seems broken to begin with...

>
> This means that we are likely to see a zero cfs_rq utilization when we
> enqueue a task on an empty CPU, or a non-zero cfs_rq utilization when
> instead, for example, we are throttling all the FAIR tasks of a CPU.
>
> While the second issue is less important, since we are less likely to

Actually I wanted behavior like the second issue, because that means
frequency will not be dropped if CPU is about to idle which is similar
to a patch I sent long time ago (skip freq update if CPU about to
idle).

> reduce frequency when CPU utilization decreases, the first issue can
> instead impact performance. Indeed, we potentially introduce a not desired

This issue would be fixed by just fixing the h_nr_running bug right?

> latency between a task enqueue on a CPU and its frequency increase.
>
> Another possible unwanted side effect is the iowait boosting of a CPU
> when we enqueue a task into a throttled cfs_rq.

Probably very very rare.

> Moreover, the current schedutil integration has these other downsides:
>
>  - schedutil updates are triggered by RQ's load updates, which makes
>sense in general but it does not allow to know exactly which other RQ
>related information has been updated (e.g. h_nr_running).

It seems broken that all information that schedutil needs isn't
updated _before_ the cpufreq update request, so that should be fixed
instead?

>
>  - increasing the chances to update schedutil does not always correspond
>to provide the most accurate information for a proper frequency
>selection, thus we can skip some updates.

Again IMO its just updated at the right time already, not just
frequently enough...

>
>  - we don't know exactly at which point a schedutil update is triggered,
>and thus potentially a frequency change started, and that's because
>the update is a side effect of cfs_rq_util_changed instead of an
>explicit call from the most suitable call path.
>
>  - cfs_rq_util_change() is mainly a wrapper function for an already
>existing "public API", cpufreq_update_util(), to ensure we actually
>update schedutil only when we are updating a root RQ. Thus, especially
>when task groups are in use, most of the calls to this wrapper
>function are really not required.
>
>  - the usage of a wrapper function is not completely consistent across
>fair.c, since we still need sometimes additional explicit calls to
>cpufreq_update_util(), for example to support the IOWAIT boot flag in
>the wakeup path
>
>  - it makes it hard to integrate new features since it could require to
>change other function prototypes just to pass in an additional flag,
>as it happened for example here:
>
>   commit ea14b57e8a18 ("sched/cpufreq: Provide migration hint")
>
> All the above considered, let's try to make schedutil updates more
> explicit in fair.c by:
>
>  - removing the cfs_rq_util_change() wrapper function to use the
>cpufreq_update_util() public API only when root cfs_rq is updated
>
>  - remove indirect and side-effect (sometimes not required) schedutil
>updates when the cfs_rq utilization is updated
>
>  - call cpufreq_update_util() explicitly in the few call sites where it
>really makes sense and all the required 

Re: [PATCH] sched/fair: schedutil: update only with all info available

[Joel Fernandes]

On Fri, Apr 6, 2018 at 10:28 AM, Patrick Bellasi
 wrote:
> Schedutil is not properly updated when the first FAIR task wakes up on a
> CPU and when a RQ is (un)throttled. This is mainly due to the current
> integration strategy, which relies on updates being triggered implicitly
> each time a cfs_rq's utilization is updated.

To me that's not implicit, but is explicitly done when the util is
updated. It seems that's the logical place..

>
> Those updates are currently provided (mainly) via
>cfs_rq_util_change()
> which is used in:
>  - update_cfs_rq_load_avg()
>when the utilization of a cfs_rq is updated
>  - {attach,detach}_entity_load_avg()
> This is done based on the idea that "we should callback schedutil
> frequently enough" to properly update the CPU frequency at every

This also I didn't get, its not called "frequently enough" but at the
right time (when the util is updated).

> utilization change.
>
> Since this recent schedutil update:
>
>   commit 8f111bc357aa ("cpufreq/schedutil: Rewrite CPUFREQ_RT support")
>
> we use additional RQ information to properly account for FAIR tasks
> utilization. Specifically, cfs_rq::h_nr_running has to be non-zero
> in sugov_aggregate_util() to sum up the cfs_rq's utilization.
>
> However, cfs_rq::h_nr_running is usually updated as:
>
> enqueue_entity()
>...
>update_load_avg()
>   ...
>   cfs_rq_util_change ==> trigger schedutil update
> ...
> cfs_rq->h_nr_running += number_of_tasks
>
> both in enqueue_task_fair() as well as in unthrottle_cfs_rq().
> A similar pattern is used also in dequeue_task_fair() and
> throttle_cfs_rq() to remove tasks.

Maybe this should be fixed then? It seems broken to begin with...

>
> This means that we are likely to see a zero cfs_rq utilization when we
> enqueue a task on an empty CPU, or a non-zero cfs_rq utilization when
> instead, for example, we are throttling all the FAIR tasks of a CPU.
>
> While the second issue is less important, since we are less likely to

Actually I wanted behavior like the second issue, because that means
frequency will not be dropped if CPU is about to idle which is similar
to a patch I sent long time ago (skip freq update if CPU about to
idle).

> reduce frequency when CPU utilization decreases, the first issue can
> instead impact performance. Indeed, we potentially introduce a not desired

This issue would be fixed by just fixing the h_nr_running bug right?

> latency between a task enqueue on a CPU and its frequency increase.
>
> Another possible unwanted side effect is the iowait boosting of a CPU
> when we enqueue a task into a throttled cfs_rq.

Probably very very rare.

> Moreover, the current schedutil integration has these other downsides:
>
>  - schedutil updates are triggered by RQ's load updates, which makes
>sense in general but it does not allow to know exactly which other RQ
>related information has been updated (e.g. h_nr_running).

It seems broken that all information that schedutil needs isn't
updated _before_ the cpufreq update request, so that should be fixed
instead?

>
>  - increasing the chances to update schedutil does not always correspond
>to provide the most accurate information for a proper frequency
>selection, thus we can skip some updates.

Again IMO its just updated at the right time already, not just
frequently enough...

>
>  - we don't know exactly at which point a schedutil update is triggered,
>and thus potentially a frequency change started, and that's because
>the update is a side effect of cfs_rq_util_changed instead of an
>explicit call from the most suitable call path.
>
>  - cfs_rq_util_change() is mainly a wrapper function for an already
>existing "public API", cpufreq_update_util(), to ensure we actually
>update schedutil only when we are updating a root RQ. Thus, especially
>when task groups are in use, most of the calls to this wrapper
>function are really not required.
>
>  - the usage of a wrapper function is not completely consistent across
>fair.c, since we still need sometimes additional explicit calls to
>cpufreq_update_util(), for example to support the IOWAIT boot flag in
>the wakeup path
>
>  - it makes it hard to integrate new features since it could require to
>change other function prototypes just to pass in an additional flag,
>as it happened for example here:
>
>   commit ea14b57e8a18 ("sched/cpufreq: Provide migration hint")
>
> All the above considered, let's try to make schedutil updates more
> explicit in fair.c by:
>
>  - removing the cfs_rq_util_change() wrapper function to use the
>cpufreq_update_util() public API only when root cfs_rq is updated
>
>  - remove indirect and side-effect (sometimes not required) schedutil
>updates when the cfs_rq utilization is updated
>
>  - call cpufreq_update_util() explicitly in the few call sites where it
>really makes sense and all the required information has been 

Re: 4.15.14 crash with iscsi target and dvd

[Wakko Warner]

Bart Van Assche wrote:
> On Thu, 2018-04-05 at 22:06 -0400, Wakko Warner wrote:
> > I know now why scsi_print_command isn't doing anything.  cmd->cmnd is null.
> > I added a dev_printk in scsi_print_command where the 2 if statements return.
> > Logs:
> > [  29.866415] sr 3:0:0:0: cmd->cmnd is NULL
> 
> That's something that should never happen. As one can see in
> scsi_setup_scsi_cmnd() and scsi_setup_fs_cmnd() both functions initialize
> that pointer. Since I have not yet been able to reproduce myself what you
> reported, would it be possible for you to bisect this issue? You will need
> to follow something like the following procedure (see also
> https://git-scm.com/docs/git-bisect):

I don't know how relevent it is, but this machine boots nfs and exports it's
dvd drives over iscsi with the target modules.  My scsi_target.lio is at the
end.  I removed the iqn name.  The options are default except for a few. 
Non default options I tabbed over.
eth0 is the nfs/localnet nic and eth1 is the
nic that iscsi goes over.
eth0 is onboard pci 8086:1502 (subsystem 1028:05d3)
eth1 is pci 8086:107d (subsystem 8086:1084)
Both use the e1000e driver

The initiator is running 4.4.107.
When running on the initiator, /dev/sr1 is the target /dev/sr0.  Therefor
cat /dev/sr1 > /dev/null seems to work.
mount /dev/sr1 /cdrom works
find /cdrom -type f | xargs cat > /dev/null immediately crashes the target.
Burning to /dev/sr1 seems to work.

I have another nic that uses igb instead, I'll see if that makes a
difference.

> git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git bisect start
> git bisect bad v4.10
> git bisect good v4.9
> 
> and then build the kernel, install it, boot the kernel and test it.
> Depending on the result, run either git bisect bad or git bisect good and
> keep going until git bisect comes to a conclusion. This can take an hour or
> more.

I'll try this.

Here's my scsi_target.lio:
storage pscsi {
disk dvd0 {
path /dev/sr0 
attribute {
emulate_3pc yes 
emulate_caw yes 
emulate_dpo no 
emulate_fua_read no 
emulate_model_alias no 
emulate_rest_reord no 
emulate_tas yes 
emulate_tpu no 
emulate_tpws no 
emulate_ua_intlck_ctrl no 
emulate_write_cache no 
enforce_pr_isids yes 
fabric_max_sectors 8192 
is_nonrot yes 
max_unmap_block_desc_count 0 
max_unmap_lba_count 0 
max_write_same_len 65535 
queue_depth 128 
unmap_granularity 0 
unmap_granularity_alignment 0 
}
}
disk dvd1 {
path /dev/sr1 
attribute {
emulate_3pc yes 
emulate_caw yes 
emulate_dpo no 
emulate_fua_read no 
emulate_model_alias no 
emulate_rest_reord no 
emulate_tas yes 
emulate_tpu no 
emulate_tpws no 
emulate_ua_intlck_ctrl no 
emulate_write_cache no 
enforce_pr_isids yes 
fabric_max_sectors 8192 
is_nonrot yes 
max_unmap_block_desc_count 0 
max_unmap_lba_count 0 
max_write_same_len 65535 
queue_depth 128 
unmap_granularity 0 
unmap_granularity_alignment 0 
}
}
disk dvd2 {
path /dev/sr2 
attribute {
emulate_3pc yes 
emulate_caw yes 
emulate_dpo no 
emulate_fua_read no 
emulate_model_alias no 
emulate_rest_reord no 
emulate_tas yes 
emulate_tpu no 
emulate_tpws no 
emulate_ua_intlck_ctrl no 
emulate_write_cache no 
enforce_pr_isids yes 
fabric_max_sectors 8192 
is_nonrot yes 
max_unmap_block_desc_count 0 
max_unmap_lba_count 0 
max_write_same_len 65535 
queue_depth 128 
unmap_granularity 0 
unmap_granularity_alignment 0 
}
}
}
fabric iscsi {
discovery_auth {
enable no 
mutual_password "" 
mutual_userid "" 
password "" 
userid "" 
}
target iqn.:dvd tpgt 1 {
enable yes 
attribute {
authentication no 
cache_dynamic_acls yes 
default_cmdsn_depth 64 
default_erl 0 
demo_mode_discovery yes 
demo_mode_write_protect no 
fabric_prot_type 0 
generate_node_acls yes 
login_timeout 15 
netif_timeout 2 
prod_mode_write_protect no 
t10_pi 0 
tpg_enabled_sendtargets 1 
}
auth {
password "" 
password_mutual "" 
userid "" 
userid_mutual "" 
}
parameter {
AuthMethod "CHAP,None" 
DataDigest "CRC32C,None" 
DataPDUInOrder yes 
DataSequenceInOrder yes 
DefaultTime2Retain 20 
DefaultTime2Wait 2 
ErrorRecoveryLevel no 
FirstBurstLength 65536 
HeaderDigest "CRC32C,None" 
IFMarkInt Reject 
IFMarker no 
ImmediateData yes 
InitialR2T yes 
MaxBurstLength 262144 
MaxConnections 1 
MaxOutstandingR2T 1 
MaxRecvDataSegmentLength 8192 
MaxXmitDataSegmentLength 262144 
OFMarkInt Reject 
OFMarker no 
TargetAlias "LIO Target" 
}
lun 0 backend pscsi:dvd0 
lun 1 backend pscsi:dvd1 
lun 2 backend pscsi:dvd2 
portal 0.0.0.0:3260 
}
}


-- 
 Microsoft has beaten Volkswagen's world record.  Volkswagen only created 22
 million bugs.

Re: 4.15.14 crash with iscsi target and dvd

[Wakko Warner]

Bart Van Assche wrote:
> On Thu, 2018-04-05 at 22:06 -0400, Wakko Warner wrote:
> > I know now why scsi_print_command isn't doing anything.  cmd->cmnd is null.
> > I added a dev_printk in scsi_print_command where the 2 if statements return.
> > Logs:
> > [  29.866415] sr 3:0:0:0: cmd->cmnd is NULL
> 
> That's something that should never happen. As one can see in
> scsi_setup_scsi_cmnd() and scsi_setup_fs_cmnd() both functions initialize
> that pointer. Since I have not yet been able to reproduce myself what you
> reported, would it be possible for you to bisect this issue? You will need
> to follow something like the following procedure (see also
> https://git-scm.com/docs/git-bisect):

I don't know how relevent it is, but this machine boots nfs and exports it's
dvd drives over iscsi with the target modules.  My scsi_target.lio is at the
end.  I removed the iqn name.  The options are default except for a few. 
Non default options I tabbed over.
eth0 is the nfs/localnet nic and eth1 is the
nic that iscsi goes over.
eth0 is onboard pci 8086:1502 (subsystem 1028:05d3)
eth1 is pci 8086:107d (subsystem 8086:1084)
Both use the e1000e driver

The initiator is running 4.4.107.
When running on the initiator, /dev/sr1 is the target /dev/sr0.  Therefor
cat /dev/sr1 > /dev/null seems to work.
mount /dev/sr1 /cdrom works
find /cdrom -type f | xargs cat > /dev/null immediately crashes the target.
Burning to /dev/sr1 seems to work.

I have another nic that uses igb instead, I'll see if that makes a
difference.

> git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git bisect start
> git bisect bad v4.10
> git bisect good v4.9
> 
> and then build the kernel, install it, boot the kernel and test it.
> Depending on the result, run either git bisect bad or git bisect good and
> keep going until git bisect comes to a conclusion. This can take an hour or
> more.

I'll try this.

Here's my scsi_target.lio:
storage pscsi {
disk dvd0 {
path /dev/sr0 
attribute {
emulate_3pc yes 
emulate_caw yes 
emulate_dpo no 
emulate_fua_read no 
emulate_model_alias no 
emulate_rest_reord no 
emulate_tas yes 
emulate_tpu no 
emulate_tpws no 
emulate_ua_intlck_ctrl no 
emulate_write_cache no 
enforce_pr_isids yes 
fabric_max_sectors 8192 
is_nonrot yes 
max_unmap_block_desc_count 0 
max_unmap_lba_count 0 
max_write_same_len 65535 
queue_depth 128 
unmap_granularity 0 
unmap_granularity_alignment 0 
}
}
disk dvd1 {
path /dev/sr1 
attribute {
emulate_3pc yes 
emulate_caw yes 
emulate_dpo no 
emulate_fua_read no 
emulate_model_alias no 
emulate_rest_reord no 
emulate_tas yes 
emulate_tpu no 
emulate_tpws no 
emulate_ua_intlck_ctrl no 
emulate_write_cache no 
enforce_pr_isids yes 
fabric_max_sectors 8192 
is_nonrot yes 
max_unmap_block_desc_count 0 
max_unmap_lba_count 0 
max_write_same_len 65535 
queue_depth 128 
unmap_granularity 0 
unmap_granularity_alignment 0 
}
}
disk dvd2 {
path /dev/sr2 
attribute {
emulate_3pc yes 
emulate_caw yes 
emulate_dpo no 
emulate_fua_read no 
emulate_model_alias no 
emulate_rest_reord no 
emulate_tas yes 
emulate_tpu no 
emulate_tpws no 
emulate_ua_intlck_ctrl no 
emulate_write_cache no 
enforce_pr_isids yes 
fabric_max_sectors 8192 
is_nonrot yes 
max_unmap_block_desc_count 0 
max_unmap_lba_count 0 
max_write_same_len 65535 
queue_depth 128 
unmap_granularity 0 
unmap_granularity_alignment 0 
}
}
}
fabric iscsi {
discovery_auth {
enable no 
mutual_password "" 
mutual_userid "" 
password "" 
userid "" 
}
target iqn.:dvd tpgt 1 {
enable yes 
attribute {
authentication no 
cache_dynamic_acls yes 
default_cmdsn_depth 64 
default_erl 0 
demo_mode_discovery yes 
demo_mode_write_protect no 
fabric_prot_type 0 
generate_node_acls yes 
login_timeout 15 
netif_timeout 2 
prod_mode_write_protect no 
t10_pi 0 
tpg_enabled_sendtargets 1 
}
auth {
password "" 
password_mutual "" 
userid "" 
userid_mutual "" 
}
parameter {
AuthMethod "CHAP,None" 
DataDigest "CRC32C,None" 
DataPDUInOrder yes 
DataSequenceInOrder yes 
DefaultTime2Retain 20 
DefaultTime2Wait 2 
ErrorRecoveryLevel no 
FirstBurstLength 65536 
HeaderDigest "CRC32C,None" 
IFMarkInt Reject 
IFMarker no 
ImmediateData yes 
InitialR2T yes 
MaxBurstLength 262144 
MaxConnections 1 
MaxOutstandingR2T 1 
MaxRecvDataSegmentLength 8192 
MaxXmitDataSegmentLength 262144 
OFMarkInt Reject 
OFMarker no 
TargetAlias "LIO Target" 
}
lun 0 backend pscsi:dvd0 
lun 1 backend pscsi:dvd1 
lun 2 backend pscsi:dvd2 
portal 0.0.0.0:3260 
}
}


-- 
 Microsoft has beaten Volkswagen's world record.  Volkswagen only created 22
 million bugs.

Re: [PATCH] blk-cgroup: remove entries in blkg_tree before queue release

[kbuild test robot]

Hi Alexandru,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on block/for-next]
[also build test ERROR on v4.16 next-20180406]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Alexandru-Moise/blk-cgroup-remove-entries-in-blkg_tree-before-queue-release/20180407-035957
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git 
for-next
config: c6x-evmc6678_defconfig (attached as .config)
compiler: c6x-elf-gcc (GCC) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=c6x 

All errors (new ones prefixed by >>):

   block/blk-sysfs.c: In function '__blk_release_queue':
>> block/blk-sysfs.c:820:2: error: implicit declaration of function 
>> 'blkg_destroy_all'; did you mean 'irq_destroy_ipi'? 
>> [-Werror=implicit-function-declaration]
 blkg_destroy_all(q);
 ^~~~
 irq_destroy_ipi
   cc1: some warnings being treated as errors

vim +820 block/blk-sysfs.c

   770  
   771  /**
   772   * __blk_release_queue - release a request queue when it is no longer 
needed
   773   * @work: pointer to the release_work member of the request queue to be 
released
   774   *
   775   * Description:
   776   * blk_release_queue is the counterpart of blk_init_queue(). It 
should be
   777   * called when a request queue is being released; typically when a 
block
   778   * device is being de-registered. Its primary task it to free the 
queue
   779   * itself.
   780   *
   781   * Notes:
   782   * The low level driver must have finished any outstanding requests 
first
   783   * via blk_cleanup_queue().
   784   *
   785   * Although blk_release_queue() may be called with preemption 
disabled,
   786   * __blk_release_queue() may sleep.
   787   */
   788  static void __blk_release_queue(struct work_struct *work)
   789  {
   790  struct request_queue *q = container_of(work, typeof(*q), 
release_work);
   791  
   792  if (test_bit(QUEUE_FLAG_POLL_STATS, >queue_flags))
   793  blk_stat_remove_callback(q, q->poll_cb);
   794  blk_stat_free_callback(q->poll_cb);
   795  
   796  blk_free_queue_stats(q->stats);
   797  
   798  blk_exit_rl(q, >root_rl);
   799  
   800  if (q->queue_tags)
   801  __blk_queue_free_tags(q);
   802  
   803  if (!q->mq_ops) {
   804  if (q->exit_rq_fn)
   805  q->exit_rq_fn(q, q->fq->flush_rq);
   806  blk_free_flush_queue(q->fq);
   807  } else {
   808  blk_mq_release(q);
   809  }
   810  
   811  blk_trace_shutdown(q);
   812  
   813  if (q->mq_ops)
   814  blk_mq_debugfs_unregister(q);
   815  
   816  if (q->bio_split)
   817  bioset_free(q->bio_split);
   818  
   819  spin_lock_irq(q->queue_lock);
 > 820  blkg_destroy_all(q);
   821  spin_unlock_irq(q->queue_lock);
   822  
   823  ida_simple_remove(_queue_ida, q->id);
   824  call_rcu(>rcu_head, blk_free_queue_rcu);
   825  }
   826  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH] blk-cgroup: remove entries in blkg_tree before queue release

[kbuild test robot]

Hi Alexandru,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on block/for-next]
[also build test ERROR on v4.16 next-20180406]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Alexandru-Moise/blk-cgroup-remove-entries-in-blkg_tree-before-queue-release/20180407-035957
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git 
for-next
config: c6x-evmc6678_defconfig (attached as .config)
compiler: c6x-elf-gcc (GCC) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=c6x 

All errors (new ones prefixed by >>):

   block/blk-sysfs.c: In function '__blk_release_queue':
>> block/blk-sysfs.c:820:2: error: implicit declaration of function 
>> 'blkg_destroy_all'; did you mean 'irq_destroy_ipi'? 
>> [-Werror=implicit-function-declaration]
 blkg_destroy_all(q);
 ^~~~
 irq_destroy_ipi
   cc1: some warnings being treated as errors

vim +820 block/blk-sysfs.c

   770  
   771  /**
   772   * __blk_release_queue - release a request queue when it is no longer 
needed
   773   * @work: pointer to the release_work member of the request queue to be 
released
   774   *
   775   * Description:
   776   * blk_release_queue is the counterpart of blk_init_queue(). It 
should be
   777   * called when a request queue is being released; typically when a 
block
   778   * device is being de-registered. Its primary task it to free the 
queue
   779   * itself.
   780   *
   781   * Notes:
   782   * The low level driver must have finished any outstanding requests 
first
   783   * via blk_cleanup_queue().
   784   *
   785   * Although blk_release_queue() may be called with preemption 
disabled,
   786   * __blk_release_queue() may sleep.
   787   */
   788  static void __blk_release_queue(struct work_struct *work)
   789  {
   790  struct request_queue *q = container_of(work, typeof(*q), 
release_work);
   791  
   792  if (test_bit(QUEUE_FLAG_POLL_STATS, >queue_flags))
   793  blk_stat_remove_callback(q, q->poll_cb);
   794  blk_stat_free_callback(q->poll_cb);
   795  
   796  blk_free_queue_stats(q->stats);
   797  
   798  blk_exit_rl(q, >root_rl);
   799  
   800  if (q->queue_tags)
   801  __blk_queue_free_tags(q);
   802  
   803  if (!q->mq_ops) {
   804  if (q->exit_rq_fn)
   805  q->exit_rq_fn(q, q->fq->flush_rq);
   806  blk_free_flush_queue(q->fq);
   807  } else {
   808  blk_mq_release(q);
   809  }
   810  
   811  blk_trace_shutdown(q);
   812  
   813  if (q->mq_ops)
   814  blk_mq_debugfs_unregister(q);
   815  
   816  if (q->bio_split)
   817  bioset_free(q->bio_split);
   818  
   819  spin_lock_irq(q->queue_lock);
 > 820  blkg_destroy_all(q);
   821  spin_unlock_irq(q->queue_lock);
   822  
   823  ida_simple_remove(_queue_ida, q->id);
   824  call_rcu(>rcu_head, blk_free_queue_rcu);
   825  }
   826  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver

[Stephen Boyd]

Quoting Lina Iyer (2018-04-06 08:13:55)
> From: Mahesh Sivasubramanian 
> 
> Command DB is a simple database in the shared memory of QCOM SoCs, that
> provides information regarding shared resources. Some shared resources
> in the SoC have properties that are probed dynamically at boot by the
> remote processor. The information pertaining to the SoC and the platform
> are made available in the shared memory. Drivers can query this
> information using predefined strings.
> 
> Signed-off-by: Mahesh Sivasubramanian 
> Signed-off-by: Lina Iyer 
> Reviewed-by: Bjorn Andersson 
> ---

I have this patch on top to fix the endian stuff. Care to test it out
and see if it still works?

From: Stephen Boyd 
Subject: soc: qcom: cmd-db: Make endian-agnostic

This driver deals with memory that is stored in little-endian format.
Update the structures with the proper little-endian types and then
do the proper conversions when reading the fields. Note that we compare
the ids with a memcmp() because we already pad out the string 'id' field
to exactly 8 bytes with the strncpy() onto the stack.

Signed-off-by: Stephen Boyd 

diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
index b5172049f608..a56dc9edab82 100644
--- a/drivers/soc/qcom/cmd-db.c
+++ b/drivers/soc/qcom/cmd-db.c
@@ -13,18 +13,10 @@
 
 #define NUM_PRIORITY   2
 #define MAX_SLV_ID 8
-#define CMD_DB_MAGIC   0x0C0330DBUL
+static const char CMD_DB_MAGIC[] = { 0xdb, 0x33, 0x03, 0x0c };
 #define SLAVE_ID_MASK  0x7
 #define SLAVE_ID_SHIFT 16
 
-#define ENTRY_HEADER(hdr)  ((void *)cmd_db_header +\
-   sizeof(*cmd_db_header) +\
-   hdr->header_offset)
-
-#define RSC_OFFSET(hdr, ent)   ((void *)cmd_db_header +\
-   sizeof(*cmd_db_header) +\
-   hdr.data_offset + ent.offset)
-
 /**
  * struct entry_header: header for each entry in cmddb
  *
@@ -35,11 +27,11 @@
  * @offset: offset from :@data_offset, start of the data
  */
 struct entry_header {
-   u64 id;
-   u32 priority[NUM_PRIORITY];
-   u32 addr;
-   u16 len;
-   u16 offset;
+   u8 id[8];
+   __le32 priority[NUM_PRIORITY];
+   __le32 addr;
+   __le16 len;
+   __le16 offset;
 };
 
 /**
@@ -53,12 +45,12 @@ struct entry_header {
  * @reserved: reserved for future use.
  */
 struct rsc_hdr {
-   u16 slv_id;
-   u16 header_offset;
-   u16 data_offset;
-   u16 cnt;
-   u16 version;
-   u16 reserved[3];
+   __le16 slv_id;
+   __le16 header_offset;
+   __le16 data_offset;
+   __le16 cnt;
+   __le16 version;
+   __le16 reserved[3];
 };
 
 /**
@@ -72,11 +64,11 @@ struct rsc_hdr {
  * @data: driver specific data
  */
 struct cmd_db_header {
-   u32 version;
-   u32 magic_num;
+   __le32 version;
+   __le32 magic_num;
struct rsc_hdr header[MAX_SLV_ID];
-   u32 checksum;
-   u32 reserved;
+   __le32 checksum;
+   __le32 reserved;
u8 data[];
 };
 
@@ -101,6 +93,22 @@ struct cmd_db_header {
 
 static struct cmd_db_header *cmd_db_header;
 
+static inline void *rsc_to_entry_header(struct rsc_hdr *hdr)
+{
+   u16 offset = le16_to_cpu(hdr->header_offset);
+
+   return cmd_db_header->data + offset;
+}
+
+static inline void *
+rsc_offset(struct rsc_hdr *hdr, struct entry_header *ent)
+{
+   u16 offset = le16_to_cpu(hdr->header_offset);
+   u16 loffset = le16_to_cpu(ent->offset);
+
+   return cmd_db_header->data + offset +  loffset;
+}
+
 /**
  * cmd_db_ready - Indicates if command DB is available
  *
@@ -110,29 +118,20 @@ int cmd_db_ready(void)
 {
if (cmd_db_header == NULL)
return -EPROBE_DEFER;
-   else if (cmd_db_header->magic_num != CMD_DB_MAGIC)
+   else if (memcmp(_db_header->magic_num, CMD_DB_MAGIC, 
sizeof(CMD_DB_MAGIC)))
return -EINVAL;
 
return 0;
 }
 EXPORT_SYMBOL(cmd_db_ready);
 
-static u64 cmd_db_get_u64_id(const char *id)
-{
-   u64 rsc_id = 0;
-   u8 *ch = (u8 *)_id;
-
-   strncpy(ch, id, min(strlen(id), sizeof(rsc_id)));
-
-   return rsc_id;
-}
-
-static int cmd_db_get_header(u64 query, struct entry_header *eh,
+static int cmd_db_get_header(const char *id, struct entry_header *eh,
 struct rsc_hdr *rh)
 {
struct rsc_hdr *rsc_hdr;
struct entry_header *ent;
int ret, i, j;
+   char query[8];
 
ret = cmd_db_ready();
if (ret)
@@ -141,18 +140,21 @@ static int cmd_db_get_header(u64 query, struct 
entry_header *eh,
if (!eh || !rh)
return -EINVAL;
 
+   /* Pad out query string to same length as in DB */
+   strncpy(query, id, sizeof(query));
+
for (i = 

[PATCH v6] earlycon: Use a pointer table to fix __earlycon_table stride

[Daniel Kurtz]

Commit 99492c39f39f ("earlycon: Fix __earlycon_table stride") tried to fix
__earlycon_table stride by forcing the earlycon_id struct alignment to 32
and asking the linker to 32-byte align the __earlycon_table symbol.  This
fix was based on commit 07fca0e57fca92 ("tracing: Properly align linker
defined symbols") which tried a similar fix for the tracing subsystem.

However, this fix doesn't quite work because there is no guarantee that
gcc will place structures packed into an array format.  In fact, gcc 4.9
chooses to 64-byte align these structs by inserting additional padding
between the entries because it has no clue that they are supposed to be in
an array.  If we are unlucky, the linker will assign symbol
"__earlycon_table" to a 32-byte aligned address which does not correspond
to the 64-byte aligned contents of section "__earlycon_table".

To address this same problem, the fix to the tracing system was
subsequently re-implemented using a more robust table of pointers approach
by commits:
 3d56e331b653 ("tracing: Replace syscall_meta_data struct array with pointer 
array")
 654986462939 ("tracepoints: Fix section alignment using pointer array")
 e4a9ea5ee7c8 ("tracing: Replace trace_event struct array with pointer array")

Let's use this same "array of pointers to structs" approach for
EARLYCON_TABLE.

Fixes: 99492c39f39f ("earlycon: Fix __earlycon_table stride")
Signed-off-by: Daniel Kurtz 
Suggested-by: Aaron Durbin 
Reviewed-by: Rob Herring 
Tested-by: Guenter Roeck 
Reviewed-by: Guenter Roeck 
---
Changes since v2:
 * Use __initconst instead of __initdata to avoid h8300 and alpha kbuild errors

Changes since v3:
 * Fixed typos in commit message

Changes since v4:
 * removed Change-Id: from commit message

Changes since v5:
 * None, just re-posting per request from greg k-h

 drivers/of/fdt.c  |  7 +--
 drivers/tty/serial/earlycon.c |  6 --
 include/asm-generic/vmlinux.lds.h |  2 +-
 include/linux/serial_core.h   | 21 ++---
 4 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 84aa9d676375..6da20b9688f7 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -942,7 +942,7 @@ int __init early_init_dt_scan_chosen_stdout(void)
int offset;
const char *p, *q, *options = NULL;
int l;
-   const struct earlycon_id *match;
+   const struct earlycon_id **p_match;
const void *fdt = initial_boot_params;
 
offset = fdt_path_offset(fdt, "/chosen");
@@ -969,7 +969,10 @@ int __init early_init_dt_scan_chosen_stdout(void)
return 0;
}
 
-   for (match = __earlycon_table; match < __earlycon_table_end; match++) {
+   for (p_match = __earlycon_table; p_match < __earlycon_table_end;
+p_match++) {
+   const struct earlycon_id *match = *p_match;
+
if (!match->compatible[0])
continue;
 
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index a24278380fec..22683393a0f2 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -169,7 +169,7 @@ static int __init register_earlycon(char *buf, const struct 
earlycon_id *match)
  */
 int __init setup_earlycon(char *buf)
 {
-   const struct earlycon_id *match;
+   const struct earlycon_id **p_match;
 
if (!buf || !buf[0])
return -EINVAL;
@@ -177,7 +177,9 @@ int __init setup_earlycon(char *buf)
if (early_con.flags & CON_ENABLED)
return -EALREADY;
 
-   for (match = __earlycon_table; match < __earlycon_table_end; match++) {
+   for (p_match = __earlycon_table; p_match < __earlycon_table_end;
+p_match++) {
+   const struct earlycon_id *match = *p_match;
size_t len = strlen(match->name);
 
if (strncmp(buf, match->name, len))
diff --git a/include/asm-generic/vmlinux.lds.h 
b/include/asm-generic/vmlinux.lds.h
index 278841c75b97..af240573e482 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -188,7 +188,7 @@
 #endif
 
 #ifdef CONFIG_SERIAL_EARLYCON
-#define EARLYCON_TABLE() STRUCT_ALIGN();   \
+#define EARLYCON_TABLE() . = ALIGN(8); \
 VMLINUX_SYMBOL(__earlycon_table) = .;  \
 KEEP(*(__earlycon_table))  \
 VMLINUX_SYMBOL(__earlycon_table_end) = .;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 1d356105f25a..b4c9fda9d833 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -351,10 +351,10 @@ struct earlycon_id {
charname[16];
charcompatible[128];
int (*setup)(struct earlycon_device *, const char *options);
-} __aligned(32);
+};
 

Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver

[Stephen Boyd]

Quoting Lina Iyer (2018-04-06 08:13:55)
> From: Mahesh Sivasubramanian 
> 
> Command DB is a simple database in the shared memory of QCOM SoCs, that
> provides information regarding shared resources. Some shared resources
> in the SoC have properties that are probed dynamically at boot by the
> remote processor. The information pertaining to the SoC and the platform
> are made available in the shared memory. Drivers can query this
> information using predefined strings.
> 
> Signed-off-by: Mahesh Sivasubramanian 
> Signed-off-by: Lina Iyer 
> Reviewed-by: Bjorn Andersson 
> ---

I have this patch on top to fix the endian stuff. Care to test it out
and see if it still works?

From: Stephen Boyd 
Subject: soc: qcom: cmd-db: Make endian-agnostic

This driver deals with memory that is stored in little-endian format.
Update the structures with the proper little-endian types and then
do the proper conversions when reading the fields. Note that we compare
the ids with a memcmp() because we already pad out the string 'id' field
to exactly 8 bytes with the strncpy() onto the stack.

Signed-off-by: Stephen Boyd 

diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
index b5172049f608..a56dc9edab82 100644
--- a/drivers/soc/qcom/cmd-db.c
+++ b/drivers/soc/qcom/cmd-db.c
@@ -13,18 +13,10 @@
 
 #define NUM_PRIORITY   2
 #define MAX_SLV_ID 8
-#define CMD_DB_MAGIC   0x0C0330DBUL
+static const char CMD_DB_MAGIC[] = { 0xdb, 0x33, 0x03, 0x0c };
 #define SLAVE_ID_MASK  0x7
 #define SLAVE_ID_SHIFT 16
 
-#define ENTRY_HEADER(hdr)  ((void *)cmd_db_header +\
-   sizeof(*cmd_db_header) +\
-   hdr->header_offset)
-
-#define RSC_OFFSET(hdr, ent)   ((void *)cmd_db_header +\
-   sizeof(*cmd_db_header) +\
-   hdr.data_offset + ent.offset)
-
 /**
  * struct entry_header: header for each entry in cmddb
  *
@@ -35,11 +27,11 @@
  * @offset: offset from :@data_offset, start of the data
  */
 struct entry_header {
-   u64 id;
-   u32 priority[NUM_PRIORITY];
-   u32 addr;
-   u16 len;
-   u16 offset;
+   u8 id[8];
+   __le32 priority[NUM_PRIORITY];
+   __le32 addr;
+   __le16 len;
+   __le16 offset;
 };
 
 /**
@@ -53,12 +45,12 @@ struct entry_header {
  * @reserved: reserved for future use.
  */
 struct rsc_hdr {
-   u16 slv_id;
-   u16 header_offset;
-   u16 data_offset;
-   u16 cnt;
-   u16 version;
-   u16 reserved[3];
+   __le16 slv_id;
+   __le16 header_offset;
+   __le16 data_offset;
+   __le16 cnt;
+   __le16 version;
+   __le16 reserved[3];
 };
 
 /**
@@ -72,11 +64,11 @@ struct rsc_hdr {
  * @data: driver specific data
  */
 struct cmd_db_header {
-   u32 version;
-   u32 magic_num;
+   __le32 version;
+   __le32 magic_num;
struct rsc_hdr header[MAX_SLV_ID];
-   u32 checksum;
-   u32 reserved;
+   __le32 checksum;
+   __le32 reserved;
u8 data[];
 };
 
@@ -101,6 +93,22 @@ struct cmd_db_header {
 
 static struct cmd_db_header *cmd_db_header;
 
+static inline void *rsc_to_entry_header(struct rsc_hdr *hdr)
+{
+   u16 offset = le16_to_cpu(hdr->header_offset);
+
+   return cmd_db_header->data + offset;
+}
+
+static inline void *
+rsc_offset(struct rsc_hdr *hdr, struct entry_header *ent)
+{
+   u16 offset = le16_to_cpu(hdr->header_offset);
+   u16 loffset = le16_to_cpu(ent->offset);
+
+   return cmd_db_header->data + offset +  loffset;
+}
+
 /**
  * cmd_db_ready - Indicates if command DB is available
  *
@@ -110,29 +118,20 @@ int cmd_db_ready(void)
 {
if (cmd_db_header == NULL)
return -EPROBE_DEFER;
-   else if (cmd_db_header->magic_num != CMD_DB_MAGIC)
+   else if (memcmp(_db_header->magic_num, CMD_DB_MAGIC, 
sizeof(CMD_DB_MAGIC)))
return -EINVAL;
 
return 0;
 }
 EXPORT_SYMBOL(cmd_db_ready);
 
-static u64 cmd_db_get_u64_id(const char *id)
-{
-   u64 rsc_id = 0;
-   u8 *ch = (u8 *)_id;
-
-   strncpy(ch, id, min(strlen(id), sizeof(rsc_id)));
-
-   return rsc_id;
-}
-
-static int cmd_db_get_header(u64 query, struct entry_header *eh,
+static int cmd_db_get_header(const char *id, struct entry_header *eh,
 struct rsc_hdr *rh)
 {
struct rsc_hdr *rsc_hdr;
struct entry_header *ent;
int ret, i, j;
+   char query[8];
 
ret = cmd_db_ready();
if (ret)
@@ -141,18 +140,21 @@ static int cmd_db_get_header(u64 query, struct 
entry_header *eh,
if (!eh || !rh)
return -EINVAL;
 
+   /* Pad out query string to same length as in DB */
+   strncpy(query, id, sizeof(query));
+
for (i = 0; i < MAX_SLV_ID; i++) {
rsc_hdr = _db_header->header[i];
if (!rsc_hdr->slv_id)

[PATCH v6] earlycon: Use a pointer table to fix __earlycon_table stride

[Daniel Kurtz]

Commit 99492c39f39f ("earlycon: Fix __earlycon_table stride") tried to fix
__earlycon_table stride by forcing the earlycon_id struct alignment to 32
and asking the linker to 32-byte align the __earlycon_table symbol.  This
fix was based on commit 07fca0e57fca92 ("tracing: Properly align linker
defined symbols") which tried a similar fix for the tracing subsystem.

However, this fix doesn't quite work because there is no guarantee that
gcc will place structures packed into an array format.  In fact, gcc 4.9
chooses to 64-byte align these structs by inserting additional padding
between the entries because it has no clue that they are supposed to be in
an array.  If we are unlucky, the linker will assign symbol
"__earlycon_table" to a 32-byte aligned address which does not correspond
to the 64-byte aligned contents of section "__earlycon_table".

To address this same problem, the fix to the tracing system was
subsequently re-implemented using a more robust table of pointers approach
by commits:
 3d56e331b653 ("tracing: Replace syscall_meta_data struct array with pointer 
array")
 654986462939 ("tracepoints: Fix section alignment using pointer array")
 e4a9ea5ee7c8 ("tracing: Replace trace_event struct array with pointer array")

Let's use this same "array of pointers to structs" approach for
EARLYCON_TABLE.

Fixes: 99492c39f39f ("earlycon: Fix __earlycon_table stride")
Signed-off-by: Daniel Kurtz 
Suggested-by: Aaron Durbin 
Reviewed-by: Rob Herring 
Tested-by: Guenter Roeck 
Reviewed-by: Guenter Roeck 
---
Changes since v2:
 * Use __initconst instead of __initdata to avoid h8300 and alpha kbuild errors

Changes since v3:
 * Fixed typos in commit message

Changes since v4:
 * removed Change-Id: from commit message

Changes since v5:
 * None, just re-posting per request from greg k-h

 drivers/of/fdt.c  |  7 +--
 drivers/tty/serial/earlycon.c |  6 --
 include/asm-generic/vmlinux.lds.h |  2 +-
 include/linux/serial_core.h   | 21 ++---
 4 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 84aa9d676375..6da20b9688f7 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -942,7 +942,7 @@ int __init early_init_dt_scan_chosen_stdout(void)
int offset;
const char *p, *q, *options = NULL;
int l;
-   const struct earlycon_id *match;
+   const struct earlycon_id **p_match;
const void *fdt = initial_boot_params;
 
offset = fdt_path_offset(fdt, "/chosen");
@@ -969,7 +969,10 @@ int __init early_init_dt_scan_chosen_stdout(void)
return 0;
}
 
-   for (match = __earlycon_table; match < __earlycon_table_end; match++) {
+   for (p_match = __earlycon_table; p_match < __earlycon_table_end;
+p_match++) {
+   const struct earlycon_id *match = *p_match;
+
if (!match->compatible[0])
continue;
 
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index a24278380fec..22683393a0f2 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -169,7 +169,7 @@ static int __init register_earlycon(char *buf, const struct 
earlycon_id *match)
  */
 int __init setup_earlycon(char *buf)
 {
-   const struct earlycon_id *match;
+   const struct earlycon_id **p_match;
 
if (!buf || !buf[0])
return -EINVAL;
@@ -177,7 +177,9 @@ int __init setup_earlycon(char *buf)
if (early_con.flags & CON_ENABLED)
return -EALREADY;
 
-   for (match = __earlycon_table; match < __earlycon_table_end; match++) {
+   for (p_match = __earlycon_table; p_match < __earlycon_table_end;
+p_match++) {
+   const struct earlycon_id *match = *p_match;
size_t len = strlen(match->name);
 
if (strncmp(buf, match->name, len))
diff --git a/include/asm-generic/vmlinux.lds.h 
b/include/asm-generic/vmlinux.lds.h
index 278841c75b97..af240573e482 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -188,7 +188,7 @@
 #endif
 
 #ifdef CONFIG_SERIAL_EARLYCON
-#define EARLYCON_TABLE() STRUCT_ALIGN();   \
+#define EARLYCON_TABLE() . = ALIGN(8); \
 VMLINUX_SYMBOL(__earlycon_table) = .;  \
 KEEP(*(__earlycon_table))  \
 VMLINUX_SYMBOL(__earlycon_table_end) = .;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 1d356105f25a..b4c9fda9d833 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -351,10 +351,10 @@ struct earlycon_id {
charname[16];
charcompatible[128];
int (*setup)(struct earlycon_device *, const char *options);
-} __aligned(32);
+};
 
-extern const struct earlycon_id __earlycon_table[];
-extern const struct earlycon_id 

Re: [PATCH v7 [RESEND] 2/2] dt-bindings: introduce Command DB for QCOM SoCs

[Stephen Boyd]

Quoting Lina Iyer (2018-04-06 08:13:56)
> From: Mahesh Sivasubramanian 
> 
> Command DB provides information on shared resources like clocks,
> regulators etc., probed at boot by the remote subsytem and made
> available in shared memory.
> 
> Cc: devicet...@vger.kernel.org
> Signed-off-by: Mahesh Sivasubramanian 
> Signed-off-by: Lina Iyer 
> Reviewed-by: Bjorn Andersson 
> ---
> 

Reviewed-by: Stephen Boyd 

> +   reserved-memory {
> +   [...]
> +   qcom,cmd-db@85fe {

Nitpick: This may want to be called 'memory@85fe' because we prefer
generic node names.

Re: [PATCH v7 [RESEND] 2/2] dt-bindings: introduce Command DB for QCOM SoCs

[Stephen Boyd]

Quoting Lina Iyer (2018-04-06 08:13:56)
> From: Mahesh Sivasubramanian 
> 
> Command DB provides information on shared resources like clocks,
> regulators etc., probed at boot by the remote subsytem and made
> available in shared memory.
> 
> Cc: devicet...@vger.kernel.org
> Signed-off-by: Mahesh Sivasubramanian 
> Signed-off-by: Lina Iyer 
> Reviewed-by: Bjorn Andersson 
> ---
> 

Reviewed-by: Stephen Boyd 

> +   reserved-memory {
> +   [...]
> +   qcom,cmd-db@85fe {

Nitpick: This may want to be called 'memory@85fe' because we prefer
generic node names.

Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver

[Stephen Boyd]

Quoting Lina Iyer (2018-04-06 08:13:55)
> From: Mahesh Sivasubramanian 
> 
> Command DB is a simple database in the shared memory of QCOM SoCs, that
> provides information regarding shared resources. Some shared resources
> in the SoC have properties that are probed dynamically at boot by the
> remote processor. The information pertaining to the SoC and the platform
> are made available in the shared memory. Drivers can query this
> information using predefined strings.
> 
> Signed-off-by: Mahesh Sivasubramanian 
> Signed-off-by: Lina Iyer 
> Reviewed-by: Bjorn Andersson 
> ---

Reviewed-by: Stephen Boyd 

Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver

[Stephen Boyd]

Quoting Lina Iyer (2018-04-06 08:13:55)
> From: Mahesh Sivasubramanian 
> 
> Command DB is a simple database in the shared memory of QCOM SoCs, that
> provides information regarding shared resources. Some shared resources
> in the SoC have properties that are probed dynamically at boot by the
> remote processor. The information pertaining to the SoC and the platform
> are made available in the shared memory. Drivers can query this
> information using predefined strings.
> 
> Signed-off-by: Mahesh Sivasubramanian 
> Signed-off-by: Lina Iyer 
> Reviewed-by: Bjorn Andersson 
> ---

Reviewed-by: Stephen Boyd 

Re: [PATCH v3 1/4] Ktest: add email support

[Tim Tianyang Chen]

I just fixed them up and pulled them in myself. ;-)

I also added the following on top of them (and testing this, live while
testing ftrace patches and other builds).

Thanks Steve!

diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
index 30a4c053f98b..837fa75cbb47 100755
--- a/tools/testing/ktest/ktest.pl
+++ b/tools/testing/ktest/ktest.pl
@@ -23,7 +23,7 @@ my %evals;
  
  #default opts

  my %default = (
-"MAILER" => "sendmail",  # default mailer
+"MAILER" => "sendmail",  # default mailer

Noticed this when I sent the new version.


  "EMAIL_ON_ERROR"=> 1,
  "EMAIL_WHEN_FINISHED"   => 1,
  "EMAIL_WHEN_CANCELED"   => 0,
@@ -218,6 +218,7 @@ my $dirname = $FindBin::Bin;
  
  my $mailto;

  my $mailer;
+my $mail_exec;
  my $email_on_error;
  my $email_when_finished;
  my $email_when_started;
@@ -1431,7 +1433,14 @@ sub do_not_reboot {
($test_type eq "config_bisect" && $opt{"CONFIG_BISECT_TYPE[$i]"} eq 
"build");
  }
  
+my $in_die = 0;

+
  sub dodie {
+
+# avoid recusion
+return if ($in_die);
+$in_die = 1;
+
  doprint "CRITICAL FAILURE... ", @_, "\n";

Good idea.
  
  my $i = $iteration;

@@ -4126,21 +4135,31 @@ sub set_test_option {
  
  sub _mailx_send {

  my ($subject, $message) = @_;
-system("$mailer -s \'$subject\' $mailto <<< \'$message\'");
+
+if (!defined($mail_exec)) {
+   $mail_exec = $mailer;
+}
+run_command "$mail_exec -s \'$subject\' $mailto <<< \'$message\'";
  }
  
  sub _sendmail_send {

  my ($subject, $message) = @_;
-system("echo -e \"Subject: $subject\n\n$message\" | sendmail -t $mailto");
+
+if (!defined($mail_exec)) {
+   $mail_exec = "/usr/sbin/sendmail";
+}
+run_command "echo \'Subject: $subject\n\n$message\' | $mail_exec -t 
$mailto";
  }
  
Not sure if I understand why $mail_exec is necessary. Doesn't $mailer 
already have a default? Wouldn't people just use $mailer to define the 
executable they want to use? What if the $mailx_exec specified doesn't 
use '-t' option?


Thanks,
Tim

Re: [PATCH v3 1/4] Ktest: add email support

[Tim Tianyang Chen]

I just fixed them up and pulled them in myself. ;-)

I also added the following on top of them (and testing this, live while
testing ftrace patches and other builds).

Thanks Steve!

diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
index 30a4c053f98b..837fa75cbb47 100755
--- a/tools/testing/ktest/ktest.pl
+++ b/tools/testing/ktest/ktest.pl
@@ -23,7 +23,7 @@ my %evals;
  
  #default opts

  my %default = (
-"MAILER" => "sendmail",  # default mailer
+"MAILER" => "sendmail",  # default mailer

Noticed this when I sent the new version.


  "EMAIL_ON_ERROR"=> 1,
  "EMAIL_WHEN_FINISHED"   => 1,
  "EMAIL_WHEN_CANCELED"   => 0,
@@ -218,6 +218,7 @@ my $dirname = $FindBin::Bin;
  
  my $mailto;

  my $mailer;
+my $mail_exec;
  my $email_on_error;
  my $email_when_finished;
  my $email_when_started;
@@ -1431,7 +1433,14 @@ sub do_not_reboot {
($test_type eq "config_bisect" && $opt{"CONFIG_BISECT_TYPE[$i]"} eq 
"build");
  }
  
+my $in_die = 0;

+
  sub dodie {
+
+# avoid recusion
+return if ($in_die);
+$in_die = 1;
+
  doprint "CRITICAL FAILURE... ", @_, "\n";

Good idea.
  
  my $i = $iteration;

@@ -4126,21 +4135,31 @@ sub set_test_option {
  
  sub _mailx_send {

  my ($subject, $message) = @_;
-system("$mailer -s \'$subject\' $mailto <<< \'$message\'");
+
+if (!defined($mail_exec)) {
+   $mail_exec = $mailer;
+}
+run_command "$mail_exec -s \'$subject\' $mailto <<< \'$message\'";
  }
  
  sub _sendmail_send {

  my ($subject, $message) = @_;
-system("echo -e \"Subject: $subject\n\n$message\" | sendmail -t $mailto");
+
+if (!defined($mail_exec)) {
+   $mail_exec = "/usr/sbin/sendmail";
+}
+run_command "echo \'Subject: $subject\n\n$message\' | $mail_exec -t 
$mailto";
  }
  
Not sure if I understand why $mail_exec is necessary. Doesn't $mailer 
already have a default? Wouldn't people just use $mailer to define the 
executable they want to use? What if the $mailx_exec specified doesn't 
use '-t' option?


Thanks,
Tim

Re: [PATCH] MIPS: vmlinuz: Fix compiler intrinsics location and build directly

[James Hogan]

On Thu, Apr 05, 2018 at 10:42:19PM +0100, James Hogan wrote:
> On Thu, Apr 05, 2018 at 11:13:14AM +0100, Matt Redfearn wrote:
> > Actually, this patch would be better inserted as patch 3 in the series 
> > since it can pull in the generic ashldi3 before the MIPS one is removed 
> > in the final patch. Here's an updated commit message:
> 
> Thanks Matt, applied.

Loongson1b/c defconfigs are still broken unfortunately. The .o files
aren't getting generated, apparently due to a different cmd_cc_o_c
definition when CONFIG_MODVERSIONS=y.

I'm gonna drop this patchset from 4.17. Hopefully an updated version can
be applied for 4.18 after the merge window.

Thanks
James


signature.asc
Description: Digital signature

Re: [PATCH] MIPS: vmlinuz: Fix compiler intrinsics location and build directly

[James Hogan]

On Thu, Apr 05, 2018 at 10:42:19PM +0100, James Hogan wrote:
> On Thu, Apr 05, 2018 at 11:13:14AM +0100, Matt Redfearn wrote:
> > Actually, this patch would be better inserted as patch 3 in the series 
> > since it can pull in the generic ashldi3 before the MIPS one is removed 
> > in the final patch. Here's an updated commit message:
> 
> Thanks Matt, applied.

Loongson1b/c defconfigs are still broken unfortunately. The .o files
aren't getting generated, apparently due to a different cmd_cc_o_c
definition when CONFIG_MODVERSIONS=y.

I'm gonna drop this patchset from 4.17. Hopefully an updated version can
be applied for 4.18 after the merge window.

Thanks
James


signature.asc
Description: Digital signature

Re: [GIT PULL] SELinux patches for v4.17

[Linus Torvalds]

On Tue, Apr 3, 2018 at 6:37 PM, Paul Moore  wrote:
>
> Everything passes the selinux-testsuite, but there are a few known
> merge conflicts.  The first is with the netdev tree and is in
> net/sctp/socket.c.  Unfortunately it is a bit ugly, thankfully Stephen
> Rothwell has already done the heavy lifting in resolving the merge for
> you, and the SCTP folks have given his merge patch a thumbs-up.

I ended up re-doing the merge, and it looks like some more sctp
changes happened after Stephen's merge anyway, so mine didn't end up
quite like his.

Adding Xin Long to see if he can verify it again, but it all *looks* sane.

While looking at it, it struck me that the new security hooks don't
seem to hook into __sctp_connect(), which also does that

scope = sctp_scope();
asoc = sctp_association_new(ep, sk, scope, GFP_KERNEL);

thing. Is that intentional? The sendmsg case does that
security_sctp_bind_connect, the actual __sctp_connect() does not.

This is not because I screwed up the merge - it's that way in the
SELinux tree too. And I obviously _left_ it that way, but while doing
the merge and trying to understand what was going on, this struck me.

I'm probably missing something really obvious why the connect case
doesn't want to do it thgere.

NOTE! I do see it being done in __sctp_setsockopt_connectx(). But
__sctp_connect() has another caller (in sctp_connect()) which doesn't
have that security_sctp_bind_connect() call.

So please check my resolution, but also somebody should tell me
"Linus, you're a cretin, sctp_connect() doesn't want that
security_sctp_bind_connect() at all because it was already done by
XYZ"

 Linus

Re: [GIT PULL] SELinux patches for v4.17

[Linus Torvalds]

On Tue, Apr 3, 2018 at 6:37 PM, Paul Moore  wrote:
>
> Everything passes the selinux-testsuite, but there are a few known
> merge conflicts.  The first is with the netdev tree and is in
> net/sctp/socket.c.  Unfortunately it is a bit ugly, thankfully Stephen
> Rothwell has already done the heavy lifting in resolving the merge for
> you, and the SCTP folks have given his merge patch a thumbs-up.

I ended up re-doing the merge, and it looks like some more sctp
changes happened after Stephen's merge anyway, so mine didn't end up
quite like his.

Adding Xin Long to see if he can verify it again, but it all *looks* sane.

While looking at it, it struck me that the new security hooks don't
seem to hook into __sctp_connect(), which also does that

scope = sctp_scope();
asoc = sctp_association_new(ep, sk, scope, GFP_KERNEL);

thing. Is that intentional? The sendmsg case does that
security_sctp_bind_connect, the actual __sctp_connect() does not.

This is not because I screwed up the merge - it's that way in the
SELinux tree too. And I obviously _left_ it that way, but while doing
the merge and trying to understand what was going on, this struck me.

I'm probably missing something really obvious why the connect case
doesn't want to do it thgere.

NOTE! I do see it being done in __sctp_setsockopt_connectx(). But
__sctp_connect() has another caller (in sctp_connect()) which doesn't
have that security_sctp_bind_connect() call.

So please check my resolution, but also somebody should tell me
"Linus, you're a cretin, sctp_connect() doesn't want that
security_sctp_bind_connect() at all because it was already done by
XYZ"

 Linus

Re: [PATCH v2] pstore: fix crypto dependencies without compression

[Kees Cook]

On Fri, Apr 6, 2018 at 12:56 AM, Arnd Bergmann  wrote:
> On Fri, Apr 6, 2018 at 9:25 AM, Tobias Regnery  
> wrote:
>> Commit 58eb5b670747 ("pstore: fix crypto dependencies") fixed up the crypto
>> dependencies but missed the case when no compression is selected.
>>
>> With CONFIG_PSTORE=y, CONFIG_PSTORE_COMPRESS=n  and CONFIG_CRYPTO=m we see
>> the following link error:
>>
>> fs/pstore/platform.o: In function `pstore_register':
>> (.text+0x1b1): undefined reference to `crypto_has_alg'
>> (.text+0x205): undefined reference to `crypto_alloc_base'
>> fs/pstore/platform.o: In function `pstore_unregister':
>> (.text+0x3b0): undefined reference to `crypto_destroy_tfm'
>>
>> Fix this by checking at compile-time if CONFIG_PSTORE_COMPRESS is enabled.
>>
>> Fixes: 58eb5b670747 ("pstore: fix crypto dependencies")
>> Signed-off-by: Tobias Regnery 
>> ---
>> v2: check the config at compile-time rather than change the
>> kconfig-dependency as suggested by Arnd.
>
> Thanks!
>
> Acked-by: Arnd Bergmann 

Thanks! I'll apply this.

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2] pstore: fix crypto dependencies without compression

[Kees Cook]

On Fri, Apr 6, 2018 at 12:56 AM, Arnd Bergmann  wrote:
> On Fri, Apr 6, 2018 at 9:25 AM, Tobias Regnery  
> wrote:
>> Commit 58eb5b670747 ("pstore: fix crypto dependencies") fixed up the crypto
>> dependencies but missed the case when no compression is selected.
>>
>> With CONFIG_PSTORE=y, CONFIG_PSTORE_COMPRESS=n  and CONFIG_CRYPTO=m we see
>> the following link error:
>>
>> fs/pstore/platform.o: In function `pstore_register':
>> (.text+0x1b1): undefined reference to `crypto_has_alg'
>> (.text+0x205): undefined reference to `crypto_alloc_base'
>> fs/pstore/platform.o: In function `pstore_unregister':
>> (.text+0x3b0): undefined reference to `crypto_destroy_tfm'
>>
>> Fix this by checking at compile-time if CONFIG_PSTORE_COMPRESS is enabled.
>>
>> Fixes: 58eb5b670747 ("pstore: fix crypto dependencies")
>> Signed-off-by: Tobias Regnery 
>> ---
>> v2: check the config at compile-time rather than change the
>> kconfig-dependency as suggested by Arnd.
>
> Thanks!
>
> Acked-by: Arnd Bergmann 

Thanks! I'll apply this.

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v3 1/4] Ktest: add email support

[Steven Rostedt]

On Fri, 6 Apr 2018 15:19:48 -0700
Tim Tianyang Chen  wrote:


> > The full path name needs to be here.
> >
> >   tools/testing/ktest/ktest.pl
> >  
> Sorry I was working on my private folder, version tracked with other 
> stuff. Just re-sent them.

I just fixed them up and pulled them in myself. ;-)

I also added the following on top of them (and testing this, live while
testing ftrace patches and other builds).

-- Steve

diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
index 30a4c053f98b..837fa75cbb47 100755
--- a/tools/testing/ktest/ktest.pl
+++ b/tools/testing/ktest/ktest.pl
@@ -23,7 +23,7 @@ my %evals;
 
 #default opts
 my %default = (
-"MAILER"   => "sendmail",  # default mailer
+"MAILER"   => "sendmail",  # default mailer
 "EMAIL_ON_ERROR"   => 1,
 "EMAIL_WHEN_FINISHED"  => 1,
 "EMAIL_WHEN_CANCELED"  => 0,
@@ -218,6 +218,7 @@ my $dirname = $FindBin::Bin;
 
 my $mailto;
 my $mailer;
+my $mail_exec;
 my $email_on_error;
 my $email_when_finished;
 my $email_when_started;
@@ -250,8 +251,9 @@ my $no_reboot = 1;
 my $reboot_success = 0;
 
 my %option_map = (
-"MAILTO"   => \$mailto,
-"MAILER"   => \$mailer,
+"MAILTO"   => \$mailto,
+"MAILER"   => \$mailer,
+"MAIL_EXEC"=> \$mail_exec,
 "EMAIL_ON_ERROR"   => \$email_on_error,
 "EMAIL_WHEN_FINISHED"  => \$email_when_finished,
 "EMAIL_WHEN_STARTED"   => \$email_when_started,
@@ -1431,7 +1433,14 @@ sub do_not_reboot {
($test_type eq "config_bisect" && $opt{"CONFIG_BISECT_TYPE[$i]"} eq 
"build");
 }
 
+my $in_die = 0;
+
 sub dodie {
+
+# avoid recusion
+return if ($in_die);
+$in_die = 1;
+
 doprint "CRITICAL FAILURE... ", @_, "\n";
 
 my $i = $iteration;
@@ -4126,21 +4135,31 @@ sub set_test_option {
 
 sub _mailx_send {
 my ($subject, $message) = @_;
-system("$mailer -s \'$subject\' $mailto <<< \'$message\'");
+
+if (!defined($mail_exec)) {
+   $mail_exec = $mailer;
+}
+run_command "$mail_exec -s \'$subject\' $mailto <<< \'$message\'";
 }
 
 sub _sendmail_send {
 my ($subject, $message) = @_;
-system("echo -e \"Subject: $subject\n\n$message\" | sendmail -t $mailto");
+
+if (!defined($mail_exec)) {
+   $mail_exec = "/usr/sbin/sendmail";
+}
+run_command "echo \'Subject: $subject\n\n$message\' | $mail_exec -t 
$mailto";
 }
 
 sub send_email {
-if (defined($mailto) && defined($mailer)) {
+if (defined($mailto)) {
+   if (!defined($mailer)) {
+   doprint "No email sent: email or mailer not specified in config.\n";
+   return;
+   }
 if ($mailer eq "mail" || $mailer eq "mailx"){ _mailx_send(@_);}
 elsif ($mailer eq "sendmail" ) { _sendmail_send(@_);}
-else { doprint "\nYour mailer: $mailer is not supported.\n" }
-} else {
-print "No email sent: email or mailer not specified in config.\n"
+else { die "\nYour mailer: $mailer is not supported.\n" }
 }
 }
 
diff --git a/tools/testing/ktest/sample.conf b/tools/testing/ktest/sample.conf
index d1a2626aaa0a..86e7cffc45c0 100644
--- a/tools/testing/ktest/sample.conf
+++ b/tools/testing/ktest/sample.conf
@@ -411,6 +411,10 @@
 # (default sendmail)
 #MAILER = sendmail
 #
+# The executable to run
+# (default: for sendmail "/usr/sbin/sendmail", otherwise equals ${MAILER})
+#MAIL_EXEC = /usr/sbin/sendmail
+#
 # Errors are defined as those would terminate the script
 # (default 1)
 #EMAIL_ON_ERROR = 1

Re: [PATCH v3 1/4] Ktest: add email support

[Steven Rostedt]

On Fri, 6 Apr 2018 15:19:48 -0700
Tim Tianyang Chen  wrote:


> > The full path name needs to be here.
> >
> >   tools/testing/ktest/ktest.pl
> >  
> Sorry I was working on my private folder, version tracked with other 
> stuff. Just re-sent them.

I just fixed them up and pulled them in myself. ;-)

I also added the following on top of them (and testing this, live while
testing ftrace patches and other builds).

-- Steve

diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
index 30a4c053f98b..837fa75cbb47 100755
--- a/tools/testing/ktest/ktest.pl
+++ b/tools/testing/ktest/ktest.pl
@@ -23,7 +23,7 @@ my %evals;
 
 #default opts
 my %default = (
-"MAILER"   => "sendmail",  # default mailer
+"MAILER"   => "sendmail",  # default mailer
 "EMAIL_ON_ERROR"   => 1,
 "EMAIL_WHEN_FINISHED"  => 1,
 "EMAIL_WHEN_CANCELED"  => 0,
@@ -218,6 +218,7 @@ my $dirname = $FindBin::Bin;
 
 my $mailto;
 my $mailer;
+my $mail_exec;
 my $email_on_error;
 my $email_when_finished;
 my $email_when_started;
@@ -250,8 +251,9 @@ my $no_reboot = 1;
 my $reboot_success = 0;
 
 my %option_map = (
-"MAILTO"   => \$mailto,
-"MAILER"   => \$mailer,
+"MAILTO"   => \$mailto,
+"MAILER"   => \$mailer,
+"MAIL_EXEC"=> \$mail_exec,
 "EMAIL_ON_ERROR"   => \$email_on_error,
 "EMAIL_WHEN_FINISHED"  => \$email_when_finished,
 "EMAIL_WHEN_STARTED"   => \$email_when_started,
@@ -1431,7 +1433,14 @@ sub do_not_reboot {
($test_type eq "config_bisect" && $opt{"CONFIG_BISECT_TYPE[$i]"} eq 
"build");
 }
 
+my $in_die = 0;
+
 sub dodie {
+
+# avoid recusion
+return if ($in_die);
+$in_die = 1;
+
 doprint "CRITICAL FAILURE... ", @_, "\n";
 
 my $i = $iteration;
@@ -4126,21 +4135,31 @@ sub set_test_option {
 
 sub _mailx_send {
 my ($subject, $message) = @_;
-system("$mailer -s \'$subject\' $mailto <<< \'$message\'");
+
+if (!defined($mail_exec)) {
+   $mail_exec = $mailer;
+}
+run_command "$mail_exec -s \'$subject\' $mailto <<< \'$message\'";
 }
 
 sub _sendmail_send {
 my ($subject, $message) = @_;
-system("echo -e \"Subject: $subject\n\n$message\" | sendmail -t $mailto");
+
+if (!defined($mail_exec)) {
+   $mail_exec = "/usr/sbin/sendmail";
+}
+run_command "echo \'Subject: $subject\n\n$message\' | $mail_exec -t 
$mailto";
 }
 
 sub send_email {
-if (defined($mailto) && defined($mailer)) {
+if (defined($mailto)) {
+   if (!defined($mailer)) {
+   doprint "No email sent: email or mailer not specified in config.\n";
+   return;
+   }
 if ($mailer eq "mail" || $mailer eq "mailx"){ _mailx_send(@_);}
 elsif ($mailer eq "sendmail" ) { _sendmail_send(@_);}
-else { doprint "\nYour mailer: $mailer is not supported.\n" }
-} else {
-print "No email sent: email or mailer not specified in config.\n"
+else { die "\nYour mailer: $mailer is not supported.\n" }
 }
 }
 
diff --git a/tools/testing/ktest/sample.conf b/tools/testing/ktest/sample.conf
index d1a2626aaa0a..86e7cffc45c0 100644
--- a/tools/testing/ktest/sample.conf
+++ b/tools/testing/ktest/sample.conf
@@ -411,6 +411,10 @@
 # (default sendmail)
 #MAILER = sendmail
 #
+# The executable to run
+# (default: for sendmail "/usr/sbin/sendmail", otherwise equals ${MAILER})
+#MAIL_EXEC = /usr/sbin/sendmail
+#
 # Errors are defined as those would terminate the script
 # (default 1)
 #EMAIL_ON_ERROR = 1

Re: x32 suspend failuer in Re: linux-next: Tree for Apr 4

[Pavel Machek]

On Wed 2018-04-04 09:50:47, Pavel Machek wrote:
> Hi!
> 
> > Please do not add any v4.18 destined stuff to your linux-next included
> > trees until after v4.17-rc1 has been released.
> 
> On thinkpad x60, suspend does not suspend at all with this -next
> version. Previous versions suspended/resumed fine but broke networking.

I bisected networking breakage to

c16add24522547bf52c189b3c0d1ab6f5c2b4375

which is slightly weird. But it modifies ACPI in strange way, so maybe
not that weird.

Networking breakage is still in next-20180406.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: x32 suspend failuer in Re: linux-next: Tree for Apr 4

[Pavel Machek]

On Wed 2018-04-04 09:50:47, Pavel Machek wrote:
> Hi!
> 
> > Please do not add any v4.18 destined stuff to your linux-next included
> > trees until after v4.17-rc1 has been released.
> 
> On thinkpad x60, suspend does not suspend at all with this -next
> version. Previous versions suspended/resumed fine but broke networking.

I bisected networking breakage to

c16add24522547bf52c189b3c0d1ab6f5c2b4375

which is slightly weird. But it modifies ACPI in strange way, so maybe
not that weird.

Networking breakage is still in next-20180406.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH] Input: synaptics-rmi4 - Fix an unchecked out of memory error path

[Dmitry Torokhov]

On Mon, Apr 02, 2018 at 05:52:52PM -0700, Andrew Duggan wrote:
> 
> On 04/02/2018 07:03 AM, Christophe JAILLET wrote:
> > When extending the rmi_spi buffers, we must check that no out of memory
> > error occurs, otherwise we may access data above the currently allocated
> > memory.
> > 
> > Propagate the error code returned by 'rmi_spi_manage_pools()' instead.
> Yep, that definitely looks like an oversight on my part. Thanks for the fix.
> 
> Andrew
> 
> > Signed-off-by: Christophe JAILLET 
> 
> Reviewed-by: Andrew Duggan 

Applied, thank you.

> 
> > ---
> >   drivers/input/rmi4/rmi_spi.c | 7 +--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/input/rmi4/rmi_spi.c b/drivers/input/rmi4/rmi_spi.c
> > index 76edbf2c1bce..082defc329a8 100644
> > --- a/drivers/input/rmi4/rmi_spi.c
> > +++ b/drivers/input/rmi4/rmi_spi.c
> > @@ -147,8 +147,11 @@ static int rmi_spi_xfer(struct rmi_spi_xport *rmi_spi,
> > if (len > RMI_SPI_XFER_SIZE_LIMIT)
> > return -EINVAL;
> > -   if (rmi_spi->xfer_buf_size < len)
> > -   rmi_spi_manage_pools(rmi_spi, len);
> > +   if (rmi_spi->xfer_buf_size < len) {
> > +   ret = rmi_spi_manage_pools(rmi_spi, len);
> > +   if (ret < 0)
> > +   return ret;
> > +   }
> > if (addr == 0)
> > /*
> 

-- 
Dmitry

Re: [PATCH] Input: synaptics-rmi4 - Fix an unchecked out of memory error path

[Dmitry Torokhov]

On Mon, Apr 02, 2018 at 05:52:52PM -0700, Andrew Duggan wrote:
> 
> On 04/02/2018 07:03 AM, Christophe JAILLET wrote:
> > When extending the rmi_spi buffers, we must check that no out of memory
> > error occurs, otherwise we may access data above the currently allocated
> > memory.
> > 
> > Propagate the error code returned by 'rmi_spi_manage_pools()' instead.
> Yep, that definitely looks like an oversight on my part. Thanks for the fix.
> 
> Andrew
> 
> > Signed-off-by: Christophe JAILLET 
> 
> Reviewed-by: Andrew Duggan 

Applied, thank you.

> 
> > ---
> >   drivers/input/rmi4/rmi_spi.c | 7 +--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/input/rmi4/rmi_spi.c b/drivers/input/rmi4/rmi_spi.c
> > index 76edbf2c1bce..082defc329a8 100644
> > --- a/drivers/input/rmi4/rmi_spi.c
> > +++ b/drivers/input/rmi4/rmi_spi.c
> > @@ -147,8 +147,11 @@ static int rmi_spi_xfer(struct rmi_spi_xport *rmi_spi,
> > if (len > RMI_SPI_XFER_SIZE_LIMIT)
> > return -EINVAL;
> > -   if (rmi_spi->xfer_buf_size < len)
> > -   rmi_spi_manage_pools(rmi_spi, len);
> > +   if (rmi_spi->xfer_buf_size < len) {
> > +   ret = rmi_spi_manage_pools(rmi_spi, len);
> > +   if (ret < 0)
> > +   return ret;
> > +   }
> > if (addr == 0)
> > /*
> 

-- 
Dmitry

Re: [PATCH v2 1/2] livepatch: Initialize shadow variables safely by a custom callback

[Josh Poimboeuf]

On Thu, Apr 05, 2018 at 02:23:14PM +0200, Petr Mladek wrote:
> @@ -150,6 +149,23 @@ static void *__klp_shadow_get_or_alloc(void *obj, 
> unsigned long id, void *data,
>   goto exists;
>   }
>  
> + new_shadow->obj = obj;
> + new_shadow->id = id;
> +
> + if (ctor) {
> + int err;
> +
> + err = ctor(obj, new_shadow->data, ctor_data);
> + if (err) {
> + spin_unlock_irqrestore(_shadow_lock, flags);
> + kfree(new_shadow);
> + WARN(1,
> +  "Failed to construct shadow variable <%p, %lx>\n",
> +  obj, id);
> + return NULL;
> + }
> + }
> +

I'm not sure why a constructor would return an error, though I guess it
doesn't hurt to allow it.

The WARN seems excessive though, IMO.  The constructor itself can warn
(or printk or whatever else) if it thinks its warranted.

Also I think the 'err' variable isn't really needed.

-- 
Josh

Re: [PATCH v2 1/2] livepatch: Initialize shadow variables safely by a custom callback

[Josh Poimboeuf]

On Thu, Apr 05, 2018 at 02:23:14PM +0200, Petr Mladek wrote:
> @@ -150,6 +149,23 @@ static void *__klp_shadow_get_or_alloc(void *obj, 
> unsigned long id, void *data,
>   goto exists;
>   }
>  
> + new_shadow->obj = obj;
> + new_shadow->id = id;
> +
> + if (ctor) {
> + int err;
> +
> + err = ctor(obj, new_shadow->data, ctor_data);
> + if (err) {
> + spin_unlock_irqrestore(_shadow_lock, flags);
> + kfree(new_shadow);
> + WARN(1,
> +  "Failed to construct shadow variable <%p, %lx>\n",
> +  obj, id);
> + return NULL;
> + }
> + }
> +

I'm not sure why a constructor would return an error, though I guess it
doesn't hurt to allow it.

The WARN seems excessive though, IMO.  The constructor itself can warn
(or printk or whatever else) if it thinks its warranted.

Also I think the 'err' variable isn't really needed.

-- 
Josh

[PATCH] block/compat_ioctl: fix range check in BLKGETSIZE

[Khazhismel Kumykov]

kernel ulong and compat_ulong_t may not be same width. Use type directly
to eliminate mismatches.

This would result in truncation rather than EFBIG for 32bit mode for
large disks.

Signed-off-by: Khazhismel Kumykov 
---
 block/compat_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/compat_ioctl.c b/block/compat_ioctl.c
index 6ca015f92766..3a2c77f07da8 100644
--- a/block/compat_ioctl.c
+++ b/block/compat_ioctl.c
@@ -388,7 +388,7 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, 
unsigned long arg)
return 0;
case BLKGETSIZE:
size = i_size_read(bdev->bd_inode);
-   if ((size >> 9) > ~0UL)
+   if ((size >> 9) > ~((compat_ulong_t)0UL))
return -EFBIG;
return compat_put_ulong(arg, size >> 9);
 
-- 
2.17.0.484.g0c8726318c-goog



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH 4.9 000/102] 4.9.93-stable review

[Dan Rue]

On Fri, Apr 06, 2018 at 03:22:41PM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 4.9.93 release.
> There are 102 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Sun Apr  8 08:42:55 UTC 2018.
> Anything received after that time might be too late.

Results from Linaro’s test farm.
No regressions on arm64, arm and x86_64.

There is a new test failure on dragonboard 410c (arm64) in
kselftest/cpu-on-off-test. However, it looks like the test was failing
but giving a false "PASS" on previous versions of 4.9. This -RC seems to
have changed the behavior enough to cause the test to actually mark a
failure.

In any event, this looks like a db410c-specific pre-existing issue that we have
already escalated to our Qualcomm team. Details can be found at
https://bugs.linaro.org/show_bug.cgi?id=3723 for those interested.


Summary


kernel: 4.9.93-rc1
git repo: 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
git branch: linux-4.9.y
git commit: 7eea6c2b79f0c0f15f8a519cd2797bcfffc47d4a
git describe: v4.9.92-103-g7eea6c2b79f0
Test details: 
https://qa-reports.linaro.org/lkft/linux-stable-rc-4.9-oe/build/v4.9.92-103-g7eea6c2b79f0


No regressions (compared to build v4.9.92-85-g6664986610a8)

Boards, architectures and test suites:
-

dragonboard-410c
* boot - pass: 24
* kselftest - skip: 27, fail: 1, pass: 35
* libhugetlbfs - skip: 1, pass: 90
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - skip: 17, pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 4
* ltp-fs-tests - skip: 2, pass: 61
* ltp-fs_bind-tests - pass: 4
* ltp-fs_perms_simple-tests - pass: 38
* ltp-fsx-tests - pass: 4
* ltp-hugetlb-tests - skip: 1, pass: 21
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - pass: 14
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 148, pass: 1002
* ltp-timers-tests - skip: 1, pass: 12

hi6220-hikey - arm64
* boot - pass: 28
* kselftest - skip: 47, pass: 78
* libhugetlbfs - skip: 2, pass: 180
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - skip: 17, pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 4
* ltp-fs-tests - skip: 4, pass: 122
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - skip: 2, pass: 42
* ltp-io-tests - pass: 6
* ltp-ipc-tests - pass: 18
* ltp-math-tests - pass: 22
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - skip: 4, pass: 10
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 151, pass: 999
* ltp-timers-tests - skip: 1, pass: 12

juno-r2 - arm64
* boot - pass: 27
* kselftest - skip: 23, pass: 40
* libhugetlbfs - skip: 1, pass: 90
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - skip: 17, pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - skip: 2, pass: 61
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - pass: 22
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 22
* ltp-nptl-tests - pass: 4
* ltp-pty-tests - pass: 8
* ltp-sched-tests - skip: 8, pass: 20
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 149, pass: 1001
* ltp-timers-tests - skip: 1, pass: 12

qemu_x86_64
* boot - pass: 22
* kselftest - skip: 28, pass: 52
* kselftest-vsyscall-mode-native - skip: 28, pass: 52
* kselftest-vsyscall-mode-none - skip: 28, pass: 52
* libhugetlbfs - skip: 1, pass: 90
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - skip: 17, pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - skip: 6, pass: 57
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - pass: 22
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - skip: 1, pass: 13
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 148, pass: 1002
* ltp-timers-tests - skip: 1, pass: 12

x15 - arm
* boot - pass: 21
* kselftest - skip: 24, pass: 38
* libhugetlbfs - skip: 1, pass: 87
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - skip: 17, pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - skip: 2, pass: 61
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - skip: 2, pass: 20
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 

[PATCH] block/compat_ioctl: fix range check in BLKGETSIZE

[Khazhismel Kumykov]

kernel ulong and compat_ulong_t may not be same width. Use type directly
to eliminate mismatches.

This would result in truncation rather than EFBIG for 32bit mode for
large disks.

Signed-off-by: Khazhismel Kumykov 
---
 block/compat_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/compat_ioctl.c b/block/compat_ioctl.c
index 6ca015f92766..3a2c77f07da8 100644
--- a/block/compat_ioctl.c
+++ b/block/compat_ioctl.c
@@ -388,7 +388,7 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, 
unsigned long arg)
return 0;
case BLKGETSIZE:
size = i_size_read(bdev->bd_inode);
-   if ((size >> 9) > ~0UL)
+   if ((size >> 9) > ~((compat_ulong_t)0UL))
return -EFBIG;
return compat_put_ulong(arg, size >> 9);
 
-- 
2.17.0.484.g0c8726318c-goog



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH 4.9 000/102] 4.9.93-stable review

[Dan Rue]

On Fri, Apr 06, 2018 at 03:22:41PM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 4.9.93 release.
> There are 102 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Sun Apr  8 08:42:55 UTC 2018.
> Anything received after that time might be too late.

Results from Linaro’s test farm.
No regressions on arm64, arm and x86_64.

There is a new test failure on dragonboard 410c (arm64) in
kselftest/cpu-on-off-test. However, it looks like the test was failing
but giving a false "PASS" on previous versions of 4.9. This -RC seems to
have changed the behavior enough to cause the test to actually mark a
failure.

In any event, this looks like a db410c-specific pre-existing issue that we have
already escalated to our Qualcomm team. Details can be found at
https://bugs.linaro.org/show_bug.cgi?id=3723 for those interested.


Summary


kernel: 4.9.93-rc1
git repo: 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
git branch: linux-4.9.y
git commit: 7eea6c2b79f0c0f15f8a519cd2797bcfffc47d4a
git describe: v4.9.92-103-g7eea6c2b79f0
Test details: 
https://qa-reports.linaro.org/lkft/linux-stable-rc-4.9-oe/build/v4.9.92-103-g7eea6c2b79f0


No regressions (compared to build v4.9.92-85-g6664986610a8)

Boards, architectures and test suites:
-

dragonboard-410c
* boot - pass: 24
* kselftest - skip: 27, fail: 1, pass: 35
* libhugetlbfs - skip: 1, pass: 90
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - skip: 17, pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 4
* ltp-fs-tests - skip: 2, pass: 61
* ltp-fs_bind-tests - pass: 4
* ltp-fs_perms_simple-tests - pass: 38
* ltp-fsx-tests - pass: 4
* ltp-hugetlb-tests - skip: 1, pass: 21
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - pass: 14
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 148, pass: 1002
* ltp-timers-tests - skip: 1, pass: 12

hi6220-hikey - arm64
* boot - pass: 28
* kselftest - skip: 47, pass: 78
* libhugetlbfs - skip: 2, pass: 180
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - skip: 17, pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 4
* ltp-fs-tests - skip: 4, pass: 122
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - skip: 2, pass: 42
* ltp-io-tests - pass: 6
* ltp-ipc-tests - pass: 18
* ltp-math-tests - pass: 22
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - skip: 4, pass: 10
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 151, pass: 999
* ltp-timers-tests - skip: 1, pass: 12

juno-r2 - arm64
* boot - pass: 27
* kselftest - skip: 23, pass: 40
* libhugetlbfs - skip: 1, pass: 90
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - skip: 17, pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - skip: 2, pass: 61
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - pass: 22
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 22
* ltp-nptl-tests - pass: 4
* ltp-pty-tests - pass: 8
* ltp-sched-tests - skip: 8, pass: 20
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 149, pass: 1001
* ltp-timers-tests - skip: 1, pass: 12

qemu_x86_64
* boot - pass: 22
* kselftest - skip: 28, pass: 52
* kselftest-vsyscall-mode-native - skip: 28, pass: 52
* kselftest-vsyscall-mode-none - skip: 28, pass: 52
* libhugetlbfs - skip: 1, pass: 90
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - skip: 17, pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - skip: 6, pass: 57
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - pass: 22
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - skip: 1, pass: 13
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 148, pass: 1002
* ltp-timers-tests - skip: 1, pass: 12

x15 - arm
* boot - pass: 21
* kselftest - skip: 24, pass: 38
* libhugetlbfs - skip: 1, pass: 87
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - skip: 17, pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - skip: 2, pass: 61
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - skip: 2, pass: 20
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 

Re: [PATCH 3.18 00/93] 3.18.103-stable review

[Shuah Khan]

On 04/06/2018 07:22 AM, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 3.18.103 release.
> There are 93 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Sun Apr  8 08:42:04 UTC 2018.
> Anything received after that time might be too late.
> 
> The whole patch series can be found in one patch at:
>   
> https://www.kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.103-rc1.gz
> or in the git tree and branch at:
>   
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> linux-3.18.y
> and the diffstat can be found below.
> 
> thanks,
> 
> greg k-h
> 

Compiled and booted on my test system. No dmesg regressions.

thanks,
-- Shuah

Re: [PATCH 3.18 00/93] 3.18.103-stable review

[Shuah Khan]

On 04/06/2018 07:22 AM, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 3.18.103 release.
> There are 93 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Sun Apr  8 08:42:04 UTC 2018.
> Anything received after that time might be too late.
> 
> The whole patch series can be found in one patch at:
>   
> https://www.kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.103-rc1.gz
> or in the git tree and branch at:
>   
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> linux-3.18.y
> and the diffstat can be found below.
> 
> thanks,
> 
> greg k-h
> 

Compiled and booted on my test system. No dmesg regressions.

thanks,
-- Shuah