[GIT PULL] lsm: fix smack_inode_removexattr and xattr_getsecurity memleak
Please pull this fix for v4.14-rc4.
It fixes a bug in xattr_getsecurity() where security_release_secctx() was
being called instead of kfree(), which leads to a memory leak in the
capabilities code. smack_inode_getsecurity is also fixed to behave
correctly when called from there.
The following changes since commit d81fa669e3de7eb8a631d7d95dac5fbcb2bf9d4e:
Merge branch 'for-4.14-fixes' of
git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq (2017-10-03 10:44:03 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git
fixes-v4.14-rc4
Casey Schaufler (1):
lsm: fix smack_inode_removexattr and xattr_getsecurity memleak
fs/xattr.c |2 +-
security/smack/smack_lsm.c | 55
2 files changed, 26 insertions(+), 31 deletions(-)
---
commit 57e7ba04d422c3d41c8426380303ec9b7533ded9
Author: Casey Schaufler
Date: Tue Sep 19 09:39:08 2017 -0700
lsm: fix smack_inode_removexattr and xattr_getsecurity memleak
security_inode_getsecurity() provides the text string value
of a security attribute. It does not provide a "secctx".
The code in xattr_getsecurity() that calls security_inode_getsecurity()
and then calls security_release_secctx() happened to work because
SElinux and Smack treat the attribute and the secctx the same way.
It fails for cap_inode_getsecurity(), because that module has no
secctx that ever needs releasing. It turns out that Smack is the
one that's doing things wrong by not allocating memory when instructed
to do so by the "alloc" parameter.
The fix is simple enough. Change the security_release_secctx() to
kfree() because it isn't a secctx being returned by
security_inode_getsecurity(). Change Smack to allocate the string when
told to do so.
Note: this also fixes memory leaks for LSMs which implement
inode_getsecurity but not release_secctx, such as capabilities.
Signed-off-by: Casey Schaufler
Reported-by: Konstantin Khlebnikov
Cc: sta...@vger.kernel.org
Signed-off-by: James Morris
diff --git a/fs/xattr.c b/fs/xattr.c
index 4424f7f..61cd28b 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -250,7 +250,7 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char
*name,
}
memcpy(value, buffer, len);
out:
- security_release_secctx(buffer, len);
+ kfree(buffer);
out_noalloc:
return len;
}
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 319add3..286171a 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1473,7 +1473,7 @@ static int smack_inode_removexattr(struct dentry *dentry,
const char *name)
* @inode: the object
* @name: attribute name
* @buffer: where to put the result
- * @alloc: unused
+ * @alloc: duplicate memory
*
* Returns the size of the attribute or an error code
*/
@@ -1486,43 +1486,38 @@ static int smack_inode_getsecurity(struct inode *inode,
struct super_block *sbp;
struct inode *ip = (struct inode *)inode;
struct smack_known *isp;
- int ilen;
- int rc = 0;
- if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) {
+ if (strcmp(name, XATTR_SMACK_SUFFIX) == 0)
isp = smk_of_inode(inode);
- ilen = strlen(isp->smk_known);
- *buffer = isp->smk_known;
- return ilen;
- }
+ else {
+ /*
+* The rest of the Smack xattrs are only on sockets.
+*/
+ sbp = ip->i_sb;
+ if (sbp->s_magic != SOCKFS_MAGIC)
+ return -EOPNOTSUPP;
- /*
-* The rest of the Smack xattrs are only on sockets.
-*/
- sbp = ip->i_sb;
- if (sbp->s_magic != SOCKFS_MAGIC)
- return -EOPNOTSUPP;
+ sock = SOCKET_I(ip);
+ if (sock == NULL || sock->sk == NULL)
+ return -EOPNOTSUPP;
- sock = SOCKET_I(ip);
- if (sock == NULL || sock->sk == NULL)
- return -EOPNOTSUPP;
-
- ssp = sock->sk->sk_security;
+ ssp = sock->sk->sk_security;
- if (strcmp(name, XATTR_SMACK_IPIN) == 0)
- isp = ssp->smk_in;
- else if (strcmp(name, XATTR_SMACK_IPOUT) == 0)
- isp = ssp->smk_out;
- else
- return -EOPNOTSUPP;
+ if (strcmp(name, XATTR_SMACK_IPIN) == 0)
+ isp = ssp->smk_in;
+ else if (strcmp(name, XATTR_SMACK_IPOUT) == 0)
+ isp = ssp->smk_out;
+ else
+ return -EOPNOTSUPP;
+ }
- ilen = strlen(isp->smk_known);
- if (rc == 0) {
- *buffer = isp->smk_known;
- rc = ilen;
+ if (alloc) {
+ *buffer = kstrdup(isp->smk_known, GFP_KERNEL);
+
Re: [PATCH] cpufreq: docs: Drop intel_pstate.txt from index.txt
On Thu, Sep 28, 2017 at 5:26 AM, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki > > Commit 33fc30b47098 (cpufreq: intel_pstate: Document the current > behavior and user interface) dropped the intel-pstate.txt file > from Documentation/cpu-freq/, but it did not update the index.txt > file in there accordingly, so do that now. > > Fixes: 33fc30b47098 (cpufreq: intel_pstate: Document the current behavior and > user interface) > Signed-off-by: Rafael J. Wysocki > --- > Documentation/cpu-freq/index.txt |2 -- > 1 file changed, 2 deletions(-) > > Index: linux-pm/Documentation/cpu-freq/index.txt > === > --- linux-pm.orig/Documentation/cpu-freq/index.txt > +++ linux-pm/Documentation/cpu-freq/index.txt > @@ -32,8 +32,6 @@ cpufreq-stats.txt - General description > > index.txt - File index, Mailing list and Links (this document) > > -intel-pstate.txt - Intel pstate cpufreq driver specific file. > - > pcc-cpufreq.txt - PCC cpufreq driver specific file. Acked-by: Viresh Kumar
Re: [RFC 4/5] ext4: add fs freezing support on suspend/hibernation
On Tue, Oct 03, 2017 at 09:42:04PM -0400, Theodore Ts'o wrote: > On Tue, Oct 03, 2017 at 10:13:21PM +0200, Luis R. Rodriguez wrote: > > > After we remove add the NEEDS_RECOVERY flag, we need to make sure > > > recovery flag is pushed out to disk before any other changes are > > > allowed to be pushed out to disk. That's why we originally did the > > > update synchronously. > > > > I see. I had to try to dig through linux-history to see why this was done, > > and > > I actually could not find an exact commit which explained what you just did. > > Thanks! > > > > Hrm, so on freeze we issue the same commit as well, so is a sync *really* > > needed > > on thaw? > > So let me explain what's going on. When we do a freeze, we make sure > all of the blocks that are written to the journal are writen to the > final location on disk, and the journal is is truncated. (This is > called a "journal checkpoint".) Then we clear the NEEDS RECOVERY > feature flag and set the EXT4_VALID_FS flags in the superblock. In > the no journal case, we flush out all metadata writes, and we set the > EXT4_VALID_FS flag. In both cases, we call ext4_commit_super(sb, 1) > to make sure the flags update is pushed out to disk. This puts the > file system into a "quiscent" state; in fact, it looks like the file > system has been unmounted, so that it becomes safe to run > dump/restore, run fsck -n on the file system, etc. > > The problem on the thaw side is that we need to make sure that > EXT4_VALID_FS flag is removed (and if journaling is enabled, the NEEDS > RECOVERY feature flag is set) and the superblock is flushed out to the > storage device before any other writes are persisted on the disk. In > the case where we have journalling enabled, we could wait until the > first journal commit to write out the superblock, since in journal > mode all metadata updates to the disk are suppressed until the journal > commit. We don't do that today, but it is a change we could make. > > However, in the no journal node we need to make sure the EXT4_VALID_FS > flag is cleared on disk before any other metadata operations can take > place, and calling ext4_commit_super(sb, 1) is the only real way to do > that. > > > No, it was am empirical evaluation done at testing, I observed bio_submit() > > stalls, we never get completion from the device. Even if we call thaw at the > > very end of resume, after the devices should be up, we still end up in the > > same > > situation. Given how I order freezing filesystems after freezing userspace > > I do > > believe we should thaw filesystems prior unfreezing userspace, its why I > > placed > > the call where it is now. > > So when we call ext4_commit_super(sb, 1), we issue the bio, and then > we block waiting for the I/O to complete. That's a blocking call. Is > the thaw context one which allows blocking? thaw_super() does down_write(), so it *must* be able to sleep. > If userspace is still > frozen, does that imply that the scheduler isn't allow to run? If > that's the case, then that's probably the problem. > > More generally, if the thawing code needs to allocate memory, or do Which is does. xfs_fs_unfreeze() queues work to a workqueue, so there's memory allocation there. > any number of things that could potentially block, this could > potentially be an issue. yup, gfs2_unfreeze does even more stuff - it releases glocks which may end up queuing work, waking other threads and freeing stuff via call_rcu()... Basically, before thawing filesystems the rest of the kernel infrastructure needs to have been restarted. i.e. the order needs to be: freeze userspace freeze filesystems freeze kernel threads freeze workqueues thaw workqueues thaw kernel threads thaw filesystems thaw userspace and it should end up that way. > Or if we have a network block device, or > something else in the storage stack that needs to run a kernel thread > context (or a workqueue, etc.) --- is the fact that userspace is > frozen mean the scheduler is going to refuse to schedule()? No. Cheers, Dave. -- Dave Chinner da...@fromorbit.com
Re: perf script : wrong symoff in callchain
On 03/10/17 14:45, Matthieu CASTET wrote:
> Le Tue, 3 Oct 2017 13:34:37 +0300,
> Adrian Hunter a écrit :
>
>> On 03/10/17 13:19, Matthieu CASTET wrote:
>>> Hi,
>>>
>>> while using perf on x86_64, I saw strange output for symoff.
>>>
>>> $ perf record -g -- sleep 1
>>> $ perf script -F comm,tid,pid,time,ip,sym,dso,symoff
>>>
>>> [...]
>>> sleep 11656/11656 1045318.546436:
>>> 7fff9542e5b5 __d_lookup_rcu+0x80006ae02035 ([kernel.kallsyms])
>>> 7fff9541e132 lookup_fast+0x80006ae02052 ([kernel.kallsyms])
>>> 7fff9541eae8 walk_component+0x80006ae02048 ([kernel.kallsyms])
>>> 7fff9541efa2 link_path_walk+0x80006ae021b2 ([kernel.kallsyms])
>>> 7fff9541c92d path_init+0x80006ae021bd ([kernel.kallsyms])
>>> 7fff9542100b path_openat+0x80006ae020fb ([kernel.kallsyms])
>>> 7fff953c14fe handle_mm_fault+0x80006ae020ee ([kernel.kallsyms])
>>> 7fff95423669 do_filp_open+0x80006ae02099 ([kernel.kallsyms])
>>> 7fff95433414 __alloc_fd+0x80006ae02044 ([kernel.kallsyms])
>>> 7fff9540ff6e do_sys_open+0x80006ae0212e ([kernel.kallsyms])
>>> 7fff9540ff6e do_sys_open+0x80006ae0212e ([kernel.kallsyms])
>>> 7fff95850abb system_call_fast_compare_end+0x80006ae0200c
>>> ([kernel.kallsyms])
>>>2aecf _nl_load_locale_from_archive+0x014b5a92841f
>>> (/lib/x86_64-linux-gnu/libc-2.24.so)
>>>
>>>
>>> I tried to revert a4eb24a49566db77ee999b46603f602a0302f481 and I got
>>> good result :
>>> [...]
>>> sleep 11656/11656 1045318.546436:
>>> 7fff9542e5b5 __d_lookup_rcu+0x35 ([kernel.kallsyms])
>>> 7fff9541e132 lookup_fast+0x52 ([kernel.kallsyms])
>>> 7fff9541eae8 walk_component+0x48 ([kernel.kallsyms])
>>> 7fff9541efa2 link_path_walk+0x1b2 ([kernel.kallsyms])
>>> 7fff9541c92d path_init+0x1bd ([kernel.kallsyms])
>>> 7fff9542100b path_openat+0xfb ([kernel.kallsyms])
>>> 7fff953c14fe handle_mm_fault+0xee ([kernel.kallsyms])
>>> 7fff95423669 do_filp_open+0x99 ([kernel.kallsyms])
>>> 7fff95433414 __alloc_fd+0x44 ([kernel.kallsyms])
>>> 7fff9540ff6e do_sys_open+0x12e ([kernel.kallsyms])
>>> 7fff9540ff6e do_sys_open+0x12e ([kernel.kallsyms])
>>> 7fff95850abb system_call_fast_compare_end+0xc
>>> ([kernel.kallsyms])
>>>2aecf _nl_load_locale_from_archive+0x41f
>>> (/lib/x86_64-linux-gnu/libc-2.24.so)
>>>
>>>
>>> Any idea ?
>>
>> It could be the problem with add_callchain_ip() described here:
>>
>> https://www.spinics.net/lists/linux-perf-users/msg03172.html
>
> The db-export code removed the remapping [1]
>
> So there a mix of mapping in callchain code :
> - add_callchain_ip() store al.addr (ip remapped)
> - fill_callchain_info do the translation (map_ip)
> - sample__fprintf_callchain do the translation
> - call_path_from_sample (db-export) don't do anymore the translation
>
>
> So what should we do :
> - assume we store al.addr and remove mapping from all consumers ?
Seems it got changed to al.addr (per below), so I guess we should
remove mapping from all consumers.
commit 5550171b2a9f8df26ff483051d060db06376b26d
Author: Andi Kleen
Date: Wed Nov 12 18:05:21 2014 -0800
perf callchain: Use al.addr to set up call chain
Use the relative address, this makes get_srcline work correctly in the
end.
Signed-off-by: Andi Kleen
Cc: Jiri Olsa
Cc: Namhyung Kim
Link:
http://lkml.kernel.org/r/1415844328-4884-4-git-send-email-a...@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 84390ee..d97309c 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1406,7 +1406,7 @@ static int add_callchain_ip(struct thread *thread,
}
}
- return callchain_cursor_append(&callchain_cursor, ip, al.map, al.sym);
+ return callchain_cursor_append(&callchain_cursor, al.addr, al.map,
al.sym);
}
struct branch_info *sample__resolve_bstack(struct perf_sample *sample,
> - store ip and do the mapping in consumers ?
>
>
>
> [1]
> commit 7a2544c004a6c576b1e307f30925b165affe6a22
> Author: Chris Phlipot
> Date: Tue May 10 20:26:48 2016 -0700
>
> perf script: Fix callchain addresses in db-export
>
> Remove the call to map_ip() to adjust al.addr, because it has already
> been called when assembling the callchain, in:
>
> thread__resolve_callchain_sample(perf_sample)
> add_callchain_ip(ip = perf_sample->callchain->ips[j])
> thread__find_addr_location(addr = ip)
> thread__find_addr_map(addr) {
> al->addr = addr
> if (al->map)
> al->addr = al->map->map_ip(al->map, al->addr);
> }
>
> Calling it a second time can re
Re: [PATCH] scsi/eh: fix hang adding ehandler wakeups after decrementing host_busy
Hi. Please tell if there is something I can do to help the patch get
processed? It is on the list without reply for almost a month.
On 09/05/2017 03:54 PM, Pavel Tikhomirov wrote:
We have a problem on several our nodes with scsi EH. Imagine such an
order of execution of two threads:
CPU1 scsi_eh_scmd_add CPU2 scsi_host_queue_ready
/* shost->host_busy == 1 initialy */
if (shost->shost_state == SHOST_RECOVERY)
/* does not get here */
return 0;
lock(shost->host_lock);
shost->shost_state = SHOST_RECOVERY;
busy = shost->host_busy++;
/* host->can_queue == 1 initialy, busy == 1
* - go to starved label */
lock(shost->host_lock) /* wait */
shost->host_failed++;
/* shost->host_busy == 2, shost->host_failed == 1 */
call scsi_eh_wakeup(shost) {
if (host_busy == host_failed) {
/* does not get here */
wake_up_process(shost->ehandler)
}
}
unlock(shost->host_lock)
/* acquire lock */
shost->host_busy--;
Finaly we do not wakeup scsi_error_handler and all other commands
coming will hang as we are in never ending recovery state as there
is no one left to wakeup handler.
So scsi disc in these host becomes unresponsive and all bio on node
hangs. (We trigger these problem when scsi cmnds to DVD drive timeout.)
Main idea of the fix is to try to do wake up every time we decrement
host_busy or increment host_failed(the latter is already OK).
Now the very *last* one of busy threads getting host_lock after
decrementing host_busy will see all write operations on host's
shost_state, host_busy and host_failed completed thanks to implied
memory barriers on spin_lock/unlock, so at the time of busy==failed
we will trigger wakeup in at least one thread. (Thats why putting
recovery and failed checks under lock)
Signed-off-by: Pavel Tikhomirov
---
drivers/scsi/scsi_lib.c | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f6097b89d5d3..6c99221d60aa 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -320,12 +320,11 @@ void scsi_device_unbusy(struct scsi_device *sdev)
if (starget->can_queue > 0)
atomic_dec(&starget->target_busy);
+ spin_lock_irqsave(shost->host_lock, flags);
if (unlikely(scsi_host_in_recovery(shost) &&
-(shost->host_failed || shost->host_eh_scheduled))) {
- spin_lock_irqsave(shost->host_lock, flags);
+(shost->host_failed || shost->host_eh_scheduled)))
scsi_eh_wakeup(shost);
- spin_unlock_irqrestore(shost->host_lock, flags);
- }
+ spin_unlock_irqrestore(shost->host_lock, flags);
atomic_dec(&sdev->device_busy);
}
@@ -1503,6 +1502,13 @@ static inline int scsi_host_queue_ready(struct
request_queue *q,
spin_unlock_irq(shost->host_lock);
out_dec:
atomic_dec(&shost->host_busy);
+
+ spin_lock_irq(shost->host_lock);
+ if (unlikely(scsi_host_in_recovery(shost) &&
+(shost->host_failed || shost->host_eh_scheduled)))
+ scsi_eh_wakeup(shost);
+ spin_unlock_irq(shost->host_lock);
+
return 0;
}
@@ -1964,6 +1970,13 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
out_dec_host_busy:
atomic_dec(&shost->host_busy);
+
+ spin_lock_irq(shost->host_lock);
+ if (unlikely(scsi_host_in_recovery(shost) &&
+(shost->host_failed || shost->host_eh_scheduled)))
+ scsi_eh_wakeup(shost);
+ spin_unlock_irq(shost->host_lock);
+
out_dec_target_busy:
if (scsi_target(sdev)->can_queue > 0)
atomic_dec(&scsi_target(sdev)->target_busy);
--
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.
Re: [RFC 5/5] pm: remove kernel thread freezing
On Wed, Oct 04, 2017 at 02:47:56AM +0200, Luis R. Rodriguez wrote:
> On Tue, Oct 03, 2017 at 11:15:07PM +0200, Rafael J. Wysocki wrote:
> > On Tuesday, October 3, 2017 8:59:00 PM CEST Rafael J. Wysocki wrote:
> > > On Tuesday, October 3, 2017 8:53:13 PM CEST Luis R. Rodriguez wrote:
> > > > Now that all filesystems which used to rely on kthread
> > > > freezing have been converted to filesystem freeze/thawing
> > > > we can remove the kernel kthread freezer.
> > > >
> > > > Signed-off-by: Luis R. Rodriguez
> > >
> > > I like this one. :-)
> >
> > However, suspend_freeze/thaw_processes() require some more work.
> >
> > In particular, the freezing of workqueues is being removed here
> > without a replacement.
>
> Hrm, where do you see that? freeze_workqueues_busy() is still called on
> try_to_freeze_tasks(). Likewise thaw_processes() also calls thaw_workqueues().
> I did forget to nuke pm_nosig_freezing though.
static int try_to_freeze_tasks(bool user_only)
{
.
if (!user_only)
freeze_workqueues_begin();
.
if (!user_only) {
wq_busy = freeze_workqueues_busy();
.
}
i.e. only try_to_freeze_tasks(false) will freeze workqueues, and the
only function that calls that - freeze_kernel_threads() - is removed
by this patch.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
Re: [PATCHv2 1/1] [tools]: android/ion: userspace test utility for ion buffer sharing
On Tue, Oct 03, 2017 at 12:48:59PM -0400, Pintu Agarwal wrote: > This is a test utility to verify ION buffer sharing in user space > between 2 independent processes. > It uses unix domain socket as IPC to transfer an FD to another process > and install it. > > This utility demonstrates how ION buffer sharing can be implemented between > two user space processes, using various heap ids. > > This utility is verified on Ubuntu 32-bit machine using 2 independent > process such as: ionapp_export (server) and ionapp_import (client). > First the server needs to be run to export FD to the client. > This utility works only if /dev/ion interface is present. > > Here is a sample demo example: > > linux/tools/android/ion$ sudo ./ionapp_export.out -i 1 -s 10 > heap_type: 2, heap_size: 10 > Fill buffer content: > 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd > Sharing fd: 6, Client fd: 5 > : buffer release successfully > > linux/tools/android/ion$ sudo ./ionapp_import.out > Received buffer fd: 4 > Read buffer content: > 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd > 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 > 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 > Fill buffer content: > 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd > 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd > 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd > : buffer release successfully Can you tie this into the kselftest framework, to make automated tests of this kernel feature easier? thanks, greg k-h
Re: [PATCH 2/2] sysctl: remove /proc/sys/vm/nr_pdflush_threads
On Tue 03-10-17 09:16:21, Jens Axboe wrote:
> This tunable has been obsolete since 2.6.32, and writes to the
> file have been failing and complaining in dmesg since then:
>
> nr_pdflush_threads exported in /proc is scheduled for removal
>
> That was 8 years ago. Remove the file ABI obsolete notice, and
> the sysfs file.
>
> Signed-off-by: Jens Axboe
Agreed. You can add:
Reviewed-by: Jan Kara
Honza
> ---
> Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads | 5 -
> kernel/sysctl.c | 5 -
> 2 files changed, 10 deletions(-)
> delete mode 100644 Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads
>
> diff --git a/Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads
> b/Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads
> deleted file mode 100644
> index b0b0eeb20fe3..
> --- a/Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads
> +++ /dev/null
> @@ -1,5 +0,0 @@
> -What:/proc/sys/vm/nr_pdflush_threads
> -Date:June 2012
> -Contact: Wanpeng Li
> -Description: Since pdflush is replaced by per-BDI flusher, the interface of
> old pdflush
> - exported in /proc/sys/vm/ should be removed.
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 6648fbbb8157..a5dd8d82c253 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1345,11 +1345,6 @@ static struct ctl_table vm_table[] = {
> .extra1 = &zero,
> },
> {
> - .procname = "nr_pdflush_threads",
> - .mode = 0444 /* read-only */,
> - .proc_handler = pdflush_proc_obsolete,
> - },
> - {
> .procname = "swappiness",
> .data = &vm_swappiness,
> .maxlen = sizeof(vm_swappiness),
> --
> 2.7.4
>
--
Jan Kara
SUSE Labs, CR
Re: [PATCH v4 19/27] x86: assembly, make some functions local
On 10/02/2017, 02:48 PM, Ard Biesheuvel wrote:
> On 2 October 2017 at 10:12, Jiri Slaby wrote:
>> There is a couple of assembly functions, which are invoked only locally
>> in the file they are defined. In C, we mark them "static". In assembly,
>> annotate them using SYM_{FUNC,CODE}_START_LOCAL (and switch their
>> ENDPROC to SYM_{FUNC,CODE}_END too). Whether FUNC or CODE depends on
>> ENDPROC/END for a particular function (C or non-C).
>>
>
> I wasn't cc'ed on the cover letter, so I am missing the rationale of
> replacing ENTRY/ENDPROC with other macros.
There was no cover letter. I am attaching what is in PATCH 1/27 instead:
Introduce new C macros for annotations of functions and data in
assembly. There is a long-standing mess in macros like ENTRY, END,
ENDPROC and similar. They are used in different manners and sometimes
incorrectly.
So introduce macros with clear use to annotate assembly as follows:
a) Support macros for the ones below
SYM_T_FUNC -- type used by assembler to mark functions
SYM_T_OBJECT -- type used by assembler to mark data
SYM_T_NONE -- type used by assembler to mark entries of unknown type
They are defined as STT_FUNC, STT_OBJECT, and STT_NOTYPE
respectively. According to the gas manual, this is the most portable
way. I am not sure about other assemblers, so we can switch this back
to %function and %object if this turns into a problem. Architectures
can also override them by something like ", @function" if they need.
SYM_A_ALIGN, SYM_A_NONE -- align the symbol?
SYM_V_GLOBAL, SYM_V_WEAK, SYM_V_LOCAL -- visibility of symbols
b) Mostly internal annotations, used by the ones below
SYM_ENTRY -- use only if you have to (for non-paired symbols)
SYM_START -- use only if you have to (for paired symbols)
SYM_END -- use only if you have to (for paired symbols)
c) Annotations for code
SYM_FUNC_START_LOCAL_ALIAS -- use where there are two local names for
one function
SYM_FUNC_START_ALIAS -- use where there are two global names for one
function
SYM_FUNC_END_ALIAS -- the end of LOCAL_ALIASed or ALIASed function
SYM_FUNC_START -- use for global functions
SYM_FUNC_START_NOALIGN -- use for global functions, w/o alignment
SYM_FUNC_START_LOCAL -- use for local functions
SYM_FUNC_START_LOCAL_NOALIGN -- use for local functions, w/o
alignment
SYM_FUNC_START_WEAK -- use for weak functions
SYM_FUNC_START_WEAK_NOALIGN -- use for weak functions, w/o alignment
SYM_FUNC_END -- the end of SYM_FUNC_START_LOCAL, SYM_FUNC_START,
SYM_FUNC_START_WEAK, ...
SYM_FUNC_INNER_LABEL -- only for labels in the middle of functions
SYM_FUNC_INNER_LABEL_NOALIGN -- only for labels in the middle of
functions, w/o alignment
For functions with special (non-C) calling conventions:
SYM_CODE_START -- use for non-C (special) functions
SYM_CODE_START_NOALIGN -- use for non-C (special) functions, w/o
alignment
SYM_CODE_START_LOCAL -- use for local non-C (special) functions
SYM_CODE_START_LOCAL_NOALIGN -- use for local non-C (special)
functions, w/o alignment
SYM_CODE_END -- the end of SYM_CODE_START_LOCAL or SYM_CODE_START
SYM_CODE_INNER_LABEL -- only for labels in the middle of code
SYM_CODE_INNER_LABEL_NOALIGN -- only for labels in the middle of code
d) For data
SYM_DATA_START -- global data symbol
SYM_DATA_END -- the end of the SYM_DATA_START symbol
SYM_DATA_END_LABEL -- the labeled end of SYM_DATA_START symbol
SYM_DATA_SIMPLE -- start+end wrapper around simple global data
SYM_DATA_SIMPLE_LOCAL -- start+end wrapper around simple local data
==
The macros allow to pair starts and ends of functions and mark functions
correctly in the output ELF objects.
All users of the old macros in x86 are converted to use these in further
patches.
thanks,
--
js
suse labs
Re: [PATCH] firmware: add firmware to new device's devres list for second time cache
On Tue, Aug 22, 2017 at 03:52:46PM +0800, Kai-Heng Feng wrote: > Currently, firmware will only be chached if assign_firmware_buf() gets > called. > > When a device loses its power or a USB device gets plugged to another > port under suspend, request_firmware() can still find cached firmware, > but firmware name no longer associates with the new device's devres. > So next time the system suspend, those firmware won't be cached. > > Hence, we should add the firmware name to the devres when the firmware > is found in cache, to make the firmware cacheable next time. > > Signed-off-by: Kai-Heng Feng > --- > drivers/base/firmware_class.c | 4 > 1 file changed, 4 insertions(+) Luis???
Re: [PATCH] Add new uio device for PCI with dynamic memory allocation
Hi Dan. Thanks for your comments. I can fix all of those.
Probably there is also some upgrade needed for the MSI stuff.
pci_disable_msi() is not there anymore, so I have to use
pci_alloc_irq_vectors(). Doing tests with my PCIe HW I had some
problems with masking the legacy IRQs. Probably uio_pci_generic
suffers from the same problems.
Can someone tell me how to properly handle legacy and MSI
interrupts in irqhandler()? Or should I implement a irqcontrol()
function?
Best regards,
Manuel Stahl
On Di, 2017-10-03 at 12:48 +0300, Dan Carpenter wrote:
> Looks good. A couple minor comments below.
>
> On Mon, Oct 02, 2017 at 03:02:09PM +, Stahl, Manuel wrote:
> > +static int open(struct uio_info *info, struct inode *inode)
> > +{
> > + struct uio_pci_dmem_dev *priv = to_uio_pci_dmem_dev(info);
> > + struct uio_mem *uiomem;
> > + int ret = 0;
> > + int dmem_region = 0;
> > +
> > + uiomem = &priv->info.mem[dmem_region];
> > +
> > + mutex_lock(&priv->alloc_lock);
> > + while (!priv->refcnt && uiomem < &priv->info.mem[MAX_UIO_MAPS]) {
> > + void *addr;
> > +
> > + if (!uiomem->size)
> > + break;
> > +
> > + addr = dma_alloc_coherent(&priv->pdev->dev, uiomem->size,
> > + (dma_addr_t *)&uiomem->addr,
> > + GFP_KERNEL);
> > + if (!addr)
> > + uiomem->addr = DMEM_MAP_ERROR;
> > +
> > + priv->dmem_region_vaddr[dmem_region++] = addr;
> > + ++uiomem;
> > + }
> > + if (pci_check_and_mask_intx(priv->pdev))
> > + dev_info(&priv->pdev->dev, "Found pending interrupt");
> > +
> > + if (!priv->refcnt)
> > + pci_set_master(priv->pdev);
> > +
> > + priv->refcnt++;
> > +
> > + mutex_unlock(&priv->alloc_lock);
> > +
> > + return ret;
>
> Just "return 0;" is more readable/explicit than "return ret;".
>
> > +}
> > +
> > +static int release(struct uio_info *info, struct inode *inode)
> > +{
> > + struct uio_pci_dmem_dev *priv = to_uio_pci_dmem_dev(info);
> > + struct uio_mem *uiomem;
> > + int dmem_region = 0;
> > +
> > + uiomem = &priv->info.mem[dmem_region];
> > +
> > + mutex_lock(&priv->alloc_lock);
> > +
> > + priv->refcnt--;
> > + while (!priv->refcnt && uiomem < &priv->info.mem[MAX_UIO_MAPS]) {
> > + if (!uiomem->size)
> > + break;
>
> I think this needs to be a continue instead of a break to match
> parse_dmem_entries() since we don't break for size == 0 in that loop.
>
> > + if (priv->dmem_region_vaddr[dmem_region]) {
> > + dma_free_coherent(&priv->pdev->dev, uiomem->size,
> > + priv->dmem_region_vaddr[dmem_region],
> > + uiomem->addr);
> > + }
> > + uiomem->addr = DMEM_MAP_ERROR;
> > + ++dmem_region;
> > + ++uiomem;
> > + }
> > + if (pci_check_and_mask_intx(priv->pdev))
> > + dev_info(&priv->pdev->dev, "Found pending interrupt");
> > +
> > + if (!priv->refcnt)
> > + pci_clear_master(priv->pdev);
> > +
> > + mutex_unlock(&priv->alloc_lock);
> > + return 0;
> > +}
> > +
> > +static int dmem_mmap(struct uio_info *info, struct vm_area_struct *vma)
> > +{
> > + struct uio_pci_dmem_dev *gdev = to_uio_pci_dmem_dev(info->priv);
> > + struct uio_mem *uiomem;
> > + int mi = vma->vm_pgoff;
> > +
> > + if (mi >= MAX_UIO_MAPS)
> > + return -EINVAL;
> > +
> > + uiomem = &info->mem[mi];
> > + if (uiomem->memtype != UIO_MEM_PHYS)
> > + return -EINVAL;
> > + if (!uiomem->size)
> > + return -EINVAL;
> > +
> > + /* DMA address */
> > + vma->vm_pgoff = 0;
> > + return dma_mmap_coherent(&gdev->pdev->dev, vma,
> > + gdev->dmem_region_vaddr[mi],
> > + uiomem->addr, uiomem->size);
> > +}
> > +
> > +/* Interrupt handler. Read/modify/write the command register to disable the
> > + * interrupt.
> > + */
> > +static irqreturn_t irqhandler(int irq, struct uio_info *info)
> > +{
> > + struct uio_pci_dmem_dev *gdev = to_uio_pci_dmem_dev(info);
> > +
> > + if (gdev->pdev->msi_enabled)
> > + return IRQ_HANDLED;
> > +
> > + if (pci_check_and_mask_intx(gdev->pdev))
> > + return IRQ_HANDLED;
> > +
> > + return IRQ_NONE;
> > +}
> > +
> > +static unsigned int uio_dmem_dma_bits = 32;
> > +static char uio_dmem_sizes[256];
> > +
> > +static int parse_dmem_entries(struct pci_dev *pdev,
> > + const struct pci_device_id *id,
> > + struct uio_pci_dmem_dev *gdev)
> > +{
> > + int ret;
> > + u32 regions = 0;
> > + u32 vendor, device;
> > + char *s, *tok, *sizes = NULL;
> > + unsigned long long size;
> > + struct uio_mem *uiomem;
> > + char * const buf = kstrdup(uio_dmem_sizes, GFP_KERNEL);
> > +
> > + if (!buf) {
> > + ret = -ENOMEM;
> > +
Re: [PATCH 1/2] writeback: eliminate work item allocation in bd_start_writeback()
On Tue 03-10-17 09:16:20, Jens Axboe wrote:
> Handle start-all writeback like we do periodic or kupdate
> style writeback - by marking the bdi_writeback as needing a full
> flush, and simply waking the thread. This eliminates the need to
> allocate and queue a specific work item just for this purpose.
>
> After this change, we truly only ever have one of them running at
> any point in time. We mark the need to start all flushes, and the
> writeback thread will clear it once it has processed the request.
>
> Signed-off-by: Jens Axboe
Just one nit below. You can add:
Reviewed-by: Jan Kara
> diff --git a/include/linux/backing-dev-defs.h
> b/include/linux/backing-dev-defs.h
> index 420de5c7c7f9..f0f1df29d6b8 100644
> --- a/include/linux/backing-dev-defs.h
> +++ b/include/linux/backing-dev-defs.h
> @@ -116,6 +116,7 @@ struct bdi_writeback {
>
> struct fprop_local_percpu completions;
> int dirty_exceeded;
> + int start_all_reason;
This should be 'enum wb_reason' instead of 'int'.
Honza
--
Jan Kara
SUSE Labs, CR
Re: [PATCH v4 4/5] cramfs: add mmap support
On Tue, Oct 03, 2017 at 11:40:28AM -0400, Nicolas Pitre wrote: > I provided that explanation several times by now in my cover letter. And > separately even to you directly at least once. What else should I do? You should do the right things instead of stating irrelevant things in your cover letter. As said in my last mail: look at the VM_MIXEDMAP flag and how it is used by DAX, and you'll get out of the vma splitting business in the fault path. If the fs/dax.c code scares you take a look at drivers/dax/device.c instead.
Re: Re: [PATCH] MAINTAINERS: update TPM driver infrastructure changes
On Fri, Sep 29, 2017 at 08:31:30PM +0200, Peter Huewe wrote: > > > > > Gesendet: Freitag, 29. September 2017 um 18:59 Uhr > Von: "Jarkko Sakkinen" > An: "Mimi Zohar" , peterhu...@gmx.de > Cc: linux-kernel@vger.kernel.org, linux-integr...@vger.kernel.org > Betreff: Re: [PATCH] MAINTAINERS: update TPM driver infrastructure changes > On Tue, Sep 26, 2017 at 12:16:20PM -0400, Mimi Zohar wrote: > > Hi Jarkko, > > > > > +Q: https://patchwork.kernel.org/project/linux-integrity/list/ > > > > Is there a way of viewing not just the posted patches, but the > > discussion as well? Do we need to set up an archive as well? > > > > Mimi > > > Peter, did you setup archiving? > Jarkko, did you read my other email? > > /Peter > > > p.s.: > Spinics is archiving us at > https://www.spinics.net/lists/linux-integrity/[https://www.spinics.net/lists/linux-integrity/] > > It would be good to add this to the vger page + documented on the wiki, which > should be added to MAINTAINERS. > http://kernsec.org/wiki/index.php/Linux_Kernel_Integrity[http://kernsec.org/wiki/index.php/Linux_Kernel_Integrity] > as wiki page Can you send a follow-up patch to update MAINTAINERS? The first one is already applied so it does not make sense to revise it and it doesn't contain any errors. /Jarkko
[PATCH for 4.15] leds: pca955x: Fix configuration on GPIO request for input
The prior patch which fixed the problem with output inversion failed to
account for a necessary change to requesting a pin for input, which is
affected by the open-drain nature of the hardware.
Previously pca955x_gpio_direction_input() called through
pca955x_set_value() to configure the direction, however continuing down
this route lead to a brain-hemorrhaging level of necessary inversions.
Instead, remove one layer by calling directly into pca955x_led_set(),
and define PCA955X_GPIO_INPUT to paper over the rest of the confusion.
Cc: Cédric Le Goater
Cc: Joel Stanley
Signed-off-by: Andrew Jeffery
Tested-by: Matt Spinler
---
Jacek:
The prior patch called out above is "eae1693fb026 leds: pca955x: Don't
invert requested value in pca955x_gpio_set_value()" (at the time of sending). I
think I've seen the leds tree be rebased in the past, so feel free to squash
this change in if desired.
drivers/leds/leds-pca955x.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
index 6dcd2d7cc6a4..78183f90820e 100644
--- a/drivers/leds/leds-pca955x.c
+++ b/drivers/leds/leds-pca955x.c
@@ -61,6 +61,7 @@
#define PCA955X_LS_BLINK0 0x2 /* Blink at PWM0 rate */
#define PCA955X_LS_BLINK1 0x3 /* Blink at PWM1 rate */
+#define PCA955X_GPIO_INPUT LED_OFF
#define PCA955X_GPIO_HIGH LED_OFF
#define PCA955X_GPIO_LOW LED_FULL
@@ -358,8 +359,11 @@ static int pca955x_gpio_get_value(struct gpio_chip *gc,
unsigned int offset)
static int pca955x_gpio_direction_input(struct gpio_chip *gc,
unsigned int offset)
{
- /* To use as input ensure pin is not driven */
- return pca955x_set_value(gc, offset, 0);
+ struct pca955x *pca955x = gpiochip_get_data(gc);
+ struct pca955x_led *led = &pca955x->leds[offset];
+
+ /* To use as input ensure pin is not driven. */
+ return pca955x_led_set(&led->led_cdev, PCA955X_GPIO_INPUT);
}
static int pca955x_gpio_direction_output(struct gpio_chip *gc,
--
2.11.0
Re: [PATCH] kbuild: Fix optimization level choice default
On Wed, Oct 4, 2017 at 1:53 AM, Ulf Magnusson wrote: > The choice containing the CC_OPTIMIZE_FOR_PERFORMANCE symbol > accidentally added a "CONFIG_" prefix when trying to make it the > default, selecting an undefined symbol as the default. > > The mistake is harmless here: Since the default symbol is not visible, > the choice falls back on using the visible symbol as the default > instead, which is CC_OPTIMIZE_FOR_PERFORMANCE, as intended. > > A patch that makes Kconfig print a warning in this case has been > submitted separately: > http://www.spinics.net/lists/linux-kbuild/msg15566.html > > Signed-off-by: Ulf Magnusson Acked-by: Arnd Bergmann
Re: [PATCH 0/3] tpm: retrieve digest size of unknown algorithms from TPM
Hi And apologies for late review. On Mon, Sep 25, 2017 at 01:19:47PM +0200, Roberto Sassu wrote: > This patch set derives from a larger patch set which modifies the TPM > driver API in order to extend a PCR with multiple digests. It can be > retrieved at the URL: > > https://sourceforge.net/p/tpmdd/mailman/message/35905412/ A patch set should be able to live on its own. Please remove this link. I don't care about that patch set at this point and I'm not going to give any distant promises. > The TPM driver currently relies on the crypto subsystem to determine the > digest size of supported TPM algorithms. In the future, TPM vendors might > implement new algorithms in their chips, and those algorithms might not > be supported by the crypto subsystem. > > Usually, vendors provide patches for the new hardware, and likely > the crypto subsystem will be updated before the new algorithm is > introduced. However, old kernels might be updated later, after patches > are included in the mainline kernel. This would leave the opportunity > for attackers to misuse PCRs, as PCR banks with an unknown algorithm > are not extended. > > This patch set provides a long term solution for this issue. If a TPM > algorithm is not known by the crypto subsystem, the TPM driver retrieves > the digest size from the TPM with a PCR read. All the PCR banks are > extended, even if the algorithm is not yet supported by the crypto > subsystem. This part makes sense to me. /Jarkko
Re: [PATCH v4 19/27] x86: assembly, make some functions local
Hello Jiri,
On 4 October 2017 at 08:22, Jiri Slaby wrote:
> On 10/02/2017, 02:48 PM, Ard Biesheuvel wrote:
>> On 2 October 2017 at 10:12, Jiri Slaby wrote:
>>> There is a couple of assembly functions, which are invoked only locally
>>> in the file they are defined. In C, we mark them "static". In assembly,
>>> annotate them using SYM_{FUNC,CODE}_START_LOCAL (and switch their
>>> ENDPROC to SYM_{FUNC,CODE}_END too). Whether FUNC or CODE depends on
>>> ENDPROC/END for a particular function (C or non-C).
>>>
>>
>> I wasn't cc'ed on the cover letter, so I am missing the rationale of
>> replacing ENTRY/ENDPROC with other macros.
>
> There was no cover letter. I am attaching what is in PATCH 1/27 instead:
> Introduce new C macros for annotations of functions and data in
> assembly. There is a long-standing mess in macros like ENTRY, END,
> ENDPROC and similar. They are used in different manners and sometimes
> incorrectly.
>
I must say, I don't share this sentiment.
In arm64, we use ENTRY/ENDPROC for functions with external linkage,
and the bare symbol name/ENDPROC for functions with local linkage. I
guess we could add ENDOBJECT if we wanted to, but we never really felt
the need.
> So introduce macros with clear use to annotate assembly as follows:
>
> a) Support macros for the ones below
>SYM_T_FUNC -- type used by assembler to mark functions
>SYM_T_OBJECT -- type used by assembler to mark data
>SYM_T_NONE -- type used by assembler to mark entries of unknown type
>
Is is necessary to mark an entry as having no type? What is the
default type for an unmarked entry?
>They are defined as STT_FUNC, STT_OBJECT, and STT_NOTYPE
>respectively. According to the gas manual, this is the most portable
>way. I am not sure about other assemblers, so we can switch this back
>to %function and %object if this turns into a problem. Architectures
>can also override them by something like ", @function" if they need.
>
>SYM_A_ALIGN, SYM_A_NONE -- align the symbol?
>SYM_V_GLOBAL, SYM_V_WEAK, SYM_V_LOCAL -- visibility of symbols
>
Linkage != visibility
> b) Mostly internal annotations, used by the ones below
>SYM_ENTRY -- use only if you have to (for non-paired symbols)
>SYM_START -- use only if you have to (for paired symbols)
>SYM_END -- use only if you have to (for paired symbols)
>
> c) Annotations for code
>SYM_FUNC_START_LOCAL_ALIAS -- use where there are two local names for
> one function
>SYM_FUNC_START_ALIAS -- use where there are two global names for one
> function
>SYM_FUNC_END_ALIAS -- the end of LOCAL_ALIASed or ALIASed function
>
>SYM_FUNC_START -- use for global functions
>SYM_FUNC_START_NOALIGN -- use for global functions, w/o alignment
>SYM_FUNC_START_LOCAL -- use for local functions
>SYM_FUNC_START_LOCAL_NOALIGN -- use for local functions, w/o
> alignment
>SYM_FUNC_START_WEAK -- use for weak functions
>SYM_FUNC_START_WEAK_NOALIGN -- use for weak functions, w/o alignment
>SYM_FUNC_END -- the end of SYM_FUNC_START_LOCAL, SYM_FUNC_START,
> SYM_FUNC_START_WEAK, ...
>
>SYM_FUNC_INNER_LABEL -- only for labels in the middle of functions
>SYM_FUNC_INNER_LABEL_NOALIGN -- only for labels in the middle of
> functions, w/o alignment
>
>For functions with special (non-C) calling conventions:
>SYM_CODE_START -- use for non-C (special) functions
>SYM_CODE_START_NOALIGN -- use for non-C (special) functions, w/o
> alignment
>SYM_CODE_START_LOCAL -- use for local non-C (special) functions
>SYM_CODE_START_LOCAL_NOALIGN -- use for local non-C (special)
> functions, w/o alignment
>SYM_CODE_END -- the end of SYM_CODE_START_LOCAL or SYM_CODE_START
>
>SYM_CODE_INNER_LABEL -- only for labels in the middle of code
>SYM_CODE_INNER_LABEL_NOALIGN -- only for labels in the middle of code
>
> d) For data
>SYM_DATA_START -- global data symbol
>SYM_DATA_END -- the end of the SYM_DATA_START symbol
>SYM_DATA_END_LABEL -- the labeled end of SYM_DATA_START symbol
>SYM_DATA_SIMPLE -- start+end wrapper around simple global data
>SYM_DATA_SIMPLE_LOCAL -- start+end wrapper around simple local data
>
I am sorry but I think this is terrible. Do we really need 20+ new
macros to wrap every single assembler directive involved in defining
symbols and setting their attributes?
Is this issue you are solving widely perceived as a problem?
Re: Re: [PATCH] MAINTAINERS: update TPM driver infrastructure changes
Will do. Peter Am 4. Oktober 2017 09:27:08 MESZ schrieb Jarkko Sakkinen : >On Fri, Sep 29, 2017 at 08:31:30PM +0200, Peter Huewe wrote: >> >> >> >> >> Gesendet: Freitag, 29. September 2017 um 18:59 Uhr >> Von: "Jarkko Sakkinen" >> An: "Mimi Zohar" , peterhu...@gmx.de >> Cc: linux-kernel@vger.kernel.org, linux-integr...@vger.kernel.org >> Betreff: Re: [PATCH] MAINTAINERS: update TPM driver infrastructure >changes >> On Tue, Sep 26, 2017 at 12:16:20PM -0400, Mimi Zohar wrote: >> > Hi Jarkko, >> > >> > > +Q: https://patchwork.kernel.org/project/linux-integrity/list/ >> > >> > Is there a way of viewing not just the posted patches, but the >> > discussion as well? Do we need to set up an archive as well? >> > >> > Mimi >> >> > Peter, did you setup archiving? >> Jarkko, did you read my other email? >> >> /Peter >> >> >> p.s.: >> Spinics is archiving us at >> >https://www.spinics.net/lists/linux-integrity/[https://www.spinics.net/lists/linux-integrity/] >> >> It would be good to add this to the vger page + documented on the >wiki, which should be added to MAINTAINERS. >> >http://kernsec.org/wiki/index.php/Linux_Kernel_Integrity[http://kernsec.org/wiki/index.php/Linux_Kernel_Integrity] >as wiki page > >Can you send a follow-up patch to update MAINTAINERS? > >The first one is already applied so it does not make sense to >revise it and it doesn't contain any errors. > >/Jarkko -- Sent from my mobile
Re: [PATCH v3 12/12] ARM: dtsi: axp81x: set pinmux for GPIO0/1 when used as LDOs
Hi Chen-Yu, Linus,
On 03/10/2017 17:08, Chen-Yu Tsai wrote:
> On Tue, Oct 3, 2017 at 10:47 PM, Maxime Ripard
> wrote:
>> Hi Linus,
>>
>> On Tue, Oct 03, 2017 at 09:27:17AM +, Linus Walleij wrote:
>>> On Mon, Oct 2, 2017 at 2:08 PM, Quentin Schulz
>>> wrote:
>>>
On AXP813/818, GPIO0 and GPIO1 can be used as LDO as (respectively)
ldo_io0 and ldo_io1.
>>> (...)
+ gpio0_ldo: gpio0_ldo {
+ pins = "GPIO0";
+ function = "ldo";
+ };
>>> (...)
+ pinctrl-names = "default";
+ pinctrl-0 = <&gpio0_ldo>;
/* Disable by default to avoid conflicts with GPIO
*/
status = "disabled";
>>>
>>> So this is still by default disabled, but you make the default
>>> mode something called "ldo".
>>>
>>> And I think that is to be understood as a low-dropout regulator?
>>>
>>> So is the idea that this should be represented as a regulator
>>> in the end?
>>>
>>> Then I think the state name should not be "default" rather
>>> something like "regulator" and "default" should be the GPIO
>>> mode, as I guess something like that exists.
>>>
>>> Activating a regulator using pin control "default" mode is
>>> not very pretty. It would probably be unintuitive and end
>>> up wasting power because people will get confused about
>>> what is going on.
>>
>> That's not really it. The PMIC has pins that can be muxed either to
>> (regular) GPIOs, an ADC or to an LDO regulator.
>>
>> This is just muxing, the regulator will be enabled and disabled
>> separately through another register. If it wasn't the case, it would
>> indeed be very messy.
>
> No. Actually they are controlled in the same register, so it is
> very messy. The muxing options are:
>
> - 0: drive low
> - 1: drive high
> - 2: input with interrupt triggering
> - 3: LDO on
> - 4: LDO off
> - 5~7: floating (or ADC)
>
Just to be a little more precise,
- 0: drive low
- 1: drive high
- 2: input with interrupt triggering
- 3: LDO on
- 4: LDO off
- 5~7: floating (or ADC)
for AXP813, and
- 0: drive low
- 1: drive high
- 2: input with interrupt triggering
- 3: LDO on
- 4: ADC
- 5~7: floating
for AXP209.
So I think what you suggested Linus is not really relevant here as the
regulator framework will take care of disabling the regulator when
needed (for AXP813 via the ldo_off "muxing" selected by the regulator
framework).
However, there is no LDO off bit for AXP209 and the LDO can't be set to
0V in any other register. What's done now in the regulator driver for
AXP209 is to select the floating "muxing" for the pin when wanting to
disable the regulator. So I guess that's a way to handle it. Should we
do it another way?
Thanks for raising the issue, I frankly haven't thought about that at all.
I have to send a v4 to update the support for AXP813 (basically setting
ADC muxing to 0x5 instead of 0x4, for AXP813) as I misread the muxing
register description when adding support for it.
Thanks,
Quentin
--
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Re: [PATCH 2/9 v2] usb: usb251xb: Add USB251x specific port count setting
On 09/16/2017 12:42 PM, Serge Semin wrote:
> USB251xb as well as USB2517 datasheet states, that all these
> hubs differ by number of ports declared as the last digit in the
> model name. So USB2512 got two ports, USB2513 - three, and so on.
> Such setting must be reflected in the device specific data
> structure and corresponding dts property should be checked whether
> it doesn't get out of available ports.
>
> Signed-off-by: Serge Semin
> ---
> drivers/usb/misc/usb251xb.c | 21 ++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
> index 96a8c20ac..5cb0e5570 100644
> --- a/drivers/usb/misc/usb251xb.c
> +++ b/drivers/usb/misc/usb251xb.c
...
>
> @@ -422,8 +431,10 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
> for (i = 0; i < len / sizeof(u32); i++) {
> u32 port = be32_to_cpu(cproperty_u32[i]);
>
> - if ((port >= 1) && (port <= 4))
> + if ((port >= 1) && (port <= data->port_cnt))
> hub->non_rem_dev |= BIT(port);
> + else
> + dev_warn(dev, "port %u doesn't exist\n", port);
I'd prefer to add the (invalid) property trying to be set in this
warning messages. So someone is able to find the invalid dt setting
faster. Something like;
dev_warn(dev, "requested NRD port %u doesn't exist\n", port);
> }
> }
>
> @@ -433,8 +444,10 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
> for (i = 0; i < len / sizeof(u32); i++) {
> u32 port = be32_to_cpu(cproperty_u32[i]);
>
> - if ((port >= 1) && (port <= 4))
> + if ((port >= 1) && (port <= data->port_cnt))
> hub->port_disable_sp |= BIT(port);
> + else
> + dev_warn(dev, "port %u doesn't exist\n", port);
... same as above. For example:
dev_warn(dev, "requested PDS port %u doesn't exist\n", port);
> }
> }
>
> @@ -444,8 +457,10 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
> for (i = 0; i < len / sizeof(u32); i++) {
> u32 port = be32_to_cpu(cproperty_u32[i]);
>
> - if ((port >= 1) && (port <= 4))
> + if ((port >= 1) && (port <= data->port_cnt))
> hub->port_disable_bp |= BIT(port);
> + else
> + dev_warn(dev, "port %u doesn't exist\n", port);
... same as above. For example:
dev_warn(dev, "requested PDB port %u doesn't exist\n", port);
Apart from that this patch looks fine for me. Thanks for the spot of the
hardcoded max ports check.
regards,
Richard.L
Re: [PATCH 1/9 v2] usb: usb251xb: Add USB2517i specific struct and IDs
Hi Serge, On 09/16/2017 12:42 PM, Serge Semin wrote: > There are USB2517 and USB2517i hubs, which have almost the same > registers space as already supported USB251xbi series. The difference > it in DIDs and in few functions. This patch adds the USB2517/i data > structures to the driver, so it would have different setting depending > on the device discovered on i2c-bus. > > Signed-off-by: Serge Semin > --- > Documentation/devicetree/bindings/usb/usb251xb.txt | 3 ++- > drivers/usb/misc/usb251xb.c | 21 > - > 2 files changed, 22 insertions(+), 2 deletions(-) ... Please also mention support for the USB2517(i) hubs in the Kconfig helptext of the driver @ drivers/usb/misc/Kconfig. Thanks. Apart from that the patch looks good to me. regards, Richard.L
Re: [PATCH V9 13/15] mmc: block: Add CQE and blk-mq support
On Fri, Sep 22, 2017 at 2:37 PM, Adrian Hunter wrote: > Add CQE support to the block driver, including: > - optionally using DCMD for flush requests > - "manually" issuing discard requests > - issuing read / write requests to the CQE > - supporting block-layer timeouts > - handling recovery > - supporting re-tuning > > CQE offers 25% - 50% better random multi-threaded I/O. There is a slight > (e.g. 2%) drop in sequential read speed but no observable change to sequential > write. > > CQE automatically sends the commands to complete requests. However it only > supports reads / writes and so-called "direct commands" (DCMD). Furthermore > DCMD is limited to one command at a time, but discards require 3 commands. > That makes issuing discards through CQE very awkward, but some CQE's don't > support DCMD anyway. So for discards, the existing non-CQE approach is > taken, where the mmc core code issues the 3 commands one at a time i.e. > mmc_erase(). Where DCMD is used, is for issuing flushes. > > For host controllers without CQE support, blk-mq support is extended to > synchronous reads/writes or, if the host supports CAP_WAIT_WHILE_BUSY, > asynchonous reads/writes. The advantage of asynchronous reads/writes is > that it allows the preparation of the next request while the current > request is in progress. > > Signed-off-by: Adrian Hunter I am trying to wrap my head around this large patch. The size makes it hard but I am doing my best. Some overarching questions: - Is the CQE only available on the MQ path (i.e. if you enabled MQ) or on both paths? I think it is reasonable that if we introduce a new feature like this it will only be available for the new block path. This reflects how the block maintainers e.g. only allow new scheduling policies to be merged on the MQ path. The old block layer is legacy and should not be extended with new cool features that can then be regarded as "regressions" if they don't work properly with MQ. Better to only implement them for MQ then. - Performance before/after path on MQ? I tested this very extensively when working with my (now dormant) MQ patch set. Better/equal/worse? (https://marc.info/?l=linux-mmc&m=148665788227015&w=2) The reason my patch set contained refactorings of async post-processing, removed the waitqueues and the context info, up to the point where I can issue requests in parallel, i.e. complete requests from the ->done() callback on the host and immediately let the core issue the next one, was due to performance issues. It's these patches from the old patch set: mmc: core: move some code in mmc_start_areq() mmc: core: refactor asynchronous request finalization mmc: core: refactor mmc_request_done() mmc: core: move the asynchronous post-processing mmc: core: add a kthread for completing requests mmc: core: replace waitqueue with worker mmc: core: do away with is_done_rcv mmc: core: do away with is_new_req mmc: core: kill off the context info mmc: queue: simplify queue logic mmc: block: shuffle retry and error handling mmc: queue: stop flushing the pipeline with NULL mmc: queue: issue struct mmc_queue_req items mmc: queue: get/put struct mmc_queue_req mmc: queue: issue requests in massive parallel I.e. I made 15 patches just to make sure the new block layer did not regress performance. The MQ-switch patch was just the final step of these 16 patches. Most energy went into that and I think it will be necessary still to work with MQ in the long haul. I am worried that this could add a second MQ execution path that performs worse than the legacy block path for the above reason, i.e. it doesn't really take advantage of the MQ speedups by marshalling the requests and not doing away with the waitqueues and not completing the requests out-of-order, so we get stuck with a lump of MQ code that doesn't perform and therefore we cannot switch seamlessly to MQ. Yours, Linus Walleij
Re: [PATCHv3] mm: Account pud page tables
On Wed, Oct 04, 2017 at 06:03:47AM +, Vlastimil Babka wrote:
> On 10/02/2017 10:04 AM, Kirill A. Shutemov wrote:
> > On machine with 5-level paging support a process can allocate
> > significant amount of memory and stay unnoticed by oom-killer and
> > memory cgroup. The trick is to allocate a lot of PUD page tables.
> > We don't account PUD page tables, only PMD and PTE.
> >
> > We already addressed the same issue for PMD page tables, see
> > dc6c9a35b66b ("mm: account pmd page tables to the process").
> > Introduction 5-level paging bring the same issue for PUD page tables.
> >
> > The patch expands accounting to PUD level.
> >
> > Signed-off-by: Kirill A. Shutemov
> > Cc: Michal Hocko
> > Cc: Vlastimil Babka
>
> Acked-by: Vlastimil Babka
>
> Small fix below:
>
> > --- a/fs/proc/task_mmu.c
> > +++ b/fs/proc/task_mmu.c
> > @@ -25,7 +25,7 @@
> >
> > void task_mem(struct seq_file *m, struct mm_struct *mm)
> > {
> > - unsigned long text, lib, swap, ptes, pmds, anon, file, shmem;
> > + unsigned long text, lib, swap, ptes, pmds, puds, anon, file, shmem;
> > unsigned long hiwater_vm, total_vm, hiwater_rss, total_rss;
> >
> > anon = get_mm_counter(mm, MM_ANONPAGES);
> > @@ -51,6 +51,7 @@ void task_mem(struct seq_file *m, struct mm_struct *mm)
> > swap = get_mm_counter(mm, MM_SWAPENTS);
> > ptes = PTRS_PER_PTE * sizeof(pte_t) * atomic_long_read(&mm->nr_ptes);
> > pmds = PTRS_PER_PMD * sizeof(pmd_t) * mm_nr_pmds(mm);
> > + puds = PTRS_PER_PUD * sizeof(pmd_t) * mm_nr_puds(mm);
>
>^ pud_t ?
Ouch. Thanks for spotting this.
Andrew, could you take this fixup:
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 0bf9e423aa99..627de66204bd 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -51,7 +51,7 @@ void task_mem(struct seq_file *m, struct mm_struct *mm)
swap = get_mm_counter(mm, MM_SWAPENTS);
ptes = PTRS_PER_PTE * sizeof(pte_t) * atomic_long_read(&mm->nr_ptes);
pmds = PTRS_PER_PMD * sizeof(pmd_t) * mm_nr_pmds(mm);
- puds = PTRS_PER_PUD * sizeof(pmd_t) * mm_nr_puds(mm);
+ puds = PTRS_PER_PUD * sizeof(pud_t) * mm_nr_puds(mm);
seq_printf(m,
"VmPeak:\t%8lu kB\n"
"VmSize:\t%8lu kB\n"
--
Kirill A. Shutemov
[PATCH v3] staging: ccree: Convert to platform_{get,set}_drvdata()
From: Suniel Mahesh
Platform devices are expected to use wrapper functions,
platform_{get,set}_drvdata() with platform_device as argument,
for getting and setting the driver data. dev_{get,set}_drvdata()
are using &plat_dev->dev.
For wrapper functions we can directly pass a struct platform_device.
dev_set_drvdata() is redundant and therefore removed. The driver core
clears the driver data to NULL after device_release or on probe failure.
Signed-off-by: Suniel Mahesh
---
Changes for v3:
- Rebased on top of staging-testing as suggested by Greg KH.
- Patch was tested and built(ARCH=arm) on staging-testing.
---
Changes for v2:
- Rebased on top of staging-testing.
---
Note:
- No build issues reported, however it was not tested on
real hardware.
---
drivers/staging/ccree/ssi_driver.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/ccree/ssi_driver.c
b/drivers/staging/ccree/ssi_driver.c
index 795a087..5f03c25 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -215,7 +215,7 @@ static int init_cc_resources(struct platform_device
*plat_dev)
rc = -ENOMEM;
goto post_drvdata_err;
}
- dev_set_drvdata(dev, new_drvdata);
+ platform_set_drvdata(plat_dev, new_drvdata);
new_drvdata->plat_dev = plat_dev;
new_drvdata->clk = of_clk_get(np, 0);
@@ -393,7 +393,6 @@ static int init_cc_resources(struct platform_device
*plat_dev)
cc_clk_off(new_drvdata);
post_drvdata_err:
dev_err(dev, "ccree init error occurred!\n");
- dev_set_drvdata(dev, NULL);
return rc;
}
@@ -407,7 +406,7 @@ void fini_cc_regs(struct ssi_drvdata *drvdata)
static void cleanup_cc_resources(struct platform_device *plat_dev)
{
struct ssi_drvdata *drvdata =
- (struct ssi_drvdata *)dev_get_drvdata(&plat_dev->dev);
+ (struct ssi_drvdata *)platform_get_drvdata(plat_dev);
ssi_aead_free(drvdata);
ssi_hash_free(drvdata);
@@ -423,7 +422,6 @@ static void cleanup_cc_resources(struct platform_device
*plat_dev)
#endif
fini_cc_regs(drvdata);
cc_clk_off(drvdata);
- dev_set_drvdata(&plat_dev->dev, NULL);
}
int cc_clk_on(struct ssi_drvdata *drvdata)
--
1.9.1
Re: [PATCH] nvme: use menu Kconfig interface
Thanks, applied to nvme-4.15.
MAP_FIXED for ELF mappings
Hi,
while studying CVE-2017-1000253 and the MAP_FIXED usage in load_elf*
code paths I have stumbled over MAP_FIXED usage for elf segments
mapping. I am not really familiar with this area much so I might draw
completely incorrect conclusions here but I am really wondering why we
are doing MAP_FIXED there at all.
I can see why some segments really have to be mapped at a specific
address but I wonder whether MAP_FIXED is the right tool to achieve
that. It seems to me that MAP_FIXED is fundamentally dangerous because
it unmaps any existing mapping. I assume that nothing should be really
mapped in the requested range that early so we can only stumble over
something when the address space randomization place things unexpectedly
(which was the case of the above mentioned CVE AFAIU).
So my primary question is whether we can/should simply drop MAP_FIXED
from elf_map at all. Instead we should test whether the mapping was
successful for the requested address and fail otherwise. I realize that
failing due to something that a user has no idea about sucks a lot but
it seems to me safer to simply complain into the log and fail is a safer
option.
Something like the following completely untested diff. Or am I
completely missing the point of the MAP_FIXED purpose?
---
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 6466153f2bf0..09456e2add18 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -341,6 +341,29 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr
*exec,
#ifndef elf_map
+static unsigned long elf_vm_mmap(struct file *filep, unsigned long addr,
+ unsigned long size, int prot, int type, unsigned long off)
+{
+ unsigned long map_addr;
+
+ /*
+* If caller requests the mapping at a specific place, make sure we fail
+* rather than potentially clobber an existing mapping which can have
+* security consequences (e.g. smash over the stack area).
+*/
+ map_addr = vm_mmap(filep, addr, size, prot, type & ~MAP_FIXED, off);
+ if (BAD_ADDR(map_addr))
+ return map_addr;
+
+ if ((type & MAP_FIXED) && map_addr != addr) {
+ pr_info("Uhuuh, elf segement at %p requested but the memory is
mapped already\n",
+ (void*)addr);
+ return -EAGAIN;
+ }
+
+ return map_addr;
+}
+
static unsigned long elf_map(struct file *filep, unsigned long addr,
struct elf_phdr *eppnt, int prot, int type,
unsigned long total_size)
@@ -366,11 +389,11 @@ static unsigned long elf_map(struct file *filep, unsigned
long addr,
*/
if (total_size) {
total_size = ELF_PAGEALIGN(total_size);
- map_addr = vm_mmap(filep, addr, total_size, prot, type, off);
+ map_addr = elf_vm_mmap(filep, addr, total_size, prot, type,
off);
if (!BAD_ADDR(map_addr))
vm_munmap(map_addr+size, total_size-size);
} else
- map_addr = vm_mmap(filep, addr, size, prot, type, off);
+ map_addr = elf_vm_mmap(filep, addr, size, prot, type, off);
return(map_addr);
}
@@ -1215,7 +1238,7 @@ static int load_elf_library(struct file *file)
eppnt++;
/* Now use mmap to map the library into memory. */
- error = vm_mmap(file,
+ error = elf_vm_mmap(file,
ELF_PAGESTART(eppnt->p_vaddr),
(eppnt->p_filesz +
ELF_PAGEOFFSET(eppnt->p_vaddr)),
--
Michal Hocko
SUSE Labs
Re: [PATCH 3/9 v2] usb: usb251xb: Add 5,6,7 ports mapping def setting
On 09/16/2017 12:42 PM, Serge Semin wrote: > USB2517 got three additionl downstream ports, which can > as well be mapped to another logical ports. USB2551xb driver typo in "USB251xb" > currently doesn't fully support such setting configuration > from dts file. This patch doesn't change this, but adds > usb2517 spcific ports default liner mapping. > > Signed-off-by: Serge Semin > --- > drivers/usb/misc/usb251xb.c | 12 +++- > 1 file changed, 11 insertions(+), 1 deletion(-) > Code itself looks good to me, therefore feel free to add: Acked-by: Richard Leitner regards, Richard.L
Re: usb/sound/bcd2000: warning in bcd2000_init_device
On Wed, Oct 04, 2017 at 08:10:59AM +0200, Takashi Iwai wrote:
> On Tue, 03 Oct 2017 19:42:21 +0200,
> Greg Kroah-Hartman wrote:
> >
> > On Tue, Oct 03, 2017 at 12:50:08PM -0400, Alan Stern wrote:
> > > On Tue, 3 Oct 2017, Takashi Iwai wrote:
> > >
> > > > > It's a dev_WARN because it indicates a potentially serious error in
> > > > > the
> > > > > driver: The driver has submitted an interrupt URB to a bulk endpoint.
> > > > >
> > > > > That may not sound bad, but the same check gets triggered if a driver
> > > > > submits a bulk URB to an isochronous endpoint, or any other invalid
> > > > > combination.
> > > > >
> > > > > Most likely the explanation here is that the driver doesn't bother to
> > > > > check the endpoint type because it expects the endpoint will always be
> > > > > interrupt. But that is not a safe strategy. USB devices and their
> > > > > firmware should not be trusted unnecessarily.
> > > > >
> > > > > The best fix is, like you said, to add a sanity check in the caller.
> > > >
> > > > OK, but then do we have some handy helper for the check?
> > > > As other bug reports by syzkaller suggest, there are a few other
> > > > drivers that do the same, submitting a urb with naive assumption of
> > > > the fixed EP for specific devices. In the end we'll need to put the
> > > > very same checks there in multiple places.
> > >
> > > Perhaps we could add a helper routine that would take a list of
> > > expected endpoint types and check that the actual endpoints match the
> > > types. But of course, all the drivers you're talking about would have
> > > to add a call to this helper routine.
> >
> > We have almost this type of function, usb_find_common_endpoints(),
> > what's wrong with using that? Johan has already swept the tree and
> > added a lot of these checks, odds are no one looked at the sound/
> > subdir...
>
> Well, what I had in my mind is just a snippet from usb_submit_urb(),
> something like:
>
> bool usb_sanity_check_urb_pipe(struct urb *urb)
> {
> struct usb_host_endpoint *ep;
> int xfertype;
> static const int pipetypes[4] = {
> PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, PIPE_INTERRUPT
> };
>
> ep = usb_pipe_endpoint(urb->dev, urb->pipe);
> xfertype = usb_endpoint_type(&ep->desc);
> return usb_pipetype(urb->pipe) != pipetypes[xfertype];
> }
>
> And calling this before usb_submit_urb() in each place that assigns
> the fixed EP as device-specific quirks.
> Does it make sense?
Yes, kind of, but checking the endpoint type/direction is what you are
expecting it to be as you "know" what the type should be for each
driver as it is unique.
Anyway, a "real" patch might make more sense to me.
thanks,
greg k-h
Re: [PATCH 3.18 00/24] 3.18.73-stable review
On Tue, Oct 03, 2017 at 01:25:32PM -0600, Shuah Khan wrote: > On 10/03/2017 06:18 AM, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 3.18.73 release. > > There are 24 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 Thu Oct 5 11:36:26 UTC 2017. > > Anything received after that time might be too late. > > > > The whole patch series can be found in one patch at: > > kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.73-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 for testing all of these and letting me know. greg k-h
Re: 4.14-rc2 on thinkpad x220: out of memory when inserting mmc card
On Tue, Oct 3, 2017 at 8:30 AM, Adrian Hunter wrote:
> On 02/10/17 17:09, Linus Walleij wrote:
>> On Sun, Oct 1, 2017 at 12:57 PM, Tetsuo Handa
>> wrote:
>>
> I inserted u-SD card, only to realize that it is not detected as it
> should be. And dmesg indeed reveals:
Tetsuo asked me to report this to linux-mm.
But 2^4 is 16 pages, IIRC that can't be expected to work reliably, and
thus this sounds like MMC bug, not mm bug.
>>
>>
>> I'm not sure I fully understand this error message:
>> "worker/2:1: page allocation failure: order:4"
>>
>> What I guess from context is that the mmc_init_request()
>> call is failing to allocate 16 pages, meaning for 4K pages
>> 64KB which is the typical bounce buffer.
>>
>> This is what the code has always allocated as bounce buffer,
>> but it used to happen upfront, when probing the MMC block layer,
>> rather than when allocating the requests.
>
> That is not exactly right. As I already wrote, the memory allocation used
> to be optional but became mandatory with:
>
> commit 304419d8a7e9204c5d19b704467b814df8c8f5b1
> Author: Linus Walleij
> Date: Thu May 18 11:29:32 2017 +0200
>
> mmc: core: Allocate per-request data using the block layer core
Yes you are right, it used to look like this, with the bounce buffer
hiding behind a Kconfig symbol:
#ifdef CONFIG_MMC_BLOCK_BOUNCE
if (host->max_segs == 1) {
unsigned int bouncesz;
bouncesz = MMC_QUEUE_BOUNCESZ;
if (bouncesz > host->max_req_size)
bouncesz = host->max_req_size;
if (bouncesz > host->max_seg_size)
bouncesz = host->max_seg_size;
if (bouncesz > (host->max_blk_count * 512))
bouncesz = host->max_blk_count * 512;
if (bouncesz > 512 &&
mmc_queue_alloc_bounce_bufs(mq, bouncesz)) {
blk_queue_bounce_limit(mq->queue, BLK_BOUNCE_ANY);
blk_queue_max_hw_sectors(mq->queue, bouncesz / 512);
blk_queue_max_segments(mq->queue, bouncesz / 512);
blk_queue_max_segment_size(mq->queue, bouncesz);
ret = mmc_queue_alloc_bounce_sgs(mq, bouncesz);
if (ret)
goto cleanup_queue;
bounce = true;
}
}
#endif
I recently concluded that I find no evidence whatsoever that anyone
turned this symbol on. Actually. (Checked defconfigs and distro configs.)
The option was just sitting there unused.
This code was never exercised except by some people who turned it
on on their custom kernels in the past. It's in practice dead code.
My patch started to allocate and use bounce buffers for all hosts
with max_segs == 1, unless specifically flagged NOT to use bounce
buffers.
That wasn't smart, I should have just deleted them. Mea culpa.
So that is why I asked Ulf to simply put the patch deleting the bounce
buffers that noone is using to fixes, and it should fix this problem.
Yours,
Linus Walleij
Re: [PATCH v2 2/2] dt-bindings: arm: amlogic: Add Tronsmart Vega S96 binding
On 03/10/2017 23:30, Rob Herring wrote: > On Tue, Oct 03, 2017 at 05:29:45PM +0200, Neil Armstrong wrote: >> Cc: supp...@tronsmart.com >> Acked-by: Jerome Brunet >> Signed-off-by: Oleg Ivanov >> Signed-off-by: Neil Armstrong >> --- >> Documentation/devicetree/bindings/arm/amlogic.txt | 1 + >> 1 file changed, 1 insertion(+) > > I acked v1. Please add acks when posting new versions. > Sorry, I misses it. Kevin, can you add the missing ack when applying ? Neil
Re: [PATCH 4.13 000/110] 4.13.5-stable review
On Tue, Oct 03, 2017 at 01:30:25PM -0700, Guenter Roeck wrote: > On Tue, Oct 03, 2017 at 02:28:22PM +0200, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 4.13.5 release. > > There are 110 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 Thu Oct 5 11:42:12 UTC 2017. > > Anything received after that time might be too late. > > > > Build results: > total: 145 pass: 145 fail: 0 > Qemu test results: > total: 123 pass: 123 fail: 0 > > Details are available at http://kerneltests.org/builders. Thanks for testing all of these and letting me know. greg k-h
Re: [PATCH V10 0/3] PM / Domains: Performance state support
On 4 October 2017 at 08:45, Viresh Kumar wrote: > On 03-10-17, 09:52, Ulf Hansson wrote: >> We sorted out things at LPC! >> >> However, the last weeks discussions at Linaro connect, raised a couple >> of more concerns with the current approach. Let me summarize them >> here. >> >> 1) >> The ->dev_get_performance_state(), which currently translates >> frequency for a device to a performance index of its PM domain, is too >> closely integrated with genpd. Instead this kind of translation rather >> belongs as a part of the OPP core, because of not limiting this only >> to translate frequencies, but perhaps *later* also voltages. >> >> 2) >> Propagating an aggregated increased requested performance state index >> for a genpd, upwards in the hierarchy of its master domains, is >> currently not needed by any existing SoCs. >> >> 3) If some day the need for 2) becomes required, we must not assume a >> 1 to 1 mapping of the supported performance state index for a >> master/subdomain. For example a domain may support 1-5, while its >> master may support 1-8. >> >> Taking this into consideration, this series need yet another round of >> re-spin. The ->dev_get_performance_state() part should be move to the >> OPP layer and the code dealing with the aggregation of the performance >> state index can be greatly simplified. > > Thanks for the summary. > > From the above, it looks like I can send this series right away instead of > waiting for the multiple genpd per device thing? Is that the case ? Yes, I think so! Kind regards Uffe
Re: [PATCH 4.4 00/41] 4.4.90-stable review
On Tue, Oct 03, 2017 at 03:30:14PM -0500, Tom Gall wrote: > > > On Oct 3, 2017, at 7:21 AM, Greg Kroah-Hartman > > wrote: > > > > This is the start of the stable review cycle for the 4.4.90 release. > > There are 41 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 Thu Oct 5 11:42:00 UTC 2017. > > Anything received after that time might be too late. > > > > The whole patch series can be found in one patch at: > > kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.4.90-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-4.4.y > > and the diffstat can be found below. > > > > thanks, > > > > greg k-h > > > > Test results from the linaro linux kernel functional test farm: > > kernel: 4.4.90-rc1 > git repo: > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > git branch: linux-4.4.y > git commit: 255b4a073a820d2f46a1ce1740f1a9be3de9661a > git describe: v4.4.89-42-g255b4a073a82 > Test details: > https://qa-reports.linaro.org/lkft/linux-stable-rc-4.4-oe/build/v4.4.89-42-g255b4a073a82 > > No regressions (compared to build v4.4.89-30-gb547584d016b) > > Boards, architectures and test suites: > - > > dell-poweredge-r200 - x86_64 > * boot - 1 pass > * kselftest - 44 pass - 24 known failures > * libhugetlbfs - 76 pass - 1 skip > * ltp-syscalls-tests - 941 pass - 175 skip - 11 known failures > > > And as a separate but related report, we have HiKey (arm64) results. These > are separate because there are some out of tree patches that didn’t quite > make 4.4 back in the day, thus for this board the kernel is a blend of the > LTS RC + some platform patches. > > Summary > > > kernel: 4.4.90-rc1 > git repo: https://git.linaro.org/lkft/arm64-stable-rc.git > git tag: 4.4.90-rc1-hikey-20171003 > git commit: 8b0848771d885b6030233931b2d2039382a1cf73 > git describe: v4.4.89-352-g8b0848771d88 > Test details: > https://qa-reports.linaro.org/lkft/linaro-hikey-stable-rc-4.4-oe/build/v4.4.89-352-g8b0848771d88 > > > No regressions (compared to build v4.4.89-340-ga6279f8aa398) > > Boards, architectures and test suites: > - > > hi6220-hikey - arm64 > * boot - 1 pass > * kselftest - 32 pass - 1 skip - 21 known failures > * libhugetlbfs - 90 pass - 1 skip > * ltp-syscalls-tests - 960 pass - 138 skip - 2 known failures Nice, thanks for letting me know, but that seems like a lot of "known failures" :( Any chance you can add a beaglebone black to your 4.4 testing to give it some "native arm" test support? That should run a "stock" 4.4 kernel, right? thanks, greg k-h
Re: [PATCH v2 4/4] KVM: LAPIC: Don't silently accept bad vectors
2017-10-04 1:53 GMT+08:00 Radim Krčmář :
> 2017-09-28 18:04-0700, Wanpeng Li:
>> From: Wanpeng Li
>>
>> Vectors 0-15 are reserved, and a physical LAPIC - upon sending or
>> receiving one - would generate an APIC error instead of doing the
>> requested action. Make our emulation behave similarly.
>>
>> Cc: Paolo Bonzini
>> Cc: Radim Krčmář
>> Signed-off-by: Wanpeng Li
>> ---
>> arch/x86/kvm/lapic.c | 30 --
>> 1 file changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index 6bafd06..a779ba9 100644
>> --- a/arch/x86/kvm/lapic.c
>> +++ b/arch/x86/kvm/lapic.c
>> @@ -935,6 +935,25 @@ bool kvm_intr_is_single_vcpu_fast(struct kvm *kvm,
>> struct kvm_lapic_irq *irq,
>> return ret;
>> }
>>
>> +static void apic_error(struct kvm_lapic *apic, unsigned long errmask)
>> +{
>> + uint32_t esr;
>> +
>> + esr = kvm_lapic_get_reg(apic, APIC_ESR);
>> +
>> + if ((esr & errmask) != errmask) {
>
> The spec makes me think that there is going to be only 1 interrupt
> (regardless of the number errors) until the software writes 0 to
> APIC_ESR. Is there a better description than the following 10.5.3?
>
> The ESR is a write/read register. Before attempt to read from the ESR,
> software should first write to it. (The value written does not affect
> the values read subsequently; only zero may be written in x2APIC
> mode.) This write clears any previously logged errors and updates the
> ESR with any errors detected since the last write to the ESR. This
> write also rearms the APIC error interrupt triggering mechanism.
>
> This also describes a different handling of APIC_ESR -- APIC_ESR is
> updated only on software writes to APIC_ESR. All errors in between seem
> to be logged internally (not sure where to migrate it).
Is there any thing need to be changed in this function?
>
>> + uint32_t lvterr = kvm_lapic_get_reg(apic, APIC_LVTERR);
>> +
>> + kvm_lapic_set_reg(apic, APIC_ESR, esr | errmask);
>> + if (!(lvterr & APIC_LVT_MASKED)) {
>> + struct kvm_lapic_irq irq;
>> +
>> + irq.vector = lvterr & 0xff;
>> + kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq,
>> NULL);
>> + }
>> + }
>> +}
>> +
>> /*
>> * Add a pending IRQ into lapic.
>> * Return 1 if successfully added and 0 if discarded.
>> @@ -946,6 +965,11 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
>> int delivery_mode,
>> int result = 0;
>> struct kvm_vcpu *vcpu = apic->vcpu;
>>
>> + if (unlikely(vector < 16) && delivery_mode == APIC_DM_FIXED) {
>> + apic_error(apic, APIC_ESR_RECVILL);
>
> The error is also triggered if lowest priority is supported and tries to
> deliver an invalid vector.
Could you point out this in SDM? :)
>
>> + return 0;
>> + }
>> +
>> trace_kvm_apic_accept_irq(vcpu->vcpu_id, delivery_mode,
>> trig_mode, vector);
>> switch (delivery_mode) {
>> @@ -1146,7 +1170,10 @@ static void apic_send_ipi(struct kvm_lapic *apic)
>> irq.trig_mode, irq.level, irq.dest_mode, irq.delivery_mode,
>> irq.vector, irq.msi_redir_hint);
>>
>> - kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
>> + if (unlikely(irq.vector < 16 && irq.delivery_mode == APIC_DM_FIXED))
>
> Please check how APICv self-IPI acceleration behaves, so we're
> consistent.
There is no vmexit for APICv self-IPI, so I think we can't intercept the vector.
Regards,
Wanpeng Li
>
> Thanks.
>
>> + apic_error(apic, APIC_ESR_SENDILL);
>> + else
>> + kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
>> }
>>
>> static u32 apic_get_tmcct(struct kvm_lapic *apic)
>> @@ -1734,7 +1761,6 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32
>> reg, u32 val)
>> case APIC_LVTPC:
>> case APIC_LVT1:
>> case APIC_LVTERR:
>> - /* TODO: Check vector */
>> if (!kvm_apic_sw_enabled(apic))
>> val |= APIC_LVT_MASKED;
>>
>> --
>> 2.7.4
>>
Re: Patch "KVM: VMX: avoid double list add with VT-d posted interrupts" has been added to the 4.13-stable tree
On Wed, Oct 04, 2017 at 12:30:21AM +0200, Stefan Lippers-Hollmann wrote: > Hi > > On 2017-10-03, Paolo Bonzini wrote: > > On 03/10/2017 09:46, Stefan Lippers-Hollmann wrote: > > > On 2017-10-02, gre...@linuxfoundation.org wrote: > > >> This is a note to let you know that I've just added the patch titled > > >> > > >> KVM: VMX: avoid double list add with VT-d posted interrupts > > >> > > >> to the 4.13-stable tree which can be found at: > > >> > > >> http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary > > >> > > >> The filename of the patch is: > > >> kvm-vmx-avoid-double-list-add-with-vt-d-posted-interrupts.patch > > >> and it can be found in the queue-4.13 subdirectory. > > > > > > This patch, as part of the current queue-4.13, breaks the build on > > > i386 (amd64/ x86_64 builds fine): > [...] > > There is another patch in the kvm tree to fix it, I'll send it to stable > > immediately. > > Thanks, I can confirm this to work in 4.13.5-rc1 (including > "KVM: VMX: use cmpxchg64") for i386 and x86_64. > > Unrelated to this specific, solved, issue I can confirm kernel > 4.9.53-rc1 to build and boot on armhf (ipq8065) and 4.4.90-rc1 > on mips (ar71xx). Nice, thanks for testing and letting me know. greg k-h
Re: [PATCH 4.9 00/64] 4.9.53-stable review
On Tue, Oct 03, 2017 at 03:29:46PM -0500, Tom Gall wrote: > > > On Oct 3, 2017, at 7:22 AM, Greg Kroah-Hartman > > wrote: > > > > This is the start of the stable review cycle for the 4.9.53 release. > > There are 64 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 Thu Oct 5 11:42:06 UTC 2017. > > Anything received after that time might be too late. > > > > The whole patch series can be found in one patch at: > > kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.9.53-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-4.9.y > > and the diffstat can be found below. > > > > thanks, > > > > greg k-h > > > > > Test results from the linaro linux kernel functional test farm: > > kernel: 4.9.53-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: aceea42c68d96c58958954e4c8c23a26f8883d62 > git describe: v4.9.52-65-gaceea42c68d9 > Test details: > https://qa-reports.linaro.org/lkft/linux-stable-rc-4.9-oe/build/v4.9.52-65-gaceea42c68d9 > > > No regressions (compared to build v4.9.51-78-ge009129d09cb) > > Boards, architectures and test suites: > - > > hi6220-hikey - arm64 > * boot - 1 pass > * kselftest - 38 pass - 1 skip - 15 known failures > * libhugetlbfs - 90 pass - 1 skip > * ltp-syscalls-tests - 964 pass - 136 skip > > juno-r2 - arm64 > * boot - 1 pass > * kselftest - 37 pass - 1 skip - 14 known failures > * libhugetlbfs - 90 pass - 1 skip > > dell-poweredge-r200 - x86_64 > * boot - 1 pass > * kselftest - 52 pass - 15 known failures > * libhugetlbfs - 76 pass - 1 skip > * ltp-syscalls-tests - 941 pass - 175 skip - 11known failures Looks like you lost a ' ' in that last line somehow :) Anyway, again, lots of "known failures", do these also show up in 4.13-stable? thanks for testing and letting me know. greg k-h
Re: [PATCH RFC hack dont apply] intel_idle: support running within a VM
On Wed, 4 Oct 2017, Michael S. Tsirkin wrote: > On Tue, Oct 03, 2017 at 11:02:55PM +0200, Thomas Gleixner wrote: > > There is the series from Audrey which makes use of the various idle > > prediction mechanisms, scheduler, irq timings, idle governor to get an idea > > about the estimated idle time. Exactly this information can be fed to the > > kvmidle driver which can act accordingly. > > > > Hacking a random hardware specific idle driver is definitely the wrong > > approach. It might be useful to chain the kvmidle driver and hardware > > specific drivers at some point, i.e. if the kvmdriver decides not to exit > > it delegates the mwait decision to the proper hardware driver in order not > > to reimplement all the required logic again. > > By making changes to idle core to allow that chaining? > Does this sound like something reasonable? At least for me it makes sense to avoid code duplication. But thats up to the cpuidle maintainers to decide at the end. Thanks, tglx
Re: [PATCH 4/9 v2] usb: usb251xb: Add 5,6,7 ports boost settings
On 09/16/2017 12:42 PM, Serge Semin wrote: > USB electrical signaling drive strength boost bit is also supported > by USB2517 hub. Since it got three addition ports, the designers > needed to add one more register for initialization. It turned out > to be formerly reserved 0xF7. As before we just initialize it with > default zeros. > > Signed-off-by: Serge Semin > --- > drivers/usb/misc/usb251xb.c | 15 ++- > 1 file changed, 10 insertions(+), 5 deletions(-) > Looks good to me. Feel free to add: Acked-by: Richard Leitner regards, Richard.L
Re: MAP_FIXED for ELF mappings
Dohh, screwed up From. Sorry for spamming.
On Wed 04-10-17 09:50:59, Michal Hocko wrote:
> Hi,
> while studying CVE-2017-1000253 and the MAP_FIXED usage in load_elf*
> code paths I have stumbled over MAP_FIXED usage for elf segments
> mapping. I am not really familiar with this area much so I might draw
> completely incorrect conclusions here but I am really wondering why we
> are doing MAP_FIXED there at all.
>
> I can see why some segments really have to be mapped at a specific
> address but I wonder whether MAP_FIXED is the right tool to achieve
> that. It seems to me that MAP_FIXED is fundamentally dangerous because
> it unmaps any existing mapping. I assume that nothing should be really
> mapped in the requested range that early so we can only stumble over
> something when the address space randomization place things unexpectedly
> (which was the case of the above mentioned CVE AFAIU).
>
> So my primary question is whether we can/should simply drop MAP_FIXED
> from elf_map at all. Instead we should test whether the mapping was
> successful for the requested address and fail otherwise. I realize that
> failing due to something that a user has no idea about sucks a lot but
> it seems to me safer to simply complain into the log and fail is a safer
> option.
>
> Something like the following completely untested diff. Or am I
> completely missing the point of the MAP_FIXED purpose?
> ---
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 6466153f2bf0..09456e2add18 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -341,6 +341,29 @@ create_elf_tables(struct linux_binprm *bprm, struct
> elfhdr *exec,
>
> #ifndef elf_map
>
> +static unsigned long elf_vm_mmap(struct file *filep, unsigned long addr,
> + unsigned long size, int prot, int type, unsigned long off)
> +{
> + unsigned long map_addr;
> +
> + /*
> + * If caller requests the mapping at a specific place, make sure we fail
> + * rather than potentially clobber an existing mapping which can have
> + * security consequences (e.g. smash over the stack area).
> + */
> + map_addr = vm_mmap(filep, addr, size, prot, type & ~MAP_FIXED, off);
> + if (BAD_ADDR(map_addr))
> + return map_addr;
> +
> + if ((type & MAP_FIXED) && map_addr != addr) {
> + pr_info("Uhuuh, elf segement at %p requested but the memory is
> mapped already\n",
> + (void*)addr);
> + return -EAGAIN;
> + }
> +
> + return map_addr;
> +}
> +
> static unsigned long elf_map(struct file *filep, unsigned long addr,
> struct elf_phdr *eppnt, int prot, int type,
> unsigned long total_size)
> @@ -366,11 +389,11 @@ static unsigned long elf_map(struct file *filep,
> unsigned long addr,
> */
> if (total_size) {
> total_size = ELF_PAGEALIGN(total_size);
> - map_addr = vm_mmap(filep, addr, total_size, prot, type, off);
> + map_addr = elf_vm_mmap(filep, addr, total_size, prot, type,
> off);
> if (!BAD_ADDR(map_addr))
> vm_munmap(map_addr+size, total_size-size);
> } else
> - map_addr = vm_mmap(filep, addr, size, prot, type, off);
> + map_addr = elf_vm_mmap(filep, addr, size, prot, type, off);
>
> return(map_addr);
> }
> @@ -1215,7 +1238,7 @@ static int load_elf_library(struct file *file)
> eppnt++;
>
> /* Now use mmap to map the library into memory. */
> - error = vm_mmap(file,
> + error = elf_vm_mmap(file,
> ELF_PAGESTART(eppnt->p_vaddr),
> (eppnt->p_filesz +
>ELF_PAGEOFFSET(eppnt->p_vaddr)),
> --
> Michal Hocko
> SUSE Labs
--
Michal Hocko
SUSE Labs
Re: [PATCH] Add new uio device for PCI with dynamic memory allocation
On Wed, Oct 04, 2017 at 07:24:49AM +, Stahl, Manuel wrote:
> Hi Dan. Thanks for your comments. I can fix all of those.
> Probably there is also some upgrade needed for the MSI stuff.
> pci_disable_msi() is not there anymore, so I have to use
> pci_alloc_irq_vectors(). Doing tests with my PCIe HW I had some
> problems with masking the legacy IRQs. Probably uio_pci_generic
> suffers from the same problems.
>
> Can someone tell me how to properly handle legacy and MSI
> interrupts in irqhandler()? Or should I implement a irqcontrol()
> function?
>
These questions about above my pay grade. I've never used the code, I
just do mechanical reviews. I'm not certain who to ask either...
regards,
dan carpenter
> Best regards,
> Manuel Stahl
>
> On Di, 2017-10-03 at 12:48 +0300, Dan Carpenter wrote:
> > Looks good. A couple minor comments below.
> >
> > On Mon, Oct 02, 2017 at 03:02:09PM +, Stahl, Manuel wrote:
> > > +static int open(struct uio_info *info, struct inode *inode)
> > > +{
> > > + struct uio_pci_dmem_dev *priv = to_uio_pci_dmem_dev(info);
> > > + struct uio_mem *uiomem;
> > > + int ret = 0;
> > > + int dmem_region = 0;
> > > +
> > > + uiomem = &priv->info.mem[dmem_region];
> > > +
> > > + mutex_lock(&priv->alloc_lock);
> > > + while (!priv->refcnt && uiomem < &priv->info.mem[MAX_UIO_MAPS]) {
> > > + void *addr;
> > > +
> > > + if (!uiomem->size)
> > > + break;
> > > +
> > > + addr = dma_alloc_coherent(&priv->pdev->dev, uiomem->size,
> > > + (dma_addr_t *)&uiomem->addr,
> > > + GFP_KERNEL);
> > > + if (!addr)
> > > + uiomem->addr = DMEM_MAP_ERROR;
> > > +
> > > + priv->dmem_region_vaddr[dmem_region++] = addr;
> > > + ++uiomem;
> > > + }
> > > + if (pci_check_and_mask_intx(priv->pdev))
> > > + dev_info(&priv->pdev->dev, "Found pending interrupt");
> > > +
> > > + if (!priv->refcnt)
> > > + pci_set_master(priv->pdev);
> > > +
> > > + priv->refcnt++;
> > > +
> > > + mutex_unlock(&priv->alloc_lock);
> > > +
> > > + return ret;
> >
> > Just "return 0;" is more readable/explicit than "return ret;".
> >
> > > +}
> > > +
> > > +static int release(struct uio_info *info, struct inode *inode)
> > > +{
> > > + struct uio_pci_dmem_dev *priv = to_uio_pci_dmem_dev(info);
> > > + struct uio_mem *uiomem;
> > > + int dmem_region = 0;
> > > +
> > > + uiomem = &priv->info.mem[dmem_region];
> > > +
> > > + mutex_lock(&priv->alloc_lock);
> > > +
> > > + priv->refcnt--;
> > > + while (!priv->refcnt && uiomem < &priv->info.mem[MAX_UIO_MAPS]) {
> > > + if (!uiomem->size)
> > > + break;
> >
> > I think this needs to be a continue instead of a break to match
> > parse_dmem_entries() since we don't break for size == 0 in that loop.
> >
> > > + if (priv->dmem_region_vaddr[dmem_region]) {
> > > + dma_free_coherent(&priv->pdev->dev, uiomem->size,
> > > + priv->dmem_region_vaddr[dmem_region],
> > > + uiomem->addr);
> > > + }
> > > + uiomem->addr = DMEM_MAP_ERROR;
> > > + ++dmem_region;
> > > + ++uiomem;
> > > + }
> > > + if (pci_check_and_mask_intx(priv->pdev))
> > > + dev_info(&priv->pdev->dev, "Found pending interrupt");
> > > +
> > > + if (!priv->refcnt)
> > > + pci_clear_master(priv->pdev);
> > > +
> > > + mutex_unlock(&priv->alloc_lock);
> > > + return 0;
> > > +}
> > > +
> > > +static int dmem_mmap(struct uio_info *info, struct vm_area_struct *vma)
> > > +{
> > > + struct uio_pci_dmem_dev *gdev = to_uio_pci_dmem_dev(info->priv);
> > > + struct uio_mem *uiomem;
> > > + int mi = vma->vm_pgoff;
> > > +
> > > + if (mi >= MAX_UIO_MAPS)
> > > + return -EINVAL;
> > > +
> > > + uiomem = &info->mem[mi];
> > > + if (uiomem->memtype != UIO_MEM_PHYS)
> > > + return -EINVAL;
> > > + if (!uiomem->size)
> > > + return -EINVAL;
> > > +
> > > + /* DMA address */
> > > + vma->vm_pgoff = 0;
> > > + return dma_mmap_coherent(&gdev->pdev->dev, vma,
> > > + gdev->dmem_region_vaddr[mi],
> > > + uiomem->addr, uiomem->size);
> > > +}
> > > +
> > > +/* Interrupt handler. Read/modify/write the command register to disable
> > > the
> > > + * interrupt.
> > > + */
> > > +static irqreturn_t irqhandler(int irq, struct uio_info *info)
> > > +{
> > > + struct uio_pci_dmem_dev *gdev = to_uio_pci_dmem_dev(info);
> > > +
> > > + if (gdev->pdev->msi_enabled)
> > > + return IRQ_HANDLED;
> > > +
> > > + if (pci_check_and_mask_intx(gdev->pdev))
> > > + return IRQ_HANDLED;
> > > +
> > > + return IRQ_NONE;
> > > +}
> > > +
> > > +static unsigned int uio_dmem_dma_bits = 32;
> > > +static char uio_dmem_sizes[256];
> > > +
> > > +static int parse_dmem_entries(struct pci_dev *pdev,
> > > + const struct pci_device_id *id,
>
[PATCH] mfd: syscon: atmel-smc: include string.h
The string.h header file is needed for the memset() definition. The RT build fails because it is not pulled in via other header files. Signed-off-by: Sebastian Andrzej Siewior --- drivers/mfd/atmel-smc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/mfd/atmel-smc.c b/drivers/mfd/atmel-smc.c index 7d77948567d7..0adbd2e796fe 100644 --- a/drivers/mfd/atmel-smc.c +++ b/drivers/mfd/atmel-smc.c @@ -12,6 +12,7 @@ */ #include +#include /** * atmel_smc_cs_conf_init - initialize a SMC CS conf -- 2.14.2
Re: 4.14-rc2 on thinkpad x220: out of memory when inserting mmc card
On 4 October 2017 at 09:53, Linus Walleij wrote:
> On Tue, Oct 3, 2017 at 8:30 AM, Adrian Hunter wrote:
>> On 02/10/17 17:09, Linus Walleij wrote:
>>> On Sun, Oct 1, 2017 at 12:57 PM, Tetsuo Handa
>>> wrote:
>>>
>> I inserted u-SD card, only to realize that it is not detected as it
>> should be. And dmesg indeed reveals:
>
> Tetsuo asked me to report this to linux-mm.
>
> But 2^4 is 16 pages, IIRC that can't be expected to work reliably, and
> thus this sounds like MMC bug, not mm bug.
>>>
>>>
>>> I'm not sure I fully understand this error message:
>>> "worker/2:1: page allocation failure: order:4"
>>>
>>> What I guess from context is that the mmc_init_request()
>>> call is failing to allocate 16 pages, meaning for 4K pages
>>> 64KB which is the typical bounce buffer.
>>>
>>> This is what the code has always allocated as bounce buffer,
>>> but it used to happen upfront, when probing the MMC block layer,
>>> rather than when allocating the requests.
>>
>> That is not exactly right. As I already wrote, the memory allocation used
>> to be optional but became mandatory with:
>>
>> commit 304419d8a7e9204c5d19b704467b814df8c8f5b1
>> Author: Linus Walleij
>> Date: Thu May 18 11:29:32 2017 +0200
>>
>> mmc: core: Allocate per-request data using the block layer core
>
> Yes you are right, it used to look like this, with the bounce buffer
> hiding behind a Kconfig symbol:
>
> #ifdef CONFIG_MMC_BLOCK_BOUNCE
> if (host->max_segs == 1) {
> unsigned int bouncesz;
>
> bouncesz = MMC_QUEUE_BOUNCESZ;
>
> if (bouncesz > host->max_req_size)
> bouncesz = host->max_req_size;
> if (bouncesz > host->max_seg_size)
> bouncesz = host->max_seg_size;
> if (bouncesz > (host->max_blk_count * 512))
> bouncesz = host->max_blk_count * 512;
>
> if (bouncesz > 512 &&
> mmc_queue_alloc_bounce_bufs(mq, bouncesz)) {
> blk_queue_bounce_limit(mq->queue, BLK_BOUNCE_ANY);
> blk_queue_max_hw_sectors(mq->queue, bouncesz / 512);
> blk_queue_max_segments(mq->queue, bouncesz / 512);
> blk_queue_max_segment_size(mq->queue, bouncesz);
>
> ret = mmc_queue_alloc_bounce_sgs(mq, bouncesz);
> if (ret)
> goto cleanup_queue;
> bounce = true;
> }
> }
> #endif
>
> I recently concluded that I find no evidence whatsoever that anyone
> turned this symbol on. Actually. (Checked defconfigs and distro configs.)
> The option was just sitting there unused.
> This code was never exercised except by some people who turned it
> on on their custom kernels in the past. It's in practice dead code.
>
> My patch started to allocate and use bounce buffers for all hosts
> with max_segs == 1, unless specifically flagged NOT to use bounce
> buffers.
>
> That wasn't smart, I should have just deleted them. Mea culpa.
>
> So that is why I asked Ulf to simply put the patch deleting the bounce
> buffers that noone is using to fixes, and it should fix this problem.
Adrian, Linus,
Thanks for looking into the problem! I am queuing up the patch
deleting bounce buffers for fixes asap!
Kind regards
Uffe
[PATCH v3 1/2] sched/fair: make capacity_margin __read_mostly
Variable 'capacity_margin' is used with read operation for most cases
to calculate the capacity margin, put it into __read_mostly section.
Cc: Dietmar Eggemann
Cc: Morten Rasmussen
Cc: Chris Redpath
Cc: Joel Fernandes
Cc: Vincent Guittot
Cc: Patrick Bellasi
Signed-off-by: Leo Yan
---
kernel/sched/fair.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 70ba32e..ad03bf4 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -129,7 +129,7 @@ unsigned int sysctl_sched_cfs_bandwidth_slice
= 5000UL;
*
* (default: ~20%)
*/
-unsigned int capacity_margin = 1280;
+unsigned int capacity_margin __read_mostly = 1280;
static inline void update_load_add(struct load_weight *lw, unsigned long inc)
{
--
2.7.4
[PATCH v3 2/2] cpufreq: schedutil: consolidate capacity margin calculation
Scheduler CFS class has variable 'capacity_margin' to represent the capacity margin, currently in the kernel 'capacity_margin' is 1280; on the other hand schedutil governor also needs to compensate the margin for frequency tipping point. Below are formulas which are used in CFS class and schedutil governor separately, from the formulas we can get to know essentially both of them uses the same margin: CFS: U` = U * capacity_margin / 1024 = U * 1.25 Schedutil: U` = U + U >> 2 = U + U * 0.25 = U * 1.25 This patch consolidates the usage of the capacity margin value and lets schedutil use the same formula as the CFS class. Thus we can avoid the mismatch between schedutil and CFS class if 'capacity_margin' is changed to other values in the future. Cc: Dietmar Eggemann Cc: Morten Rasmussen Cc: Chris Redpath Cc: Joel Fernandes Cc: Vincent Guittot Cc: Patrick Bellasi Cc: Rafael J. Wysocki Signed-off-by: Leo Yan --- kernel/sched/cpufreq_schedutil.c | 7 +-- kernel/sched/sched.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c index 9209d83..79abbaa 100644 --- a/kernel/sched/cpufreq_schedutil.c +++ b/kernel/sched/cpufreq_schedutil.c @@ -155,7 +155,9 @@ static void sugov_update_commit(struct sugov_policy *sg_policy, u64 time, * * next_freq = C * curr_freq * util_raw / max * - * Take C = 1.25 for the frequency tipping point at (util / max) = 0.8. + * Take C = capacity_margin >> SCHED_CAPACITY_SHIFT; if capacity_margin is + * 1280 and SCHED_CAPACITY_SHIFT is 10, this results in C = 1.25 and the + * frequency tipping point at (util / max) = 0.8. * * The lowest driver-supported frequency which is equal or greater than the raw * next_freq (as calculated above) is returned, subject to policy min/max and @@ -168,7 +170,8 @@ static unsigned int get_next_freq(struct sugov_policy *sg_policy, unsigned int freq = arch_scale_freq_invariant() ? policy->cpuinfo.max_freq : policy->cur; - freq = (freq + (freq >> 2)) * util / max; + freq = freq * capacity_margin >> SCHED_CAPACITY_SHIFT; + freq = freq * util / max; if (freq == sg_policy->cached_raw_freq && sg_policy->next_freq != UINT_MAX) return sg_policy->next_freq; diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index 14db76c..cf75bdc 100644 --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -52,6 +52,7 @@ struct cpuidle_state; #define TASK_ON_RQ_MIGRATING 2 extern __read_mostly int scheduler_running; +extern unsigned int capacity_margin __read_mostly; extern unsigned long calc_load_update; extern atomic_long_t calc_load_tasks; -- 2.7.4
Re: usb/sound/bcd2000: warning in bcd2000_init_device
On Wed, 04 Oct 2017 09:52:36 +0200,
Greg Kroah-Hartman wrote:
>
> On Wed, Oct 04, 2017 at 08:10:59AM +0200, Takashi Iwai wrote:
> > On Tue, 03 Oct 2017 19:42:21 +0200,
> > Greg Kroah-Hartman wrote:
> > >
> > > On Tue, Oct 03, 2017 at 12:50:08PM -0400, Alan Stern wrote:
> > > > On Tue, 3 Oct 2017, Takashi Iwai wrote:
> > > >
> > > > > > It's a dev_WARN because it indicates a potentially serious error in
> > > > > > the
> > > > > > driver: The driver has submitted an interrupt URB to a bulk
> > > > > > endpoint.
> > > > > > That may not sound bad, but the same check gets triggered if a
> > > > > > driver
> > > > > > submits a bulk URB to an isochronous endpoint, or any other invalid
> > > > > > combination.
> > > > > >
> > > > > > Most likely the explanation here is that the driver doesn't bother
> > > > > > to
> > > > > > check the endpoint type because it expects the endpoint will always
> > > > > > be
> > > > > > interrupt. But that is not a safe strategy. USB devices and their
> > > > > > firmware should not be trusted unnecessarily.
> > > > > >
> > > > > > The best fix is, like you said, to add a sanity check in the caller.
> > > > >
> > > > > OK, but then do we have some handy helper for the check?
> > > > > As other bug reports by syzkaller suggest, there are a few other
> > > > > drivers that do the same, submitting a urb with naive assumption of
> > > > > the fixed EP for specific devices. In the end we'll need to put the
> > > > > very same checks there in multiple places.
> > > >
> > > > Perhaps we could add a helper routine that would take a list of
> > > > expected endpoint types and check that the actual endpoints match the
> > > > types. But of course, all the drivers you're talking about would have
> > > > to add a call to this helper routine.
> > >
> > > We have almost this type of function, usb_find_common_endpoints(),
> > > what's wrong with using that? Johan has already swept the tree and
> > > added a lot of these checks, odds are no one looked at the sound/
> > > subdir...
> >
> > Well, what I had in my mind is just a snippet from usb_submit_urb(),
> > something like:
> >
> > bool usb_sanity_check_urb_pipe(struct urb *urb)
> > {
> > struct usb_host_endpoint *ep;
> > int xfertype;
> > static const int pipetypes[4] = {
> > PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, PIPE_INTERRUPT
> > };
> >
> > ep = usb_pipe_endpoint(urb->dev, urb->pipe);
> > xfertype = usb_endpoint_type(&ep->desc);
> > return usb_pipetype(urb->pipe) != pipetypes[xfertype];
> > }
> >
> > And calling this before usb_submit_urb() in each place that assigns
> > the fixed EP as device-specific quirks.
> > Does it make sense?
>
> Yes, kind of, but checking the endpoint type/direction is what you are
> expecting it to be as you "know" what the type should be for each
> driver as it is unique.
Yes, it can be simplified, but if we want a common helper function,
this style would have an advantage that it can be used generically for
all drivers.
> Anyway, a "real" patch might make more sense to me.
I can cook up a patch if you find it a good idea to add such a common
function to usb core side. OTOH, if each driver should open-code this
in each place, I can work on that, too. Which would you prefer?
thanks,
Takashi
Re: [PATCH v3] doc: coresight: correct usage for disabling idle states
On Mon, Oct 02, 2017 at 11:14:46AM -0700, Mathieu Poirier wrote: > On 19 September 2017 at 21:46, Leo Yan wrote: > > In the coresight CPU debug document it suggests to use 'echo' command > > to set latency request to /dev/cpu_dma_latency so can disable all CPU > > idle states, but in fact this doesn't work. > > > > This is because when the command 'echo' exits, it releases the device > > node's file descriptor and the kernel release function removes the QoS > > constraint; finally when the command 'echo' finished there have no > > constraint imposed on cpu_dma_latency. > > > > This patch changes to use 'exec' to access '/dev/cpu_dma_latency', the > > command 'exec' can avoid the file descriptor to be closed so we can > > keep the constraint on cpu_dma_latency. > > > > This patch also adds the info for reference docs for PM QoS and cpuidle > > sysfs. > > > > Cc: Jonathan Corbet > > Cc: Sudeep Holla > > Reported-by: Kim Phillips > > Suggested-by: Mathieu Poirier > > Signed-off-by: Leo Yan > > --- > > Documentation/trace/coresight-cpu-debug.txt | 22 +- > > 1 file changed, 17 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/trace/coresight-cpu-debug.txt > > b/Documentation/trace/coresight-cpu-debug.txt > > index b3da1f9..2b9b51c 100644 > > --- a/Documentation/trace/coresight-cpu-debug.txt > > +++ b/Documentation/trace/coresight-cpu-debug.txt > > @@ -149,11 +149,23 @@ If you want to limit idle states at boot time, you > > can use "nohlt" or > > > > At the runtime you can disable idle states with below methods: > > > > -Set latency request to /dev/cpu_dma_latency to disable all CPUs specific > > idle > > -states (if latency = 0uS then disable all idle states): > > -# echo "what_ever_latency_you_need_in_uS" > /dev/cpu_dma_latency > > - > > -Disable specific CPU's specific idle state: > > +It is possible to disable CPU idle states by way of the PM QoS > > +subsystem, more specifically by using the "/dev/cpu_dma_latency" > > +interface (see Documentation/power/pm_qos_interface.txt for more > > +details). As specified in the PM QoS documentation the requested > > +parameter will stay in effect until the file descriptor is released. > > +For example: > > + > > +# exec 3<> /dev/cpu_dma_latency; echo 0 >&3 > > +... > > +Do some work... > > +... > > +# exec 3<>- > > + > > +The same can also be done from an application program. > > + > > +Disable specific CPU's specific idle state from cpuidle sysfs (see > > +Documentation/cpuidle/sysfs.txt): > > # echo 1 > /sys/devices/system/cpu/cpu$cpu/cpuidle/state$state/disable > > > > Applied. Thanks a lot, Mathieu. > Thanks, > Mathieu > > > > > -- > > 2.7.4 > >
Re: [PATCH 8/9 v2] usb: usb251xb: Add max power/current dts property support
On 09/21/2017 07:10 PM, Serge Semin wrote:
> On Thu, Sep 21, 2017 at 11:26:04AM -0500, Rob Herring wrote:
>> On Wed, Sep 20, 2017 at 4:27 PM, Serge Semin wrote:
...
>>> These are different parameters of the device. They got different
>>> configuration
>>> registers and descriptions:
>>> max_power* - ... This value also includes the power consumption of a
>>> permanently attached peripheral if the hub is configured as a compound
>>> device, and the embedded peripheral reports 0mA in its descriptors.
>>> max_current* - ... This value does NOT include the power consumption of a
>>> permanently attached peripheral if the hub is configured as a compound
>>> device.
>>
>> Then the names here should somehow reflect the above. Perhaps
>> "composite-current" and "hub-current" or something like that.
>>
>
> I left the naming in accordance with the device datasheet. I thought it would
> be
> better since the driver user would still need to consult with the device
> documentation to properly set them. I don't really get how the difference is
> reflected
> with the naming declared there though. So what naming would you prefer then?
> Might be
> something like:
> {sp,bp}-max-total-current - for so named {sp,bp}-max-power, since it includes
> all the
> permanently attached peripherals.
> {sp,bp}-max-removable-current - for so named {sp,bp}-max-current, since it
> doesn't
> include the permanently attached peripherals.
>
> Or is it better to leave it in compliance with the documentation naming?
I'd prefer the naming to be in accordance with the device datasheet too.
Changing it will IMHO generate avoidable misunderstandings...
But if we should go with Robs proposal please make sure the name from
the device datasheet is mentioned somewhere in the description of the
binding.
>
>>>
>>> Additionally as you can see, they both are measured in "mA", so it isn't
>>> a real physical power.
>>
>> Well, I can't because there's no units.
>>
>
> What this line means then?
> - sp-max-{power,current} : ... The value is given in mA in a 0 - 100 range
> (default is 1mA).
> - bp-max-{power,current} : ... The value is given in mA in a 0 - 510 range
> (default is 100mA).
>
> Maybe I don't know something and the description line should state the units
> somehow
> clearer?
Append the unit to the binding name, just like in "power-on-time-ms". As
there's no "Standard Unit Suffix" for mA stated in the documentation
(Documentation/devicetree/bindings/property-units.txt) I think it should
be "-milliamp"?
regards,
Richard.L
Re: [PATCH 6/7] phy: samsung: Remove dependency on obsolete SoC support
On Wed, Oct 4, 2017 at 8:38 AM, Marek Szyprowski
wrote:
> Support for Exynos4212 SoCs has been removed by commit bca9085e0ae9 ("ARM:
> dts: exynos: remove Exynos4212 support (dead code)"), so there is no need
> to keep remaining dead code related to this SoC version.
>
> Signed-off-by: Marek Szyprowski
> ---
> drivers/phy/samsung/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Acked-by: Krzysztof Kozlowski
Best regards,
Krzysztof
Re: [PATCH 1/1] drivers/tty: check for null pointer
On Wed, Sep 06, 2017 at 09:57:20PM +0800, Li, Zhenhua wrote: > when I tried to boot linux on a system with virtual bios and vortual keyboard, > it crashes. > > The root cause is the bios does not initialize devices correctly. Then please fix the virtual bios :) thanks, greg k-h
Re: [PATCH] tty: vt: remove multi-fetch, derive font.height from font.data
On Sun, Sep 24, 2017 at 12:59:42PM -0400, Meng Xu wrote: > In con_font_set(), when we need to guess font height (for > compat reasons?), the current approach uses multiple userspace > fetches, i.e., get_user(tmp, &charmap[32*i+h-1]), to derive > the height. This has two drawbacks: > > 1. performance: accessing userspace memory is less efficient than > directly de-reference the byte > > 2. security: a more critical problem is that the height derived > might not match with the actual font.data. This is because a user > thread might race condition to change the memory of op->data after > the op->height guessing but before the second fetch: font.data = > memdup_user(op->data, size). Leaving font.height = 32 while the > actual height is 1 or vice-versa. > > This patch tries to resolve both issues by re-locating the height > guessing part after the font.data is fetched in. In this way, the > userspace data is fetched in one shot and we directly dereference > the font.data in kernel space to probe for the height. > > Signed-off-by: Meng Xu > --- > drivers/tty/vt/vt.c | 48 > 1 file changed, 28 insertions(+), 20 deletions(-) Please always run your patches through checkpatch.pl so that you don't get a grumpy maintainer telling you to use checkpatch.pl :( Can you fix the obvious issues up and resend? thanks, greg k-h
[PATCH] jfs: Add missing NULL pointer check in __get_metapage
alloc_metapage can return a NULL pointer so check for that. And also emit
an error message if that happens.
Signed-off-by: Juerg Haefliger
---
fs/jfs/jfs_metapage.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/fs/jfs/jfs_metapage.c b/fs/jfs/jfs_metapage.c
index 1c4b9ad4d7ab..00f21af66872 100644
--- a/fs/jfs/jfs_metapage.c
+++ b/fs/jfs/jfs_metapage.c
@@ -187,14 +187,18 @@ static inline struct metapage *alloc_metapage(gfp_t
gfp_mask)
{
struct metapage *mp = mempool_alloc(metapage_mempool, gfp_mask);
- if (mp) {
- mp->lid = 0;
- mp->lsn = 0;
- mp->data = NULL;
- mp->clsn = 0;
- mp->log = NULL;
- init_waitqueue_head(&mp->wait);
+ if (!mp) {
+ jfs_err("mempool_alloc failed!\n");
+ return NULL;
}
+
+ mp->lid = 0;
+ mp->lsn = 0;
+ mp->data = NULL;
+ mp->clsn = 0;
+ mp->log = NULL;
+ init_waitqueue_head(&mp->wait);
+
return mp;
}
@@ -663,6 +667,8 @@ struct metapage *__get_metapage(struct inode *inode,
unsigned long lblock,
} else {
INCREMENT(mpStat.pagealloc);
mp = alloc_metapage(GFP_NOFS);
+ if (!mp)
+ goto unlock;
mp->page = page;
mp->sb = inode->i_sb;
mp->flag = 0;
--
2.14.1
Re: [PATCHv3 1/7] switch dereference_function_descriptor() to `unsigned long'
On Sat 2017-09-30 11:53:13, Sergey Senozhatsky wrote:
> Convert dereference_function_descriptor() to accept and return
> `unsigned long'. There will be two new ARCH function for kernel
> and module function pointer dereference, which will work with
> `unsigned long', so the patch unifies interfaces.
>
> Besides, dereference_function_descriptor() mostly work with
> `unsigned long':
>
> Convert dereference_function_descriptor() users tree-wide.
I am not sure if this is a real win. If I count correctly,
the net result is 6 additional casts in this patch. Many
casts are still needed also in the other patches.
> diff --git a/arch/ia64/include/asm/sections.h
> b/arch/ia64/include/asm/sections.h
> index 2ab2003698ef..de6bfa1ef8fb 100644
> --- a/arch/ia64/include/asm/sections.h
> +++ b/arch/ia64/include/asm/sections.h
> @@ -27,13 +27,13 @@ extern char __start_unwind[], __end_unwind[];
> extern char __start_ivt_text[], __end_ivt_text[];
>
> #undef dereference_function_descriptor
> -static inline void *dereference_function_descriptor(void *ptr)
> +static inline unsigned long dereference_function_descriptor(unsigned long
> ptr)
I would also expect that a function called "dereference*" would work with
a pointer. The parameter is called ptr but it is unsigned long.
> {
> - struct fdesc *desc = ptr;
> + struct fdesc *desc = (struct fdesc *)ptr;
> void *p;
>
> if (!probe_kernel_address(&desc->ip, p))
> - ptr = p;
> + ptr = (unsigned long)p;
> return ptr;
> }
>
> diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c
> index 1ca9a2b4239f..06e1b79e2946 100644
> --- a/arch/parisc/mm/init.c
> +++ b/arch/parisc/mm/init.c
> @@ -389,10 +389,10 @@ static void __init setup_bootmem(void)
> static int __init parisc_text_address(unsigned long vaddr)
> {
> static unsigned long head_ptr __initdata;
> + unsigned long addr = (unsigned long)&parisc_kernel_start;
>
> if (!head_ptr)
> - head_ptr = PAGE_MASK & (unsigned long)
> - dereference_function_descriptor(&parisc_kernel_start);
> + head_ptr = PAGE_MASK & dereference_function_descriptor(addr);
IMHO, this is harder to read than the original. You need to
search the definition of "addr" and check its manipulation
to understand the meaning.
>
> return core_kernel_text(vaddr) || vaddr == head_ptr;
> }
To make it clear. All these comments are not a big deal and I do
not want to invalidate all the acked-by and tested-by just because
of them.
But please, consider removing this change if we need to do
another iteration of this patchset. IMHO, there is no real win
and it would just pollute the git history.
Best Regards,
Petr
Re: usb/sound/bcd2000: warning in bcd2000_init_device
On Wed, Oct 04, 2017 at 10:08:24AM +0200, Takashi Iwai wrote:
> On Wed, 04 Oct 2017 09:52:36 +0200,
> Greg Kroah-Hartman wrote:
> >
> > On Wed, Oct 04, 2017 at 08:10:59AM +0200, Takashi Iwai wrote:
> > > On Tue, 03 Oct 2017 19:42:21 +0200,
> > > Greg Kroah-Hartman wrote:
> > > >
> > > > On Tue, Oct 03, 2017 at 12:50:08PM -0400, Alan Stern wrote:
> > > > > On Tue, 3 Oct 2017, Takashi Iwai wrote:
> > > > >
> > > > > > > It's a dev_WARN because it indicates a potentially serious error
> > > > > > > in the
> > > > > > > driver: The driver has submitted an interrupt URB to a bulk
> > > > > > > endpoint.
> > > > > > > That may not sound bad, but the same check gets triggered if a
> > > > > > > driver
> > > > > > > submits a bulk URB to an isochronous endpoint, or any other
> > > > > > > invalid
> > > > > > > combination.
> > > > > > >
> > > > > > > Most likely the explanation here is that the driver doesn't
> > > > > > > bother to
> > > > > > > check the endpoint type because it expects the endpoint will
> > > > > > > always be
> > > > > > > interrupt. But that is not a safe strategy. USB devices and
> > > > > > > their
> > > > > > > firmware should not be trusted unnecessarily.
> > > > > > >
> > > > > > > The best fix is, like you said, to add a sanity check in the
> > > > > > > caller.
> > > > > >
> > > > > > OK, but then do we have some handy helper for the check?
> > > > > > As other bug reports by syzkaller suggest, there are a few other
> > > > > > drivers that do the same, submitting a urb with naive assumption of
> > > > > > the fixed EP for specific devices. In the end we'll need to put the
> > > > > > very same checks there in multiple places.
> > > > >
> > > > > Perhaps we could add a helper routine that would take a list of
> > > > > expected endpoint types and check that the actual endpoints match the
> > > > > types. But of course, all the drivers you're talking about would
> > > > > have
> > > > > to add a call to this helper routine.
> > > >
> > > > We have almost this type of function, usb_find_common_endpoints(),
> > > > what's wrong with using that? Johan has already swept the tree and
> > > > added a lot of these checks, odds are no one looked at the sound/
> > > > subdir...
> > >
> > > Well, what I had in my mind is just a snippet from usb_submit_urb(),
> > > something like:
> > >
> > > bool usb_sanity_check_urb_pipe(struct urb *urb)
> > > {
> > > struct usb_host_endpoint *ep;
> > > int xfertype;
> > > static const int pipetypes[4] = {
> > > PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, PIPE_INTERRUPT
> > > };
> > >
> > > ep = usb_pipe_endpoint(urb->dev, urb->pipe);
> > > xfertype = usb_endpoint_type(&ep->desc);
> > > return usb_pipetype(urb->pipe) != pipetypes[xfertype];
> > > }
> > >
> > > And calling this before usb_submit_urb() in each place that assigns
> > > the fixed EP as device-specific quirks.
> > > Does it make sense?
> >
> > Yes, kind of, but checking the endpoint type/direction is what you are
> > expecting it to be as you "know" what the type should be for each
> > driver as it is unique.
>
> Yes, it can be simplified, but if we want a common helper function,
> this style would have an advantage that it can be used generically for
> all drivers.
>
> > Anyway, a "real" patch might make more sense to me.
>
> I can cook up a patch if you find it a good idea to add such a common
> function to usb core side. OTOH, if each driver should open-code this
> in each place, I can work on that, too. Which would you prefer?
A common function is good, open-coding is bad :)
Try it with a driver or two to see what it looks like?
thanks,
greg k-h
Re: [PATCH 6/7] phy: samsung: Remove dependency on obsolete SoC support
On 10/04/2017 08:38 AM, Marek Szyprowski wrote:
> Support for Exynos4212 SoCs has been removed by commit bca9085e0ae9 ("ARM:
> dts: exynos: remove Exynos4212 support (dead code)"), so there is no need
> to keep remaining dead code related to this SoC version.
>
> Signed-off-by: Marek Szyprowski
Reviewed-by: Sylwester Nawrocki
Re: [PATCH 07/11] powerpc: make dma_cache_sync a no-op
Le 03/10/2017 à 13:43, Christoph Hellwig a écrit : On Tue, Oct 03, 2017 at 01:24:57PM +0200, Christophe LEROY wrote: powerpc does not implement DMA_ATTR_NON_CONSISTENT allocations, so it doesn't make any sense to do any work in dma_cache_sync given that it must be a no-op when dma_alloc_attrs returns coherent memory. What about arch/powerpc/mm/dma-noncoherent.c ? Powerpc 8xx doesn't have coherent memory. It doesn't implement the DMA_ATTR_NON_CONSISTENT interface either, so if it really doesn't have a way to provide dma coherent allocation (although the code in __dma_alloc_coherent suggests it does provide dma coherent allocations) I have no idea how it could ever have worked. Yes indeed it provides coherent memory by allocation non cached memory. And drivers aiming at using non coherent memory do it by using kmalloc() with GFP_DMA then dma_map_single(). Then they use dma_sync_single_for_xxx(), which calls __dma_sync() on the 8xx and is a nop on other powerpcs. Christophe
Re: [PATCH 4.4 00/41] 4.4.90-stable review
Hi Greg, On 4 October 2017 at 00:55, Greg Kroah-Hartman wrote: > On Tue, Oct 03, 2017 at 03:30:14PM -0500, Tom Gall wrote: >> >> > On Oct 3, 2017, at 7:21 AM, Greg Kroah-Hartman >> > wrote: >> > >> > This is the start of the stable review cycle for the 4.4.90 release. >> > There are 41 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 Thu Oct 5 11:42:00 UTC 2017. >> > Anything received after that time might be too late. >> > >> > The whole patch series can be found in one patch at: >> > kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.4.90-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-4.4.y >> > and the diffstat can be found below. >> > >> > thanks, >> > >> > greg k-h >> > >> >> Test results from the linaro linux kernel functional test farm: >> >> kernel: 4.4.90-rc1 >> git repo: >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git >> git branch: linux-4.4.y >> git commit: 255b4a073a820d2f46a1ce1740f1a9be3de9661a >> git describe: v4.4.89-42-g255b4a073a82 >> Test details: >> https://qa-reports.linaro.org/lkft/linux-stable-rc-4.4-oe/build/v4.4.89-42-g255b4a073a82 >> >> No regressions (compared to build v4.4.89-30-gb547584d016b) >> >> Boards, architectures and test suites: >> - >> >> dell-poweredge-r200 - x86_64 >> * boot - 1 pass >> * kselftest - 44 pass - 24 known failures >> * libhugetlbfs - 76 pass - 1 skip >> * ltp-syscalls-tests - 941 pass - 175 skip - 11 known failures >> >> >> And as a separate but related report, we have HiKey (arm64) results. These >> are separate because there are some out of tree patches that didn’t quite >> make 4.4 back in the day, thus for this board the kernel is a blend of the >> LTS RC + some platform patches. >> >> Summary >> >> >> kernel: 4.4.90-rc1 >> git repo: https://git.linaro.org/lkft/arm64-stable-rc.git >> git tag: 4.4.90-rc1-hikey-20171003 >> git commit: 8b0848771d885b6030233931b2d2039382a1cf73 >> git describe: v4.4.89-352-g8b0848771d88 >> Test details: >> https://qa-reports.linaro.org/lkft/linaro-hikey-stable-rc-4.4-oe/build/v4.4.89-352-g8b0848771d88 >> >> >> No regressions (compared to build v4.4.89-340-ga6279f8aa398) >> >> Boards, architectures and test suites: >> - >> >> hi6220-hikey - arm64 >> * boot - 1 pass >> * kselftest - 32 pass - 1 skip - 21 known failures >> * libhugetlbfs - 90 pass - 1 skip >> * ltp-syscalls-tests - 960 pass - 138 skip - 2 known failures > > Nice, thanks for letting me know, but that seems like a lot of "known > failures" :( Most of these failures result from our "mainline kselftest + lts kernel" combination - this is also visible from the failures seen on the x86 box as well. We are working on getting these tests to not run on 4.4 specifically, as we see most such failures on these kernels. > > Any chance you can add a beaglebone black to your 4.4 testing to give it > some "native arm" test support? That should run a "stock" 4.4 kernel, > right? > > thanks, > > greg k-h Best, Sumit.
Re: [PATCH 1/2] uio: dt-bindings: document binding for uio-pdrv-genirq
On Thu, Sep 21, 2017 at 12:53:25PM +1200, Chris Packham wrote: > Signed-off-by: Chris Packham > --- I can't take patches without any changelog text, sorry.
Re: [PATCH] dt-bindings: i2c: Add armada-38x i2c binding
Hi Rob, On mar., oct. 03 2017, Rob Herring wrote: > On Wed, Sep 27, 2017 at 09:33:00AM +0200, Gregory CLEMENT wrote: >> Hi Chris, >> >> On mer., sept. 27 2017, Chris Packham >> wrote: >> >> > Hi Gregory, >> > >> > On 27/09/17 00:56, Gregory CLEMENT wrote: >> >> Hi Kalyan, >> >> >> >> Please try avoid top-posting. >> >> >> >> On dim., sept. 24 2017, Kalyan Kinthada >> >> wrote: >> >> >> >>> Hi Gregory, >> >>> >> >>> I got this information from Armada-38x functional errata document. >> >> >> >> OK but in any case just adding a new compatible was not enough you have >> >> to update the driver in the same time, however for this case we won't >> >> need it, see below. >> >> >> >>> >> >>> I can add the "marvell,mv78230-i2c" compatible string to the appropriate >> >>> device tree files >> >>> but the i2c-mv64xxx driver enables an additional feature (offload i2c >> >>> transactions) >> >>> based on the compatible string "marvell,mv78230-i2c". >> >>> >> >>> I am not sure if this feature (offload i2c transactions) should be >> >>> enabled for Armada-38x devices. >> >>> That is the reason I felt for the need of a new compatible string >> >>> specifically for Armada-38x SoCs. >> >> >> >> Indeed the Armada-38x SoCs does not support hardware offloading (at >> >> least according the datasheet). But it happens that in the earlier >> >> version of the Armada XP the hardware offloading was buggy, so we >> >> introduced a compatible for this case: marvell,mv78230-a0-i2c. This >> >> compatible enable the errata fix but not the offloading feature. That >> >> means that it is exactly the compatible you need for Armada 38x (and >> >> Armada 39x and 375 I think). >> > >> > The "mv78230-a0-i2c" dt-binding has the following note >> > >> >Note: Only use "marvell,mv78230-a0-i2c" for a >> >very rare, initial version of the SoC which >> >had broken offload support. Linux >> >auto-detects this and sets it appropriately. >> > >> > If we are going to re-use this binding for armada-38x we should probably >> > remove this note. Personally my preference would be an armada-38x >> >> Updating the documentation is the right thing to do yes. >> >> >> > compatible string (or 370 if that's the common base of these SoCs). But >> > of course we'll go with whatever your preference is as maintainer. >> >> If the IP is compatible then there is no reason to add a new one, else >> we will end with a compatible for each SoC and the compatible property >> will just loose its meaning. > > If you all had added compatibles for each SoC in the first place, then > we wouldn't be having this dicussion. Really?? For Armada 38x we have already 6 flavor 381, 382, 383, 385, 388. Then we have 9 SoC families on mvebu: orion5x, kirkwood, dove, Armada 370, Armada XP, Armada 375, Armada 39x, Armada 7K, Armada 8K. Of course in each family we have several flavors. But then Allwiner also use this driver in 8 SoC families: sun4i, sun5i, sun6i, sun7i, sun8i, sun9i, sunxi, sun50i. Here again each family have their own flavor. So you are just suggesting to blot the i2c driver with more than 50 compatible string! That also mean updating the i2c driver each time a new SoC flavor appear, so more or less for each release. Really this just don't scale. Gregory > > Rob -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
Re: tty crash due to auto-failing vmalloc
On Tue 03-10-17 18:55:04, Johannes Weiner wrote:
[...]
> commit 5d17a73a2ebeb8d1c6924b91e53ab2650fe86ffb
> Author: Michal Hocko
> Date: Fri Feb 24 14:58:53 2017 -0800
>
> vmalloc: back off when the current task is killed
>
> __vmalloc_area_node() allocates pages to cover the requested vmalloc
> size. This can be a lot of memory. If the current task is killed by
> the OOM killer, and thus has an unlimited access to memory reserves, it
> can consume all the memory theoretically. Fix this by checking for
> fatal_signal_pending and back off early.
>
> Link: http://lkml.kernel.org/r/20170201092706.9966-4-mho...@kernel.org
> Signed-off-by: Michal Hocko
> Reviewed-by: Christoph Hellwig
> Cc: Tetsuo Handa
> Cc: Al Viro
> Signed-off-by: Andrew Morton
> Signed-off-by: Linus Torvalds
>
> This talks about the oom killer and memory exhaustion, but most fatal
> signals don't happen due to the OOM killer.
Now that we have cd04ae1e2dc8 ("mm, oom: do not rely on TIF_MEMDIE for
memory reserves access") the risk of the memory depletion is much
smaller so reverting the above commit should be acceptable. On the other
hand the failure is still possible and the caller should be prepared for
that.
--
Michal Hocko
SUSE Labs
Re: [lockdep] b09be676e0 BUG: unable to handle kernel NULL pointer dereference at 000001f2
On Tue, Oct 03, 2017 at 10:05:38AM -0500, Josh Poimboeuf wrote:
> I don't know the lockdep code, but one more comment from the peanut
> gallery. This code looks suspect to me:
>
>
> /*
>* Stop saving stack_trace if save_trace() was
>* called at least once:
>*/
> if (save && ret == 2)
> save = NULL;
>
>
> From looking at check_prev_add(), a return value of 2 doesn't
> necessarily imply that save_trace() was called. If the
> check_redundant() call returns 0, then check_prev_add() can return 2,
> and the trace will still be uninitialized, but save will be set to NULL
> even though save_trace() hasn't been called. Then a subsequent call to
> check_prev_add() could add an uninitialized stack_trace struct to the
> dependency list.
Right, and print_circular_bug() uses @trace before it ever can be set,
although I suspect the intention is that that only ever gets called from
commit_xhlock() where we pass in an initialized @trace. A comment
would've been good :/
So the whole point of that trace wankery here is to not do save_trace()
when we'll not in fact use the stack trace.
It seems to me we can do that much saner by actually initializing our
@trace buffer and testing if it contains an actual stacktrace.
Also killed that verbose thing, because dropping that graph_lock thing
hurts my brain -- although I think it should work.
Does this make the corruption thing go away?
---
kernel/locking/lockdep.c | 48
1 file changed, 20 insertions(+), 28 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 44c8d0d17170..e36e652d996f 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1873,10 +1873,10 @@ check_prev_add(struct task_struct *curr, struct
held_lock *prev,
struct held_lock *next, int distance, struct stack_trace *trace,
int (*save)(struct stack_trace *trace))
{
+ struct lock_list *uninitialized_var(target_entry);
struct lock_list *entry;
- int ret;
struct lock_list this;
- struct lock_list *uninitialized_var(target_entry);
+ int ret;
/*
* Prove that the new -> dependency would not
@@ -1890,8 +1890,17 @@ check_prev_add(struct task_struct *curr, struct
held_lock *prev,
this.class = hlock_class(next);
this.parent = NULL;
ret = check_noncircular(&this, hlock_class(prev), &target_entry);
- if (unlikely(!ret))
+ if (unlikely(!ret)) {
+ if (!trace->entries) {
+ /*
+* If @save fails here, the printing might trigger
+* a WARN but because of the !nr_entries it should
+* not do bad things.
+*/
+ save(trace);
+ }
return print_circular_bug(&this, target_entry, next, prev,
trace);
+ }
else if (unlikely(ret < 0))
return print_bfs_bug(ret);
@@ -1938,7 +1947,7 @@ check_prev_add(struct task_struct *curr, struct held_lock
*prev,
return print_bfs_bug(ret);
- if (save && !save(trace))
+ if (!trace->entries && !save(trace))
return 0;
/*
@@ -1958,20 +1967,6 @@ check_prev_add(struct task_struct *curr, struct
held_lock *prev,
if (!ret)
return 0;
- /*
-* Debugging printouts:
-*/
- if (verbose(hlock_class(prev)) || verbose(hlock_class(next))) {
- graph_unlock();
- printk("\n new dependency: ");
- print_lock_name(hlock_class(prev));
- printk(KERN_CONT " => ");
- print_lock_name(hlock_class(next));
- printk(KERN_CONT "\n");
- dump_stack();
- if (!graph_lock())
- return 0;
- }
return 2;
}
@@ -1986,8 +1981,12 @@ check_prevs_add(struct task_struct *curr, struct
held_lock *next)
{
int depth = curr->lockdep_depth;
struct held_lock *hlock;
- struct stack_trace trace;
- int (*save)(struct stack_trace *trace) = save_trace;
+ struct stack_trace trace = {
+ .nr_entries = 0,
+ .max_entries = 0,
+ .entries = NULL,
+ .skip = 0,
+ };
/*
* Debugging checks.
@@ -2018,17 +2017,10 @@ check_prevs_add(struct task_struct *curr, struct
held_lock *next)
*/
if (hlock->read != 2 && hlock->check) {
int ret = check_prev_add(curr, hlock, next,
-distance, &trace,
save);
+distance, &trace,
save_trace);
Re: [PATCH v5 00/10] arm, arm64, cpufreq: frequency- and cpu-invariant accounting support for task scheduler
On Tue, Sep 26, 2017 at 05:41:05PM +0100, Dietmar Eggemann wrote: > For a more accurate (i.e. frequency- and cpu-invariant) accounting > the task scheduler needs a frequency-scaling and on a heterogeneous > system a cpu-scaling correction factor. > > This patch-set implements a Frequency Invariance Engine (FIE) > based on the ratio of current frequency and maximum supported frequency > (topology_get_freq_scale()) in the arch topology driver (arm, arm64) to > provide such a frequency-scaling correction factor. > This is a solution to get frequency-invariant accounting support for > platforms without hardware-based performance tracking. > > The Cpu Invariance Engine (CIE) (topology_get_cpu_scale()) providing a > cpu-scaling correction factor was already introduced by the "Fix issues > and factorize arm/arm64 capacity information code" patch-set [1] which > went into v4.13. > > This patch-set also enables the frequency- and cpu-invariant accounting > support. Enabling here means to associate (wire) the task scheduler > function name arch_scale_freq_capacity and arch_scale_cpu_capacity with > the FIE and CIE function names from drivers/base/arch_topology.c. This > replaces the scheduler's default FIE and CIE in kernel/sched/sched.h. > > v4: review results: > > There were no further comments during the v4 [2] review. This patchset crosses a bunch of different subsystems, who do you want/expect to be taking this through their tree? thanks, greg k-h
[PATCH] KEYS: checking the input id parameters before finding asymmetric key
For finding asymmetric key, the input id_0 and id_1 parameters can
not be NULL at the same time. This patch adds the BUG_ON checking
for id_0 and id_1.
Cc: David Howells
Cc: Herbert Xu
Cc: "David S. Miller"
Signed-off-by: "Lee, Chun-Yi"
---
crypto/asymmetric_keys/asymmetric_type.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/crypto/asymmetric_keys/asymmetric_type.c
b/crypto/asymmetric_keys/asymmetric_type.c
index e4b0ed3..3a3b028 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -57,6 +57,8 @@ struct key *find_asymmetric_key(struct key *keyring,
char *req, *p;
int len;
+ BUG_ON(!id_0 && !id_1);
+
if (id_0) {
lookup = id_0->data;
len = id_0->len;
--
2.10.2
Re: [PATCH v9 12/12] mm: stop zeroing memory during allocation in vmemmap
On Tue 03-10-17 16:26:51, Pasha Tatashin wrote: > Hi Michal, > > I decided not to merge these two patches, because in addition to sparc > optimization move, we have this dependancies: optimizations can and should go on top of the core patch. > mm: zero reserved and unavailable struct pages > > must be before > > mm: stop zeroing memory during allocation in vmemmap. > > Otherwise, we can end-up with struct pages that are not zeroed properly. Right and you can deal with it easily. Just introduce the mm_zero_struct_page earlier along with its user in "stop zeroing ..." I think that moving the zeroying in one go is more reasonable than adding it to __init_single_page with misleading numbers and later dropping the zeroying from the memmap path. -- Michal Hocko SUSE Labs
Re: [PATCH v9 06/12] mm: zero struct pages during initialization
On Tue 03-10-17 11:22:35, Pasha Tatashin wrote: > > > On 10/03/2017 09:08 AM, Michal Hocko wrote: > > On Wed 20-09-17 16:17:08, Pavel Tatashin wrote: > > > Add struct page zeroing as a part of initialization of other fields in > > > __init_single_page(). > > > > > > This single thread performance collected on: Intel(R) Xeon(R) CPU E7-8895 > > > v3 @ 2.60GHz with 1T of memory (268400646 pages in 8 nodes): > > > > > > BASEFIX > > > sparse_init 11.244671836s 0.007199623s > > > zone_sizes_init 4.879775891s 8.355182299s > > >-- > > > Total 16.124447727s 8.362381922s > > > > Hmm, this is confusing. This assumes that sparse_init doesn't zero pages > > anymore, right? So these number depend on the last patch in the series? > > Correct, without the last patch sparse_init time won't change. THen this is just misleading. -- Michal Hocko SUSE Labs
Re: [PATCH v9 03/12] mm: deferred_init_memmap improvements
On Tue 03-10-17 12:01:08, Pasha Tatashin wrote:
> Hi Michal,
>
> Are you OK, if I replace DEFERRED_FREE() macro with a function like this:
>
> /*
> * Helper for deferred_init_range, free the given range, and reset the
> * counters
> */
> static inline unsigned long __def_free(unsigned long *nr_free,
>unsigned long *free_base_pfn,
>struct page **page)
> {
> unsigned long nr = *nr_free;
>
> deferred_free_range(*free_base_pfn, nr);
> *free_base_pfn = 0;
> *nr_free = 0;
> *page = NULL;
>
> return nr;
> }
>
> Since it is inline, and we operate with non-volatile counters, compiler will
> be smart enough to remove all the unnecessary de-references. As a plus, we
> won't be adding any new branches, and the code is still going to stay
> compact.
OK. It is a bit clunky but we are holding too much state there. I
haven't checked whether that can be simplified but this can be always
done later.
--
Michal Hocko
SUSE Labs
Re: [PATCH 2/2] Drivers: hv: vmbus: Expose per-channel interrupts and events counters
On Thu, Sep 21, 2017 at 08:58:50PM -0700, k...@exchange.microsoft.com wrote: > From: Stephen Hemminger > > When investigating performance, it is useful to be able to look at > the number of host and guest events per-channel. This is equivalent > to per-device interrupt statistics. > > Signed-off-by: Stephen Hemminger > Signed-off-by: K. Y. Srinivasan > --- > Documentation/ABI/stable/sysfs-bus-vmbus | 14 ++ > drivers/hv/connection.c | 2 ++ > drivers/hv/vmbus_drv.c | 18 ++ > include/linux/hyperv.h | 4 > 4 files changed, 38 insertions(+) This patch didn't apply to my tree, can you rebase and resend it? thanks, greg k-h
Re: [kernel-hardening] [RFC V2 4/6] lib: vsprintf: default kptr_restrict to the maximum value
On Sun, Oct 01, 2017 at 11:06:48AM +1100, Tobin C. Harding wrote: > Set the initial value of kptr_restrict to the maximum > setting rather than the minimum setting, to ensure that > early boot logging is not leaking information. > > Signed-off-by: Tobin C. Harding Signed-off-by: Greg Kroah-Hartman
Re: [kernel-hardening] [RFC V2 1/6] lib: vsprintf: additional kernel pointer filtering options
On Sun, Oct 01, 2017 at 11:06:45AM +1100, Tobin C. Harding wrote: > Add the kptr_restrict setting of 3 which results in both > %p and %pK values being replaced by zeros. > > Add an additional %pP value inspired by the Grsecurity > option which explicitly whitelists pointers for output. > > Amend scripts/checkpatch.pl to handle %pP. > > This patch is based on work by William Roberts > > > Signed-off-by: Tobin C. Harding Signed-off-by: Greg Kroah-Hartman
Re: [PATCH v9 08/12] mm: zero reserved and unavailable struct pages
On Tue 03-10-17 11:29:16, Pasha Tatashin wrote: > On 10/03/2017 09:18 AM, Michal Hocko wrote: > > On Wed 20-09-17 16:17:10, Pavel Tatashin wrote: > > > Some memory is reserved but unavailable: not present in memblock.memory > > > (because not backed by physical pages), but present in memblock.reserved. > > > Such memory has backing struct pages, but they are not initialized by > > > going > > > through __init_single_page(). > > > > Could you be more specific where is such a memory reserved? > > > > I know of one example: trim_low_memory_range() unconditionally reserves from > pfn 0, but e820__memblock_setup() might provide the exiting memory from pfn > 1 (i.e. KVM). Then just initialize struct pages for that mapping rigth there where a special API is used. > But, there could be more based on this comment from linux/page-flags.h: > > 19 * PG_reserved is set for special pages, which can never be swapped out. > Some > 20 * of them might not even exist (eg empty_bad_page)... I have no idea wht empty_bad_page is but a quick grep shows that this is never used. I might be wrong here but if somebody is reserving a memory in a special way then we should handle the initialization right there. E.g. create an API for special memblock reservations. -- Michal Hocko SUSE Labs
Re: [kernel-hardening] [RFC V2 2/6] lib: vsprintf: whitelist stack traces
On Mon, Oct 02, 2017 at 11:42:05AM +0100, Will Deacon wrote:
> On Sun, Oct 01, 2017 at 11:06:46AM +1100, Tobin C. Harding wrote:
> > Use the %pP functionality to explicitly allow kernel
> > pointers to be logged for stack traces.
> >
> > Signed-off-by: Tobin C. Harding
> > ---
> > arch/arm64/kernel/traps.c | 4 ++--
> > kernel/printk/printk.c| 2 +-
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> > index 5ea4b85..fe09660 100644
> > --- a/arch/arm64/kernel/traps.c
> > +++ b/arch/arm64/kernel/traps.c
> > @@ -147,7 +147,7 @@ void dump_backtrace(struct pt_regs *regs, struct
> > task_struct *tsk)
> > struct stackframe frame;
> > int skip;
> >
> > - pr_debug("%s(regs = %p tsk = %p)\n", __func__, regs, tsk);
> > + pr_debug("%s(regs = %pP tsk = %pP)\n", __func__, regs, tsk);
>
> Why do we care for pr_debug?
Because you really want the real value? Seems to make sense to me...
thanks,
greg k-h
Re: [kernel-hardening] [RFC V2 3/6] lib: vsprintf: physical address kernel pointer filtering options
On Sun, Oct 01, 2017 at 11:06:47AM +1100, Tobin C. Harding wrote: > Add the kptr_restrict setting of 4 which results in %pa and > %p[rR] values being cleansed. > > Address types printed with %pa are replaced by zeros. Resources printed > with %p[rR] have the starting address replaced by zeros, resource size > is still shown. > > Signed-off-by: Tobin C. Harding Signed-off-by: Greg Kroah-Hartman
Re: [kernel-hardening] [RFC V2 5/6] lib: vsprintf: add "%paP", "%papP", and "%padP" specifiers
On Sun, Oct 01, 2017 at 11:06:49AM +1100, Tobin C. Harding wrote: > Add %papP and %padP for address types that need to always be shown > regardless of kptr restrictions. Add %paP is a synonym for %papP, this > is inline with current implementation (%pa is a synonym for %pap). > > Signed-off-by: Tobin C. Harding Signed-off-by: Greg Kroah-Hartman
Re: [kernel-hardening] [RFC V2 0/6] add more kernel pointer filter options
On Sun, Oct 01, 2017 at 11:11:05AM +1100, Tobin C. Harding wrote:
> On Sun, Oct 01, 2017 at 11:06:44AM +1100, Tobin C. Harding wrote:
> > Version 2 of Greg's patch series with changes made as suggested by comments
> > to V1.
>
> Patch set tested by setting /proc/sys/kernel/kptr_restrict and inserting the
> following module
>
> #include
> #include
>
> #define DRIVER_AUTHOR "Tobin C. Harding "
> #define DRIVER_DESC "Testing module"
>
> static void test_printk(void)
> {
> char *ptr = "";
>
> pr_alert("printing with p: %p\n", ptr);
> pr_alert("printing with pK: %pK\n", ptr);
> pr_alert("printing with pP: %pP\n", ptr);
>
> pr_alert("printing with pa: %pa\n", ptr);
> pr_alert("printing with pad: %pad\n", ptr);
> pr_alert("printing with pap: %pap\n", ptr);
>
> pr_alert("printing with paP: %paP\n", ptr);
> pr_alert("printing with padP: %padP\n", ptr);
> pr_alert("printing with papP: %papP\n", ptr);
>
> pr_alert("printing with pr: %pr\n", ptr);
> pr_alert("printing with pR: %pR\n", ptr);
> }
>
> static int hello_init(void)
> {
> pr_alert("Hello, world\n");
>
> test_printk();
>
> return 0;
> }
> module_init(hello_init);
>
> static void hello_exit(void)
> {
> pr_alert("Goodbye, world\n");
> }
> module_exit(hello_exit);
>
> MODULE_DESCRIPTION(DRIVER_DESC);
> MODULE_AUTHOR(DRIVER_AUTHOR);
> MODULE_LICENSE("GPL");
Hm, any way to add something like this to the testing infrastructure?
Or maybe just at boot time? It's nice to keep tests around to ensure
things do not break :)
thanks,
greg k-h
Re: [kernel-hardening] [RFC V2 2/6] lib: vsprintf: whitelist stack traces
On Wed, Oct 04, 2017 at 10:56:57AM +0200, Greg KH wrote:
> On Mon, Oct 02, 2017 at 11:42:05AM +0100, Will Deacon wrote:
> > On Sun, Oct 01, 2017 at 11:06:46AM +1100, Tobin C. Harding wrote:
> > > Use the %pP functionality to explicitly allow kernel
> > > pointers to be logged for stack traces.
> > >
> > > Signed-off-by: Tobin C. Harding
> > > ---
> > > arch/arm64/kernel/traps.c | 4 ++--
> > > kernel/printk/printk.c| 2 +-
> > > 2 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> > > index 5ea4b85..fe09660 100644
> > > --- a/arch/arm64/kernel/traps.c
> > > +++ b/arch/arm64/kernel/traps.c
> > > @@ -147,7 +147,7 @@ void dump_backtrace(struct pt_regs *regs, struct
> > > task_struct *tsk)
> > > struct stackframe frame;
> > > int skip;
> > >
> > > - pr_debug("%s(regs = %p tsk = %p)\n", __func__, regs, tsk);
> > > + pr_debug("%s(regs = %pP tsk = %pP)\n", __func__, regs, tsk);
> >
> > Why do we care for pr_debug?
>
> Because you really want the real value? Seems to make sense to me...
Just seems like anybody debugging the kernel using pr_debug can probably
change /proc/sys/kernel/kptr_restrict...
Will
Re: [kernel-hardening] [RFC V2 6/6] drivers: uio: un-restrict sysfs pointers for UIO
On Sun, Oct 01, 2017 at 11:06:50AM +1100, Tobin C. Harding wrote: > The address and size on the UIO devices are required by userspace to > function properly. Let's un-restrict these by adding the 'P' modifier > to %pa. > > Signed-off-by: Tobin C. Harding Signed-off-by: Greg Kroah-Hartman
Re: [kernel-hardening] [RFC V2 0/6] add more kernel pointer filter options
On Sun, Oct 01, 2017 at 11:06:44AM +1100, Tobin C. Harding wrote: > Version 2 of Greg's patch series with changes made as suggested by comments > to V1. > > Applies on top of Linus' current development tree > > a8c964eacb21288b2dbfa9d80cee5968a3b8fb21 > > V1 cover letter: > > Here's a short patch series from Chris Fries and Dave Weinstein that > implements some new restrictions when printing out kernel pointers, as > well as the ability to whitelist kernel pointers where needed. > > These patches are based on work from William Roberts, and also are > inspired by grsecurity's %pP to specifically whitelist a kernel pointer, > where it is always needed, like the last patch in the series shows, in > the UIO drivers (UIO requires that you know the address, it's a hardware > address, nothing wrong with seeing that...) > > I haven't done much to this patch series, only forward porting it from > an older kernel release (4.4) and a few minor tweaks. [snip] Nice! Thanks for doing this work, looks great to me. Care to resend the next version as a "real" one (i.e. no RFC)? thanks, greg k-h
Re: [PATCH v2 0/4] mmc: sdhci-msm: Corrections to implementation of power irq
On 27 September 2017 at 07:34, Vijay Viswanath wrote: > Register writes which change voltage of IO lines or turn the IO bus on/off > require sdhc controller to be ready before progressing further. Once a > register write which affects IO lines is done, the driver should wait for > power irq from controller. Once the irq comes, the driver should acknowledge > the irq by writing to power control register. If the acknowledgement is not > given to controller, the controller may not complete the corresponding > register write action and this can mess up the controller if drivers proceeds > without power irq completing. > > Changes since v1: > Patch enabling MMC_IO_ACCESSORS in Kconfig moved up the patch list. > Also corrected a mistake in the patch. > Removed all ifdef CONFIG_MMC_IO_ACCESSORS since the patches using it > will come after MMC_IO_ACCESSORS are enabled. > Merged the patches 3 & 4 of earlier series into 1 patch (patch 4 in > current series). > Corrected a mistake in a comment text in patch 3 of previous series. > > Changes since RFC: > wait_for_completion_timeout replaced with wait_event_timeout when > waiting for power irq. > Removed the spinlock within power irq handler and API which waits > for power irq. > Added comments to sdhci msm register write functions, warning that > they > can sleep. > Sdhci msm register write functions will do a memory barrier before > writing to the register if the particular register can trigger > power irq. > Instead of enabling SDHCI IO ACCESSORS config in arm64/defconfig, it > will be selected in mmc/host/Kconfig if the platform is MMC_SDHCI_MSM. > > Sahitya Tummala (1): > mmc: sdhci-msm: Fix HW issue with power IRQ handling during reset > > Subhash Jadavani (1): > mmc: sdhci-msm: fix issue with power irq > > Vijay Viswanath (2): > mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS > mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr > irq > > drivers/mmc/host/Kconfig | 1 + > drivers/mmc/host/sdhci-msm.c | 235 > ++- > 2 files changed, 231 insertions(+), 5 deletions(-) > Thanks, applied for next! Kind regards Uffe
Re: [PATCH 0/3] mmc: meson-gx: fixes for v4.14
On 2 October 2017 at 14:27, Jerome Brunet wrote: > This patchset fixes the issues reported by Heiner [0] with the odroid-c2. > In a nutshell, this series does the following: > > * Make sure the mmc clock rate is not set higher than what is requested > * Reset the tuning values on mmc power cycle instead of POWER_ON > * Add tuning of the tx phase to the tuning function. > > This fixes the problem seen on the odroid-c2, both for hs200 and hs400. > To check for regression, this series has also been tested on the gxl > libretech-cc > > Ulf, could you please pick this as fixes for v4.14 ? Thanks, applied for fixes! Kind regards Uffe > > [0]: https://lkml.kernel.org/r/e2171288-84ef-82db-2efc-300935e6c...@gmail.com > > Jerome Brunet (3): > mmc: meson-gx: make sure the clock is rounded down > mmc: meson-gx: fix rx phase reset > mmc: meson-gx: include tx phase in the tuning process > > drivers/mmc/host/meson-gx-mmc.c | 26 ++ > 1 file changed, 22 insertions(+), 4 deletions(-) > > -- > 2.13.5 >
Re: [PATCH] mmc: dw_mmc-k3: make array hs_timing_cfg static
On 3 October 2017 at 11:56, Colin King wrote:
> From: Colin Ian King
>
> The array hs_timing_cfg is local to the source and does not need to
> be in global scope, so make it static.
>
> Cleans up sparse warning:
> symbol 'hs_timing_cfg' was not declared. Should it be static?
>
> Signed-off-by: Colin Ian King
Thanks, applied for next!
Kind regards
Uffe
> ---
> drivers/mmc/host/dw_mmc-k3.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c
> index 64cda84b2302..73fd75c3c824 100644
> --- a/drivers/mmc/host/dw_mmc-k3.c
> +++ b/drivers/mmc/host/dw_mmc-k3.c
> @@ -75,7 +75,7 @@ struct hs_timing {
> u32 smpl_phase_min;
> };
>
> -struct hs_timing hs_timing_cfg[TIMING_MODE][TIMING_CFG_NUM] = {
> +static struct hs_timing hs_timing_cfg[TIMING_MODE][TIMING_CFG_NUM] = {
> { /* reserved */ },
> { /* SD */
> {7, 0, 15, 15,}, /* 0: LEGACY 400k */
> --
> 2.14.1
>
[PATCH] Staging: rtl8723bs: Externs should be avoided in .C file
This patch fixes the following checkpatch.pl warning.
WARNING: externs should be avoided in .c files
Signed-off-by: Srinivasan Shanmugam
---
drivers/staging/rtl8723bs/core/rtw_ap.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c
b/drivers/staging/rtl8723bs/core/rtw_ap.c
index 0b530ea..c66fed1 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ap.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ap.c
@@ -17,12 +17,6 @@
#include
#include
-extern unsigned char RTW_WPA_OUI[];
-extern unsigned char WMM_OUI[];
-extern unsigned char WPS_OUI[];
-extern unsigned char P2P_OUI[];
-extern unsigned char WFD_OUI[];
-
void init_mlme_ap_info(struct adapter *padapter)
{
struct mlme_priv *pmlmepriv = &(padapter->mlmepriv);
--
1.9.1
Re: [PATCH] mmc: sdhci-of-at91: make function sdhci_at91_set_uhs_signaling static
On 3 October 2017 at 12:06, Colin King wrote:
> From: Colin Ian King
>
> The function sdhci_at91_set_uhs_signaling is local to the source and does
> not need to be in global scope, so make it static.
>
> Cleans up sparse warning:
> symbol 'sdhci_at91_set_uhs_signaling' was not declared. Should it be
> static?
>
> Signed-off-by: Colin Ian King
Thanks, applied for next!
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci-of-at91.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-at91.c
> b/drivers/mmc/host/sdhci-of-at91.c
> index 4e47ed6bc716..682c573e20a7 100644
> --- a/drivers/mmc/host/sdhci-of-at91.c
> +++ b/drivers/mmc/host/sdhci-of-at91.c
> @@ -114,7 +114,8 @@ static void sdhci_at91_set_power(struct sdhci_host *host,
> unsigned char mode,
> sdhci_set_power_noreg(host, mode, vdd);
> }
>
> -void sdhci_at91_set_uhs_signaling(struct sdhci_host *host, unsigned int
> timing)
> +static void sdhci_at91_set_uhs_signaling(struct sdhci_host *host,
> +unsigned int timing)
> {
> if (timing == MMC_TIMING_MMC_DDR52)
> sdhci_writeb(host, SDMMC_MC1R_DDR, SDMMC_MC1R);
> --
> 2.14.1
>
RE: [PATCH 04/11] ia64: make dma_cache_sync a no-op
From: Christoph Hellwig
> Sent: 03 October 2017 11:43
>
> ia64 does not implement DMA_ATTR_NON_CONSISTENT allocations, so it doesn't
> make any sense to do any work in dma_cache_sync given that it must be a
> no-op when dma_alloc_attrs returns coherent memory.
>
> Signed-off-by: Christoph Hellwig
> ---
> arch/ia64/include/asm/dma-mapping.h | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/arch/ia64/include/asm/dma-mapping.h
> b/arch/ia64/include/asm/dma-mapping.h
> index 3ce5ab4339f3..99dfc1aa9d3c 100644
> --- a/arch/ia64/include/asm/dma-mapping.h
> +++ b/arch/ia64/include/asm/dma-mapping.h
> @@ -48,11 +48,6 @@ static inline void
> dma_cache_sync (struct device *dev, void *vaddr, size_t size,
> enum dma_data_direction dir)
> {
> - /*
> - * IA-64 is cache-coherent, so this is mostly a no-op. However, we do
> need to
> - * ensure that dma_cache_sync() enforces order, hence the mb().
> - */
> - mb();
> }
Are you sure about this one?
It looks as though you are doing a mechanical change for all architectures.
Some of them are probably stranger than you realise.
Even with cache coherent memory any cpu 'store/write buffer' may not
be snooped by dma reads.
Something needs to flush the store buffer between the last cpu write
to the dma buffer and the write (probably a device register) that
tells the device it can read the memory.
My guess from the comment is that dma_cache_synch() is expected to
include that barrier - and it might not be anywhere else.
David
Re: [PATCHv3 2/7] sections: split dereference_function_descriptor()
On Sat 2017-09-30 11:53:14, Sergey Senozhatsky wrote: > There are two format specifiers to print out a pointer in symbolic > format: '%pS/%ps' and '%pF/%pf'. On most architectures, the two > mean exactly the same thing, but some architectures (ia64, ppc64, > parisc64) use an indirect pointer for C function pointers, where > the function pointer points to a function descriptor (which in > turn contains the actual pointer to the code). The '%pF/%pf, when > used appropriately, automatically does the appropriate function > descriptor dereference on such architectures. > > The "when used appropriately" part is tricky. Basically this is > a subtle ABI detail, specific to some platforms, that made it to > the API level and people can be unaware of it and miss the whole > "we need to dereference the function" business out. [1] proves > that point (note that it fixes only '%pF' and '%pS', there might > be '%pf' and '%ps' cases as well). > > It appears that we can handle everything within the affected > arches and make '%pS/%ps' smart enough to retire '%pF/%pf'. > Function descriptors live in .opd elf section and all affected > arches (ia64, ppc64, parisc64) handle it properly for kernel > and modules. So we, technically, can decide if the dereference > is needed by simply looking at the pointer: if it belongs to > .opd section then we need to dereference it. Great catch. I really like this approach! > diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h > index e5da44eddd2f..387f22c41e0d 100644 > --- a/include/asm-generic/sections.h > +++ b/include/asm-generic/sections.h > @@ -29,6 +29,7 @@ > * __ctors_start, __ctors_end > * __irqentry_text_start, __irqentry_text_end > * __softirqentry_text_start, __softirqentry_text_end > + * __start_opd, __end_opd > */ > extern char _text[], _stext[], _etext[]; > extern char _data[], _sdata[], _edata[]; > @@ -47,12 +48,15 @@ extern char __softirqentry_text_start[], > __softirqentry_text_end[]; > /* Start and end of .ctors section - used for constructor calls. */ > extern char __ctors_start[], __ctors_end[]; > > +/* Start and end of .opd section - used for function descriptors. */ > +extern char __start_opd[], __end_opd[]; > + > extern __visible const void __nosave_begin, __nosave_end; > > -/* function descriptor handling (if any). Override > - * in asm/sections.h */ > +/* Function descriptor handling (if any). Override in asm/sections.h */ > #ifndef dereference_function_descriptor > #define dereference_function_descriptor(p) (p) > +#define dereference_kernel_function_descriptor(p) (p) > #endif > > /* random extra sections (if any). Override > diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h > index 4d0cb9bba93e..172904e9cded 100644 > --- a/include/linux/moduleloader.h > +++ b/include/linux/moduleloader.h > @@ -85,6 +85,10 @@ void module_arch_cleanup(struct module *mod); > /* Any cleanup before freeing mod->module_init */ > void module_arch_freeing_init(struct module *mod); > > +/* Dereference module function descriptor */ > +unsigned long dereference_module_function_descriptor(struct module *mod, > + unsigned long addr); > + The function is used when the module is already loaded. IMHO, include/linux/module.h would be a better place. One advantage would be that we could use the same trick as in include/asm-generic/sections.h. I mean: #define dereference_module_function_descriptor(mod, addr) (addr) and redefine it in the three affected arch//include/asm/module.h headers. Then it might be completely optimized out on all architectures. Anyway, we need to get ack from Jessica for this change. Best Regards, Petr
[PATCH 2/2] USB: serial: console: fix use-after-free after failed setup
Make sure to reset the USB-console port pointer when console setup fails
in order to avoid having the struct usb_serial be prematurely freed by
the console code when the device is later disconnected.
Fixes: 73e487fdb75f ("[PATCH] USB console: fix disconnection issues")
Cc: stable # 2.6.18
Signed-off-by: Johan Hovold
---
drivers/usb/serial/console.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/serial/console.c b/drivers/usb/serial/console.c
index ed8ba3ef5c79..43a862a90a77 100644
--- a/drivers/usb/serial/console.c
+++ b/drivers/usb/serial/console.c
@@ -186,6 +186,7 @@ static int usb_console_setup(struct console *co, char
*options)
tty_kref_put(tty);
reset_open_count:
port->port.count = 0;
+ info->port = NULL;
usb_autopm_put_interface(serial->interface);
error_get_interface:
usb_serial_put(serial);
--
2.14.2
[PATCH 0/2] USB: serial: console: fix two use-after-free bugs
A recent clean-up patch of mine did more than intended and introduced the potential for a use-after-free on disconnect under some very specific circumstances. Fortunately those circumstances were not obscure enough to prevent Andrey's fuzzing from triggering the bug. While fixing this one I found a related issue through inspection which dates back to 2006. Johan Johan Hovold (2): USB: serial: console: fix use-after-free on disconnect USB: serial: console: fix use-after-free after failed setup drivers/usb/serial/console.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 2.14.2
Re: [kernel-hardening] [RFC V2 2/6] lib: vsprintf: whitelist stack traces
On Wed, Oct 04, 2017 at 09:58:10AM +0100, Will Deacon wrote:
> On Wed, Oct 04, 2017 at 10:56:57AM +0200, Greg KH wrote:
> > On Mon, Oct 02, 2017 at 11:42:05AM +0100, Will Deacon wrote:
> > > On Sun, Oct 01, 2017 at 11:06:46AM +1100, Tobin C. Harding wrote:
> > > > Use the %pP functionality to explicitly allow kernel
> > > > pointers to be logged for stack traces.
> > > >
> > > > Signed-off-by: Tobin C. Harding
> > > > ---
> > > > arch/arm64/kernel/traps.c | 4 ++--
> > > > kernel/printk/printk.c| 2 +-
> > > > 2 files changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> > > > index 5ea4b85..fe09660 100644
> > > > --- a/arch/arm64/kernel/traps.c
> > > > +++ b/arch/arm64/kernel/traps.c
> > > > @@ -147,7 +147,7 @@ void dump_backtrace(struct pt_regs *regs, struct
> > > > task_struct *tsk)
> > > > struct stackframe frame;
> > > > int skip;
> > > >
> > > > - pr_debug("%s(regs = %p tsk = %p)\n", __func__, regs, tsk);
> > > > + pr_debug("%s(regs = %pP tsk = %pP)\n", __func__, regs, tsk);
> > >
> > > Why do we care for pr_debug?
> >
> > Because you really want the real value? Seems to make sense to me...
>
> Just seems like anybody debugging the kernel using pr_debug can probably
> change /proc/sys/kernel/kptr_restrict...
Ok, fair enough :)
[PATCH 1/2] USB: serial: console: fix use-after-free on disconnect
A clean-up patch removing removing two redundant NULL-checks from the
console disconnect handler inadvertently also removed a third check.
This could lead to the struct usb_serial being prematurely freed by the
console code when a driver accepts but does not register any ports for
an interface which also lacks endpoint descriptors.
Fixes: 0e517c93dc02 ("USB: serial: console: clean up sanity checks")
Cc: stable # 4.11
Reported-by: Andrey Konovalov
Signed-off-by: Johan Hovold
---
drivers/usb/serial/console.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/serial/console.c b/drivers/usb/serial/console.c
index fdf89800ebc3..ed8ba3ef5c79 100644
--- a/drivers/usb/serial/console.c
+++ b/drivers/usb/serial/console.c
@@ -265,7 +265,7 @@ static struct console usbcons = {
void usb_serial_console_disconnect(struct usb_serial *serial)
{
- if (serial->port[0] == usbcons_info.port) {
+ if (serial->port[0] && serial->port[0] == usbcons_info.port) {
usb_serial_console_exit();
usb_serial_put(serial);
}
--
2.14.2
Re: [PATCH v3 1/2] dt-bindings: add device tree binding for Allwinner XR819 SDIO Wi-Fi
Icenowy Zheng writes: > Allwinner XR819 is a SDIO Wi-Fi chip, which has the functionality to use > an out-of-band interrupt pin instead of SDIO in-band interrupt. > > Add the device tree binding of this chip, in order to make it possible > to add this interrupt pin to device trees. > > Signed-off-by: Icenowy Zheng > Acked-by: Rob Herring > --- > Changes in v3: > - Renames the node name. > - Adds ACK from Rob. > Changes in v2: > - Removed status property in example. > - Added required property reg. > > .../bindings/net/wireless/allwinner,xr819.txt | 38 > ++ > 1 file changed, 38 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt Like I asked already last time, AFAICS there is no upstream xr819 wireless driver in drivers/net/wireless directory. Do we still accept bindings like this for out-of-tree drivers? -- Kalle Valo
[tip:core/watchdog] watchdog/core, powerpc: Replace watchdog_nmi_reconfigure()
Commit-ID: 6b9dc4806b28214a4a260517e59439e0ac12a15e
Gitweb: https://git.kernel.org/tip/6b9dc4806b28214a4a260517e59439e0ac12a15e
Author: Thomas Gleixner
AuthorDate: Mon, 2 Oct 2017 12:34:50 +0200
Committer: Thomas Gleixner
CommitDate: Wed, 4 Oct 2017 10:53:53 +0200
watchdog/core, powerpc: Replace watchdog_nmi_reconfigure()
The recent cleanup of the watchdog code split watchdog_nmi_reconfigure()
into two stages. One to stop the NMI and one to restart it after
reconfiguration. That was done by adding a boolean 'run' argument to the
code, which is functionally correct but not necessarily a piece of art.
Replace it by two explicit functions: watchdog_nmi_stop() and
watchdog_nmi_start().
Fixes: 6592ad2fcc8f ("watchdog/core, powerpc: Make watchdog_nmi_reconfigure()
two stage")
Requested-by: Linus 'Nursing his pet-peeve' Torvalds
Signed-off-by: Thomas 'Mopping up garbage' Gleixner
Acked-by: Michael Ellerman
Cc: Peter Zijlstra
Cc: Don Zickus
Cc: Benjamin Herrenschmidt
Cc: Nicholas Piggin
Cc: linuxppc-...@lists.ozlabs.org
Link: http://lkml.kernel.org/r/alpine.DEB.2.20.1710021957480.2114@nanos
---
arch/powerpc/kernel/watchdog.c | 23 ++-
include/linux/nmi.h| 3 ++-
kernel/watchdog.c | 33 ++---
3 files changed, 34 insertions(+), 25 deletions(-)
diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
index dfb0677..2673ec8 100644
--- a/arch/powerpc/kernel/watchdog.c
+++ b/arch/powerpc/kernel/watchdog.c
@@ -355,19 +355,24 @@ static void watchdog_calc_timeouts(void)
wd_timer_period_ms = watchdog_thresh * 1000 * 2 / 5;
}
-void watchdog_nmi_reconfigure(bool run)
+void watchdog_nmi_stop(void)
{
int cpu;
cpus_read_lock();
- if (!run) {
- for_each_cpu(cpu, &wd_cpus_enabled)
- stop_wd_on_cpu(cpu);
- } else {
- watchdog_calc_timeouts();
- for_each_cpu_and(cpu, cpu_online_mask, &watchdog_cpumask)
- start_wd_on_cpu(cpu);
- }
+ for_each_cpu(cpu, &wd_cpus_enabled)
+ stop_wd_on_cpu(cpu);
+ cpus_read_unlock();
+}
+
+void watchdog_nmi_start(void)
+{
+ int cpu;
+
+ cpus_read_lock();
+ watchdog_calc_timeouts();
+ for_each_cpu_and(cpu, cpu_online_mask, &watchdog_cpumask)
+ start_wd_on_cpu(cpu);
cpus_read_unlock();
}
diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index 89ba8b2..0c9ed49 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -109,7 +109,8 @@ static inline int hardlockup_detector_perf_init(void) {
return 0; }
# endif
#endif
-void watchdog_nmi_reconfigure(bool run);
+void watchdog_nmi_stop(void);
+void watchdog_nmi_start(void);
/**
* touch_nmi_watchdog - restart NMI watchdog timeout.
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index f6ef163..6ad6226 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -123,24 +123,27 @@ int __weak __init watchdog_nmi_probe(void)
}
/**
- * watchdog_nmi_reconfigure - Optional function to reconfigure NMI watchdogs
- * @run: If false stop the watchdogs on all enabled CPUs
- * If true start the watchdogs on all enabled CPUs
+ * watchdog_nmi_stop - Stop the watchdog for reconfiguration
*
- * The core call order is:
- * watchdog_nmi_reconfigure(false);
+ * The reconfiguration steps are:
+ * watchdog_nmi_stop();
* update_variables();
- * watchdog_nmi_reconfigure(true);
+ * watchdog_nmi_start();
+ */
+void __weak watchdog_nmi_stop(void) { }
+
+/**
+ * watchdog_nmi_start - Start the watchdog after reconfiguration
*
- * The second call which starts the watchdogs again guarantees that the
- * following variables are stable across the call.
+ * Counterpart to watchdog_nmi_stop().
+ *
+ * The following variables have been updated in update_variables() and
+ * contain the currently valid configuration:
* - watchdog_enabled
* - watchdog_thresh
* - watchdog_cpumask
- *
- * After the call the variables can be changed again.
*/
-void __weak watchdog_nmi_reconfigure(bool run) { }
+void __weak watchdog_nmi_start(void) { }
/**
* lockup_detector_update_enable - Update the sysctl enable bit
@@ -551,13 +554,13 @@ static void softlockup_unpark_threads(void)
static void softlockup_reconfigure_threads(void)
{
- watchdog_nmi_reconfigure(false);
+ watchdog_nmi_stop();
softlockup_park_all_threads();
set_sample_period();
lockup_detector_update_enable();
if (watchdog_enabled && watchdog_thresh)
softlockup_unpark_threads();
- watchdog_nmi_reconfigure(true);
+ watchdog_nmi_start();
}
/*
@@ -602,9 +605,9 @@ static inline void watchdog_disable_all_cpus(void) { }
static inline void softlockup_init_threads(void) { }
static void softlockup_reconfigure_threads(void)
{
- watchdog_nmi_reconfigure(false);
+ watchdog_nmi_stop();
loc
Re: [PATCH v3 1/2] dt-bindings: add device tree binding for Allwinner XR819 SDIO Wi-Fi
于 2017年10月4日 GMT+08:00 下午5:02:17, Kalle Valo 写到: >Icenowy Zheng writes: > >> Allwinner XR819 is a SDIO Wi-Fi chip, which has the functionality to >use >> an out-of-band interrupt pin instead of SDIO in-band interrupt. >> >> Add the device tree binding of this chip, in order to make it >possible >> to add this interrupt pin to device trees. >> >> Signed-off-by: Icenowy Zheng >> Acked-by: Rob Herring >> --- >> Changes in v3: >> - Renames the node name. >> - Adds ACK from Rob. >> Changes in v2: >> - Removed status property in example. >> - Added required property reg. >> >> .../bindings/net/wireless/allwinner,xr819.txt | 38 >++ >> 1 file changed, 38 insertions(+) >> create mode 100644 >Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt > >Like I asked already last time, AFAICS there is no upstream xr819 >wireless driver in drivers/net/wireless directory. Do we still accept >bindings like this for out-of-tree drivers? See esp8089. There's also no in-tree driver for it.
[tip:core/watchdog] watchdog/core, powerpc: Lock cpus across reconfiguration
Commit-ID: e31d6883f21c1cdfe5bc64e28411f8a92b783fde
Gitweb: https://git.kernel.org/tip/e31d6883f21c1cdfe5bc64e28411f8a92b783fde
Author: Thomas Gleixner
AuthorDate: Tue, 3 Oct 2017 16:37:53 +0200
Committer: Thomas Gleixner
CommitDate: Wed, 4 Oct 2017 10:53:54 +0200
watchdog/core, powerpc: Lock cpus across reconfiguration
Instead of dropping the cpu hotplug lock after stopping NMI watchdog and
threads and reaquiring for restart, the code and the protection rules
become more obvious when holding cpu hotplug lock across the full
reconfiguration.
Suggested-by: Linus Torvalds
Signed-off-by: Thomas Gleixner
Acked-by: Michael Ellerman
Cc: Peter Zijlstra
Cc: Don Zickus
Cc: Benjamin Herrenschmidt
Cc: Nicholas Piggin
Cc: linuxppc-...@lists.ozlabs.org
Link: http://lkml.kernel.org/r/alpine.DEB.2.20.1710022105570.2114@nanos
---
arch/powerpc/kernel/watchdog.c | 4
kernel/smpboot.c | 3 +--
kernel/watchdog.c | 10 +-
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
index 2673ec8..f9b4c63 100644
--- a/arch/powerpc/kernel/watchdog.c
+++ b/arch/powerpc/kernel/watchdog.c
@@ -359,21 +359,17 @@ void watchdog_nmi_stop(void)
{
int cpu;
- cpus_read_lock();
for_each_cpu(cpu, &wd_cpus_enabled)
stop_wd_on_cpu(cpu);
- cpus_read_unlock();
}
void watchdog_nmi_start(void)
{
int cpu;
- cpus_read_lock();
watchdog_calc_timeouts();
for_each_cpu_and(cpu, cpu_online_mask, &watchdog_cpumask)
start_wd_on_cpu(cpu);
- cpus_read_unlock();
}
/*
diff --git a/kernel/smpboot.c b/kernel/smpboot.c
index ed7507b..5043e74 100644
--- a/kernel/smpboot.c
+++ b/kernel/smpboot.c
@@ -351,7 +351,7 @@ void smpboot_update_cpumask_percpu_thread(struct
smp_hotplug_thread *plug_thread
static struct cpumask tmp;
unsigned int cpu;
- get_online_cpus();
+ lockdep_assert_cpus_held();
mutex_lock(&smpboot_threads_lock);
/* Park threads that were exclusively enabled on the old mask. */
@@ -367,7 +367,6 @@ void smpboot_update_cpumask_percpu_thread(struct
smp_hotplug_thread *plug_thread
cpumask_copy(old, new);
mutex_unlock(&smpboot_threads_lock);
- put_online_cpus();
}
static DEFINE_PER_CPU(atomic_t, cpu_hotplug_state) =
ATOMIC_INIT(CPU_POST_DEAD);
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 6ad6226..fff90fe 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -535,7 +535,6 @@ static void softlockup_update_smpboot_threads(void)
smpboot_update_cpumask_percpu_thread(&watchdog_threads,
&watchdog_allowed_mask);
- __lockup_detector_cleanup();
}
/* Temporarily park all watchdog threads */
@@ -554,6 +553,7 @@ static void softlockup_unpark_threads(void)
static void softlockup_reconfigure_threads(void)
{
+ cpus_read_lock();
watchdog_nmi_stop();
softlockup_park_all_threads();
set_sample_period();
@@ -561,6 +561,12 @@ static void softlockup_reconfigure_threads(void)
if (watchdog_enabled && watchdog_thresh)
softlockup_unpark_threads();
watchdog_nmi_start();
+ cpus_read_unlock();
+ /*
+* Must be called outside the cpus locked section to prevent
+* recursive locking in the perf code.
+*/
+ __lockup_detector_cleanup();
}
/*
@@ -605,9 +611,11 @@ static inline void watchdog_disable_all_cpus(void) { }
static inline void softlockup_init_threads(void) { }
static void softlockup_reconfigure_threads(void)
{
+ cpus_read_lock();
watchdog_nmi_stop();
lockup_detector_update_enable();
watchdog_nmi_start();
+ cpus_read_unlock();
}
#endif /* !CONFIG_SOFTLOCKUP_DETECTOR */
Re: [PATCHv3 3/7] ia64: Add .opd based function descriptor dereference
On Sat 2017-09-30 11:53:15, Sergey Senozhatsky wrote: > We are moving towards separate kernel and module function descriptor > dereference callbacks. This patch enables it for IA64. > > For pointers that belong to the kernel > - Added __start_opd and __end_opd pointers, to track the kernel >.opd section address range; > > - Added dereference_kernel_function_descriptor(). Now we >will dereference only function pointers that are within >[__start_opd, __end_opd]; > > For pointers that belong to a module > - Added dereference_module_function_descriptor() to handle module >function descriptor dereference. Now we will dereference only >pointers that are within [module->opd.start, module->opd.end]. > > Signed-off-by: Sergey Senozhatsky > Tested-by: Helge Deller # parisc64 > Tested-by: Santosh Sivaraj # powerpc64 > Acked-by: Michael Ellerman # powerpc64 > Tested-by: Tony Luck # ia64 Looks good to me. Reviewed-by: Petr Mladek Best Regards, Petr
Re: usb/serial: use-after-free in usb_serial_disconnect/__lock_acquire
On Wed, Sep 27, 2017 at 04:36:11PM +0200, Andrey Konovalov wrote: > Hi! > > I've got the following report while fuzzing the kernel with syzkaller. > > On commit e19b205be43d11bff638cad4487008c48d21c103 (4.14-rc2). > > gadgetfs: bound to dummy_udc driver > usb 1-1: new full-speed USB device number 2 using dummy_hcd > gadgetfs: connected > gadgetfs: disconnected > gadgetfs: connected > usb 1-1: config 4 has an invalid interface number: 1 but max is 0 > usb 1-1: config 4 has an invalid interface number: 153 but max is 0 > usb 1-1: config 4 has 2 interfaces, different from the descriptor's value: 1 > usb 1-1: config 4 has no interface number 0 > usb 1-1: config 4 interface 1 altsetting 255 has an invalid endpoint > with address 0x0, skipping > usb 1-1: config 4 interface 1 altsetting 255 has an invalid endpoint > with address 0xFF, skipping > usb 1-1: config 4 interface 1 altsetting 255 has an invalid endpoint > with address 0x56, skipping > usb 1-1: too many endpoints for config 4 interface 153 altsetting 67: > 174, using maximum allowed: 30 > usb 1-1: config 4 interface 153 altsetting 67 has 0 endpoint > descriptors, different from the interface d > escriptor's value: 174 > usb 1-1: config 4 interface 1 has no altsetting 0 > usb 1-1: config 4 interface 153 has no altsetting 0 > usb 1-1: New USB device found, idVendor=1199, idProduct=6832 > usb 1-1: New USB device strings: Mfr=4, Product=20, SerialNumber=3 > usb 1-1: Product: a > usb 1-1: Manufacturer: a > usb 1-1: SerialNumber: a > gadgetfs: configuration #4 > sierra 1-1:4.1: Sierra USB modem converter detected > usb 1-1: Sierra USB modem converter now attached to ttyUSB0 > sierra 1-1:4.153: Sierra USB modem converter detected > gadgetfs: disconnected > usb 1-1: USB disconnect, device number 2 > sierra ttyUSB0: Sierra USB modem converter now disconnected from ttyUSB0 > sierra 1-1:4.1: device disconnected > == > BUG: KASAN: use-after-free in __lock_acquire+0x4504/0x4550 > Read of size 8 at addr 8800674df790 by task kworker/1:2/1846 > > CPU: 1 PID: 1846 Comm: kworker/1:2 Not tainted > 4.14.0-rc2-42660-g24b7bd59eec0 #277 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > Workqueue: usb_hub_wq hub_event > Call Trace: > __dump_stack lib/dump_stack.c:16 > dump_stack+0x292/0x395 lib/dump_stack.c:52 > print_address_description+0x78/0x280 mm/kasan/report.c:252 > kasan_report_error mm/kasan/report.c:351 > kasan_report+0x23d/0x350 mm/kasan/report.c:409 > __asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:430 > __lock_acquire+0x4504/0x4550 kernel/locking/lockdep.c:3376 > lock_acquire+0x259/0x620 kernel/locking/lockdep.c:4002 > __mutex_lock_common kernel/locking/mutex.c:756 > __mutex_lock+0x18e/0x1a50 kernel/locking/mutex.c:893 > mutex_lock_nested+0x1b/0x20 kernel/locking/mutex.c:908 > usb_serial_disconnect+0x69/0x2e0 drivers/usb/serial/usb-serial.c:1084 > usb_unbind_interface+0x21c/0xa90 drivers/usb/core/driver.c:423 > __device_release_driver drivers/base/dd.c:861 > device_release_driver_internal+0x4f4/0x5c0 drivers/base/dd.c:893 > device_release_driver+0x1e/0x30 drivers/base/dd.c:918 > bus_remove_device+0x2f4/0x4b0 drivers/base/bus.c:565 > device_del+0x5c4/0xab0 drivers/base/core.c:1985 > usb_disable_device+0x1e9/0x680 drivers/usb/core/message.c:1170 > usb_disconnect+0x260/0x7a0 drivers/usb/core/hub.c:2124 > hub_port_connect drivers/usb/core/hub.c:4754 > hub_port_connect_change drivers/usb/core/hub.c:5009 > port_event drivers/usb/core/hub.c:5115 > hub_event+0x1318/0x3740 drivers/usb/core/hub.c:5195 > process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119 > worker_thread+0x221/0x1850 kernel/workqueue.c:2253 > kthread+0x3a1/0x470 kernel/kthread.c:231 > ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431 Thanks for reporting this. I was able to reproduce the bug from the information you provided here, and incidentally discovered a long-standing related issue which I've now also fixed. Thanks, Johan
Re: [PATCH] [media] ov5645: I2C address change (fwd)
Hello,
It seems that an unlock is missing on line 764.
julia
-- Forwarded message --
Date: Wed, 4 Oct 2017 05:59:09 +0800
From: kbuild test robot
To: kbu...@01.org
Cc: Julia Lawall
Subject: Re: [PATCH] [media] ov5645: I2C address change
CC: kbuild-...@01.org
In-Reply-To: <1506950925-13924-1-git-send-email-todor.to...@linaro.org>
TO: Todor Tomov
CC: mche...@kernel.org, sakari.ai...@linux.intel.com, hansv...@cisco.com,
linux-me...@vger.kernel.org, linux-kernel@vger.kernel.org, Todor Tomov
CC: linux-me...@vger.kernel.org, linux-kernel@vger.kernel.org, Todor Tomov
Hi Todor,
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.14-rc3 next-20170929]
[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/Todor-Tomov/ov5645-I2C-address-change/20171003-234231
base: git://linuxtv.org/media_tree.git master
:: branch date: 6 hours ago
:: commit date: 6 hours ago
>> drivers/media/i2c/ov5645.c:806:1-7: preceding lock on line 760
#
https://github.com/0day-ci/linux/commit/c222075023642217170e2ef95f48efef079f9bcd
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout c222075023642217170e2ef95f48efef079f9bcd
vim +806 drivers/media/i2c/ov5645.c
9cae9722 Todor Tomov 2017-04-11 747
9cae9722 Todor Tomov 2017-04-11 748 static int ov5645_s_power(struct
v4l2_subdev *sd, int on)
9cae9722 Todor Tomov 2017-04-11 749 {
9cae9722 Todor Tomov 2017-04-11 750struct ov5645 *ov5645 = to_ov5645(sd);
9cae9722 Todor Tomov 2017-04-11 751int ret = 0;
9cae9722 Todor Tomov 2017-04-11 752
9cae9722 Todor Tomov 2017-04-11 753mutex_lock(&ov5645->power_lock);
9cae9722 Todor Tomov 2017-04-11 754
9cae9722 Todor Tomov 2017-04-11 755/* If the power count is modified from
0 to != 0 or from != 0 to 0,
9cae9722 Todor Tomov 2017-04-11 756 * update the power state.
9cae9722 Todor Tomov 2017-04-11 757 */
9cae9722 Todor Tomov 2017-04-11 758if (ov5645->power_count == !on) {
9cae9722 Todor Tomov 2017-04-11 759if (on) {
c2220750 Todor Tomov 2017-10-02 @760
mutex_lock(&ov5645_lock);
c2220750 Todor Tomov 2017-10-02 761
9cae9722 Todor Tomov 2017-04-11 762ret =
ov5645_set_power_on(ov5645);
9cae9722 Todor Tomov 2017-04-11 763if (ret < 0)
9cae9722 Todor Tomov 2017-04-11 764goto exit;
9cae9722 Todor Tomov 2017-04-11 765
c2220750 Todor Tomov 2017-10-02 766ret =
ov5645_write_reg_to(ov5645, 0x3100,
c2220750 Todor Tomov 2017-10-02 767
ov5645->i2c_client->addr, 0x78);
c2220750 Todor Tomov 2017-10-02 768if (ret < 0) {
c2220750 Todor Tomov 2017-10-02 769
dev_err(ov5645->dev,
c2220750 Todor Tomov 2017-10-02 770"could
not change i2c address\n");
c2220750 Todor Tomov 2017-10-02 771
ov5645_set_power_off(ov5645);
c2220750 Todor Tomov 2017-10-02 772
mutex_unlock(&ov5645_lock);
c2220750 Todor Tomov 2017-10-02 773goto exit;
c2220750 Todor Tomov 2017-10-02 774}
c2220750 Todor Tomov 2017-10-02 775
c2220750 Todor Tomov 2017-10-02 776
mutex_unlock(&ov5645_lock);
c2220750 Todor Tomov 2017-10-02 777
9cae9722 Todor Tomov 2017-04-11 778ret =
ov5645_set_register_array(ov5645,
9cae9722 Todor Tomov 2017-04-11 779
ov5645_global_init_setting,
9cae9722 Todor Tomov 2017-04-11 780
ARRAY_SIZE(ov5645_global_init_setting));
9cae9722 Todor Tomov 2017-04-11 781if (ret < 0) {
9cae9722 Todor Tomov 2017-04-11 782
dev_err(ov5645->dev,
9cae9722 Todor Tomov 2017-04-11 783"could
not set init registers\n");
9cae9722 Todor Tomov 2017-04-11 784
ov5645_set_power_off(ov5645);
9cae9722 Todor Tomov 2017-04-11 785goto exit;
9cae9722 Todor Tomov 2017-04-11 786}
9cae9722 Todor Tomov 2017-04-11 787
9cae9722 Todor Tomov 2017-04-11 788ret =
ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
9cae9722 Todor Tomov 2017-04-11 789
OV5645_SYSTEM_CTRL0_STOP);
9cae9722 Todor Tomov 2017-04-11 790if (ret < 0) {
9cae9722 Todor Tomov 2017-04-11 791
ov5645_set_power_off(ov5645);
9cae9722 Todor Tomov 2017-04-11 792goto exit;
9cae9722 Todor Tomov 2017-04-11 793}
9cae9722 Todor Tomov 2017-04-11 794} else {
9cae9722 Todor Tomov 2017-04-11 795
ov5645_set
Re: [PATCH 04/11] ia64: make dma_cache_sync a no-op
On Wed, Oct 04, 2017 at 08:59:24AM +, David Laight wrote: > Are you sure about this one? Yes. And if you are not you haven't read either of the cover letter, the DMA-API documentation, or the reply from Thomas to your mail yesterday.
Re: [PATCH 3/3] early_printk: Add simple serialization to early_vprintk()
On Tue, Oct 03, 2017 at 06:24:22PM -0400, Steven Rostedt wrote:
> On Thu, 28 Sep 2017 14:18:26 +0200
> Peter Zijlstra wrote:
> > static int early_vprintk(const char *fmt, va_list args)
> > {
> > + int n, cpu, old;
> > char buf[512];
> > +
> > + cpu = get_cpu();
> > + /*
> > +* Test-and-Set inter-cpu spinlock with recursion.
> > +*/
> > + for (;;) {
> > + /*
> > +* c-cas to avoid the exclusive bouncing on spin.
> > +* Depends on the memory barrier implied by cmpxchg
> > +* for ACQUIRE semantics.
> > +*/
> > + old = READ_ONCE(early_printk_cpu);
> > + if (old == -1) {
>
> If old != -1 and old != cpu, is it possible that the CPU could have
> fetched an old value, and never try to fetch it again?
What? If old != -1 and old != cpu, we'll hit the cpu_relax() and do the
READ_ONCE() again. The READ_ONCE() guarantees we'll do the load again,
as does the barrier() implied by cpu_relax().
> The cmpxchg memory barrier only happens when old == -1.
Yeah, so?
> > + old = cmpxchg(&early_printk_cpu, -1, cpu);
> > + if (old == -1)
> > + break;
> > + }
> > + /*
> > +* Allow recursion for interrupts and the like.
> > +*/
> > + if (old == cpu)
> > + break;
> > +
> > + cpu_relax();
> > + }
> >
> > n = vscnprintf(buf, sizeof(buf), fmt, args);
> > early_console->write(early_console, buf, n);
> >
> > + /*
> > +* Unlock -- in case @old == @cpu, this is a no-op.
> > +*/
> > + smp_store_release(&early_printk_cpu, old);
> > + put_cpu();
> > +
> > return n;
> > }
Re: [PATCH v4 03/10] kexec_file: factor out arch_kexec_kernel_*() from x86, powerpc
Hi AKASHI,
[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on v4.14-rc3 next-20170929]
[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/AKASHI-Takahiro/arm64-kexec-add-kexec_file_load-support/20171004-163130
base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git
for-next/core
config: x86_64-randconfig-x000-201740 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
>> arch/x86/kernel/kexec-bzimage64.c:541:29: error: conflicting type qualifiers
>> for 'kexec_bzImage64_ops'
const struct kexec_file_ops kexec_bzImage64_ops = {
^~~
In file included from arch/x86/kernel/kexec-bzimage64.c:29:0:
arch/x86/include/asm/kexec-bzimage64.h:4:30: note: previous declaration of
'kexec_bzImage64_ops' was here
extern struct kexec_file_ops kexec_bzImage64_ops;
^~~
vim +/kexec_bzImage64_ops +541 arch/x86/kernel/kexec-bzimage64.c
540
> 541 const struct kexec_file_ops kexec_bzImage64_ops = {
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: ath9k: make const array reg_hole_list static, reduces object code size
Colin Ian King wrote: > Don't populate the read-only array reg_hole_list on the stack, instead make > it static. Makes the object code smaller by over 200 bytes: > > Before: >text data bss dec hex filename > 57518 15248 0 72766 11c3e debug.o > > After: >text data bss dec hex filename > 57218 15344 0 72562 11b72 debug.o > > Signed-off-by: Colin Ian King > Signed-off-by: Kalle Valo Patch applied to ath-next branch of ath.git, thanks. eba0f28473b2 ath9k: make const array reg_hole_list static, reduces object code size -- https://patchwork.kernel.org/patch/9960183/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
